Closed thaJeztah closed 6 days ago
Also logs / output before/after
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 61.46%. Comparing base (
3837aa6
) to head (ab23024
). Report is 3 commits behind head on master.
💡 👉 As some of the sub-test changes introduce whitespace noise; the diff is easier to read when ignoring white-spaces; use this URL to see the diff with white-space suppressed; https://github.com/docker/cli/pull/5224/files?w=1 (i.e. add ?w=1
to the URL)
This makes a quick pass through our tests;
Discard output/err
Many tests were testing for error-conditions, but didn't discard output. This produced a lot of noise when running the tests, and made it hard to discover if there were actual failures, or if the output was expected. For example:
And after discarding output:
Use sub-tests where possible
Some tests were already set-up to use test-tables, and even had a usable name (or in some cases "error" to check for). Change them to actual sub- tests. Same test as above, but now with sub-tests and output discarded:
It's not perfect in all cases (in the above, there's duplicate "expected" errors, but Go conveniently adds "#01" for the duplicate). There's probably also various tests I missed that could still use the same changes applied; we can improve these in follow-ups.
Set cmd.Args to prevent test-failures
When running tests from my IDE, it compiles the tests before running, then executes the compiled binary to run the tests. Cobra doesn't like that, because in that situation
os.Args
is taken as argument for the command that's executed. The command that's tested now sees the test- flags as arguments (-test.v -test.run ..
), which causes various tests to fail ("Command XYZ does not accept arguments").The Cobra maintainers ran into the same situation, and for their own use have added a special case to ignore
os.Args
in these cases; https://github.com/spf13/cobra/blob/v1.8.1/command.go#L1078-L1083Unfortunately, that exception is too specific (only checks for
cobra.test
), so doesn't automatically fix the issue for other test-binaries. They did provide acmd.SetArgs()
utility for this purpose https://github.com/spf13/cobra/blob/v1.8.1/command.go#L276-L280And the fix is to explicitly set the command's args to an empty slice to prevent Cobra from falling back to using
os.Args[1:]
as arguments.Some tests already take this issue into account, and I updated some tests for this, but there's likely many other ones that can use the same treatment.
Perhaps the Cobra maintainers would accept a contribution to make their condition less specific and to look for binaries ending with a
.test
suffix (which is what compiled binaries usually are named as).- A picture of a cute animal (not mandatory but encouraged)