Use t.Run for /pkg/cri tests #7001

dcantah · 2022-05-30T00:57:32Z

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 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.

samuelkarp · 2022-05-30T01:01:56Z

I'd probably drop the "TestCase " prefix; I don't think it adds much value. (There could be people parsing the existing t.Log-based output, but...that seems reasonably unlikely as test output and test names can change as we develop them; they're not an interface or a contract.)

Also, thanks for doing this!

dcantah · 2022-05-30T01:05:33Z

I'd probably drop the "TestCase " prefix; I don't think it adds much value. (There could be people parsing the existing t.Log-based output, but...that seems reasonably unlikely as test output and test names can change as we develop them; they're not an interface or a contract.)

I'll drop 'em then as I'm in agreeance

Also, thanks for doing this!

Of course!

dcantah · 2022-05-30T01:16:44Z

@samuelkarp Removed the TestCase prefix, thanks!

pkg/cri/server/container_stats_list_linux_test.go

pkg/cri/server/container_update_resources_linux_test.go

pkg/cri/server/helpers_linux_test.go

pkg/cri/server/helpers_selinux_linux_test.go

pkg/cri/server/sandbox_run_linux_test.go

dcantah · 2022-05-30T01:21:13Z

@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 <dcanter@microsoft.com>

samuelkarp

Thanks for doing this! LGTM.

mikebrow

LGTM

dcantah force-pushed the cri-trun branch from 392f6f4 to 6bf2dab Compare May 30, 2022 01:14

samuelkarp reviewed May 30, 2022

View reviewed changes

dcantah force-pushed the cri-trun branch from 6bf2dab to b5e1b8f Compare May 30, 2022 01:32

samuelkarp approved these changes May 30, 2022

View reviewed changes

mikebrow approved these changes May 31, 2022

View reviewed changes

kzys approved these changes May 31, 2022

View reviewed changes

kzys merged commit 78cd9d3 into containerd:main May 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use t.Run for /pkg/cri tests #7001

Use t.Run for /pkg/cri tests #7001

Uh oh!

dcantah commented May 30, 2022 •

edited

Loading

Uh oh!

samuelkarp commented May 30, 2022 •

edited

Loading

Uh oh!

dcantah commented May 30, 2022

Uh oh!

dcantah commented May 30, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dcantah commented May 30, 2022

Uh oh!

samuelkarp left a comment

Uh oh!

mikebrow left a comment

Uh oh!

Uh oh!

Use t.Run for /pkg/cri tests #7001

Use t.Run for /pkg/cri tests #7001

Uh oh!

Conversation

dcantah commented May 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samuelkarp commented May 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcantah commented May 30, 2022

Uh oh!

dcantah commented May 30, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dcantah commented May 30, 2022

Uh oh!

samuelkarp left a comment

Choose a reason for hiding this comment

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dcantah commented May 30, 2022 •

edited

Loading

samuelkarp commented May 30, 2022 •

edited

Loading