Repository navigation
Make AsyncWrapperTest deterministic (#40448) - #40449
Conversation
testBufferStopsAcceptingItems asserted the buffer count after a fixed 100ms sleep, which failed on slow Windows runners (expected 5 but was 0). Add an optional CountDownLatch gate to BasicDofn so tests hold elements in-flight until they release it, instead of relying on wall-clock sleeps. testBufferStopsAcceptingItems now joins all producers before asserting that exactly 5 items are buffered, then verifies the 5 throttled items are rescheduled and emitted. testBufferCount, testBufferWithCancellation and testLongItem use the same gate; testSlowDuplicates waits for the buffer to drain instead of sleeping. Fixes apache#40448. Supersedes the timing-based approach in apache#40397.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40449 +/- ##
============================================
- Coverage 59.04% 58.74% -0.31%
+ Complexity 15624 13907 -1717
============================================
Files 2797 2584 -213
Lines 280750 271444 -9306
Branches 12488 11229 -1259
============================================
- Hits 165764 159453 -6311
+ Misses 108540 105995 -2545
+ Partials 6446 5996 -450
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @chamikaramj for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
R: @Abacn since @chamikaramj is out for a few days |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
Codecov/patch is stuck, but ready to review. Thanks |
3 participants
testBufferStopsAcceptingItemsasserted the buffer count after a fixed 100ms sleep and failed on loaded Windows runners withexpected:<5> but was:<0>. The count is decremented inwhenCompleteas soon as a future finishes, so the assertion raced both the producer threads starting up and the buffered items completing. #38970 shortened the sleeps, which made the window smaller.This change makes the test deterministic instead of widening the sleep:
BasicDofntakes an optionalCountDownLatchgate, so a test can keep elements in flight until it releases them.testBufferStopsAcceptingItemsjoins all 10 producers before asserting. With the gate closed nothing can complete, so exactly 5 items are buffered, 10 are in state and 0 are processed. After releasing the gate it checks that the first commit emits 5 and reschedules the 5 throttled items, and the second commit emits all 10. The scheduling timeout is shortened so throttled producers give up fast; the assertions do not depend on it.testBufferCount,testBufferWithCancellationandtestLongItemhad the same sleep based race (10ms and 500ms DoFns) and now use the gate.testSlowDuplicateswaits for the buffer to drain instead of sleeping.Test only, no production change. Ran the class repeatedly on Linux, including with the build pinned to 2 cores under extra CPU load. The Windows failure itself does not reproduce locally, so the fix relies on the ordering above rather than on a repro.
Fixes #40448. This supersedes the timing based approach in #40397.
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.