[damccorm]

Note: this PR was auto-generated by tooling. Before moving it out of draft, the author has done a full review of the PR, specifically evaluating it on the following criteria:

• The PR is an appropriate fix for the issue (e.g. we're not changing a test to avoid a bug)
• The PR is a simple fix to the issue (e.g. we're not adding infra when we could just modify a test)
• The PR is not covering for other problems (e.g. if we're bumping timeouts, it is not because we're covering for a performance problem)

Full details of the methodology of generation and review can be found here - https://docs.google.com/document/d/14lj7ECVov4T4EdwmIzNlWrRGE3lLDMOwX5xUMGzaLoc/edit?usp=sharing

If you have concerns with the PR or with the underlying methodology used to generate this, please comment below - they will be addressed by the author not the tooling :)

Summary

• Annotates GroupByKeyTest.BasicTests.testAfterProcessingTimeContinuationTriggerUsingState with @BeamParallelJunit4Runner.SerialTest and increases waitMillis from 500 to 2000.

Root Cause

In PostCommit Java ValidatesRunner Direct JavaVersions (run 43366517998), GroupByKeyTest$BasicTests > testAfterProcessingTimeContinuationTriggerUsingState failed with:

java.lang.AssertionError: Second Triggered sum/Values/Values/Map/ParMultiDo(Anonymous).output:
Expected: iterable with items [<42>] in any order
     but: no item matches: <42> in []

testAfterProcessingTimeContinuationTriggerUsingState cannot use TestStream processing-time advancement (because it runs across ValidatesRunner suites), so it holds the event-time watermark using a real wall-clock Thread.sleep(waitMillis) (waitMillis = 500) inside an @OnTimer callback (onDelay) while two sequential downstream GroupByKey transforms (Triggered sum and Second Triggered sum) fire AfterProcessingTime.pastFirstElementInPane().plusDelayOf(Duration.millis(1)) early panes (PAssert.that(triggeredSums).inEarlyGlobalWindowPanes().containsInAnyOrder(42)).

After GroupByKeyTest.BasicTests was migrated to @RunWith(BeamParallelJunit4Runner.class) (#40292) and :runners:direct-java:validatesRunner enabled maxParallelForks = 4 with beam.test.parallelThreads = 8 (#40324), up to 32 DirectRunner pipelines execute concurrently on a 4-vCPU runner. Under that CPU contention, propagating elements and firing processing-time timers across two consecutive GroupByKey stages can take longer than the 500ms wall-clock sleep in onDelay. When onDelay wakes up first, the input watermark advances to TIMESTAMP_MAX_VALUE, causing Second Triggered sum to emit its result in an ON_TIME pane rather than an EARLY pane.

Annotating the test with @BeamParallelJunit4Runner.SerialTest and increasing waitMillis to 2000 prevents intra-class thread contention during the real-time sleep and provides sufficient headroom for both processing-time trigger stages to fire before the watermark is released.

Verification

• Verified locally with ./gradlew :sdks:java:core:spotlessCheck :runners:direct-java:validatesRunner --tests "org.apache.beam.sdk.transforms.GroupByKeyTest\$BasicTests.testAfterProcessingTimeContinuationTriggerUsingState" :sdks:java:core:checkstyleTest.

Verification

• Triggered beam_PostCommit_Java_ValidatesRunner_Direct_JavaVersions.yml workflow run on branch: 43379443238

[damccorm]

/gemini review

[github-actions]

📋 Review Summary

This PR resolves a flakiness issue in GroupByKeyTest.BasicTests.testAfterProcessingTimeContinuationTriggerUsingState caused by CPU contention under parallel test execution. Marking the test with @BeamParallelJunit4Runner.SerialTest and increasing waitMillis from 500ms to 2000ms is a clean, targeted fix that ensures processing-time triggers across both consecutive GroupByKey stages have sufficient wall-clock headroom before the event-time watermark advances.

🔍 General Feedback

• The root cause analysis and fix are sound; no functional issues were found.
• Consider updating the Javadoc comment right above the test (lines 239–241), which still states "Currently the sleep is sub-second", to stay consistent with the new 2-second sleep duration.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[damccorm]

Updated the Javadoc comment above testAfterProcessingTimeContinuationTriggerUsingState to reflect the 2-second sleep.

/gemini review

[github-actions]

📋 Review Summary

This PR resolves a timing flake in GroupByKeyTest.BasicTests.testAfterProcessingTimeContinuationTriggerUsingState under parallel ValidatesRunner execution by annotating the test with @BeamParallelJunit4Runner.SerialTest and increasing the real-time watermark hold (waitMillis) from 500ms to 2000ms. The change is well-scoped, updates the corresponding Javadoc comment, and directly addresses CPU contention during wall-clock processing-time trigger propagation.

🔍 General Feedback

• Using @BeamParallelJunit4Runner.SerialTest on this wall-clock-sensitive test inside BasicTests (@RunWith(BeamParallelJunit4Runner.class)) properly drains in-flight parallel tests in the class before running and avoids intra-class thread contention.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.74%. Comparing base (d7d31e9) to head (1296592).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #41281      +/-   ##
============================================
- Coverage     59.26%   57.74%   -1.52%     
+ Complexity     4925     2633    -2292     
============================================
  Files          1462     1203     -259     
  Lines        186313   171888   -14425     
  Branches       3669     2182    -1487     
============================================
- Hits         110417    99257   -11160     
+ Misses        72540    69966    -2574     
+ Partials       3356     2665     -691     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[damccorm]

Do we still need to bump the timeout if we're running this serially?

[damccorm]

Good call — running it with @BeamParallelJunit4Runner.SerialTest alone is sufficient so it doesn't contend with testLargeKeys10KB/100KB/1MB/10MB in GroupByKeyTest.BasicTests inside the same JVM, and keeping waitMillis = 500 avoids adding 1.5s of wall-clock sleep across every ValidatesRunner suite.

I have applied and verified that change locally (waitMillis = 500 with only @BeamParallelJunit4Runner.SerialTest, 12/12 passing in :runners:direct-java:validatesRunner --tests "org.apache.beam.sdk.transforms.GroupByKeyTest\$BasicTests") — let me know if you'd like me to push that commit!