refactor: migrate multi_platform_linux_test.go to nerdtest.Setup#4897
Conversation
8e7b2bf to
3fed4ad
Compare
| return helpers.Command("push", "--platform=amd64,arm64,linux/arm/v7", imageName) | ||
| } | ||
|
|
||
| testCase.Expected = test.Expects(0, nil, nil) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
better to explain or use a variable for 0 constant for readbility
|
Thanks for the review. Replaced all hardcoded exit code literals in test.Expects() calls with expect.ExitCodeSuccess. |
|
Addressed. Added |
|
https://github.com/containerd/nerdctl/actions/runs/25974269421/job/78060013893?pr=4897 |
|
@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. |
|
Still constantly failing Can you try rebasing? |
e10045d to
5ee828b
Compare
|
@AkihiroSuda rebased onto latest main. The docker < linux job you linked is green now, so TestImagesFilterDangling and TestRunSeccompCapSysPtrace pass - they were failing on the old base, not from this PR (the diff is a single file, multi_platform_linux_test.go). The 5 red checks are all in other jobs hitting unrelated tests: almalinux-8 rootless and FreeBSD / 14 are the labeled flakes (#3988), rootful canary is TestIPFSAddrWithKubo (IPFS daemon), rootful old-ubuntu is TestUpdateRestartPolicy, and gomodjail failed at the "register QEMU (binfmt)" setup step. None of them touch the file this PR changes. |
| testCase := nerdtest.Setup() | ||
|
|
||
| testCase.Require = require.All( | ||
| require.Not(nerdtest.Docker), |
There was a problem hiding this comment.
Please keep the comment line to explain the reason
| testCase := nerdtest.Setup() | ||
|
|
||
| testCase.Require = require.All( | ||
| require.Not(nerdtest.Docker), |
There was a problem hiding this comment.
Please keep the comment line to explain the reason
|
Friendly bump — CI only has the FreeBSD / 14 job red which is the known flake (#3988, unrelated to this diff). The diff is a single file (multi_platform_linux_test.go). Happy to make further changes if needed. |
Please read the comments from @ChengyuZhu6 and me |
|
@AkihiroSuda done — added the explanation comments back to both TestMultiPlatformBuildPush and TestMultiPlatformBuildPushNoRun (commit 4c90ff9). |
|
Please also squash the commits? |
Replace the legacy testutil.NewBase pattern with the nerdtest.Setup /
Tigron test.Case framework throughout multi_platform_linux_test.go.
- Migrate all five test cases to test.Case{Setup, Cleanup, Command, Expected}
- Replace SubTests with sequential assertMultiPlatformRun calls to fix
concurrent platform invocations under the docker driver
- Extract randomPort constant and requireMultiPlatformExec requirement
- Use Tigron expect constants for exit codes
- Add comments explaining why require.Not(nerdtest.Docker) is needed
(non-buildx docker build lacks multi-platform; docker push lacks --platform)
Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
4c90ff9 to
83b54f0
Compare
|
@AkihiroSuda done — squashed to a single commit (83b54f0). |
Part of #4613.
Migrates
cmd/nerdctl/container/multi_platform_linux_test.gofromtestutil.NewBaseto the Tigronnerdtest.Setupframework, following the pattern established in #4641.Changes:
testutil.RequireExecPlatformreplaced with a customrequireMultiPlatformExecrequirement backed byplatformutil.CanExecProbablytestregistry.NewWithNoAuthreplaced withnerdtest.RegistryWithNoAuthdata.Temp().Save()TestMultiPlatformRunconverted to useSubTestsfor per-platform isolationtestMultiPlatformRunhelper updated to usetest.HelpersCleanupcallbackstestutil.DockerIncompatiblereplaced withrequire.Not(nerdtest.Docker)testutil.RequiresBuildreplaced withnerdtest.Build