Use testing/synctest in dedup queue tests#357
Merged
Conversation
Run TestDedupQueueParallel and TestWriteDedupQueueParallelReadWrite inside synctest bubbles. The fake clock makes the previously timing-dependent behavior deterministic: - TestDedupQueueParallel: the sleep in GetChunkFunc now guarantees all goroutines have registered as waiters before the first request completes, so the assertion can require exactly one upstream request instead of "ideally just one". - TestWriteDedupQueueParallelReadWrite: the test now asserts that no fake time elapses across StoreChunk(), proving the write does not queue behind the in-flight slow read. The 1s sleep no longer costs wall time and the background read goroutine no longer leaks past the test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Runs
TestDedupQueueParallelandTestWriteDedupQueueParallelReadWriteinsidetesting/synctestbubbles (stable since Go 1.25, which this module already requires). Under the bubble's fake clock,time.Sleeponly returns once every other goroutine in the bubble is durably blocked, which turns both tests' timing heuristics into guarantees.TestDedupQueueParallel: The 1ms sleep in
GetChunkFuncpreviously just made the store "artificially slow" and hoped all 10 goroutines piled up as waiters in time — a goroutine scheduled late would make a second store request, which is why the assertion hedged withrequests <= 1("ideally just one"). The sleep now provably cannot complete until all other goroutines are blocked waiting on the in-flight request, so the assertion becomes a deterministicrequests == 1. Also modernizes the counter toatomic.Int64and the loop towg.Go().TestWriteDedupQueueParallelReadWrite: Previously, if reads and writes wrongly shared a queue,
StoreChunk()would have just taken a second longer and still passed — the test couldn't actually detect the regression it was written for. It now asserts that zero fake time elapses acrossStoreChunk(), which fails if the write waits on the in-flight slow read. The 1s sleep no longer costs wall time, and the background read goroutine no longer leaks past the end of the test.Both tests pass with
-race -count=5.