time: mockable time support
jmhodges opened this issue · 30 comments
With https://github.com/jmhodges/clock and the broader https://github.com/benbjohnson/clock API, there's a desire for the timing systems in Go to be fakeable[1]. Using a fake clock instead of time.Now directly is useful when testing code that stores timestamps, caches data, etc. The benbjohnson clock package attempts to make Ticker and Timer calls (as well as their related AfterFunc and Sleep methods) work against a fake time that can be set and updated in tests. However, it relies on some micro-sleeps and runtime.Gosched calls that are obviously going to be flaky. But there is a desire to able to test code that uses Tickers and Timers, not by adjusting the durations they work for (which can induce flaky testing), but by adjusting when they think they need to wake up. To do that, we seem to need more runtime magic to help developers out. In fact, it might be best if a clock package like these lived in the stdlib so that it could be tied more carefully and thoughtfully to the scheduler. [1] or "mockable", whatever language you prefer. The summary of this issue is to distinguish it from issue #5356.
I'm not entirely sure this is a good idea. I'm also not sure what it would look like. If the goal is simply to provide Ticker and Timer calls for testing, I think that could be done entirely independently of the time package, along the lines of the old playground code. The testing package would keep its own queue of events, and step forward to the next event without actually waiting.
Labels changed: added repo-main, release-none.
Any chance of something happening for this?
(I'm okay with hearing no! It's just an open cycle in my head I'm trying to clear out.)
It's unlikely to happen any time soon.
One possible step toward this, for the common time.Now() case, would be to add two exported symbols to the time package:
type Clock interface {
Now() time.Time
}
var DefaultClock = /* an (unexported?) default implementation of Clock */
Then the existing time.Now() function (and possibly others) would become a de facto alias for time.DefaultClock.Now().
Whether the ticker/timer implementations could take advantage of such a Clock interface, or if it would need to be expanded, I don't know.
The primary motivation here, of course, is to provide a standard place to mock out a clock, while honoring the Go compatibility promise. It would, obviously, require altering any existing code that needs mocks, but that's already happening, so this would just provide a defined, supported way to do such mocks.
One other (possible) implication of such a Clock interface, is that it would be possible to alter the default timezone from the system default, by configuring a custom Clock instance. There may be other non-mocking uses as well, which I haven't considered yet.
Having a caller-replaceable time.DefaultClock sounds both racy and like a step in the wrong direction. If this is about testability, we should live in a world where tests are parallelizable and don't muck with globals. (I'm ok with eating the costs of needing to write code to be testable; that's going to be true anyway, regardless of this yet another reason to write "hermetic" libraries.)
My reading of this issue is to provide better means for such a mock-clock library to simulate the effects of time passing "until next interesting event" in the scheduler.
Having a caller-replaceable time.DefaultClock sounds both racy
Perhaps so, but it's a pattern commonly used elsewhere in the Go standard library.
If this is about testability, we should live in a world where tests are parallelizable and don't muck with globals.
Code which relies on this functionality for testing shouldn't mess with globals (the same holds for the other places in the stdlib where this is done), and should instead use dependency injection.
The goal isn't to make it possible to modify time.DefaultClock in tests, but rather to provide a standard way to inject a clock into code that needs to be tested, while allowing legacy code, or code which doesn't need to be tested, to rely on the time.DefaultClock implementation.
I think of this as the same as http.DefaultClient. While it's possible to override http.DefaultClient in tests, I've never seen anyone advocate this. I wouldn't advocate this for time.DefaultClock, either.
Note that the suggestion of var DefaultClock could easily be removed, and my proposal would still satisfy my purpose, without adding the "scary" race opportunities. It just wouldn't be as transparent (I suppose) that the package level Now() is the equivalent of using a default implementation of the Clock interface.
Perhaps so, but it's a pattern commonly used elsewhere in the Go standard library.
If you dig around the issues enough, you will find core devs sometimes expressing regret about those decisions. Just because it was done in the early days doesn't mean it should be the standard going forward.
If you dig around the issues enough, you will find core devs sometimes expressing regret about those decisions.
That's fair. I've often wondered about the wisdom of that myself.
But I still don't think that renders my (entire) suggestion untenable. I'd welcome your feedback on the remainder of my proposal. Specifically on the Clock interface idea.
For the sake of discussion, the DefaultClock can be turned into a function which returns a default implementation. This eliminates the user-replaceable, racy aspects.
I feel like this conversation got derailed by this (IMO, minor) aspect of my proposal. I'd rather address the core of it.
I think the two existing & used libraries mentioned in the first comment are plenty to inform the API design; as far as I understand the challenge here is purely the integration with runtime. See mentions of "flaky".
Yeah, my first paragraph in this ticket was really to set up the latter two that describe why we want and what needs stdlib or runtime support to write better time-dependent tests. Clock interfaces are easy to implement outside of the stdlib and runtime (there's been a proliferation of these beyond just the packages listed now) but the real trick is giving people the ability to test code that involves Tickers and Timers.
There are likely lots of levels of ideas to address that Ticker and Timer problem.
One is to give Tickers and Timers new constructor funcs that take some kind of expanded Clock interface that can inform the Tickers and Timers when to run (instead of having them burn a bunch of CPU checking it in a loop). This would be similar to the addition of Context functions to database/sql.
This ticket pre-dates the Go proposal mechanisms and we might be getting to the time for a formal proposal to resolve this one.
It would be cool to hear ideas different from the rough sketch I gave above. I'm fairly sure there is one.
When this issue was filed, context.Context was not in the standard library; Context was added in Go 1.7 in 2016.
I like the idea of attaching mocks to the context. It may be considered a non-starter for some people as an abuse of context.
The following assumes this is reasonable:
Here's a precedent based on attaching a Clock interface on a context.Context. This is not racy and permits very well scoped tests:
- time replacement package: https://godoc.org/go.chromium.org/luci/common/clock#Clock
- time mock package: https://godoc.org/go.chromium.org/luci/common/clock/testclock
main.go
package main
import (
"context"
"fmt"
"go.chromium.org/luci/common/clock"
)
func getFormattedTime(ctx context.Context) string {
clk := clock.Get(ctx)
return clk.Now().String()
}
func main() {
fmt.Println(getFormattedTime(context.Background()))
}
main_test.go
package main
import (
"context"
"fmt"
"time"
"go.chromium.org/luci/common/clock/testclock"
)
func ExampleGetFormattedTime() {
fixed := time.Date(2006, 1, 2, 15, 4, 5, 6, time.UTC)
ctx, tm := testclock.UseTime(context.Background(), fixed)
fmt.Println(getFormattedTime(ctx))
later := fixed.Add(time.Hour)
tm.Set(later)
fmt.Println(getFormattedTime(ctx))
// Output:
// 2006-01-02 15:04:05.000000006 +0000 UTC
// 2006-01-02 16:04:05.000000006 +0000 UTC
}
IMHO this is quite clean. I'm not sure the Clock interface here should be used as-is. I'm not sure it's a good idea to pass a context in to Sleep and NewTimer. Still, I think something inspired by this could be added to the standard library without any regression.
up you go
is this going to ever happen or not?
@ivanjaros As far as I know nobody is actively working on this.
salutations from 2022. almost 2023. any news on this ? There is an interesting piece on this issue here:
https://dmitryfrank.com/articles/mocking_time_in_go
The author says that atleast a way to wait for all the goroutines to run would still improve the situation.
If something changes, people will comment here. Nothing has changed.
@ianlancetaylor any suggestion about what it would take for something to change ? Would it be worth it to do a proposal for the new review process ?
For reference, I've been using benbjohnson/clock too, and it works fairly well for all my needs - and those of many others too, judging by the star count - but it would be nice to have an idea of where stdlib is going or not. IMHO even a definite "won't happen" would be more useful than having this issue remain in limbo.
A good first step would be to write down exactly what an API to address this issue would look like. That would likely become a proposal at some point, but it doesn't need to start as a proposal.
I think it's clear that the core Go team is not going to work on this issue. But that in itself doesn't mean that this issue should be closed, because it is possible that someone else will develop a workable approach that could be incorporated into the standard library. At least, that doesn't seem to me to be clearly impossible.
So it seems to me that if you are concerned that this issue is in limbo, you should simply assume that it will never be addressed. But if somebody wants to address it, please feel free to give it a try.
Alright, since I got nerd-sniped by this, here are two fleshed-out proposals. I'd love to hear feedback on them both from the community and from core devs.
Standard Interface Proposal
Add a new interface type, Clock, which requires a set of methods equivalent to the current top-level functions in the time package. Test helper packages can provide fake clock implementations that match the Clock interface, and packages which wish to use such helpers can design their functions/structs to take/store values matching the Clock interface. This is inspired by jmhodges’ fakeclock.
type Clock interface {
func Now() Time
func After(d Duration) <-chan Time
func Sleep(d Duration)
func Since(t Time) Duration
func Until(t Time) Duration
func NewTicker(d Duration) Tocker
func AfterFunc(d Duration, f func()) Tocker
func NewTimer(d Duration) Tocker
}Additionally, a new non-exported type clock, which matches the Clock interface, and which is returned by NewClock(). This type can use the existing package-level functions to implement all of its methods.
type clock struct {}
func NewClock() Clock { return &clock{} }
func (c *clock) Sleep(d Duration) { Sleep(d) }
// etcNote that the NewTicker, AfterFunc, and NewTimer methods above all return a Tocker (name to be bikeshedded). This is also a new interface type, nearly already satisfied by the existing Ticker and Timer concrete types. This allows test helper packages to additionally provide fake tickers and timers that send messages on the channel returned by the Listen() method.
type Tocker interface { // name to be bike-shedded
func Listen() <-chan Time
func Reset(d Duration) bool
func Stop() bool
}Finally, add .Listen() methods to the existing Ticker and Timer types which return their .C member, to satisfy the Tocker interface.
I believe this plan is fully backwards-compatible: only new types and methods are added to the time package, no existing exported symbols are changed.
I believe this accomplishes the major testing goals, by allowing test helper packages to provide a variety of fake tickers and timers which implement a stdlib-supported interface (Tocker). For example, a FastClock’s NewTicker method could return a Tocker which sends a message on its channel 100x faster than a traditional ticker would. Or a ManualClock could expose a SendTick method, which causes any Tocker returned by its NewTicker method to send a message on its channel. Or an AutoClock could expose an Advance method which moves its own internal sense of time forward, and causes any Tockers returned by its NewTicker and NewTimer methods to send messages on their channels, if time was advanced far enough forward for them to fire.
This approach has two major disadvantages. First, it massively increases the API surface of the time package by defining two new interfaces. Second, it requires all code which currently calls (e.g.) time.Sleep() to instead carry a Clock around and call c.Sleep(). Although there is lots of code (e.g. any code which already uses jmhodges’ library) which does this, it would become a requirement for any new code that wants to adopt this system.
Customizable Time Proposal
Add two new methods, NewTimerCustom and NewTickerCustom, which replace the Timer/Ticker’s underlying runtimeTimer with something that can be controlled externally (i.e. by the test code, to manipulate the code-under-test).
type RuntimeTimer interface { // name to be bike-shedded
func Start(c chan<- Time)
func Stop() bool
func Reset(d Duration) bool
}
func NewTimerCustom(d Duration, r RuntimeTimer) *Timer {
c := make(chan Time, 1)
t := &Timer{
C: c,
r: r,
}
t.r.Start(c)
return t
}
// etcUpdate the existing Ticker and Timer implementations to use this interface rather than runtimeTimer directly. For example:
type Timer struct {
C <-chan Time
r RuntimeTimer
}
type runtimeTimerWrapper struct {
inner runtimeTimer
}
func (rt runtimeTimerWrapper) Start(c chan<- Time) { rt.inner.arg = c; startTimer(&rt.inner) }
func (rt runtimeTimerWrapper) Stop() bool { return stopTimer(&rt.inner) }
func (rt runtimeTimerWrapper) Reset(d Duration) bool { return resetTimer(&rt.inner, d) }
func NewTimer(d Duration) *Timer {
c := make(chan Time, 1)
t := &Timer{
C: c,
r: runtimeTimerWrapper{
inner: runtimeTimer{
when: when(d),
f: sendTime,
},
},
}
t.r.Start(c)
return t
}
func (t *Timer) Stop() bool {
if t.r == nil {
panic("time: Stop called on uninitialized Timer")
}
return t.r.Stop()
}
// etcI believe this plan is fully backwards-compatible: although it modifies existing types and functions, it only touches their non-exported runtimeTimer implementation details.
I believe this accomplishes most of the testing goals, by allowing test helper packages to provide a variety of RuntimeTimer implementations that can send messages on their output channel without waiting for the system clock to advance. For example, there could be a ManualRuntimeTimer which exposes a .Tick(t time.Time) method, and sends that time on its channel every time that method is called. Or there could be an AutoRuntimeTimer which uses a channel to receive updates from a fake clock, and sends messages on its channel when time advances forward.
The biggest disadvantage of this approach is that it may be difficult for testing code to supply custom RuntimeTimers to code-under-test. For example, the code under test may propagate a time.Duration value many layers deep in a call stack before finally calling time.NewTicker(d). Replacing that with time.NewTickerCustom would require plumbing the test-only RuntimeTimer implementation through that whole call stack as well. Perhaps the best solution to this problem is the approach taken by jmhodges’ package, where the code under test uses a custom Clock interface type, which can be implemented either by a “real'' clock or a fake one, and for the fake clock’s NewTicker method to call NewTickerCustom behind the scenes. This resembles the Clock interface in the first proposal above, but the definition of that interface would be left to individual test helper packages.
Another approach to solving this problem would be to allow there to be a global variable of type RuntimeTimer which defaults to the runtimeTimerWrapper, and to allow test code to replace it with a custom RuntimeTimer. The time.NewTicker and time.NewTimer functions would automatically use the current global value. This would allow the code under test to be completely agnostic of the fact that its time is being messed with, at the cost of introducing a global.
Overall, I prefer the second proposal. I believe it is a smaller and more elegant change, that more directly reflects the idea put forward by @jmhodges in this comment: #8869 (comment)
@aarongable I'm surprised to not see Now() in either of the proposals. Was this an oversight, or is there a reason so obvious I'm not seeing it without my coffee?
While the second version is less intrusive, the first version appears to be more general, and looks a lot like the clock.Clock interface in benbjohnson/clock, mostly missing Clock.Now, Clock.Sleep, the With(Deadline|Timeout) functions, and changing the clock.Timer name to clock.Tocker. Any reason to omit them, regardless of the actual implementation in that package ?
@flimzy @fgm Thank you both, Clock.Now() was omitted by an accidental copy-paste error. I've updated the post above to include it.
@fgm The post above already includes Clock.Sleep(). The Clock.WithDeadline() and Clock.WithTimeout() methods are solely part of benbjohnson's package, not part of the time package's public interface, so I didn't introduce them here.
I have a couple of concerns/comments about the "Standard Interface Proposal":
- The duration-based functions (
After,Sleep,NewTicker,AfterFunc, andNewTimer) seem like a separate class of function than those based on absolute time. Is it worth creating two interfaces? I think the desire to mock/replace these duration-based functions are exceedling rare, and requiring every implementation to provide them may be onerous. - Further, IME, seems that
time.Now()is far and above the most commonly mocked/replaced function. It might be nice to have that in its own single-method interface for ease of use.
Further expanding on point 2 above, Until and Since are really just extensions of Now(), which suggests that there could be a type that takes a interface { Now() Time } as an argument, and produces a interface { Now() Time; Since(Time) Duration; Until(Time) Duration }. Although that would likely clutter the stdlib too much.
For whomever is following this, Ben B. Johnson archived the repository mentioned in the original issue but https://github.com/jonboulle/clockwork (which is about the same age, but more active) has emerged as another alternative to provide ideas for possible evolutions of the new "Standard API" proposal.