-
Notifications
You must be signed in to change notification settings - Fork 4.6k
try to fix forward tests for java 17 #39058
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
Changes from all commits
b39db41
a3fea20
83f3984
c5f170d
4ae65b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -978,6 +978,9 @@ class BeamModulePlugin implements Plugin<Project> { | |
|
|
||
| // set compiler options for java version overrides to compile with a different java version | ||
| project.ext.setJavaVerOptions = { CompileOptions options, String ver -> | ||
| if (!project.findProperty("java${ver}Home")) { | ||
| return | ||
| } | ||
| if (ver == '8') { | ||
| def java8Home = project.findProperty("java8Home") | ||
| options.fork = true | ||
|
|
@@ -1161,16 +1164,16 @@ class BeamModulePlugin implements Plugin<Project> { | |
| // If compiled on older SDK, compile with JDK configured with compatible javaXXHome | ||
| // The order is intended here | ||
| if (requireJavaVersion.compareTo(JavaVersion.VERSION_11) <= 0 && | ||
| project.hasProperty('java11Home')) { | ||
| project.findProperty('java11Home')) { | ||
| forkJavaVersion = '11' | ||
| } else if (requireJavaVersion.compareTo(JavaVersion.VERSION_17) <= 0 && | ||
| project.hasProperty('java17Home')) { | ||
| project.findProperty('java17Home')) { | ||
| forkJavaVersion = '17' | ||
| } else if (requireJavaVersion.compareTo(JavaVersion.VERSION_21) <= 0 && | ||
| project.hasProperty('java21Home')) { | ||
| project.findProperty('java21Home')) { | ||
| forkJavaVersion = '21' | ||
| } else if (requireJavaVersion.compareTo(JavaVersion.VERSION_25) <= 0 && | ||
| project.hasProperty('java25Home')) { | ||
| project.findProperty('java25Home')) { | ||
| forkJavaVersion = '25' | ||
| } else { | ||
| logger.config("Module ${project.name} disabled. To enable, either " + | ||
|
|
@@ -1628,7 +1631,7 @@ class BeamModulePlugin implements Plugin<Project> { | |
| } | ||
| // if specified test java version, modify the compile and runtime versions accordingly | ||
| if (['11', '17', '21', '25'].contains(testJavaVersion)) { | ||
| def testJavaHome = project.getProperty("java${testJavaVersion}Home") | ||
| def testJavaHome = project.findProperty("java${testJavaVersion}Home") | ||
|
|
||
| // redirect java compiler to specified version for compileTestJava only | ||
| project.tasks.compileTestJava { | ||
|
|
@@ -2710,7 +2713,8 @@ class BeamModulePlugin implements Plugin<Project> { | |
|
|
||
| def pythonDir = project.project(":sdks:python").projectDir | ||
| def usesDataflowRunner = config.pythonPipelineOptions.contains("--runner=TestDataflowRunner") || config.pythonPipelineOptions.contains("--runner=DataflowRunner") | ||
| def javaContainerSuffix = getSupportedJavaVersion() | ||
| String ver = project.findProperty('testJavaVersion') | ||
| def javaContainerSuffix = ver ? getSupportedJavaVersion(ver) : getSupportedJavaVersion() | ||
|
|
||
| // Sets up, collects, and runs Python pipeline tests | ||
| project.tasks.register(config.name+"PythonUsingJava") { | ||
|
|
@@ -2790,7 +2794,8 @@ class BeamModulePlugin implements Plugin<Project> { | |
| ] | ||
| def serviceArgs = project.project(':sdks:python').mapToArgString(expansionServiceOpts) | ||
| def pythonContainerSuffix = project.project(':sdks:python').pythonVersion.replace('.', '') | ||
| def javaContainerSuffix = getSupportedJavaVersion() | ||
| String ver = project.findProperty('testJavaVersion') | ||
| def javaContainerSuffix = ver ? getSupportedJavaVersion(ver) : getSupportedJavaVersion() | ||
| def setupTask = project.tasks.register(config.name+"Setup", Exec) { | ||
| dependsOn ':sdks:java:container:'+javaContainerSuffix+':docker' | ||
| dependsOn ':sdks:python:container:py'+pythonContainerSuffix+':docker' | ||
|
|
@@ -2979,10 +2984,11 @@ class BeamModulePlugin implements Plugin<Project> { | |
| ] | ||
| def serviceArgs = project.project(':sdks:python').mapToArgString(transformServiceOpts) | ||
| def pythonContainerSuffix = project.project(':sdks:python').pythonVersion.replace('.', '') | ||
| def javaContainerSuffix = getSupportedJavaVersion() | ||
| String ver = project.findProperty('testJavaVersion') | ||
| def javaContainerSuffix = ver ? getSupportedJavaVersion(ver) : getSupportedJavaVersion() | ||
|
|
||
| // Transform service delivers transforms that refer to SDK harness containers with following sufixes. | ||
| def transformServiceJavaContainerSuffix = 'java11' | ||
| def transformServiceJavaContainerSuffix = 'java17' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoding |
||
| def transformServicePythonContainerSuffix = pythonContainerSuffix | ||
|
|
||
| def setupTask = project.tasks.register(config.name+"Setup", Exec) { | ||
|
|
||
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
testJavaVersionis specified but the correspondingjava${testJavaVersion}Homeproperty is not defined,project.findPropertywill returnnull. This can lead to aNullPointerExceptionorIllegalArgumentExceptionwhen configuring the compilation and test tasks later in this block.\n\nWe can make this safer by checking if the property is defined directly in theifcondition.