Skip to content
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

Merged
merged 1 commit into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 10 additions & 9 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,6 @@ def initializeEnvironment() {
. ${JABBA_SHELL}
jabba which 1.8''', returnStdout: true).trim()

env.TEST_JAVA_HOME = sh(label: 'Get TEST_JAVA_HOME',script: '''#!/bin/bash -le
. ${JABBA_SHELL}
jabba which ${JABBA_VERSION}''', returnStdout: true).trim()
env.TEST_JAVA_VERSION = sh(label: 'Get TEST_JAVA_VERSION',script: '''#!/bin/bash -le
echo "${JABBA_VERSION##*.}"''', returnStdout: true).trim()

sh label: 'Download Apache CassandraⓇ or DataStax Enterprise',script: '''#!/bin/bash -le
. ${JABBA_SHELL}
jabba use 1.8
Expand Down Expand Up @@ -115,7 +109,12 @@ def buildDriver(jabbaVersion) {
}

def executeTests() {
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()
Copy link
Contributor Author

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 testJavaVersion = (JABBA_VERSION =~ /.*\.(\d+)/)[0][1]
Copy link
Contributor Author

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


def executeTestScript = '''#!/bin/bash -le
# Load CCM environment variables
set -o allexport
. ${HOME}/environment.txt
Expand All @@ -137,8 +136,8 @@ def executeTests() {
printenv | sort

mvn -B -V ${INTEGRATION_TESTS_FILTER_ARGUMENT} -T 1 verify \
-Ptest-jdk-${TEST_JAVA_VERSION} \
-DtestJavaHome=${TEST_JAVA_HOME} \
-Ptest-jdk-'''+testJavaVersion+''' \
-DtestJavaHome='''+testJavaHome+''' \
Copy link
Contributor Author

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.

-DfailIfNoTests=false \
-Dmaven.test.failure.ignore=true \
-Dmaven.javadoc.skip=${SKIP_JAVADOCS} \
Expand All @@ -149,6 +148,8 @@ def executeTests() {
${ISOLATED_ITS_ARGUMENT} \
${PARALLELIZABLE_ITS_ARGUMENT}
'''
echo "Invoking Maven with parameters test-jdk-${testJavaVersion} and testJavaHome = ${testJavaHome}"
Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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. :)

sh label: 'Execute tests', script: executeTestScript
}

def executeCodeCoverage() {
Expand Down
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,7 @@ limitations under the License.]]></inlineHeader>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<jvm>${testing.jvm}/bin/java</jvm>
Copy link
Contributor Author

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.

<systemPropertyVariables>
<logback.configurationFile>${project.basedir}/src/test/resources/logback-test.xml</logback.configurationFile>
</systemPropertyVariables>
Expand Down