c8d integration: skip TestImportWithCustomPlatform

[laurazard]

- What I did

Fix TestImportWithCustomPlatform (failing in https://github.com/moby/moby/actions/runs/6506688442/job/17672826153)

Skip failing TestImportWithCustomPlatform test cases when using the containerd image store.

- How I did it

Reuse the same validation logic from here

that's already used in here

Split out the "reject" cases from TestImportWithCustomPlatform into a new test TestImportWithCustomPlatformReject, which is skipped when using the containerd image store.

- How to verify it

go test -run TestImportWithCustomPlatform ./integration/image/...
go test -run TestImportWithCustomPlatformReject ./integration/image/...

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[rumpl]

With containerd and its shims I don't think we should be checking the platform any more, for example with this change I wouldn't be able to import a Wasm image any more. Maybe it's best to skip this test?

[laurazard]

Oh, I totally got ahead of myself. Yeah, that makes sense – I'll skip it.

However, this goes back to the discussion about whether we want to

1. never do any validation for the platform when using the c8d backend (and then throw an error when we try to run the image with no means for running it?) or
2. do some kind of validation we can run the image before importing it

Regardless I'll remove the check and skip this particular test as it doesn't make sense.

[thaJeztah]

Do we have any test to test the feature? i.e. to verify that I can import a rootfs, and specify what platform to use for it?

[thaJeztah]

Because this test-table contains both "successful" (expectedErr = "") and "failing" imports, so we probably could still run the successful ones

[laurazard]

I can split the test out into two (the "successful" cases and the ones that aren't supposed to work, and just skip the "failure" test w/ c8d image store.

[thaJeztah]

Yes, it's probably worth having some tests for this. Maybe as alternative, we could have an extra field in the test-table, so that we can skip based on that one

[laurazard]

I'd rather split the test and just straight up skip one rather than decide whether to skip dynamically based on an attribute of the test case, for readability. (but if you feel strongly about keeping it as one test I can do that 😅 )

[thaJeztah]

Either one is fine

[laurazard]

@thaJeztah PTAL

[thaJeztah]

I see this one failing https://github.com/moby/moby/actions/runs/6535667219/job/17745687263?pr=46652, but it's in "test (graphdriver) / integration (ubuntu-20.04, rootless)"; wondering what broke it, or if somehow GitHub actions is reporting results in the wrong place (or running with the wrong options)

=== FAIL: amd64.integration.image TestImportWithCustomPlatformReject/linux/sparc64 (0.01s)
    import_test.go:173: assertion failed: expected an error, got nil
    --- FAIL: TestImportWithCustomPlatformReject/linux/sparc64 (0.01s)

=== FAIL: amd64.integration.image TestImportWithCustomPlatformReject (0.02s)

[laurazard]

Ahh I slipped while copy-pasting 😅 1 sec

[laurazard]

I don't think the remaining failures have to do with this test – @thaJeztah can you TAL?

[thaJeztah]

LOL, I was trying to find the failing tests in the report (as the logs output was a bit hard to find them all), and then realised they weren't shown as Github was hiding them https://github.com/moby/moby/actions/runs/6536940074

[thaJeztah]

LGTM