Skip to content

test: refactor container_run_linux_test.go to use Tigron#5071

Merged
AkihiroSuda merged 1 commit into
containerd:mainfrom
papidb:test/refactor-container-run-linux-test-to-tigron
Jul 19, 2026
Merged

test: refactor container_run_linux_test.go to use Tigron#5071
AkihiroSuda merged 1 commit into
containerd:mainfrom
papidb:test/refactor-container-run-linux-test-to-tigron

Conversation

@papidb

@papidb papidb commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Rewrite tests in container_run_linux_test.go from testutil.NewBase to nerdtest.Setup (Tigron framework). Part of #4613.

@AkihiroSuda

Copy link
Copy Markdown
Member

Marking as a draft, as CI including lint is failing.
Please click "Ready for review" when ready.

@AkihiroSuda
AkihiroSuda marked this pull request as draft July 17, 2026 10:14
@papidb
papidb force-pushed the test/refactor-container-run-linux-test-to-tigron branch from a46f144 to 5fda100 Compare July 17, 2026 21:05
@papidb
papidb marked this pull request as ready for review July 18, 2026 10:58

@AkihiroSuda AkihiroSuda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please squash the commits


// Was thinking of os.ReadLink("/proc/1/ns/uts")
// but you'd get EPERM for rootless. Just validate the
// hostname is the same.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The comment strings should be preserved

}

func TestRunAddHost(t *testing.T) {
// Not parallelizable (https://github.com/containerd/nerdctl/issues/1127)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please retain the comments

Description: "stty with -it",
Cleanup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("rm", "-f", data.Identifier())
helpers.Anyhow("rm", "-f", data.Identifier())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?


func TestRunDeviceCDI(t *testing.T) {
t.Parallel()
// Although CDI injection is supported by Docker, specifying the --cdi-spec-dirs on the command line is not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please retain the comments


func TestRunDeviceCDIWithNerdctlConfig(t *testing.T) {
t.Parallel()
// Although CDI injection is supported by Docker, specifying the --cdi-spec-dirs on the command line is not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please retain the comments

// TestRunGPU tests GPU injection using the --gpus flag.
func TestRunGPU(t *testing.T) {
t.Parallel()
// Although CDI injection is supported by Docker, specifying the --cdi-spec-dirs on the command line is not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same

// TestRunGPUWithOtherCDIDevices tests GPU CDI injection along with other CDI devices.
func TestRunGPUWithOtherCDIDevices(t *testing.T) {
t.Parallel()
// Although CDI injection is supported by Docker, specifying the --cdi-spec-dirs on the command line is not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same

return subTests
}

func assertAddHostEntry(stdout, expected string) error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs a comment to explain what it does

return nil
}

func assertAddHostEntries(stdout string, expected []string) error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same. How is this different from assertAddHostEntry?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there's none, I've deleted assertAddHostEntry in favor of assertAddHostEntries.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also added comments to assertAddHostEntries

@papidb
papidb force-pushed the test/refactor-container-run-linux-test-to-tigron branch from bca2734 to d635153 Compare July 18, 2026 22:23

func TestRunDeviceCDI(t *testing.T) {
t.Parallel()
// Although CDI injection is supported by Docker, specifying the --cdi-spec-dirs on the command line is not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be commented around the line of require.Not(nerdtest.Docker).

Same for other tests too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I understand you correctly, you're asking me to move the // Although CDI injection is supported by Docker... comment to sit above the testCase.Require = require.Not(nerdtest.Docker) line, yes?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes

@papidb
papidb force-pushed the test/refactor-container-run-linux-test-to-tigron branch 2 times, most recently from cf13079 to 656035d Compare July 18, 2026 23:34
Signed-off-by: Daniel Benjamin <benjamindaniel706@gmail.com>
@papidb
papidb force-pushed the test/refactor-container-run-linux-test-to-tigron branch from 656035d to d4f4231 Compare July 18, 2026 23:38

@AkihiroSuda AkihiroSuda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda
AkihiroSuda merged commit ae5bc5d into containerd:main Jul 19, 2026
69 of 72 checks passed
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