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
CASSANDRA-19504 Improve state management for Java versions in Jenkinsfile #1923
Conversation
sh label: 'Execute tests', script: '''#!/bin/bash -le | ||
def testJavaHome = sh(label: 'Get TEST_JAVA_HOME',script: '''#!/bin/bash -le | ||
. ${JABBA_SHELL} | ||
jabba which ${JABBA_VERSION}''', returnStdout: true).trim() |
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.
Must be a shell invocation since we actually need output from jabba
def testJavaHome = sh(label: 'Get TEST_JAVA_HOME',script: '''#!/bin/bash -le | ||
. ${JABBA_SHELL} | ||
jabba which ${JABBA_VERSION}''', returnStdout: true).trim() | ||
def testJavaVersion = (JABBA_VERSION =~ /.*\.(\d+)/)[0][1] |
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.
There wasn't any reason to do this via a shell op; it's just a regex eval
-Ptest-jdk-${TEST_JAVA_VERSION} \ | ||
-DtestJavaHome=${TEST_JAVA_HOME} \ | ||
-Ptest-jdk-'''+testJavaVersion+''' \ | ||
-DtestJavaHome='''+testJavaHome+''' \ |
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.
We can't use string interpolation here since the ${} vals are intended to be passed through literally to the shell (so that the shell can do variable subsitution based on env vars). So instead we just add whatever we got for the two vars above directly into the multi-line string here and then continue on with a new multi-line string.
@@ -149,6 +148,8 @@ def executeTests() { | |||
${ISOLATED_ITS_ARGUMENT} \ | |||
${PARALLELIZABLE_ITS_ARGUMENT} | |||
''' | |||
echo "Invoking Maven with parameters test-jdk-${testJavaVersion} and testJavaHome = ${testJavaHome}" |
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.
Seemed useful to log what we're actually passing to Maven.
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.
I solved this by using bash -lex
instead. It will print out the whole maven command that is passed to the shell. It may provide more info, but either works.
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.
Jenkins does something similar in the BlueOcean interface, so we actually had it already. I was just trying to make it a bit more verbose here. :)
@@ -728,6 +728,7 @@ limitations under the License.]]></inlineHeader> | |||
<plugin> | |||
<artifactId>maven-surefire-plugin</artifactId> | |||
<configuration> | |||
<jvm>${testing.jvm}/bin/java</jvm> |
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.
Earlier work on the PR for JAVA-3042 had added the "jvm" param to all subprojects that had their own pom.xml... but that didn't cover anything that inherited settings from the parent POM.
Internal fix for Jenkins functionality, calling this good and keeping it moving :) |
…sfile patch by Bret McGuire; reviewed by Bret McGuire for CASSANDRA-19504
5670e73
to
f836f15
Compare
A simple fix which doesn't alter the main flow of the existing Jenkinsfile