Description
What happened:
Consider the following code (playground):
package main
import (
"time"
"k8s.io/utils/clock"
clocktesting "k8s.io/utils/clock/testing"
)
func main() {
clk := clocktesting.NewFakeClock(time.Now())
go func(clk clock.Clock) {
clk.Sleep(time.Second * 2)
println("after 2 fakeclock seconds")
}(clk)
time.Sleep(time.Millisecond) // used to emulate race condition
clk.Sleep(time.Second)
println("after 1 fakeclock second")
}The code above prints
after 2 fakeclock seconds
after 1 fakeclock second
When clk is changed to clock.RealClock{} (playground), the println lines are in correct order (1 -> 2).
What you expected to happen:
Intuitively, one would expect clk.Sleep(duration) to be identical to <-clk.After(duration). However, (*FakeClock).Sleep actually calls clk.Step instead, which immediately modifies the clock.
Suggested fix:
It would be a huge BC break to many testing packages if (*FakeClock).Sleep is changed to be passive. Documentation improvement would suffice, although deprecating Sleep would be great since Sleep doesn't always do what users intend. (imo, clk.Step is usually only intended in the main control flow of the unit test rather than other goroutines, so calling clk.Sleep from non-testing code is often unintended).
Alternatively, consider adding a ref-counted clock, where Sleep and After have identical behavior and the clock is only stepped when all refs are sleeping.
Environment:
k8s.io/utils: fe8a2dd