-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FakeClock.Sleep calling FakeClock.Step may be unexpected #309
Comments
The Kubernetes project currently lacks enough contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
What happened:
Consider the following code (playground):
The code above prints
When
clk
is changed toclock.RealClock{}
(playground), theprintln
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 callsclk.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 deprecatingSleep
would be great sinceSleep
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 callingclk.Sleep
from non-testing code is often unintended).Alternatively, consider adding a ref-counted clock, where
Sleep
andAfter
have identical behavior and the clock is only stepped when all refs are sleeping.Environment:
k8s.io/utils: fe8a2dd
The text was updated successfully, but these errors were encountered: