main: unif/refactor std{out,err} test handling#450
Open
lzap wants to merge 2 commits intoosbuild:mainfrom
Open
main: unif/refactor std{out,err} test handling#450lzap wants to merge 2 commits intoosbuild:mainfrom
lzap wants to merge 2 commits intoosbuild:mainfrom
Conversation
Contributor
Author
|
TestManifestIntegrationSmoke fails, need to take a look. |
For historic reasons we have two way to test the std{out,err}
output. One is to just monkey patch `var osStdout = os.Stdout`
and then replace it in the tests, the other to have a helper
that works like a `with` in python that replaces the stdout/stderr
dynamically, e.g.:
```
stdout, stderr := testutil.CaptureStdio(t, func() {
err = main.Run()
require.NoError(t, err)
})
```
The trouble is that they are not quite compatible. In places
where we use the monkey patches `osStdout` the
testutil.CaptureStdio() will not work as it (temporarely)
replaces the "real" `os.Stdout` (and vice-versa).
So this commit unifies all tests to use testutil.CaptureStdio()
which feels slightly nicer than the other approach in the
sense that one does not need to remember to use `osStdout` in
the code. Its mostly mechanical.
Co-authored-by: Lukas Zapletal <lzap+git@redhat.com>
These smoke tests do JSON matching on the pretty-printed manifest which is not ideal, but it is pretty effective for smoke tests. This fixes a bug that is unrelated to the previous commit.
Contributor
Author
|
Rebased, fixed the test, it seems unrelated so something wrong was merged? I do not think it is worth implementing any complex JSON testing for these smoke tests, at least for now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For historic reasons we have two way to test the std{out,err} output. One is to just monkey patch
var osStdout = os.Stdoutand then replace it in the tests, the other to have a helper that works like awithin python that replaces the stdout/stderr dynamically, e.g.:The trouble is that they are not quite compatible. In places where we use the monkey patches
osStdoutthetestutil.CaptureStdio() will not work as it (temporarely) replaces the "real"
os.Stdout(and vice-versa).So this commit unifies all tests to use testutil.CaptureStdio() which feels slightly nicer than the other approach in the sense that one does not need to remember to use
osStdoutin the code. Its mostly mechanical.I rebased #416 on top of
main, review carefully thanks :)