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

Integration tests: write more logs, check exit code #152

Merged
merged 1 commit into from
May 27, 2020

Conversation

matzf
Copy link
Contributor

@matzf matzf commented May 27, 2020

Integration tests

  • write both stdout and stderr to a log file
  • store logs as artifact in circleci. Put logs in a common subfolder under /tmp/ to make this easier
  • check process exit code to find obvious problems more easily; for now, no option to signal an expected failure, instead remove the only test that was explicitly creating failures

This change is Reviewable

Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)


_examples/helloworld/helloworld_integration_test.go, line 63 at r1 (raw file):

client_error

for now, no option to signal an expected failure, instead remove the only test that was explicitly creating failures

If you don't want to add it in this PR, I think as separate PR could add the expected exit codes to the testcases and have them checked as part of the appsWaiter. Also some apps with non-failed commands exit with non-zero exit codes.


pkg/integration/apps.go, line 300 at r1 (raw file):

io.TeeReader

Nice solution, makes this much more concise.

Copy link
Contributor Author

@matzf matzf left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @FR4NK-W)


_examples/helloworld/helloworld_integration_test.go, line 63 at r1 (raw file):

Previously, FR4NK-W wrote…
client_error

for now, no option to signal an expected failure, instead remove the only test that was explicitly creating failures

If you don't want to add it in this PR, I think as separate PR could add the expected exit codes to the testcases and have them checked as part of the appsWaiter. Also some apps with non-failed commands exit with non-zero exit codes.

Agreed. I think it will make most sense to add this feature once we want to add a test that would make use of it.

Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@matzf matzf merged commit 688fb0d into master May 27, 2020
@matzf matzf deleted the matzf/integration-logs branch May 27, 2020 12:23
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.

2 participants