-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Dynamically create test jars for JarCommandTest #4210
Conversation
jib-cli/src/integration-test/java/com/google/cloud/tools/jib/cli/JarCommandTest.java
Outdated
Show resolved
Hide resolved
jib-cli/src/integration-test/java/com/google/cloud/tools/jib/cli/JarCommandTest.java
Outdated
Show resolved
Hide resolved
JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, null, compilationUnits); | ||
boolean success = task.call(); | ||
assertThat(success).isTrue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we use an if-else statement that proceeds with the jar building stage below if task.call()
is true otherwise logs an error message if the compilation fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we just silently log the message, then we might not notice the failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, good point!
[Optional] Alternatively, to have a clear separation between the "arrange", "act" and "assert" blocks in our tests - consider using Preconditions.checkState(task.call(), "<error message>");
.
jib-cli/src/integration-test/java/com/google/cloud/tools/jib/cli/JarCommandTest.java
Outdated
Show resolved
Hide resolved
Thanks for addressing this @meltsufin! Running |
Follow-up to #4205.