Repository navigation
Conversation
mafredri
force-pushed
the
mafredri/context-advance-error
branch
from
November 3, 2026 15:16
f2fc24f to
e553042
Compare
After code moves from context.WithTimeout to Clock.WithTimeout, a pending mock context stops Advance and Set at its deadline, but the failure said "beyond next timer/ticker event", which points away from the context. AdvanceNext's failure also listed only timers and tickers. The failures now list each event due at the next event time by kind and by the call that created it, for example "mock context from WithTimeout(..., [a])". The call is printed the way Trap.String prints it, so the function and tags match the code under test and Trap(). Each event keeps the function and tags of its creating apiCall, which costs one field per event type and leaves scheduling unchanged. Rewording the message to mention contexts would give the kind but not which call, which matters when a test has several. The description leaves out the duration because Timer.Reset and Ticker.Reset change it after creation, and the message already says when the event is due.
The README and the Advance, Set, AdvanceNext, Peek, and NewMock godoc described the limits on moving the clock in terms of timers and tickers only. A pending context from Clock.WithTimeout or WithDeadline imposes the same limits, and a reader moving code off context.WithTimeout had no way to learn that from the docs. The Mock godoc now says which things are events, and the methods refer to events. The README Contexts section links to the Advance limit.
eventSource kept the apiCall's tags, which alias the caller's slice when it calls with tags.... A caller that reused that slice after the call returned changed the tags that a later Advance or Set failure printed, and reusing it while another goroutine called Advance was a data race, because describe reads the tags long after the call. The Advance subtests now reuse the slice after each creation call and expect the original tags.
The AdvanceWaiter, Wait, MustWait, and Done godoc said they wait for ticks and timers. They also wait for an expiring mock context: fire cancels it and runs the functions registered through its AfterFunc method before returning, so the context and the contexts derived from it directly are done when the waiter completes.
"Derived from it directly" did not say which contexts count, and it undersold the guarantee: stdlib contexts derived through further WithCancel, WithTimeout, or WithDeadline layers are done too. It also hid the case that is not covered. A stdlib context derived through context.WithValue is canceled by the context package on its own goroutine, so it may still be live when Wait returns; in 200 runs it was not done right after MustWait in 178.
Nothing in the API "sets" a mock context, so "begin setting" did not describe what the reader does.
The AdvanceNext subtest of TestMock_FailureNamesBlockingEvents mostly checked that AdvanceNext fires a pending mock context, which is what TestWithTimeout_FiresOnAdvance checks for Advance. That test now runs once per way of moving the clock, so AdvanceNext also gets the check that a derived context is done after MustWait, and its row checks the duration AdvanceNext reports. The failure-message test is now one table of how the clock is moved and what blocks it. Set, setting the clock backward, and AdvanceNext with nothing scheduled are rows next to the Advance rows, so they share the setup and the check that reused tags do not change the message.
Since #30, contexts derived through context.WithValue are canceled on the canceling goroutine, so they are done when Wait returns like every other derived context. In 500 runs under -race, none of the derived contexts tried (WithValue, WithCancel or WithTimeout over WithValue, a struct embedding the context) was live after MustWait; before #30's changes, the WithValue ones were live in about a quarter of runs.
"The contexts derived from it" claimed too much. In 300 runs under -race, a context from context.WithoutCancel was live after MustWait in all of them, as designed, and context.WithCancel over a Context type with its own Done channel was live in 88, because the context package cancels it from a goroutine. Contexts from the other context.With functions, also over WithValue, and from the Mock's With methods were done in every run.
Set moves the clock backward once nothing is scheduled again, as its godoc says. A probe set it back an hour without an error after stopping a timer and a ticker, canceling one mock context, letting another expire, and canceling the stdlib parent of a third.
Clock.WithDeadline on #30 now lists which derived contexts are done when MustWait returns, so AdvanceWaiter refers to that list instead of keeping its own copy, which had also said "this Mock's" where contexts from another Mock's With methods are done too.
The message was produced by running the TestAdvanceTooFar example, with its context.WithTimeout call given a parent so it compiles.
mafredri
force-pushed
the
mafredri/context-advance-error
branch
from
November 4, 2026 19:17
e553042 to
1ce3be2
Compare
mafredri
added a commit
that referenced
this pull request
Nov 4, 2026
The section now follows the Deadline() subsection, so it no longer inserts text where #31 and #32 do. The timer and ticker paragraph says what to do instead of how their cleanups work, and the BaseContext advice says to set it on an unstarted server before Start, as the probe that showed srv.Close returning did. On main's timer and ticker code, a reader of C got no tick or value in a cleanup registered before the ticker or the fire, and a reader that also selected on t.Context() exited.
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.
With #30, a pending mock context is a scheduled event, so it blocks
Advancepast its deadline the same way a timer does. When code moves fromcontext.WithTimeouttoclock.WithTimeout, tests that used to advance freely start failing with a message that names only timers and tickers:Advance,Set, and setting the time into the past now list the events due next, naming each one's kind and the call that created it, in the same format asTrapstrings:AdvanceNextwith nothing scheduled now names mock contexts alongside timers and tickers.Each event records the function and tags of the call that created it. The tags are copied, so a caller that reuses its tags slice cannot change them or race with the failure message. Once #42 lands,
newCallcopies the tags, and the copy innewEventSourcecan go. Rewording the message alone would show the kind but not which call, and a test can have several pending. The message does not show durations becauseResetcan change them after creation.The godoc and README that described Advance's limit in terms of timers and tickers now say "events" and include mock contexts, and the README shows the new message. The README no longer says the clock can never go back once a timer, ticker, or mock context exists; as
Set's godoc says, it can once none is scheduled. TheAdvanceWaitergodoc now says it waits for a mock context and the derived contexts thatClock.WithDeadlinelists.Stacked on #30. No coder/coder code asserts on the old strings.
When squash merging, please replace the commit body with this description.