Use t.Run for /pkg/cri tests#7001
Conversation
|
I'd probably drop the "TestCase " prefix; I don't think it adds much value. (There could be people parsing the existing Also, thanks for doing this! |
I'll drop 'em then as I'm in agreeance
Of course! |
|
@samuelkarp Removed the TestCase prefix, thanks! |
|
@samuelkarp Seems my editor didn't feel like removing unused imports today. Fixing shortly |
A majority of the tests in /pkg/cri are testing/validating multiple things per test (generally spec or options validations). This flow lends itself well to using *testing.T's Run method to run each thing as a subtest so `go test` output can actually display which subtest failed/passed. Some of the tests in the packages in pkg/cri already did this, but a bunch simply logged what sub-testcase was currently running without invoking t.Run. Signed-off-by: Daniel Canter <[email protected]>
samuelkarp
left a comment
samuelkarp
left a comment
There was a problem hiding this comment.
Thanks for doing this! LGTM.
4 participants
A majority of the tests in /pkg/cri are testing/validating multiple things per test (generally spec or options validations). This flow lends itself well to using *testing.T's Run method to run each thing as a subtest so
go testoutput can actually display which subtest failed/passed.Some of the tests in the packages in pkg/cri already did this, but a bunch simply logged what testcase was currently running without invoking
t.Run().Thanks to @samuelkarp for suggesting this on another PR #6996 (comment). I'm not married to the "TestCase" prefix for the subtest names, but it was already used for the log and a couple other
t.Run()uses so mostly following how things already were.