Skip to content

refactor: migrate multi_platform_linux_test.go to nerdtest.Setup#4897

Open
ogulcanaydogan wants to merge 6 commits into
containerd:mainfrom
ogulcanaydogan:refactor/nerdctl-tigron-multi-platform-test
Open

refactor: migrate multi_platform_linux_test.go to nerdtest.Setup#4897
ogulcanaydogan wants to merge 6 commits into
containerd:mainfrom
ogulcanaydogan:refactor/nerdctl-tigron-multi-platform-test

Conversation

@ogulcanaydogan
Copy link
Copy Markdown
Contributor

Part of #4613.

Migrates cmd/nerdctl/container/multi_platform_linux_test.go from testutil.NewBase to the Tigron nerdtest.Setup framework, following the pattern established in #4641.

Changes:

  • testutil.RequireExecPlatform replaced with a custom requireMultiPlatformExec requirement backed by platformutil.CanExecProbably
  • testregistry.NewWithNoAuth replaced with nerdtest.RegistryWithNoAuth
  • Dockerfiles and compose files written via data.Temp().Save()
  • TestMultiPlatformRun converted to use SubTests for per-platform isolation
  • testMultiPlatformRun helper updated to use test.Helpers
  • Builder cache prune registered in Cleanup callbacks
  • testutil.DockerIncompatible replaced with require.Not(nerdtest.Docker)
  • testutil.RequiresBuild replaced with nerdtest.Build

Replace testutil.NewBase with nerdtest.Setup throughout
multi_platform_linux_test.go, following the Tigron framework
pattern (containerd#4641).

Changes:
- Use nerdtest.RegistryWithNoAuth instead of testregistry.NewWithNoAuth
- Use data.Temp().Save() for Dockerfiles and compose files
- Convert testMultiPlatformRun helper to use test.Helpers
- Use SubTests in TestMultiPlatformRun for per-platform isolation
- Register builder cache cleanup in Cleanup callbacks
- Add requireMultiPlatformExec requirement using platformutil

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
nerdtest.RegistryWithNoAuth returns a Server struct with Setup/Cleanup
closures but does not start the registry container automatically. Add
reg.Setup(data, helpers) in each test's Setup callback to start the
registry before pushing, and reg.Cleanup(data, helpers) in the Cleanup
callback to remove the container after the test.

Affected: TestMultiPlatformBuildPush, TestMultiPlatformBuildPushNoRun,
TestMultiPlatformPullPushAllPlatforms.

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
return helpers.Command("push", "--platform=amd64,arm64,linux/arm/v7", imageName)
}

testCase.Expected = test.Expects(0, nil, nil)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use exit code conatnts from Tigron framework and there are others as well

var reg *registry.Server

testCase.Setup = func(data test.Data, helpers test.Helpers) {
reg = nerdtest.RegistryWithNoAuth(data, helpers, 0, false)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

better to explain or use a variable for 0 constant for readbility

…m tests

Replace hardcoded 0 literals passed to test.Expects() with
expect.ExitCodeSuccess for consistency with the rest of the test suite.

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
@ogulcanaydogan
Copy link
Copy Markdown
Contributor Author

Thanks for the review. Replaced all hardcoded exit code literals in test.Expects() calls with expect.ExitCodeSuccess.

The third argument 0 means "let the OS pick any available port". Add an
inline comment to make this clear at the call sites.

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
@ogulcanaydogan
Copy link
Copy Markdown
Contributor Author

Addressed. Added /* random port */ inline comment on all three RegistryWithNoAuth(data, helpers, 0, false) call sites.

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
@AkihiroSuda
Copy link
Copy Markdown
Member

test / in-host / docker is failing

2026-05-27T10:54:50.0120633Z === �[31mFAIL�[0m: cmd/nerdctl/container TestMultiPlatformRun (0.53s)
2026-05-27T10:54:50.0120788Z     multi_platform_linux_test.go:96: 
2026-05-27T10:54:50.0121047Z         ⤵️️ "TestMultiPlatformRun": into subtests prep
2026-05-27T10:54:50.0121194Z     multi_platform_linux_test.go:96: 
2026-05-27T10:54:50.0121475Z         ↩️️ "TestMultiPlatformRun": done with subtests prep

https://github.com/containerd/nerdctl/actions/runs/25974269421/job/78060013893?pr=4897

SubTests run via t.Parallel() in Tigron, which caused concurrent docker
runs across platforms to fail in the in-host/docker CI job. The original
test iterated platforms sequentially; use the existing assertMultiPlatformRun
helper inside Setup to preserve that behavior.

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
@ogulcanaydogan
Copy link
Copy Markdown
Contributor Author

@AkihiroSuda thanks for the catch. Root cause was Tigron's SubTests path calling subT.Parallel() (mod/tigron/test/case.go:184), which made the docker driver run the three platform invocations concurrently and step on each other.

Pushed e10045d: dropped the SubTests map and run assertMultiPlatformRun sequentially from the Setup callback (same pattern for all five testcases in the file). On the new run, in-host / docker < linux skips TestMultiPlatformRun cleanly at the requireMultiPlatformExec gate (binfmt not present in that runner). The three remaining failures in that job (TestImagesFilterDangling, TestRunSeccompCapSysPtrace) don't touch this file.

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.

3 participants