Skip to content
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

Add support for fake tickers #8

Merged
merged 4 commits into from Jul 13, 2018
Merged

Add support for fake tickers #8

merged 4 commits into from Jul 13, 2018

Conversation

tflach
Copy link
Contributor

@tflach tflach commented Aug 3, 2016

I spend a little time on extending your library to add support for fake tickers.
Currently this requires surrounding the Advance() calls with BlockUntil() to make sure that the ticker can go to sleep before Advance() is called and afterwards wake up before we start consuming ticks.
Let me know what you think.

@tflach
Copy link
Contributor Author

tflach commented Aug 16, 2016

Hello Jon,
did you have a chance to take a look at this?

Copy link

@pierrebeaucamp pierrebeaucamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me. It seems to be missing a time.Tick(d) function, but that is only a wrapper around NewTicker(d).Chan() at this point.

@@ -12,6 +12,7 @@ type Clock interface {
Sleep(d time.Duration)
Now() time.Time
Since(t time.Time) time.Duration
NewTicker(d time.Duration) Ticker
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically not following the exact definition of NewTicker from the stdlib. There it is func NewTicker(d Duration) *Ticker. But I don't know if this is a concern

edit: I just realized that Ticker is an interface. Sorry about the confusion

@pierrebeaucamp
Copy link

I think that the BlockUntil() requirement could be resolved by #12
Also, I couldn't help but notice that the standard library implements tickers using timers, which might get added in #13.

@pierrebeaucamp pierrebeaucamp mentioned this pull request Jul 12, 2018
@jonboulle jonboulle merged commit df1a715 into jonboulle:master Jul 13, 2018
@jonboulle
Copy link
Owner

LGTM. Sorry about the horrendous delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants