[jonathansudhakar1]

Fixes #40992.

TestPersistBundle_ResidualResumeDelay/parks_with_hold_and_schedules is flaky. It failed on master in this "Go tests" run with residual release scheduled 5.000001877s after persisting, want within (0, 5s].

Cause. The test takes before := time.Now(), at nanosecond precision, and checks that the scheduled release time fireAt is within (0, delay] of it. PersistBundle computes fireAt as emNow.Add(delay). With EnableRTC, emNow is mtime.Now(), which is read a little later and truncated to milliseconds. When a millisecond boundary falls between the two reads, fireAt lands a few microseconds past before + delay. The runner behaves correctly; only the test's bound is wrong.

Change (test only). The test now also takes after := time.Now() once PersistBundle returns. It checks fireAt against [mtime.FromTime(before).Add(delay), mtime.FromTime(after).Add(delay)]. That is the exact range PersistBundle can produce, so the check is still strict: a release time that is immediate, doubled, or off by a millisecond still fails.

Testing

Run from sdks/ on macOS/arm64 with Go 1.26:

• Before: go test ./go/pkg/beam/runners/prism/internal/engine/ -run 'TestPersistBundle_ResidualResumeDelay/parks_with_hold_and_schedules' -count=20000 failed 13 times, with the same message as CI (for example, 5.000001s after persisting). A separate -count=3000 run failed once.
• After: the same command with -count=20000, run twice: 0 failures, ok.
• Mutants of partitionResiduals, each run once against the new check, all failed as expected:
  • fireAt := emNow.Add(r.Delay + time.Millisecond) failed.
  • fireAt := emNow.Add(r.Delay - time.Millisecond) failed.
  • fireAt := emNow.Add(2 * r.Delay) failed.
• go test ./go/pkg/beam/runners/prism/internal/engine/: ok (53.5s). go vet and gofmt -l on the package are clean.

🤖 Generated with Claude Code

[github-actions]

📋 Review Summary

This PR resolves a flaky test failure in TestPersistBundle_ResidualResumeDelay/parks_with_hold_and_schedules (#40992) caused by comparing a millisecond-truncated mtime.Time sampled inside PersistBundle against a single nanosecond-precision time.Now() taken before the call. Bounding fireAt between [mtime.FromTime(before).Add(delay), mtime.FromTime(after).Add(delay)] accurately reflects the runner's millisecond-granularity processing time calculation while keeping the assertion strict.

🔍 General Feedback

• The root cause analysis and bounds check adjustment are sound, clean, and well-tested.

---

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

[github-actions]

Assigning reviewers:

R: @jrmccluskey for label go.
Backup reviewer: lostluck (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.60%. Comparing base (12bbb17) to head (254e695).
⚠️ Report is 65 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #40993       +/-   ##
===========================================
- Coverage   79.83%   56.60%   -23.23%     
===========================================
  Files         494      877      +383     
  Lines       83575   154949    +71374     
===========================================
+ Hits        66722    87710    +20988     
- Misses      16853    65381    +48528     
- Partials        0     1858     +1858     

Flag	Coverage Δ
go	29.08% <ø> (?)
python	80.08% <ø> (+0.25%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

[jrmccluskey]

Broadly LGTM, although I do think the (0, delay] form for the error message in the failure case is a little friendlier for human readability.

[jrmccluskey]

Nit: we don't really need comments explaining the behavior here