jonboulle / clockwork Goto Github PK
View Code? Open in Web Editor NEWa fake clock for golang
License: Apache License 2.0
a fake clock for golang
License: Apache License 2.0
BlockUntil currently works for blockers like Timers, but not Tickers. Would it be possible to make Tickers a blocker?
Is there an option to make it so that the callbacks from timers are invoked synchronously instead of in a goroutine? Essentially, remove go
from here:
Line 43 in 8a29bc1
My issue with the current method is demonstrated by this workaround https://github.com/temporalio/temporal/blob/15974517a5ea2987a7f9a02afefc423418ba658a/service/matching/liveness_test.go#L44
Because Advance
is not synchronous, and we are trying to verify that the callback is not fired, there is no event for us to actually wait on in the test, so we have to resort to a time.Sleep
.
The docs don't sufficiently explain the behavior of BlockUntil, at least when it comes to using Timers and Tickers.
I've found the behavior of BlockUntil for After and Sleep to be intuitive due to their blocking nature.
I'd be pleased to take a stab at contributing these improvements, but I wanted to check-in first to get a read on the attitude of the maintainers towards both the behavior of BlockUntil, and general documentation philosophy.
Here are the existing references to BlockUntil in the docs:
// BlockUntil blocks until the FakeClock has the given number of waiters.
Users can call BlockUntil to block until the clock has an expected number of waiters.
Here are the existing references to waiters in the docs:
// Advance advances the FakeClock to a new point in time, ensuring any existing
// waiters are notified appropriately before returning.
FakeClock maintains a list of "waiters," which consists of all callers waiting on the underlying clock (i.e. Tickers and Timers including callers of Sleep or After).
When I was recently using BlockUntil with Timer, I discovered that creating a NewTimer immediately created a waiter, rather than say, the call to the Chan. When I went to the docs to understand more, it was hardly discussed.
Here's some example code that I initially found unexpected, which I'd like the documentation to explain:
fakeClock := clockwork.NewFakeClock()
timer := fakeClock.NewTimer(5 * time.Second)
fakeClock.BlockUntil(1) // does not block because NewTimer created a waiter
Similarly, explaining how Stop deregisters a waiter, and Reset reregisters it again.
Again, for Tickers I haven't tested it, but my understanding is that a Ticker behave similarly wrt to Stop and Reset.
Doing all of the following may be overkill, but here are some ideas:
I wrote a simple function that executes a certain piece of code at a specified interval.
func ScheduledTask(c clockwork.Clock) {
ticker := c.NewTicker(1 * time.Hour)
go func() {
defer ticker.Stop()
for range ticker.Chan() {
fmt.Printf("responsed\n")
}
}()
}
I referred to the Readme and wrote a unit test like this.
func TestScheduledTask(t *testing.T) {
fakeClock := clockwork.NewFakeClock()
var wg sync.WaitGroup
wg.Add(1)
go func() {
ScheduledTask(fakeClock)
wg.Done()
}()
fakeClock.BlockUntil(1)
for i := 0; i < 24; i++ {
fakeClock.Advance(1*time.Hour + 1*time.Second)
//time.Sleep(1 * time.Millisecond)
}
wg.Wait()
}
The program cannot produce the expected output.
=== RUN TestScheduledTask
--- PASS: TestScheduledTask (0.00s)
PASS
But when I set a delay of one millisecond for each loop, the output of the program is as follows
=== RUN TestScheduledTask
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
--- PASS: TestScheduledTask (0.03s)
PASS
Is this a bug or a usage issue?
GitHub workflows run with write-all permissions. This makes repositories vulnerable to supply-chain attacks.
Given clockwork' workflows are for SAST and unit testing, they don't require such broad permissions. I'd therefore like to help the project close this vulnerability.
This can be done in two ways:
I'll send a PR along with this issue setting the top-level permissions on the workflows.
Or, if you'd rather (or also wish to) change the repository's default settings:
Disclosure: My name is Pedro and I work with Google and the Open Source Security Foundation (OpenSSF) to improve the supply-chain security of the open-source ecosystem.
Using NewFakeClock() to make a Clock at a specific point in time requires a custom implementation (since fakeClock is internal) or calling Advance while depending on the internal detail that the clock starts in 1900. Is this within the desired project scope?
I previously considered this package for some testing which depended on a specific timestamp for validations.
I found a large number of authors are writing tests that depend on the deterministic time returned by NewFakeClock
. I found this out the hard way when I upgraded us to #52, which changed this default.
I believe using a deterministic time when none is specified is an anti-pattern. If a test requires a deterministic time, authors should use NewFackClockAt
to make the timestamp requirement explicit. This is similar to how Go's map iterating order is non-deterministic.
I made #55 as a suggestion, which uses a random time from the year 1970 (excluding the Unix epoch time), but I am open to other thoughts/suggestions.
Hi,
fc := clockwork.NewFakeClock()
ft := fc.NewTicker(1)
fc.Advance(3)
select {
case x := <-ft.Chan():
fmt.Println("1-------------", x)
default:
}
I am expecting have 3 print in above code, but it only print once, looks like the ticker is only triggered once, how can I trigger multiple times?
I know that the real clock doesn't guarantee that timers will go off exactly on time, but in practice they go off somewhat close to on schedule.
I just created a pull request to add AfterFunc
. I noticed when testing it that I can advance the clock well past when a timer is supposed to fire. The timer fires, but it thinks the time is well past when it should be.
So then, the question is: should the clock pause for real time as it advances past timers? I think that for AfterFunc
, it should be willing to wait for f()
to return or for one real second to pass, whichever comes first.
I don't think any of the other APIs provide any way to know how long to pause.
According to the golang documentation for time.Sleep, when passed a zero or negative duration, the function returns immediately.
FakeClock correctly handles a zero duration, but ends up blocking for negative durations, thus requiring the clock to be advanced by zero to unblock the sleeper (which should never have been blocked to begin with).
to allow a code like this
t := s.clock.NewTicker(s.config.InitialDelay)
var resetOnce sync.Once
defer t.Stop()
for {
select {
case <-s.newSlotCh:
t.Reset(s.config.InitialDelay)
case <-t.Chan()():
resetOnce.Do(func() { t.Reset(s.config.PeriodicDelay) })
Hello!
First - thank you for the great package! It has been very helpful.
My coworkers and I wrapped the package and added some context helper methods. Was curious if you would be interested in a PR that would add these helpers to the package.
Can check out the code: https://gist.github.com/connyay/917cb5c6e5d43d08df8bb6a076e5dab9
I can open a PR next week if there is interest.
for this - change clockwork.Clock type from interface to struct or pointer with call system time functions by default (for zero value).
It allow make my own types usable with zero value without wrap time calls in my code.
Hi, thanks for the nice package.
I was trying to fix some flaky tests by converting them to use this. However, the tests use Context.WithTimeout
and it looks like the fake clock, even if injected into a context, is not respected by that code?
I can imagine how that might be impossible to fix without core library changes but it might be nice to document it to save people time?
Fast submission just to get this on record:
Test clockwork/clockwork_test
: FAILED in 114 out of 1000 in 13.3s
at commit 79f9aa2:
TestFakeTicker_DeliveryOrder
: fails in 113 out of 1000 runs
third_party/golang/clockwork/ticker_test.go:119: Expected ticker event didn't arrive!
TestFakeTicker_DeliveryOrder: fails 1 out of 1000 runs
third_party/golang/clockwork/ticker_test.go:45: expected tick!
third_party/golang/clockwork/ticker_test.go:55: wrong tick time, got: 0001-01-01 00:00:00.000000001 +0000 UTC, want: 0001-01-01 00:00:00.000000003 +0000 UTC
These issues are not present in v0.3.0, commit 6a247c3
:
10000 runs: clockwork:clockwork_test PASSED in 18.4s
Hey, it's Pedro and I'm back (see #75) with another security suggestion.
A security policy (SECURITY.md) explains how the project wishes to receive and handle responsible disclosure of potential vulnerabilities. GitHub recommends that projects have one.
The security policy can be found by users who enter the project's "Security" panel. They'll also see references to it in the "New issue" page.
There are two main ways to receive disclosures:
If you want to use GitHub's reporting system, it must be activated for the repository:
I'll send a PR with a draft policy along with this issue.
As per #82, BlockUntil blocks on the number of active waiters, and for Timers/Tickers this means construction. Stop removes the waiter, and Reset adds it again. In practice I've found BlockUntil is insufficient to test code using Timers and Tickers because it can't distinguish between before a call to Stop, and after a call to Reset.
I'm curious if maintainers would be interested in the proposed addition. I've been using this in my test code implemented as a wrapper around clockwork, but supporting it in the library itself would be preferable. Happy to contribute the change.
Imagine you want to write a test for the following state machine:
// Sends an incrementing counter on the counters channel every 3 seconds.
// If a heartbeat isn't received within 5 seconds, exits with an error.
func runStateMachine(
ctx context.Context,
clock clockwork.Clock,
counters chan int,
heartbeats chan struct{},
) error {
counterTimer := clock.NewTimer(3 * time.Second)
defer counterTimer.Stop()
heartbeatTimeout := clock.NewTimer(5 * time.Second)
defer heartbeatTimeout.Stop()
counter := 1
for {
select {
case <-counterTimer.Chan():
counterTimer = clock.NewTimer(3 * time.Second)
counters <- counter
counter++
case <-heartbeats:
heartbeatTimeout.Stop()
heartbeatTimeout = clock.NewTimer(5 * time.Second)
case <-heartbeatTimeout.Chan():
return fmt.Errorf("Timeout at %d", clock.Now().Unix())
case <-ctx.Done():
return ctx.Err()
}
}
}
You'll end up running into a situation where there's no primitive in the clockwork library to ensure that Advance doesn't race with the resetting of a timer:
func TestRunStateMachine_SendingHeartbeatResetsTimeout(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
fakeClock := clockwork.NewFakeClockAt(time.Unix(0, 0))
counters := make(chan int)
heartbeats := make(chan struct{})
done := make(chan error, 1)
go func() {
err := runStateMachine(ctx, fakeClock, counters, heartbeats)
done <- err
}()
// Ensure state machine timers are initialized at time zero before advancing.
fakeClock.BlockUntil(2)
fakeClock.Advance(3 * time.Second)
counter := <-counters
require.Equal(t, 1, counter)
// Sending a heartbeat gives us another 5 seconds, allowing for another counter to be
// produced at 6 seconds in.
heartbeats <- struct{}{}
log.Println("Trigger race by removing this print statement (at least on my machine)")
// PROBLEM: What do we block on here to ensure that the heartbeat timer is reset at time 3
// rather than time 6? We can't use BlockUntil(2) because the condition is already true
// before heartbeatTimeout.Stop() is called.
fakeClock.Advance(3 * time.Second)
counter = <-counters
require.Equal(t, 2, counter)
fakeClock.Advance(2 * time.Second)
err := <-done
require.Error(t, err)
require.Contains(t, err.Error(), "Timeout at 8")
}
Sometimes it's possible to write your system under test such that this race is avoided. In the above example, we reset the counterTimer before sending on the counters channel, ensuring that the reset always happens before the reset. However, in-practice I've found situations where it's just not reasonable to contort your system under test to solve for this race condition. In the above example, the heartbeatTimeout reset is similar to a real world situation I ran into, and ultimately solved with the solution proposed below.
What I've found works quite well, is a BlockUntilMonotonic method on FakeClock which blocks on the monotonically increasing number of watchers the clock has ever seen.
In the example above, the usage would look like this:
heartbeats <- struct{}{}
fakeClock.BlockUntilMonotonic(4)
// Unblocks after the new Timer is created.
In-practice, I've found reasoning about the total number of watchers that have been created over the life of the test to be tedious, so I've created a helper method that avoids me having to count.
seen := fakeClock.CurrentMonotonic()
heartbeats <- struct{}{}
fakeClock.BlockUntilMonotonic(seen + 1)
// Unblocks after the new Timer is created.
I suspect the implementation will be relatively simple: just keep track of the total number of waiters the FakeClock has ever seen, and use a similar blocker primitive to unblock when enough new waiters are seen.
I don't have strong feelings about the naming. So far I've liked BlockUntilSeenSoFar or BlockUntilMonotonic the most as far as naming goes.
Hi @jonboulle,
First of all, thanks for this package, I think it's a quite popular one!
I noticed there are a couple open issues and PRs and I wonder if you need help with maintenance. Either as a contributor to this repo, or if you don't want to maintain it anymore, as a managed fork somewhere (eg. https://github.com/gofrs).
Let me know what you think!
Consider the following code:
func TestFakeClock(t *testing.T) {
c := clockwork.NewFakeClock()
go func(){
c.Sleep(time.Second)
}()
go func(){
c.Sleep(time.Second)
}()
c.BlockUntil(1)
}
Most of the time this code will pass! But non-deterministically (if the goroutines are scheduled at just the right time), the BlockUntil will block forever.
The problem is that numExpected != numActual (2 != 1
), so the BlockUntil call will block forever. I think it should be changed so that it blocks until numExpected <= numActual. Would you accept a PR to change this?
I could also imagine other fixes, like BlockUntil panic-ing if there are more sleepers than numExpected, rather than blocking forever.
fakeClock
does not handle negative durations passed to After()
correctly.
This looks very similar to #19
func TestNegativeTimer(t *testing.T) {
fakeClock := clockwork.NewFakeClock()
timer := time.NewTimer(2 * time.Second)
defer timer.Stop()
select {
case <-fakeClock.After(-1 * time.Second):
// Success
case <-timer.C:
t.Fatalf("Timed out!")
}
}
Output:
$ go test -v -run TestNegativeTimer
=== RUN TestNegativeTimer
--- FAIL: TestNegativeTimer (2.00s)
negativetimer_test.go:18: Timed out!
FAIL
exit status 1
The godoc comments for NewFakeClockAt should refer to that function, rather than NewFakeClock.
Adding go.mod and go.sum is nice, but more important (and simpler) is tagging new version to include "recent" code changes. Without it go take version 0.10 which is quite old.
I discovered a rather nasty little race condition (technically several) in the fakeTicker. It's in the fakeTicker.tick()
function and how it references the fake clock.
This fails pretty much every time I run this, but you can also throw this in a for loop to test it a bunch of times and you'll be pretty much guaranteed to reproduce it.
func TestFakeTicker_Race(t *testing.T) {
fc := NewFakeClock()
tickTime := 1 * time.Millisecond
ticker := fc.NewTicker(tickTime)
defer ticker.Stop()
fc.Advance(tickTime)
timeout := time.NewTimer(500 * time.Millisecond)
defer timeout.Stop()
select {
case <-ticker.Chan():
// Pass
case <-timeout.C:
t.Fatalf("Ticker didn't detect the clock advance!")
}
}
I've found two races so far. Both of them are basically the same, just in two different places.
The very first thing the tick
function does is retrieve the current time from the fake clock for use in the service loop. Unfortunately, when this is actually executed depends on when the ticker goroutine is run by the scheduler. In the test code above, this order of operations can happen:
This race can be solved by passing in the "now" timestamp to the function when the goroutine starts. A simple two line fix to change fakeTicker.tick
to take in tick time.Time
and update the fakeClock.NewTicker
to call go ft.tick(fc.Now())
.
During the service loop in fakeTicker.tick
the remaining time is calculated based off of a call to ft.clock.Now()
. Later on in the loop, ft.clock.After(remaining)
is called. Unfortunately, the clock can be advanced between those two calls resulting in a lost tick:
5ms
)remaining := tick.Sub(ft.clock.Now())
is called (remaining = 5ms
because the clock hasn't advanced yet)5ms
remaining
is greater than 0, so it doesn't fire a tick. It then falls through to the second select
and ultimately calls ft.clock.After(5ms)
even though the clock has already advanced to 5ms
.With the current implementation of FakeClock
updates are lost if more than one is generated at a time.
If, for example, one has a ticker that fires every five minutes and advances the FakeClock
by one hour they will get one tick of five minutes.
This is due to the implementation relying on a lossy channel that is 1-buffered.
My proposal here would be to change this implementation to schedule all needed updates with an unbounded queue.
Simulating time in tests should behave as time should in the actual lifecycle of a program and FakeClock
currently does not (during real execution the code has time to consume the messages, they don't get sent all at once)
One other possibility would be to allow users to specify a buffer size for tickers when the fake clock is constructed (it's reasonable to assume who is writing the test might know an approximation of the amount of ticks they expect).
Note: In case you are interested in such a change I can send a PR for it.
Would be great to see clockwork implement more features of the time
library.
I noticed that there appears to be a race condition in the fake timer implementation. If Reset
gets called multiple times then apparently it can schedule multiple channel events. I will upload a commit demonstrating the issue.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.