Add skip parameter to base plugin configuration #911
Conversation
- can use system property -Djib.skip=true to skip if part of lifecycle - still requires "required parameters" on jib to be configured even if skipping, since maven doesn't provide a api for actual goal skipping
|
If anyone has any decent testing strategies for this, let me know. It's kind of hard to do since maven doesn't actually define "skipping" a goal like gradle does. |
|
You could run something like the |
|
@briandealwis I was hoping to ensure that the return was called, maybe I can ensure the log has no more writes after that? |
| verifier.setSystemProperty("jib.skip", "true"); | ||
|
|
||
| verifier.executeGoal("jib:" + BuildDockerMojo.GOAL_NAME); | ||
| File logFile = new File(verifier.getBasedir(), verifier.getLogFileName()); |
There was a problem hiding this comment.
nit: use Path? And new String(Files.readAllBytes(logFile), StandardCharsets.UTF_8)
There was a problem hiding this comment.
Yeah, that way, it will remove the Guava dependency.
There was a problem hiding this comment.
It's a test class, and we use Guava everywhere!
If I had a concern about this code, it would be with the line endings. You could consider using java.nio.file.Files#readAllLines(), which also handles multiple line endings and then compare the last three lines.
|
|
||
| verifier.executeGoal("jib:" + BuildDockerMojo.GOAL_NAME); | ||
| File logFile = new File(verifier.getBasedir(), verifier.getLogFileName()); | ||
| Assert.assertTrue( |
There was a problem hiding this comment.
Assert.assertThat(string, CoreMatchers.containsString())
|
|
||
| Path logFile = Paths.get(verifier.getBasedir(), verifier.getLogFileName()); | ||
| Assert.assertThat( | ||
| new String(Files.readAllBytes(logFile), Charset.forName("UTF-8")), |
| import org.hamcrest.CoreMatchers; | ||
| import org.junit.Assert; | ||
|
|
||
| public class SkippedGoalVerifier { |
|
|
||
| public class SkippedGoalVerifier { | ||
|
|
||
| public static void verifyGoalIsSkipped(TestProject testProject, String goal) |
There was a problem hiding this comment.
visibility can all be package-private?
jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/SkippedGoalVerifier.java
Outdated
Show resolved
Hide resolved
| String targetImage = "neverbuilt:maven" + System.nanoTime(); | ||
|
|
||
| Verifier verifier = new Verifier(testProject.getProjectRoot().toString()); | ||
| verifier.setSystemProperty("_TARGET_IMAGE", targetImage); |
There was a problem hiding this comment.
I think we can just remove this line?
There was a problem hiding this comment.
without target image specified, it will fail? oh I see, yeah lemme look.
jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/SkippedGoalVerifier.java
Show resolved
Hide resolved
| /** A simple verifier utility to test goal skipping accross all our jib goals. */ | ||
| class SkippedGoalVerifier { | ||
|
|
||
| private SkippedGoalVerifier() {} |
There was a problem hiding this comment.
nit: non-static below static
| /** Verify that a jib goal is skipped */ | ||
| static void verifyGoalIsSkipped(TestProject testProject, String goal) | ||
| throws VerificationException, IOException { | ||
|
|
| @@ -0,0 +1,51 @@ | |||
| /* | |||
| * Copyright 2018 Google LLC. All rights reserved. | |||
There was a problem hiding this comment.
I meant to ask about this All rights reserved. It's not normally seen on ASL headers, and @elharo indicated that there was some internal discussion about whether to include it or not.
There was a problem hiding this comment.
Okay, yeah I just looked at the updating docs, and All rights reserved is gone. I'll update that, and then push a larger update across the codebase.
lifecycle
even if skipping, since maven doesn't provide a api for actual
goal skipping
fixes #865