Fix beamTestPipelineOptions quote loss on Windows for ValidatesRunner tasks#39365
Conversation
… tasks On Windows, Gradle passes -DbeamTestPipelineOptions=<compact JSON> to the forked test JVM unquoted, so the Windows C runtime strips the JSON quotes and TestPipeline fails to parse the options; every test in the task fails at TestPipeline.create(). Consolidate the existing Windows quoting treatment (BEAM-10682, BEAM-11365) into a shared helper and use it in createPortableValidatesRunnerTask and createCrossLanguageValidatesRunnerTask. On non-Windows platforms the emitted JSON is unchanged. Fixes apache#39332
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 a critical issue on Windows where Gradle-forked test JVMs fail to parse 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. 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 introduces a helper method toBeamTestPipelineOptionsJson in BeamModulePlugin.groovy to centralize the serialization of pipeline options to JSON, handling Windows-specific quote-wrapping consistently. Feedback on the changes suggests adding a null check for the options parameter in this new helper method to prevent a potential NullPointerException when calling collect.
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.
| static def toBeamTestPipelineOptionsJson(def options) { | ||
| if (System.properties['os.name'].toLowerCase().contains('windows')) { | ||
| return JsonOutput.toJson(options.collect { "\"$it\"" }) | ||
| } | ||
| return JsonOutput.toJson(options) | ||
| } |
There was a problem hiding this comment.
If options is null, calling options.collect will throw a NullPointerException. Adding a null check ensures the method is robust against null inputs.
static def toBeamTestPipelineOptionsJson(def options) {
if (options && System.properties['os.name'].toLowerCase().contains('windows')) {
return JsonOutput.toJson(options.collect { "\"$it\"" })
}
return JsonOutput.toJson(options)
}
2 participants
Fixes #39332.
On Windows, Gradle forks the test JVM with
-DbeamTestPipelineOptions=<compact JSON>on the command line.ProcessBuilderonly quotes arguments that contain whitespace, so the argument is passed unquoted, the Windows C runtime strips the JSON's"characters, andTestPipeline.testingPipelineOptions()fails withJsonParseExceptionbefore any pipeline runs. Every test in the task fails atTestPipeline.create().The repo already handles this in two places by wrapping each option in literal quotes so one quoting layer survives:
runners/direct-java/build.gradle(pipelineOptionsStringCrossPlatformHandling, BEAM-11365) and the inline handling inenableJavaPerformanceTesting(BEAM-10682). This PR consolidates that treatment into a shared static helpertoBeamTestPipelineOptionsJsoninBeamModulePluginand applies it to the two ValidatesRunner paths that were missing it:createPortableValidatesRunnerTaskcreateCrossLanguageValidatesRunnerTaskThe existing inline handling in
enableJavaPerformanceTestingis refactored onto the same helper. On non-Windows platforms the helper emits exactly the same JSON as before (JsonOutput.toJson(options)), so CI behavior is unchanged; the wrapping branch only executes on Windows.Verified on Windows 11, JDK 11:
ProcessBuilderreproduction from [Bug]: Java ValidatesRunner tests cannot run on Windows, beamTestPipelineOptions loses its JSON quotes #39332, using the exact options of the Spark portable VR batch task: the current format loses every quote in the child JVM; the wrapped format arrives byte-identical to the intended JSON.gradlew :runners:spark:3:job-server:validatesPortableRunnerBatch --tests "*SplittableDoFnTest*", which previously failed 9/9 withJsonParseExceptionatTestPipeline.create(): after this change theJsonParseExceptioncount is 0/9. The forked worker's command line shows the property arriving intact (jps -v:-DbeamTestPipelineOptions=["--runner=org.apache.beam.runners.portability.testing.TestPortableRunner",...]), and all 9 tests parse their options, create theTestPipeline, and submit pipelines to the job server. They then hit [Bug]: Portable Java pipelines hang forever in artifact staging on Windows, staged file name embeds environment id colons #39364, a separate pre-existing Windows bug in artifact staging that this fix unmasked; this change is a prerequisite for any local Java VR signal on Windows.