From 2cc289cc0d0fd46c40502ffcc6d9de377b485b7f Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:00:16 +0000 Subject: [PATCH 01/28] fix: end ticker run loops when NewMock's cleanup runs A ticker restarted after its cleanup had run, by Reset or by NewTicker on a finished test, started a runLoop that nothing ended. It leaked outside a synctest bubble and made synctest panic inside one. runLoop now also returns when Mock.done closes, as timer writes do. fire selects on done too, because interrupt stays set when runLoop returns that way and the send to internalTicks would block with the lock held. On 5eaccd7 the new TickerResetAfterTestEnds subtest leaks runLoop and the leak check fails. --- mock.go | 4 ++++ mock_test.go | 18 ++++++++++++++++++ ticker.go | 21 +++++++++++++++------ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/mock.go b/mock.go index 6325cd8..b93c58a 100644 --- a/mock.go +++ b/mock.go @@ -33,6 +33,8 @@ type Mock struct { // writes holds the timers that have a goroutine sending their fired value on C. The cleanup // registered in NewMock ends those goroutines. writes map[*Timer]struct{} + // done is closed by the cleanup registered in NewMock to end ticker run loops. + done chan struct{} // cur is the current time cur time.Time @@ -503,6 +505,7 @@ func NewMock(tb TestingT) *Mock { logger: tb, cur: cur, writes: make(map[*Timer]struct{}), + done: make(chan struct{}), } tb.Cleanup(func() { m.mu.Lock() @@ -511,6 +514,7 @@ func NewMock(tb TestingT) *Mock { for t := range m.writes { t.endWriteLocked() } + close(m.done) m.logger.Logf("Mock Clock - test cleanup; will no longer log clock events") }) return m diff --git a/mock_test.go b/mock_test.go index 5c7ac26..43a4cb2 100644 --- a/mock_test.go +++ b/mock_test.go @@ -787,6 +787,24 @@ func TestNoLeak_Go123(t *testing.T) { t.Fatal(err) } }) + + t.Run("TickerResetAfterTestEnds", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + var mClock *quartz.Mock + var tkr *quartz.Ticker + t.Run("Inner", func(t *testing.T) { + mClock = quartz.NewMock(t) + tkr = mClock.NewTicker(time.Second) + tkr.Stop() + }) + tkr.Reset(time.Second) + if err := mClock.Advance(time.Second).Wait(ctx); err != nil { + t.Fatal(err) + } + }) } func TestTickerStop_Go123(t *testing.T) { diff --git a/ticker.go b/ticker.go index f4a4b06..9f2916a 100644 --- a/ticker.go +++ b/ticker.go @@ -20,10 +20,11 @@ type Ticker struct { // not to be reading. That means we need to start a new goroutine to do the channel write (runLoop) if we are a // channel-based ticker. // - // They also are not supposed to leak even if they are never read or stopped (Go runtime can garbage collect them). - // We can't garbage-collect because we can't check if any other code besides the mock references, but we can ensure - // that we don't leak goroutines so that the garbage collector can do its job when the mock is no longer - // referenced. The channels below allow us to interrupt the runLoop goroutine. + // They also are not supposed to leak even if they are never read or stopped (Go runtime can + // garbage collect them). We can't garbage-collect because we can't check if any other code + // besides the mock references, but we can ensure that we don't leak goroutines so that the + // garbage collector can do its job when the mock is no longer referenced. interrupt lets Stop + // end the runLoop goroutine; Mock.done ends it when NewMock's cleanup runs. interrupt chan struct{} } @@ -37,8 +38,12 @@ func (t *Ticker) fire(tt time.Time) { t.nxt = t.nxt.Add(t.d) } t.mock.recomputeNextLocked() - if t.interrupt != nil { // implies runLoop is still going. - t.internalTicks <- tt + // A non-nil interrupt means runLoop is running or has returned because done was closed. + if t.interrupt != nil { + select { + case t.internalTicks <- tt: + case <-t.mock.done: + } } } @@ -108,10 +113,14 @@ outer: // Discard future ticks until we can send this one. case interrupt <- struct{}{}: return + case <-t.mock.done: + return } } case interrupt <- struct{}{}: return + case <-t.mock.done: + return } } } From c7d2613198e45c7ef6f005404a781351b64ded98 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:04:55 +0000 Subject: [PATCH 02/28] refactor(ticker.go): drop the per-ticker cleanup done now ends every runLoop, so the cleanup NewTicker registered only ended runLoop before NewMock's cleanup does. A ticker now ticks until NewMock's cleanup runs, including during cleanups registered between NewMock and NewTicker, as timers do. Without the done case in runLoop's inner select, FiredDuringTest now leaks the ticker's runLoop. Without the one in the outer select, TestNewTicker does. --- ticker.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ticker.go b/ticker.go index 9f2916a..d605efb 100644 --- a/ticker.go +++ b/ticker.go @@ -147,14 +147,6 @@ func newMockTickerLocked(m *Mock, d time.Duration) *Ticker { internalTicks: make(chan time.Time), } m.addEventLocked(t) - m.tb.Cleanup(func() { - m.mu.Lock() - defer m.mu.Unlock() - if t.interrupt != nil { - <-t.interrupt - t.interrupt = nil - } - }) t.startRunLoopLocked() return t } From 090f084d6ed917739e4fd98e911cc17cda629ea3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:30:00 +0000 Subject: [PATCH 03/28] test(mock_test.go): cover each way a ticker starts after its test TickerResetAfterTestEnds only covered Reset after Stop. Reset without Stop and NewTicker after the test has ended also start a runLoop that nothing ends on 5eaccd7, so the subtest is now a table over all three. Each row fails the leak check with 5eaccd7's ticker.go and passes here. --- mock_test.go | 55 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/mock_test.go b/mock_test.go index 43a4cb2..d329f5c 100644 --- a/mock_test.go +++ b/mock_test.go @@ -788,21 +788,50 @@ func TestNoLeak_Go123(t *testing.T) { } }) - t.Run("TickerResetAfterTestEnds", func(t *testing.T) { + // Code that outlives a test can start or reset a ticker after the test's cleanups have run. + t.Run("TickerStartedAfterTestEnds", func(t *testing.T) { t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + for _, tc := range []struct { + name string + inner func(*quartz.Mock) *quartz.Ticker + after func(*quartz.Mock, *quartz.Ticker) + }{ + { + name: "StopThenReset", + inner: func(m *quartz.Mock) *quartz.Ticker { + tkr := m.NewTicker(time.Second) + tkr.Stop() + return tkr + }, + after: func(_ *quartz.Mock, tkr *quartz.Ticker) { tkr.Reset(time.Second) }, + }, + { + name: "ResetWithoutStop", + inner: func(m *quartz.Mock) *quartz.Ticker { return m.NewTicker(time.Second) }, + after: func(_ *quartz.Mock, tkr *quartz.Ticker) { tkr.Reset(time.Second) }, + }, + { + name: "NewTicker", + inner: func(*quartz.Mock) *quartz.Ticker { return nil }, + after: func(m *quartz.Mock, _ *quartz.Ticker) { _ = m.NewTicker(time.Second) }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() - var mClock *quartz.Mock - var tkr *quartz.Ticker - t.Run("Inner", func(t *testing.T) { - mClock = quartz.NewMock(t) - tkr = mClock.NewTicker(time.Second) - tkr.Stop() - }) - tkr.Reset(time.Second) - if err := mClock.Advance(time.Second).Wait(ctx); err != nil { - t.Fatal(err) + var mClock *quartz.Mock + var tkr *quartz.Ticker + t.Run("Inner", func(t *testing.T) { + mClock = quartz.NewMock(t) + tkr = tc.inner(mClock) + }) + tc.after(mClock, tkr) + if err := mClock.Advance(time.Second).Wait(ctx); err != nil { + t.Fatal(err) + } + }) } }) } From ed7f83297cb5345ad3498e215910aed3a46fadcd Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:49:00 +0000 Subject: [PATCH 04/28] docs(mock.go): say when a mock ticker stops sending ticks With the per-ticker cleanup gone, a ticker stops when NewMock's cleanup closes done, and cleanups registered after NewMock still receive ticks. The old text said ticks stop at the end of the test and stay suppressed after it; Mock.done already notes that a reader waiting on C can still get a write. --- mock.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mock.go b/mock.go index b93c58a..ce593ec 100644 --- a/mock.go +++ b/mock.go @@ -74,9 +74,9 @@ func (m *Mock) TickerFunc(ctx context.Context, d time.Duration, f func() error, } // NewTicker creates a mocked ticker attached to this Mock. The ticker stops sending ticks when the -// cleanup that NewTicker registers runs, and advancing the clock after that sends none. A Reset -// after that cleanup starts the ticker again, and only Stop ends it then. Best practice is to only -// manipulate the mock time in the main goroutine of the test. +// cleanup registered by NewMock runs, so its goroutine does not outlive the test. Cleanups +// registered after NewMock run before that cleanup and can still receive ticks. Best practice is to +// only manipulate the mock time in the main goroutine of the test. func (m *Mock) NewTicker(d time.Duration, tags ...string) *Ticker { if d <= 0 { panic("NewTicker called with negative or zero duration") From 5cf253feb2bc6f57e3b42338a13a8efef7c51a1a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:49:05 +0000 Subject: [PATCH 05/28] docs(ticker.go): say why fire also selects on done The comment said what a non-nil interrupt means but not why the send needs the done case: once runLoop has returned on done, nothing receives from internalTicks, and a plain send blocks with the lock held. --- ticker.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ticker.go b/ticker.go index d605efb..8bdc4b9 100644 --- a/ticker.go +++ b/ticker.go @@ -38,7 +38,8 @@ func (t *Ticker) fire(tt time.Time) { t.nxt = t.nxt.Add(t.d) } t.mock.recomputeNextLocked() - // A non-nil interrupt means runLoop is running or has returned because done was closed. + // A non-nil interrupt means runLoop is running or has returned because done was closed. In the + // second case nothing receives from internalTicks, so the send must also select on done. if t.interrupt != nil { select { case t.internalTicks <- tt: From d4448bac8e5fe9f56dcf927e5471758b066475bd Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 19:55:58 +0000 Subject: [PATCH 06/28] test(mock_test.go): merge the after-test-end leak cases into a table TimerFiredAfterTestEnds and TickerStartedAfterTestEnds set up the same way, so AfterTestEnds now runs both as rows. ResetWithoutStop becomes TickerResetAfterTick. Advancing inside Inner leaves the run loop parked in its select with an unread tick, so closing done ends it and the later Advance finds no receiver. With a plain send in fire this row fails 20/20; before, fire's done case was only covered when the run loop happened to return first. --- mock_test.go | 61 +++++++++++++++++++++++----------------------------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/mock_test.go b/mock_test.go index d329f5c..5147155 100644 --- a/mock_test.go +++ b/mock_test.go @@ -772,48 +772,41 @@ func TestNoLeak_Go123(t *testing.T) { } }) - // A test that stops waiting on an Advance can end before its timers fire. - t.Run("TimerFiredAfterTestEnds", func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - var mClock *quartz.Mock - t.Run("Inner", func(t *testing.T) { - mClock = quartz.NewMock(t) - _ = mClock.NewTimer(time.Second) - }) - if err := mClock.Advance(time.Second).Wait(ctx); err != nil { - t.Fatal(err) - } - }) - - // Code that outlives a test can start or reset a ticker after the test's cleanups have run. - t.Run("TickerStartedAfterTestEnds", func(t *testing.T) { + // Code that outlives a test, or an Advance it stopped waiting on, can use the Mock afterward. + t.Run("AfterTestEnds", func(t *testing.T) { t.Parallel() for _, tc := range []struct { - name string - inner func(*quartz.Mock) *quartz.Ticker - after func(*quartz.Mock, *quartz.Ticker) + name string + during func(context.Context, *quartz.Mock) (after func()) }{ { - name: "StopThenReset", - inner: func(m *quartz.Mock) *quartz.Ticker { + name: "TimerFired", + during: func(_ context.Context, m *quartz.Mock) func() { + _ = m.NewTimer(time.Second) + return func() {} + }, + }, + { + name: "TickerResetAfterStop", + during: func(_ context.Context, m *quartz.Mock) func() { tkr := m.NewTicker(time.Second) tkr.Stop() - return tkr + return func() { tkr.Reset(time.Second) } }, - after: func(_ *quartz.Mock, tkr *quartz.Ticker) { tkr.Reset(time.Second) }, }, { - name: "ResetWithoutStop", - inner: func(m *quartz.Mock) *quartz.Ticker { return m.NewTicker(time.Second) }, - after: func(_ *quartz.Mock, tkr *quartz.Ticker) { tkr.Reset(time.Second) }, + name: "TickerResetAfterTick", + during: func(ctx context.Context, m *quartz.Mock) func() { + tkr := m.NewTicker(time.Second) + m.Advance(time.Second).MustWait(ctx) + return func() { tkr.Reset(time.Second) } + }, }, { - name: "NewTicker", - inner: func(*quartz.Mock) *quartz.Ticker { return nil }, - after: func(m *quartz.Mock, _ *quartz.Ticker) { _ = m.NewTicker(time.Second) }, + name: "NewTicker", + during: func(_ context.Context, m *quartz.Mock) func() { + return func() { _ = m.NewTicker(time.Second) } + }, }, } { t.Run(tc.name, func(t *testing.T) { @@ -822,12 +815,12 @@ func TestNoLeak_Go123(t *testing.T) { defer cancel() var mClock *quartz.Mock - var tkr *quartz.Ticker + var after func() t.Run("Inner", func(t *testing.T) { mClock = quartz.NewMock(t) - tkr = tc.inner(mClock) + after = tc.during(ctx, mClock) }) - tc.after(mClock, tkr) + after() if err := mClock.Advance(time.Second).Wait(ctx); err != nil { t.Fatal(err) } From a3f86db1b26fcfdf909f0441ebeb4b4c142c7145 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 21:27:53 +0000 Subject: [PATCH 07/28] fix: stop ticker run loops through their interrupt channels Mock.done was created by NewMock, outside any synctest bubble, and every runLoop selected on it. When a Mock created with the outer test's t had a ticker created inside a bubble, runLoop waited on done, which synctest does not count as durably blocked, so the test hung until go test's timeout instead of panicking at once as on main. Timer writes stopped using done for the same reason. NewMock's cleanup now stops each running runLoop by receiving on its interrupt channel, as Stop does. The Mock tracks tickers with a running runLoop, and Stop removes them. After the cleanup, Reset and NewTicker start no runLoop, so interrupt is set only while runLoop runs and fire no longer needs a done case. --- mock.go | 19 +++++++++++-------- ticker.go | 35 +++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/mock.go b/mock.go index ce593ec..feb2adc 100644 --- a/mock.go +++ b/mock.go @@ -33,8 +33,9 @@ type Mock struct { // writes holds the timers that have a goroutine sending their fired value on C. The cleanup // registered in NewMock ends those goroutines. writes map[*Timer]struct{} - // done is closed by the cleanup registered in NewMock to end ticker run loops. - done chan struct{} + // tickers holds the tickers whose runLoop goroutine is running. The cleanup registered in + // NewMock stops those goroutines. + tickers map[*Ticker]struct{} // cur is the current time cur time.Time @@ -501,11 +502,11 @@ func NewMock(tb TestingT) *Mock { panic(err) } m := &Mock{ - tb: tb, - logger: tb, - cur: cur, - writes: make(map[*Timer]struct{}), - done: make(chan struct{}), + tb: tb, + logger: tb, + cur: cur, + writes: make(map[*Timer]struct{}), + tickers: make(map[*Ticker]struct{}), } tb.Cleanup(func() { m.mu.Lock() @@ -514,7 +515,9 @@ func NewMock(tb TestingT) *Mock { for t := range m.writes { t.endWriteLocked() } - close(m.done) + for t := range m.tickers { + t.stopRunLoopLocked() + } m.logger.Logf("Mock Clock - test cleanup; will no longer log clock events") }) return m diff --git a/ticker.go b/ticker.go index 8bdc4b9..8761947 100644 --- a/ticker.go +++ b/ticker.go @@ -24,7 +24,7 @@ type Ticker struct { // garbage collect them). We can't garbage-collect because we can't check if any other code // besides the mock references, but we can ensure that we don't leak goroutines so that the // garbage collector can do its job when the mock is no longer referenced. interrupt lets Stop - // end the runLoop goroutine; Mock.done ends it when NewMock's cleanup runs. + // and the cleanup registered by NewMock end the runLoop goroutine. interrupt chan struct{} } @@ -38,13 +38,8 @@ func (t *Ticker) fire(tt time.Time) { t.nxt = t.nxt.Add(t.d) } t.mock.recomputeNextLocked() - // A non-nil interrupt means runLoop is running or has returned because done was closed. In the - // second case nothing receives from internalTicks, so the send must also select on done. - if t.interrupt != nil { - select { - case t.internalTicks <- tt: - case <-t.mock.done: - } + if t.interrupt != nil { // implies runLoop is still going. + t.internalTicks <- tt } } @@ -68,10 +63,7 @@ func (t *Ticker) Stop(tags ...string) { t.mock.removeEventLocked(t) t.stopped = true // check if we've already fired, and if so, interrupt it. - if t.interrupt != nil { - <-t.interrupt - t.interrupt = nil - } + t.stopRunLoopLocked() } // Reset stops a ticker and resets its period to the specified duration. The @@ -114,14 +106,10 @@ outer: // Discard future ticks until we can send this one. case interrupt <- struct{}{}: return - case <-t.mock.done: - return } } case interrupt <- struct{}{}: return - case <-t.mock.done: - return } } } @@ -131,11 +119,26 @@ func (t *Ticker) startRunLoopLocked() { if t.interrupt != nil { t.mock.tb.Error("called startRunLoopLocked when interrupt suggests we are already running") } + // NewMock's cleanup has run and would not stop the runLoop, so the ticker sends no ticks. + if t.mock.testOver { + return + } interrupt := make(chan struct{}) t.interrupt = interrupt + t.mock.tickers[t] = struct{}{} go t.runLoop(interrupt) } +// stopRunLoopLocked ends the runLoop goroutine, if it is running. +func (t *Ticker) stopRunLoopLocked() { + if t.interrupt == nil { + return + } + <-t.interrupt + t.interrupt = nil + delete(t.mock.tickers, t) +} + func newMockTickerLocked(m *Mock, d time.Duration) *Ticker { // no buffer follows Go 1.23+ behavior ticks := make(chan time.Time) From 235137bed8d7ee528a1294c11acc2494d4481cd0 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 21:28:54 +0000 Subject: [PATCH 08/28] fix: send a tick to a waiting reader first When a reader was waiting on C and Stop or NewMock's cleanup was waiting on interrupt by the time runLoop reached its select, select picked between them at random. In a synctest bubble, with a reader waiting and the ticker fired as the test's last step, the reader lost the tick in 967 of 2000 runs, and in 971 of 2000 with Stop right after the Advance. main loses it about as often. runLoop now tries a non-blocking send first, as timer writes do, and the reader got the tick in all 2000 runs of both. --- ticker.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ticker.go b/ticker.go index 8761947..3060d6a 100644 --- a/ticker.go +++ b/ticker.go @@ -98,6 +98,14 @@ outer: for { select { case tt := <-t.internalTicks: + // select picks at random among ready cases. Send first to a reader that is already + // waiting, so it gets the tick even when Stop or NewMock's cleanup is waiting on + // interrupt. + select { + case t.c <- tt: + continue outer + default: + } for { select { case t.c <- tt: From 9f9eae88cd6c29b4994bd0365bdf8557636e7b6a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 12:54:19 +0000 Subject: [PATCH 09/28] test(mock_test.go): cover a ticker read at NewMock's cleanup The test becomes TestReaderWaitingAtMockCleanup with a ticker row that fires as the test's last step, so runLoop often reaches its send only after NewMock's cleanup waits on interrupt. Without runLoop's first send, TickerFiredBeforeCleanup fails 100 of 100 runs, and 46 of 100 with -race. There is no ticker row for firing after the cleanup, because a ticker sends no ticks then. --- mock_test.go | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/mock_test.go b/mock_test.go index 5147155..fcf945c 100644 --- a/mock_test.go +++ b/mock_test.go @@ -689,27 +689,31 @@ func TestTimer_ReadableDuringCleanup(t *testing.T) { }) } -// TestTimer_ReaderWaitingAtMockCleanup checks that a reader waiting on C when NewMock's cleanup -// runs receives the value of a timer that fires just before or after that cleanup. synctest.Wait -// makes sure the reader is waiting before the timer fires and that the send has ended before the -// check. -func TestTimer_ReaderWaitingAtMockCleanup(t *testing.T) { +// TestReaderWaitingAtMockCleanup checks that a reader waiting on C when NewMock's cleanup runs +// receives a timer value or tick sent just before that cleanup, or a timer value sent after it. A +// ticker sends no ticks after the cleanup. synctest.Wait makes sure the reader is waiting before +// the clock fires and that the send has ended before the check. +func TestReaderWaitingAtMockCleanup(t *testing.T) { t.Parallel() + newTimer := func(m *quartz.Mock) <-chan time.Time { return m.NewTimer(time.Second).C } + newTicker := func(m *quartz.Mock) <-chan time.Time { return m.NewTicker(time.Second).C } for _, tc := range []struct { name string + start func(*quartz.Mock) <-chan time.Time fireAfterCleanup bool }{ - // The test fires the timer as its last step. In a bubble, the goroutine that sends the - // value often starts only after NewMock's cleanup is waiting to end it. - {name: "FiredBeforeCleanup"}, - {name: "FiredAfterCleanup", fireAfterCleanup: true}, + // These rows fire as the test's last step. In a bubble, the goroutine that sends on C + // often reaches its send only after NewMock's cleanup is waiting to end it. + {name: "TimerFiredBeforeCleanup", start: newTimer}, + {name: "TickerFiredBeforeCleanup", start: newTicker}, + {name: "TimerFiredAfterCleanup", start: newTimer, fireAfterCleanup: true}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - // When the sending goroutine starts late, select picks at random between the reader - // and the cleanup, so a send that does not try the reader first loses the value about - // half the time. Each round is a new bubble, so without the race detector, which - // changes scheduling, the test then fails in nearly every run. + // When it does, select picks at random between the reader and the cleanup, so a send + // that does not try the reader first loses the value about half the time. Each round + // is a new bubble, so without the race detector, which changes scheduling, the test + // then fails in nearly every run. for range 10 { synctest.Test(t, func(t *testing.T) { var mClock *quartz.Mock @@ -723,14 +727,14 @@ func TestTimer_ReaderWaitingAtMockCleanup(t *testing.T) { synctest.Wait() close(quit) if len(received) == 0 { - t.Error("the waiting reader did not receive the timer value") + t.Error("the waiting reader did not receive the value") } }) mClock = quartz.NewMock(t) - tmr := mClock.NewTimer(time.Second) + c := tc.start(mClock) go func() { select { - case <-tmr.C: + case <-c: received <- struct{}{} case <-quit: } From 6e3d7b13a29ad7acadbbddb7b0a489e39f05a2b9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 12:55:05 +0000 Subject: [PATCH 10/28] test(mock_test.go): read a fired ticker from a later cleanup TestTimer_ReadableDuringCleanup becomes TestReadableDuringCleanup, with a Timer and a Ticker row. Its server cleanup is registered before NewTicker, as when a server creates the ticker. With the ticker code before this PR, NewTicker's own cleanup ran first and stopped runLoop, so the worker never got the tick and the Ticker row fails. --- mock_test.go | 81 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/mock_test.go b/mock_test.go index fcf945c..f45745e 100644 --- a/mock_test.go +++ b/mock_test.go @@ -651,42 +651,57 @@ func TestTimerReset_NegativeDuration(t *testing.T) { } } -// TestTimer_ReadableDuringCleanup checks that a cleanup registered after NewMock, which runs before -// NewMock's cleanup, can still receive from a timer that fired during the test. Tests that stop a -// server in such a cleanup rely on this when a worker reads the timer on its way out. -func TestTimer_ReadableDuringCleanup(t *testing.T) { +// TestReadableDuringCleanup checks that a cleanup registered after NewMock, which runs before +// NewMock's cleanup, can still receive from a timer or ticker that fired during the test. Tests +// that stop a server in such a cleanup rely on this when a worker reads the channel on its way out. +func TestReadableDuringCleanup(t *testing.T) { t.Parallel() - synctest.Test(t, func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + newTimer := func(m *quartz.Mock) <-chan time.Time { return m.NewTimer(time.Second).C } + newTicker := func(m *quartz.Mock) <-chan time.Time { return m.NewTicker(time.Second).C } + for _, tc := range []struct { + name string + start func(*quartz.Mock) <-chan time.Time + }{ + {name: "Timer", start: newTimer}, + {name: "Ticker", start: newTicker}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() - mClock := quartz.NewMock(t) - busy := make(chan struct{}) - quit := make(chan struct{}) - exited := make(chan struct{}) - // Stands in for a server's Close: it unblocks the worker and waits for it to exit. Bubble - // time moves once every goroutine is blocked, so a worker that never receives fails the - // test at once. quit then lets the worker exit, so the bubble can end. - t.Cleanup(func() { - close(busy) - select { - case <-exited: - case <-time.After(10 * time.Second): - t.Error("worker did not receive the timer value during cleanup") - } - close(quit) + mClock := quartz.NewMock(t) + busy := make(chan struct{}) + quit := make(chan struct{}) + exited := make(chan struct{}) + // Stands in for a server's Close: it unblocks the worker and waits for it to exit. It + // is registered before the timer or ticker is created, as when the server creates + // them. Bubble time moves once every goroutine is blocked, so a worker that never + // receives fails the test at once. quit then lets the worker exit, so the bubble can + // end. + t.Cleanup(func() { + close(busy) + select { + case <-exited: + case <-time.After(10 * time.Second): + t.Error("worker did not receive from the channel during cleanup") + } + close(quit) + }) + c := tc.start(mClock) + go func() { + defer close(exited) + <-busy + select { + case <-c: + case <-quit: + } + }() + mClock.Advance(time.Second).MustWait(ctx) + }) }) - tmr := mClock.NewTimer(time.Second) - go func() { - defer close(exited) - <-busy - select { - case <-tmr.C: - case <-quit: - } - }() - mClock.Advance(time.Second).MustWait(ctx) - }) + } } // TestReaderWaitingAtMockCleanup checks that a reader waiting on C when NewMock's cleanup runs From afdade2c4bfde7e7332d434b751f255edc55ddf3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 21:30:33 +0000 Subject: [PATCH 11/28] docs(ticker.go): wrap the Ticker channel comment to 100 columns --- ticker.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ticker.go b/ticker.go index 3060d6a..4c3f8ec 100644 --- a/ticker.go +++ b/ticker.go @@ -14,11 +14,12 @@ type Ticker struct { stopped bool // true if the ticker is not running internalTicks chan time.Time // used to deliver ticks to the runLoop goroutine - // As of Go 1.23, ticker channels are unbuffered and guaranteed to block forever after a call to stop. + // As of Go 1.23, ticker channels are unbuffered and guaranteed to block forever after a call to + // stop. // - // When a mocked ticker fires, we don't want to block on a channel write, because it's fine for the code under test - // not to be reading. That means we need to start a new goroutine to do the channel write (runLoop) if we are a - // channel-based ticker. + // When a mocked ticker fires, we don't want to block on a channel write, because it's fine for + // the code under test not to be reading. That means we need to start a new goroutine to do the + // channel write (runLoop) if we are a channel-based ticker. // // They also are not supposed to leak even if they are never read or stopped (Go runtime can // garbage collect them). We can't garbage-collect because we can't check if any other code From 8ae4b6bac2652c242b84af0bf8d55a89e7a392f4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 3 Nov 2026 21:30:45 +0000 Subject: [PATCH 12/28] docs(mock.go): say NewMock's cleanup also drops ticks Tickers stop when NewMock's cleanup runs, as pending timer writes end, so a cleanup registered before NewMock that waits for code reading a ticker hangs in the same way. The NewMock doc now covers tickers too. --- mock.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/mock.go b/mock.go index feb2adc..8d06ea1 100644 --- a/mock.go +++ b/mock.go @@ -490,12 +490,9 @@ func (m *Mock) WithLogger(l Logger) *Mock { // You may re-set the time earlier than this, but only before timers or tickers // are created. // -// Cleanups registered after NewMock run before NewMock's own cleanup, so they can still receive -// the value of a timer that fired during the test. NewMock's cleanup drops timer values that have -// not been received, and a timer from NewTimer that fires after it sends only to a reader already -// waiting on C. Call NewMock before registering cleanups that wait for code reading the Mock's -// timers. Ticks from NewTicker stop earlier, unless Reset starts the ticker again; see -// [Mock.NewTicker]. +// A timer value or tick that has not been received when the cleanup registered by NewMock runs is +// dropped, and tickers send no ticks after it. Call NewMock before registering cleanups that wait +// for code reading the Mock's timers or tickers. func NewMock(tb TestingT) *Mock { cur, err := time.Parse(time.RFC3339, "2024-01-01T00:00:00Z") if err != nil { From c86466ddcb85bd39b22829111baa955ee6b0c7c9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 12:55:20 +0000 Subject: [PATCH 13/28] test(mock_test.go): say which send the rounds catch The comment above the rounds started with "When it does", which pointed at a row comment in the table above. It now names the send it means. --- mock_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mock_test.go b/mock_test.go index f45745e..4fe03b7 100644 --- a/mock_test.go +++ b/mock_test.go @@ -725,10 +725,11 @@ func TestReaderWaitingAtMockCleanup(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - // When it does, select picks at random between the reader and the cleanup, so a send - // that does not try the reader first loses the value about half the time. Each round - // is a new bubble, so without the race detector, which changes scheduling, the test - // then fails in nearly every run. + // A send that reaches its select after NewMock's cleanup is waiting to end it finds the + // reader and the cleanup both ready, and select picks between them at random. A send + // that does not try the reader first then loses the value about half the time. Each + // round is a new bubble, so without the race detector, which changes scheduling, the + // test then fails in nearly every run. for range 10 { synctest.Test(t, func(t *testing.T) { var mClock *quartz.Mock From 26c4c34432dd0c82c202dec3f67ab4f1e811a719 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 12:55:33 +0000 Subject: [PATCH 14/28] fix: send a tick after NewMock's cleanup to a waiting reader After NewMock's cleanup, a timer that fires still sends to a reader already waiting on C, but a ticker sent nothing: the cleanup had stopped runLoop and none starts after it. fire now tries one non-blocking send to C when no runLoop is running, which happens only after the cleanup, so tickers match timers and still start no goroutine. The new TickerFiredAfterCleanup row fails 100 of 100 runs without this, with and without -race. On main a ticker's own cleanup also dropped ticks for a waiting reader, so this changes what such a reader sees after the test. --- mock.go | 12 ++++++------ mock_test.go | 6 +++--- ticker.go | 9 ++++++++- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/mock.go b/mock.go index 8d06ea1..79fce95 100644 --- a/mock.go +++ b/mock.go @@ -74,10 +74,10 @@ func (m *Mock) TickerFunc(ctx context.Context, d time.Duration, f func() error, return t } -// NewTicker creates a mocked ticker attached to this Mock. The ticker stops sending ticks when the -// cleanup registered by NewMock runs, so its goroutine does not outlive the test. Cleanups -// registered after NewMock run before that cleanup and can still receive ticks. Best practice is to -// only manipulate the mock time in the main goroutine of the test. +// NewTicker creates a mocked ticker attached to this Mock. After the cleanup registered by NewMock +// runs, a tick reaches only a reader that is already waiting on C. Cleanups registered after +// NewMock run before that cleanup and receive ticks as usual. Best practice is to only manipulate +// the mock time in the main goroutine of the test. func (m *Mock) NewTicker(d time.Duration, tags ...string) *Ticker { if d <= 0 { panic("NewTicker called with negative or zero duration") @@ -491,8 +491,8 @@ func (m *Mock) WithLogger(l Logger) *Mock { // are created. // // A timer value or tick that has not been received when the cleanup registered by NewMock runs is -// dropped, and tickers send no ticks after it. Call NewMock before registering cleanups that wait -// for code reading the Mock's timers or tickers. +// dropped. Call NewMock before registering cleanups that wait for code reading the Mock's timers +// or tickers. func NewMock(tb TestingT) *Mock { cur, err := time.Parse(time.RFC3339, "2024-01-01T00:00:00Z") if err != nil { diff --git a/mock_test.go b/mock_test.go index 4fe03b7..4803eb8 100644 --- a/mock_test.go +++ b/mock_test.go @@ -705,9 +705,8 @@ func TestReadableDuringCleanup(t *testing.T) { } // TestReaderWaitingAtMockCleanup checks that a reader waiting on C when NewMock's cleanup runs -// receives a timer value or tick sent just before that cleanup, or a timer value sent after it. A -// ticker sends no ticks after the cleanup. synctest.Wait makes sure the reader is waiting before -// the clock fires and that the send has ended before the check. +// receives a timer value or tick sent just before or after that cleanup. synctest.Wait makes sure +// the reader is waiting before the clock fires and that the send has ended before the check. func TestReaderWaitingAtMockCleanup(t *testing.T) { t.Parallel() newTimer := func(m *quartz.Mock) <-chan time.Time { return m.NewTimer(time.Second).C } @@ -722,6 +721,7 @@ func TestReaderWaitingAtMockCleanup(t *testing.T) { {name: "TimerFiredBeforeCleanup", start: newTimer}, {name: "TickerFiredBeforeCleanup", start: newTicker}, {name: "TimerFiredAfterCleanup", start: newTimer, fireAfterCleanup: true}, + {name: "TickerFiredAfterCleanup", start: newTicker, fireAfterCleanup: true}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() diff --git a/ticker.go b/ticker.go index 4c3f8ec..c43b5c1 100644 --- a/ticker.go +++ b/ticker.go @@ -41,6 +41,13 @@ func (t *Ticker) fire(tt time.Time) { t.mock.recomputeNextLocked() if t.interrupt != nil { // implies runLoop is still going. t.internalTicks <- tt + return + } + // NewMock's cleanup has stopped runLoop, so only a reader that is already waiting gets the + // tick. + select { + case t.c <- tt: + default: } } @@ -128,7 +135,7 @@ func (t *Ticker) startRunLoopLocked() { if t.interrupt != nil { t.mock.tb.Error("called startRunLoopLocked when interrupt suggests we are already running") } - // NewMock's cleanup has run and would not stop the runLoop, so the ticker sends no ticks. + // NewMock's cleanup has run and would not stop the runLoop, so fire sends to C itself. if t.mock.testOver { return } From e890a4ec6085e9e7df74036762b056f807a32efc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 12:56:55 +0000 Subject: [PATCH 15/28] fix: receive no stale tick after Ticker.Reset A time.Ticker delivers no tick from the old schedule after Reset returns. The mock did, in two ways. runLoop kept a tick it was holding for a reader, because Reset left a running runLoop alone. And a fire that Advance started could get the lock after Reset and send its tick anyway: a Reset right after Advance returned got a stale tick in 2000 of 2000 runs. Reset now stops runLoop, which drops a held tick, and starts a new one. fire returns without effect when the ticker was stopped or nxt no longer matches the fire's time, as timer fires do. TestTicker_StopAndReset replaces TestTickerStop_Go123 with a table: ResetTickedUnread fails without the restart, and ResetDuringAdvance without the check in fire. --- mock_test.go | 63 ++++++++++++++++++++++++++++++++++++++++------------ ticker.go | 12 ++++++---- 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/mock_test.go b/mock_test.go index 4803eb8..8b91baa 100644 --- a/mock_test.go +++ b/mock_test.go @@ -849,22 +849,57 @@ func TestNoLeak_Go123(t *testing.T) { }) } -func TestTickerStop_Go123(t *testing.T) { +// TestTicker_StopAndReset checks that no tick is received after Stop or Reset returns, and that +// Reset's period applies, as with a time.Ticker. +func TestTicker_StopAndReset(t *testing.T) { t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - mClock := quartz.NewMock(t) - - tkr := mClock.NewTicker(1 * time.Second) - mClock.Advance(1 * time.Second).MustWait(ctx) - tkr.Stop() + ticked := func(ctx context.Context, m *quartz.Mock) *quartz.Ticker { + tkr := m.NewTicker(time.Second) + m.Advance(time.Second).MustWait(ctx) + return tkr + } + // Advance fires the ticker from a new goroutine, which can run before or after the call. + advancing := func(_ context.Context, m *quartz.Mock) *quartz.Ticker { + tkr := m.NewTicker(time.Second) + _ = m.Advance(time.Second) + return tkr + } + stop := func(tkr *quartz.Ticker) { tkr.Stop() } + reset := func(tkr *quartz.Ticker) { tkr.Reset(time.Hour) } + for _, tc := range []struct { + name string + start func(context.Context, *quartz.Mock) *quartz.Ticker + call func(*quartz.Ticker) + // wantNext is how long after the call the ticker ticks, or 0 if it does not. + wantNext time.Duration + }{ + {name: "StopTickedUnread", start: ticked, call: stop}, + {name: "ResetTickedUnread", start: ticked, call: reset, wantNext: time.Hour}, + {name: "StopDuringAdvance", start: advancing, call: stop}, + {name: "ResetDuringAdvance", start: advancing, call: reset, wantNext: time.Hour}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() - select { - case tme := <-tkr.C: - t.Fatalf("got channel read after stop: %s", tme) - default: - // OK! + mClock := quartz.NewMock(t) + tkr := tc.start(ctx, mClock) + tc.call(tkr) + // Lets any goroutine that could still send on C run before the checks. + synctest.Wait() + select { + case tme := <-tkr.C: + t.Errorf("received %s after the call returned", tme) + default: + } + if next, _ := mClock.Peek(); next != tc.wantNext { + t.Errorf("ticker ticks in %s, want %s", next, tc.wantNext) + } + tkr.Stop() + }) + }) } } diff --git a/ticker.go b/ticker.go index c43b5c1..b39028e 100644 --- a/ticker.go +++ b/ticker.go @@ -32,7 +32,10 @@ type Ticker struct { func (t *Ticker) fire(tt time.Time) { t.mock.mu.Lock() defer t.mock.mu.Unlock() - if t.stopped { + // A fire that gets the lock after Stop or Reset belongs to the schedule they replaced: Stop + // stops the ticker, and Reset moves nxt. This happens when Stop or Reset gets the lock before + // a fire that Advance started. + if t.stopped || !t.nxt.Equal(tt) { return } for !t.nxt.After(t.mock.cur) { @@ -95,9 +98,10 @@ func (t *Ticker) Reset(d time.Duration, tags ...string) { } else { t.mock.recomputeNextLocked() } - if t.interrupt == nil { - t.startRunLoopLocked() - } + // A tick that runLoop holds for a reader belongs to the old schedule, and stopping runLoop + // drops it. + t.stopRunLoopLocked() + t.startRunLoopLocked() } func (t *Ticker) runLoop(interrupt chan struct{}) { From 86484a1dd3902a2c15539867f20d6e664a1713d3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 12:58:10 +0000 Subject: [PATCH 16/28] test(mock_test.go): share the channel helpers and drop a redundant row newTimer and newTicker were defined in two tests; they are now package functions. The row comment in TestReaderWaitingAtMockCleanup said all rows fire as the test's last step, which only the two FiredBeforeCleanup rows do. AfterTestEnds/TickerResetAfterTick reached the same Reset path as TickerResetAfterStop once fire no longer selected on done, so it is removed, and the rows no longer need a context. Two comment lines in TestReadableDuringCleanup were longer than 100 columns. --- mock_test.go | 42 ++++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/mock_test.go b/mock_test.go index 8b91baa..c37b4fd 100644 --- a/mock_test.go +++ b/mock_test.go @@ -651,13 +651,16 @@ func TestTimerReset_NegativeDuration(t *testing.T) { } } +// newTimer and newTicker create a timer or ticker that fires after one second and return its +// channel. +func newTimer(m *quartz.Mock) <-chan time.Time { return m.NewTimer(time.Second).C } +func newTicker(m *quartz.Mock) <-chan time.Time { return m.NewTicker(time.Second).C } + // TestReadableDuringCleanup checks that a cleanup registered after NewMock, which runs before // NewMock's cleanup, can still receive from a timer or ticker that fired during the test. Tests // that stop a server in such a cleanup rely on this when a worker reads the channel on its way out. func TestReadableDuringCleanup(t *testing.T) { t.Parallel() - newTimer := func(m *quartz.Mock) <-chan time.Time { return m.NewTimer(time.Second).C } - newTicker := func(m *quartz.Mock) <-chan time.Time { return m.NewTicker(time.Second).C } for _, tc := range []struct { name string start func(*quartz.Mock) <-chan time.Time @@ -675,11 +678,11 @@ func TestReadableDuringCleanup(t *testing.T) { busy := make(chan struct{}) quit := make(chan struct{}) exited := make(chan struct{}) - // Stands in for a server's Close: it unblocks the worker and waits for it to exit. It - // is registered before the timer or ticker is created, as when the server creates - // them. Bubble time moves once every goroutine is blocked, so a worker that never - // receives fails the test at once. quit then lets the worker exit, so the bubble can - // end. + // Stands in for a server's Close: it unblocks the worker and waits for it to exit. + // It is registered before the timer or ticker is created, as when the server + // creates them. Bubble time moves once every goroutine is blocked, so a worker that + // never receives fails the test at once. quit then lets the worker exit, so the + // bubble can end. t.Cleanup(func() { close(busy) select { @@ -709,17 +712,16 @@ func TestReadableDuringCleanup(t *testing.T) { // the reader is waiting before the clock fires and that the send has ended before the check. func TestReaderWaitingAtMockCleanup(t *testing.T) { t.Parallel() - newTimer := func(m *quartz.Mock) <-chan time.Time { return m.NewTimer(time.Second).C } - newTicker := func(m *quartz.Mock) <-chan time.Time { return m.NewTicker(time.Second).C } for _, tc := range []struct { name string start func(*quartz.Mock) <-chan time.Time fireAfterCleanup bool }{ - // These rows fire as the test's last step. In a bubble, the goroutine that sends on C - // often reaches its send only after NewMock's cleanup is waiting to end it. + // These two fire as the test's last step. In a bubble, the goroutine that sends on C often + // reaches its send only after NewMock's cleanup is waiting to end it. {name: "TimerFiredBeforeCleanup", start: newTimer}, {name: "TickerFiredBeforeCleanup", start: newTicker}, + // These two fire from the cleanup registered before NewMock, which runs after NewMock's. {name: "TimerFiredAfterCleanup", start: newTimer, fireAfterCleanup: true}, {name: "TickerFiredAfterCleanup", start: newTicker, fireAfterCleanup: true}, } { @@ -797,34 +799,26 @@ func TestNoLeak_Go123(t *testing.T) { t.Parallel() for _, tc := range []struct { name string - during func(context.Context, *quartz.Mock) (after func()) + during func(*quartz.Mock) (after func()) }{ { name: "TimerFired", - during: func(_ context.Context, m *quartz.Mock) func() { + during: func(m *quartz.Mock) func() { _ = m.NewTimer(time.Second) return func() {} }, }, { name: "TickerResetAfterStop", - during: func(_ context.Context, m *quartz.Mock) func() { + during: func(m *quartz.Mock) func() { tkr := m.NewTicker(time.Second) tkr.Stop() return func() { tkr.Reset(time.Second) } }, }, - { - name: "TickerResetAfterTick", - during: func(ctx context.Context, m *quartz.Mock) func() { - tkr := m.NewTicker(time.Second) - m.Advance(time.Second).MustWait(ctx) - return func() { tkr.Reset(time.Second) } - }, - }, { name: "NewTicker", - during: func(_ context.Context, m *quartz.Mock) func() { + during: func(m *quartz.Mock) func() { return func() { _ = m.NewTicker(time.Second) } }, }, @@ -838,7 +832,7 @@ func TestNoLeak_Go123(t *testing.T) { var after func() t.Run("Inner", func(t *testing.T) { mClock = quartz.NewMock(t) - after = tc.during(ctx, mClock) + after = tc.during(mClock) }) after() if err := mClock.Advance(time.Second).Wait(ctx); err != nil { From 0eb9a1dbed1c5c955e5305a022499f570eb87bbf Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 12:58:24 +0000 Subject: [PATCH 17/28] docs: point NewTicker at NewMock and drop a wrong Stop comment NewMock's doc says what happens to timer values and ticks when the test's cleanups run, so NewTicker now refers to it instead of repeating part of it. The comment in Ticker.Stop said runLoop is interrupted only if the ticker has fired, but Stop ends runLoop either way. --- mock.go | 7 +++---- ticker.go | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/mock.go b/mock.go index 79fce95..bcf01af 100644 --- a/mock.go +++ b/mock.go @@ -74,10 +74,9 @@ func (m *Mock) TickerFunc(ctx context.Context, d time.Duration, f func() error, return t } -// NewTicker creates a mocked ticker attached to this Mock. After the cleanup registered by NewMock -// runs, a tick reaches only a reader that is already waiting on C. Cleanups registered after -// NewMock run before that cleanup and receive ticks as usual. Best practice is to only manipulate -// the mock time in the main goroutine of the test. +// NewTicker creates a mocked ticker attached to this Mock. See NewMock for what happens to ticks +// when the test's cleanups run. Best practice is to only manipulate the mock time in the main +// goroutine of the test. func (m *Mock) NewTicker(d time.Duration, tags ...string) *Ticker { if d <= 0 { panic("NewTicker called with negative or zero duration") diff --git a/ticker.go b/ticker.go index b39028e..63106ff 100644 --- a/ticker.go +++ b/ticker.go @@ -73,7 +73,6 @@ func (t *Ticker) Stop(tags ...string) { defer close(c.complete) t.mock.removeEventLocked(t) t.stopped = true - // check if we've already fired, and if so, interrupt it. t.stopRunLoopLocked() } From 114313388966d72644c2382d32407b6e65ffa7fa Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 15:31:57 +0000 Subject: [PATCH 18/28] fix: panic on Ticker.Reset with a non-positive duration Ticker.Reset's doc says it panics when d is not positive, as time.Ticker.Reset and NewTicker do, but the mock accepted it. The next fire then looped forever advancing nxt by a zero or negative period while holding the Mock's lock: after Reset(0), Advance(0) never completed and a later Stop blocked forever. --- mock_test.go | 19 +++++++++++++++++++ ticker.go | 3 +++ 2 files changed, 22 insertions(+) diff --git a/mock_test.go b/mock_test.go index c37b4fd..6075868 100644 --- a/mock_test.go +++ b/mock_test.go @@ -843,6 +843,25 @@ func TestNoLeak_Go123(t *testing.T) { }) } +// TestTickerReset_NonPositiveDuration checks that Reset panics for a duration that is not positive, +// as time.Ticker.Reset and NewTicker do. A mock ticker with such a period made the next fire loop +// forever with the Mock's lock held. +func TestTickerReset_NonPositiveDuration(t *testing.T) { + t.Parallel() + for _, d := range []time.Duration{0, -time.Second} { + t.Run(d.String(), func(t *testing.T) { + t.Parallel() + tkr := quartz.NewMock(t).NewTicker(time.Second) + defer func() { + if recover() == nil { + t.Errorf("Reset(%s) did not panic", d) + } + }() + tkr.Reset(d) + }) + } +} + // TestTicker_StopAndReset checks that no tick is received after Stop or Reset returns, and that // Reset's period applies, as with a time.Ticker. func TestTicker_StopAndReset(t *testing.T) { diff --git a/ticker.go b/ticker.go index 63106ff..f1748ab 100644 --- a/ticker.go +++ b/ticker.go @@ -84,6 +84,9 @@ func (t *Ticker) Reset(d time.Duration, tags ...string) { t.ticker.Reset(d) return } + if d <= 0 { + panic("Ticker.Reset called with negative or zero duration") + } t.mock.mu.Lock() defer t.mock.mu.Unlock() c := newCall(clockFunctionTickerReset, tags, withDuration(d)) From 1dad7f8ec3db58bdb57ae7dfe3aeef9002a6a8f2 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 15:32:06 +0000 Subject: [PATCH 19/28] docs(mock.go): say what NewMock's cleanup leaves of later fires The NewMock doc said what happens to values not received when its cleanup runs, but not to timers and tickers that fire after it: they send only to a reader that is already waiting on C, and nothing is kept for a later receive. --- mock.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mock.go b/mock.go index bcf01af..53b1358 100644 --- a/mock.go +++ b/mock.go @@ -490,8 +490,9 @@ func (m *Mock) WithLogger(l Logger) *Mock { // are created. // // A timer value or tick that has not been received when the cleanup registered by NewMock runs is -// dropped. Call NewMock before registering cleanups that wait for code reading the Mock's timers -// or tickers. +// dropped. After that cleanup, a timer or ticker that fires sends only to a reader already waiting +// on C. Call NewMock before registering cleanups that wait for code reading the Mock's timers or +// tickers. func NewMock(tb TestingT) *Mock { cur, err := time.Parse(time.RFC3339, "2024-01-01T00:00:00Z") if err != nil { From 1eea60c2a5aded9113458ece3da798f0ddc14146 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 16:34:00 +0000 Subject: [PATCH 20/28] fix: drop a held tick on Ticker.Reset without a new runLoop Reset stopped runLoop and started a new one to drop a tick held for a reader. The new runLoop belongs to the caller's synctest bubble, so a ticker created outside a bubble and Reset inside one left a goroutine in the bubble waiting on channels created outside it, and synctest.Test hung until go test's timeout. Before the restart, Reset of a running ticker started no goroutine and that test passed. Reset now sends on a channel that both of runLoop's selects receive from, and runLoop drops a held tick when it gets it. A stopped ticker still gets a new runLoop, as before this PR. ResetTickedUnread fails if runLoop keeps the tick, and the tests hang if the outer select does not receive. --- ticker.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/ticker.go b/ticker.go index f1748ab..795baf6 100644 --- a/ticker.go +++ b/ticker.go @@ -13,6 +13,8 @@ type Ticker struct { mock *Mock // mock clock, if set stopped bool // true if the ticker is not running internalTicks chan time.Time // used to deliver ticks to the runLoop goroutine + // resets is how Reset tells runLoop to drop a tick from the old schedule. + resets chan struct{} // As of Go 1.23, ticker channels are unbuffered and guaranteed to block forever after a call to // stop. @@ -100,10 +102,14 @@ func (t *Ticker) Reset(d time.Duration, tags ...string) { } else { t.mock.recomputeNextLocked() } - // A tick that runLoop holds for a reader belongs to the old schedule, and stopping runLoop - // drops it. - t.stopRunLoopLocked() - t.startRunLoopLocked() + if t.interrupt == nil { + t.startRunLoopLocked() + return + } + // A tick that runLoop holds for a reader belongs to the old schedule, so runLoop drops it. A + // new runLoop would belong to the caller's synctest bubble and wait on channels created + // outside it, which keeps the bubble from ending. + t.resets <- struct{}{} } func (t *Ticker) runLoop(interrupt chan struct{}) { @@ -126,10 +132,13 @@ outer: continue outer case <-t.internalTicks: // Discard future ticks until we can send this one. + case <-t.resets: + continue outer case interrupt <- struct{}{}: return } } + case <-t.resets: case interrupt <- struct{}{}: return } @@ -171,6 +180,7 @@ func newMockTickerLocked(m *Mock, d time.Duration) *Ticker { nxt: m.cur.Add(d), mock: m, internalTicks: make(chan time.Time), + resets: make(chan struct{}), } m.addEventLocked(t) t.startRunLoopLocked() From 68cabafcb84f34ce12d39ff053aa2e5186d56a53 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 16:59:49 +0000 Subject: [PATCH 21/28] docs(mock.go): name the timers and tickers NewMock's cleanup limits "A timer or ticker that fires sends only to a reader already waiting on C" did not hold for AfterFunc timers, which have no C, or for TickerFunc, which keeps calling its function after NewMock's cleanup until its context ends. The doc now names NewTimer and NewTicker, and says the same holds for a ticker reset after the cleanup: Reset starts no runLoop then, and a later reader gets nothing. --- mock.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mock.go b/mock.go index 53b1358..e47dc13 100644 --- a/mock.go +++ b/mock.go @@ -490,9 +490,9 @@ func (m *Mock) WithLogger(l Logger) *Mock { // are created. // // A timer value or tick that has not been received when the cleanup registered by NewMock runs is -// dropped. After that cleanup, a timer or ticker that fires sends only to a reader already waiting -// on C. Call NewMock before registering cleanups that wait for code reading the Mock's timers or -// tickers. +// dropped. After that cleanup, a timer from NewTimer or a ticker from NewTicker that fires, also +// after Reset, sends only to a reader already waiting on C. Call NewMock before registering +// cleanups that wait for code reading the Mock's timers or tickers. func NewMock(tb TestingT) *Mock { cur, err := time.Parse(time.RFC3339, "2024-01-01T00:00:00Z") if err != nil { From 487178a0578931e07655915e812e1693db05c42a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:02:31 +0000 Subject: [PATCH 22/28] test(mock_test.go): reset a ticker from a bubble that did not create it A Reset that started a new runLoop left a goroutine in the caller's synctest bubble waiting on the Mock's channels, so synctest.Test never returned. With 08e4a02's ticker.go both rows of the new TestTicker_ResetInBubble hang until go test -timeout, which stops the run with their synctest.Test call in the stack, as deadlock tests in this repo already rely on. The TickedUnread row also advances to the new schedule and checks that the first tick received is on it, which fails when runLoop keeps the old tick. The rows do not fit TestTicker_StopAndReset, which creates the Mock in the bubble and checks with synctest.Wait, which cannot see goroutines outside the bubble. --- mock_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/mock_test.go b/mock_test.go index 6075868..5fa9d3b 100644 --- a/mock_test.go +++ b/mock_test.go @@ -862,6 +862,41 @@ func TestTickerReset_NonPositiveDuration(t *testing.T) { } } +// TestTicker_ResetInBubble resets a running ticker from a synctest bubble that did not create its +// Mock. Reset must start no goroutine there: one waiting on the Mock's channels, which belong to no +// bubble, keeps synctest.Test from returning, and go test -timeout then stops the run with this +// test's synctest.Test call in the stack. +func TestTicker_ResetInBubble(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + tick bool + }{ + {name: "Running"}, + // runLoop holds the tick for a reader when Reset is called. + {name: "TickedUnread", tick: true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + mClock := quartz.NewMock(t) + tkr := mClock.NewTicker(time.Second) + defer tkr.Stop() + if tc.tick { + mClock.Advance(time.Second).MustWait(ctx) + } + synctest.Test(t, func(*testing.T) { tkr.Reset(time.Hour) }) + mClock.Advance(time.Hour).MustWait(ctx) + // A tick kept from the old schedule would be received first. + if got, want := <-tkr.C, mClock.Now(); !got.Equal(want) { + t.Errorf("received the tick for %s, want the one for %s", got, want) + } + }) + } +} + // TestTicker_StopAndReset checks that no tick is received after Stop or Reset returns, and that // Reset's period applies, as with a time.Ticker. func TestTicker_StopAndReset(t *testing.T) { From 295fadbff3ee3eef1b875464db35c2f487480c80 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:32:05 +0000 Subject: [PATCH 23/28] docs(mock.go): lead the NewMock doc with when to call it The paragraph now starts with what the caller does, calling NewMock before cleanups that wait for code reading its timers or tickers, and then gives the reason. "also after Reset" is gone: "after that cleanup" already covers a ticker reset then. --- mock.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mock.go b/mock.go index e47dc13..aefd17d 100644 --- a/mock.go +++ b/mock.go @@ -489,10 +489,10 @@ func (m *Mock) WithLogger(l Logger) *Mock { // You may re-set the time earlier than this, but only before timers or tickers // are created. // -// A timer value or tick that has not been received when the cleanup registered by NewMock runs is -// dropped. After that cleanup, a timer from NewTimer or a ticker from NewTicker that fires, also -// after Reset, sends only to a reader already waiting on C. Call NewMock before registering -// cleanups that wait for code reading the Mock's timers or tickers. +// Call NewMock before registering cleanups that wait for code reading the Mock's timers or +// tickers: NewMock's cleanup drops any timer value or tick that has not been received. After that +// cleanup, a timer from NewTimer or a ticker from NewTicker sends only to a reader already waiting +// on C. func NewMock(tb TestingT) *Mock { cur, err := time.Parse(time.RFC3339, "2024-01-01T00:00:00Z") if err != nil { From 36e05d628bf906814f02b50613369827e90d5c8a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:32:05 +0000 Subject: [PATCH 24/28] docs(ticker.go): say that a stopped ticker's Reset still starts runLoop The Reset comment explained why a running ticker is signalled instead of getting a new runLoop, but not that a stopped ticker still gets one, so a stopped ticker created outside a synctest bubble and reset inside it still keeps synctest.Test from returning. --- ticker.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ticker.go b/ticker.go index 795baf6..476bc4a 100644 --- a/ticker.go +++ b/ticker.go @@ -106,9 +106,11 @@ func (t *Ticker) Reset(d time.Duration, tags ...string) { t.startRunLoopLocked() return } - // A tick that runLoop holds for a reader belongs to the old schedule, so runLoop drops it. A - // new runLoop would belong to the caller's synctest bubble and wait on channels created - // outside it, which keeps the bubble from ending. + // Tell runLoop to drop a tick it holds for a reader, since that tick belongs to the old + // schedule. Starting a new runLoop instead would put a goroutine in the caller's synctest + // bubble that waits on channels created outside it, and synctest.Test would not return. The + // branch above still starts one for a stopped ticker, so a Reset inside a bubble of a stopped + // ticker created outside it hangs. t.resets <- struct{}{} } From 81398ed6469ada7e10cad061de80d741b05e4fa5 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:33:52 +0000 Subject: [PATCH 25/28] test(mock_test.go): keep one case in TestTicker_ResetInBubble The TickedUnread row checked for a stale tick after Reset, which TestTicker_StopAndReset/ResetTickedUnread already covers, so the test is now the Running case alone. Its receive now gives up when the context ends, so a missing tick fails the test instead of waiting for go test -timeout. With 697264f's ticker.go, which starts a new runLoop on Reset, the test still hangs until go test -timeout with its synctest.Test call in the stack. --- mock_test.go | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/mock_test.go b/mock_test.go index 5fa9d3b..a267009 100644 --- a/mock_test.go +++ b/mock_test.go @@ -868,32 +868,18 @@ func TestTickerReset_NonPositiveDuration(t *testing.T) { // test's synctest.Test call in the stack. func TestTicker_ResetInBubble(t *testing.T) { t.Parallel() - for _, tc := range []struct { - name string - tick bool - }{ - {name: "Running"}, - // runLoop holds the tick for a reader when Reset is called. - {name: "TickedUnread", tick: true}, - } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() - mClock := quartz.NewMock(t) - tkr := mClock.NewTicker(time.Second) - defer tkr.Stop() - if tc.tick { - mClock.Advance(time.Second).MustWait(ctx) - } - synctest.Test(t, func(*testing.T) { tkr.Reset(time.Hour) }) - mClock.Advance(time.Hour).MustWait(ctx) - // A tick kept from the old schedule would be received first. - if got, want := <-tkr.C, mClock.Now(); !got.Equal(want) { - t.Errorf("received the tick for %s, want the one for %s", got, want) - } - }) + mClock := quartz.NewMock(t) + tkr := mClock.NewTicker(time.Second) + defer tkr.Stop() + synctest.Test(t, func(*testing.T) { tkr.Reset(time.Hour) }) + mClock.Advance(time.Hour).MustWait(ctx) + select { + case <-tkr.C: + case <-ctx.Done(): + t.Fatal("timed out waiting for the tick after Reset") } } From fddb3d8be1d223d82665b5ee7dc7d2e4fdad8bdc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:34:22 +0000 Subject: [PATCH 26/28] test(mock_test.go): drop Stop calls that NewMock's cleanup covers NewMock's cleanup ends every ticker's runLoop, so the Stop at the end of TestTicker_ResetInBubble and of each TestTicker_StopAndReset row did nothing that the leak check in TestMain would miss. --- mock_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/mock_test.go b/mock_test.go index a267009..27c76e2 100644 --- a/mock_test.go +++ b/mock_test.go @@ -873,7 +873,6 @@ func TestTicker_ResetInBubble(t *testing.T) { mClock := quartz.NewMock(t) tkr := mClock.NewTicker(time.Second) - defer tkr.Stop() synctest.Test(t, func(*testing.T) { tkr.Reset(time.Hour) }) mClock.Advance(time.Hour).MustWait(ctx) select { @@ -931,7 +930,6 @@ func TestTicker_StopAndReset(t *testing.T) { if next, _ := mClock.Peek(); next != tc.wantNext { t.Errorf("ticker ticks in %s, want %s", next, tc.wantNext) } - tkr.Stop() }) }) } From ea4df235427710c5af7b960df4ae4c272a622d2e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 17:34:35 +0000 Subject: [PATCH 27/28] test(mock_test.go): put TestTickerReset_NonPositiveDuration last It now follows TestTicker_StopAndReset, so the tests of a ticker's Stop and Reset results come before the test of Reset's argument check. --- mock_test.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/mock_test.go b/mock_test.go index 27c76e2..8669872 100644 --- a/mock_test.go +++ b/mock_test.go @@ -843,25 +843,6 @@ func TestNoLeak_Go123(t *testing.T) { }) } -// TestTickerReset_NonPositiveDuration checks that Reset panics for a duration that is not positive, -// as time.Ticker.Reset and NewTicker do. A mock ticker with such a period made the next fire loop -// forever with the Mock's lock held. -func TestTickerReset_NonPositiveDuration(t *testing.T) { - t.Parallel() - for _, d := range []time.Duration{0, -time.Second} { - t.Run(d.String(), func(t *testing.T) { - t.Parallel() - tkr := quartz.NewMock(t).NewTicker(time.Second) - defer func() { - if recover() == nil { - t.Errorf("Reset(%s) did not panic", d) - } - }() - tkr.Reset(d) - }) - } -} - // TestTicker_ResetInBubble resets a running ticker from a synctest bubble that did not create its // Mock. Reset must start no goroutine there: one waiting on the Mock's channels, which belong to no // bubble, keeps synctest.Test from returning, and go test -timeout then stops the run with this @@ -935,6 +916,25 @@ func TestTicker_StopAndReset(t *testing.T) { } } +// TestTickerReset_NonPositiveDuration checks that Reset panics for a duration that is not positive, +// as time.Ticker.Reset and NewTicker do. A mock ticker with such a period made the next fire loop +// forever with the Mock's lock held. +func TestTickerReset_NonPositiveDuration(t *testing.T) { + t.Parallel() + for _, d := range []time.Duration{0, -time.Second} { + t.Run(d.String(), func(t *testing.T) { + t.Parallel() + tkr := quartz.NewMock(t).NewTicker(time.Second) + defer func() { + if recover() == nil { + t.Errorf("Reset(%s) did not panic", d) + } + }() + tkr.Reset(d) + }) + } +} + func TestMain(m *testing.M) { verifyNoLeakTestMain(m) } From 95527a4d68f8edd6faeea6a9b9edbe1f5fd1a857 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 4 Nov 2026 18:10:46 +0000 Subject: [PATCH 28/28] docs(ticker.go): reword which Reset of a stopped ticker hangs The sentence named the stopped ticker twice. It now says once that the hang needs Reset called inside a bubble on a stopped ticker created outside that bubble. --- ticker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ticker.go b/ticker.go index 476bc4a..3600ba8 100644 --- a/ticker.go +++ b/ticker.go @@ -109,8 +109,8 @@ func (t *Ticker) Reset(d time.Duration, tags ...string) { // Tell runLoop to drop a tick it holds for a reader, since that tick belongs to the old // schedule. Starting a new runLoop instead would put a goroutine in the caller's synctest // bubble that waits on channels created outside it, and synctest.Test would not return. The - // branch above still starts one for a stopped ticker, so a Reset inside a bubble of a stopped - // ticker created outside it hangs. + // branch above still starts one, so Reset, called inside a bubble, of a stopped ticker created + // outside that bubble, hangs. t.resets <- struct{}{} }