fix: remove default platform option from linter tests #5103
Conversation
The progress stream would set the platform to `linux/amd64` and `linux/arm64` during testing. This would trigger the `InvalidBaseImagePlatform` rule whenever an image was pulled (usually alpine). This removes that option as it shouldn't be necessary to run the tests. Signed-off-by: Jonathan A. Sternberg <[email protected]>
|
I have a small concern with this PR. It seems to fix the tests, but I'm concerned that it may be masking a bug in |
| attrs := lintTest.FrontendAttrs | ||
| if attrs == nil { | ||
| attrs = map[string]string{ | ||
| "platform": "linux/amd64,linux/arm64", |
There was a problem hiding this comment.
@daghack That was the original intention for this? I guess to check som warning deduplication. In this is needed then it should only be ignored on certain tests (although if it is needed then why isn't the same checked for unmarshal tests). Alternatively there should be some specific tests that specifically check for multi-platform cases.
There was a problem hiding this comment.
It seems like it was just a way to keep an old default for the tests when a specific test didn't pass in specific frontend attributes. The old default came because I was emulating where I saw it elsewhere in tests—there shouldn't be any problems with removing this.
There was a problem hiding this comment.
But do we have tests that cover deduplication on warnings. So that when user is building multiple platforms when they do not get the same warnings twice?
The progress stream would set the platform to
linux/amd64andlinux/arm64during testing. This would trigger theInvalidBaseImagePlatformrule whenever an image was pulled (usually alpine).This removes that option as it shouldn't be necessary to run the tests.