fix: error when base image doesn't support target platforms #3707
Conversation
|
Thank you; we'll review next week. |
There was a problem hiding this comment.
Thank you for the fix @gsquared94! The macos failure is related to some kokoro changes we recently made. Rebasing with main should help address this.
jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java
Outdated
Show resolved
Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java
Outdated
Show resolved
Hide resolved
| public void testCheckManifestPlatform_noWarningIfDefaultAmd64Linux() | ||
| throws PlatformNotFoundInBaseImageException { |
There was a problem hiding this comment.
Oops sorry I missed this earlier. This test no longer performs an assertion so just invoking the method in the test may not give us many insights into what is expected from the method's behavior. How about we replace this test with the following?: testCheckManifestPlatform_arm64Linux where the architecture doesn't match but the os matches. This will help us test the && in the following condition: !(platform.getArchitecture().equals("amd64") && platform.getOs().equals("linux"))
There was a problem hiding this comment.
just invoking the method in the test may not give us many insights into what is expected from the method's behavior
On the contrary I think that the passing test implies the expectation that it should not throw any exception. Also this yaqs answer recommends that.
jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed! |
|
LGTM! Thanks again @gsquared94! |
Fixes #3563: Converts warnings to exceptions.