Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TAP output with multiple RuntimeInsideValidate #658

Merged
merged 3 commits into from
Aug 21, 2018

Conversation

alban
Copy link
Contributor

@alban alban commented Jul 17, 2018

Getting proper TAP output with RuntimeInsideValidate was not possible when running several containers in a single test file. With this PR, we can optionally pass the TAP object in RuntimeInsideValidate and then runtimetest in the container will no longer directly print its own TAP output on stdout but it will be intercepted and encapsulated in the outer TAP output.

Signed-off-by: Alban Crequy alban@kinvolk.io

Copy link
Contributor

@dongsupark dongsupark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. (but I'm not maintainer)
Tested it, and it works.
See below for minor things:


#### Using `util.RuntimeInsideValidate` and encapsulation

Similar to previously but the test consumes the output from `runtimetest` and re-emit a single TAP result for the container run.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it more precise to write Similar to the passthrough variant but...?

Example:
```go
err = util.RuntimeOutsideValidate(g, t, func(config *rspec.Spec, t *tap.T, state *rspec.State) error {
err := test_foo()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: s/test_foo/testFoo/ for the idiomatic Go coding style?

Signed-off-by: Alban Crequy <alban@kinvolk.io>
Signed-off-by: Alban Crequy <alban@kinvolk.io>
Signed-off-by: Alban Crequy <alban@kinvolk.io>
@alban
Copy link
Contributor Author

alban commented Aug 14, 2018

Thanks for the review. Fixed & rebased.

@dongsupark
Copy link
Contributor

👍

@zhouhao3
Copy link

zhouhao3 commented Aug 21, 2018

LGTM

Approved with PullApprove

@zhouhao3 zhouhao3 merged commit 2e13089 into opencontainers:master Aug 21, 2018
@dongsupark dongsupark deleted the alban/inceptiontap branch August 22, 2018 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants