Repository navigation
test: make timer-dependent tests deterministic with testing/synctest - #63
Merged
Merged
Conversation
TestWithTimeout and TestIsCanceled slept 5ms past a 1ms deadline and then checked the context. The deadline is enforced by time.AfterFunc, which cancels from its own goroutine; when the process is descheduled across both deadlines, both timers fire in one scheduler pass and the test goroutine runs before the cancel goroutine. Under parallel load the old tests failed 57 times in 4800 runs. Move every test that depends on real elapsed time to testing/synctest (Go 1.25): the fake clock advances only while all goroutines are blocked and synctest.Wait lets the cancel goroutine finish. - util: context timeout checked exactly at the boundary (alive 1ns before, DeadlineExceeded at the timeout); exact deadline value - core: TestContextTimeout/TestContextDeadline, same race - util: UUIDv7 ordering asserted on consecutive millisecond timestamps instead of the OS wall clock tick; embedded timestamp checked exactly - core: TestWithHealthCheck slept 150ms and asserted nothing; it now checks the first tick pings and records the time, and that a closed database is reported unhealthy with the error kept - core/health.go: drop the nolint on lastError, now used by the test
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
TestWithTimeoutandTestIsCanceledininternal/utilfailed intermittently: they set a 1ms timeout, slept 5ms and expected the context to be canceled.context.WithTimeoutcancels from atime.AfterFuncgoroutine. When the process is descheduled across both deadlines, both timers fire in the same scheduler pass and the test goroutine runs before the cancel goroutine. Under parallel load the old tests failed 57 times in 4800 runs; with a goroutine holding the only P past the deadlines, 151 of 200.Every unit test that depended on real elapsed time now runs in
testing/synctest(Go 1.25, stdlib). The fake clock advances only while all goroutines are blocked, andsynctest.Waitlets the cancel goroutine finish, so the checks are exact and take no wall time:internal/util: the context is alive 1ns before the timeout andDeadlineExceededexactly at it; the deadline value is checked for equality instead of within 100ms.internal/core:TestContextTimeout/TestContextDeadlinehad the same race with a wider window.internal/util: UUIDv7 ordering relied on the wall clock advancing within a 2ms sleep, which depends on the OS clock tick (15.6ms by default on Windows). It now asserts consecutive millisecond timestamps, so a constant timestamp can no longer pass half the time; the embedded timestamp is checked exactly.internal/core:TestWithHealthCheckslept 150ms and asserted nothing. It now checks that the first tick pings and records the time, and that a closed database is reported unhealthy with the error kept.lastError()lost its//nolint:unusedbecause the test uses it.No library behaviour changes.
Verification:
-cpu 1,4).IsCanceledalways false, constant UUID timestamp, health checker not pinging, losing the error, wrong timestamp.-raceclean on Linux (go1.27.1).go test ./...and golangci-lint pass on a clean checkout (go1.25.5), plus linux and darwin/arm64 builds.