try to fix forward tests for java 17#39058
Conversation
derrickaw
changed the title
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses failures in PostCommit TransformService and XVR Spark3 tests following recent changes. The modifications primarily focus on upgrading the Transform Service to Java 17 and enhancing the build infrastructure to better handle Java version-specific configurations during test execution. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Java version configurations in the Gradle build plugin and upgrades the base image of the transform service controller container from Java 11 to Java 17. Specifically, it replaces hasProperty and getProperty calls with findProperty to safely resolve Java home paths, and dynamically resolves the Java container suffix when testJavaVersion is specified. Feedback suggests making transformServiceJavaContainerSuffix dynamic rather than hardcoded to 'java17' to prevent test failures on other Java versions, and adding a safety check to ensure java${testJavaVersion}Home is defined before configuring test tasks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| // Transform service delivers transforms that refer to SDK harness containers with following sufixes. | ||
| def transformServiceJavaContainerSuffix = 'java11' | ||
| def transformServiceJavaContainerSuffix = 'java17' |
There was a problem hiding this comment.
Hardcoding transformServiceJavaContainerSuffix to 'java17' will cause test failures when running tests on other Java versions (such as Java 11 or Java 21). This is because the setup task only builds the container image specified by javaContainerSuffix (which is dynamic based on testJavaVersion). If the transform service tries to use 'java17' but only 'java11' was built, the test will fail with an image not found error.\n\nWe should make transformServiceJavaContainerSuffix dynamic by setting it to javaContainerSuffix so that it always matches the container version being built and tested.
def transformServiceJavaContainerSuffix = javaContainerSuffix
| if (['11', '17', '21', '25'].contains(testJavaVersion)) { | ||
| def testJavaHome = project.getProperty("java${testJavaVersion}Home") | ||
| def testJavaHome = project.findProperty("java${testJavaVersion}Home") |
There was a problem hiding this comment.
If testJavaVersion is specified but the corresponding java${testJavaVersion}Home property is not defined, project.findProperty will return null. This can lead to a NullPointerException or IllegalArgumentException when 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 the if condition.
if (['11', '17', '21', '25'].contains(testJavaVersion) && project.findProperty("java${testJavaVersion}Home")) {\n def testJavaHome = project.findProperty("java${testJavaVersion}Home")
* try to fix forward with 17 only * address just two workflows * Revert XVR Spark3 workflow java-version override * Add Java 17 and 11 setup and -Pjava17Home across all XVR workflows * Revert redundant java-version override in TransformService Direct workflow
* try to fix forward with 17 only * address just two workflows * Revert XVR Spark3 workflow java-version override * Add Java 17 and 11 setup and -Pjava17Home across all XVR workflows * Revert redundant java-version override in TransformService Direct workflow
2 participants
https://github.com/apache/beam/actions/runs/27968245213
https://github.com/apache/beam/actions/runs/27968262679/job/82767635828 - one part times out and may require increasing timeout for test workflow
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.