Repository navigation
Replace timing-based waits in tests with synchronisation - #5
Merged
Merged
Conversation
Previously, the shutdown tests slept for a fixed period and then asserted that the injected exit function had been called, which fails on a loaded machine whenever the scheduler runs the timer later than the sleep. The injected exit now reports through a channel, so each test waits exactly as long as it needs to and fails on a deadline instead. The second-signal case ran with a 500ms shutdown timeout while asserting on an exit that the second signal was meant to cause, so the timeout could produce that exit on its own; its timeout is now long enough that only the signal can. The concurrent-signals case keeps a short timeout on purpose, as os/signal drops a send to a full channel and a burst may therefore be delivered once, but it now runs on a signal of its own and joins its senders, so nothing it sends can reach a later case. The timeout accuracy case measured from before the signal that starts the timer and asserted a 150ms window around it. It now measures from the shutdown callback, runs on its own signal, keeps the meaningful bound that the forced exit never happens early, and leaves the upper one loose enough to survive a delayed timer. The HTTP tests no longer sleep before the first request, as the listener is created before the server starts and already accepts connections; their requests carry a client timeout, so a stuck server fails the test instead of hanging it.
umputun
approved these changes
Aug 19, 2026
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.
Previously, the shutdown tests slept for a fixed period and then asserted that the injected exit function had been called. That fails whenever the scheduler runs the timer later than the sleep, which is what happens on a loaded CI runner. The injected exit now reports through a channel, so each test waits only as long as it actually needs and fails on a deadline instead of on a guess.
Three cases needed more than a mechanical replacement:
os/signaldrops a send to a full channel, so a burst of signals may well be delivered once and the exit has to come from the timeout; the deterministic claim, that a second signal forces the exit, is what the case above covers. It now runs on a signal no other case sends and joins its senders, so nothing it sends can reach a later case.The HTTP tests no longer sleep before the first request either: the listener is created before the server starts and already accepts connections, so the request blocks until
Servepicks it up. Those requests now carry a client timeout, so a stuck server fails the test instead of hanging it. The sleeps in the example tests stay, as they pace the printed output and removing the one inExample_httpServerWithContextwould race the server start against the cancellation.