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

proposal: Eventually checker #121

Open
rogpeppe opened this issue Jan 18, 2022 · 3 comments
Open

proposal: Eventually checker #121

rogpeppe opened this issue Jan 18, 2022 · 3 comments

Comments

@rogpeppe
Copy link
Contributor

rogpeppe commented Jan 18, 2022

Here's an idea for an API to make it easier to wait for a given condition to become true:

package quicktest

import "github.com/rogpeppe/retry"

// Eventually calls its provided function repeatedly,
// passing its returned value to the checker c until the checker
// succeeds.
//
// The retry argument governs how often the function is
// called; if it's nil, the default strategy is to use an exponential
// backoff that times out after about 5s.
//
// For example:
//
//	qt.Assert(t, func() int64 {
//		return atomic.LoadInt64(&foo)
//	}, qt.Eventually(qt.Equals, nil), int64(1234))
func Eventually(c Checker, retry *retry.Strategy) Checker

// EventuallyStable is like Eventually except that it also runs the checker
// for a time after the checker succeeds to make sure that it doesn't
// fail again. The stableRetry argument governs how that is done;
// the default is to run for about 100ms.
//
// In general stableRetry should complete in a much shorter
// period of time than retry because it will always be run to completion.
func EventuallyStable(c Checker, retry, stableRetry *retry.Strategy) Checker
@mvdan
Copy link
Contributor

mvdan commented Jan 18, 2022

For both APIs, my mind immediately goes to: you mention the default limits (5s and 100ms), but not the initial frequency at which the tries are done. Does it start at every 1ms? 100ms?

// In general stableRetry should complete in a much shorter
// period of time than retry because it will always be run to completion.

This sentence confused me at first; I wasn't sure if you meant that each check try should complete fast, or that the entire retry strategy should have a shorter total duration. I think you mean the latter; perhaps best to be explicit, like:

// In general, stableRetry should have a smaller maximum duration than retry
// because it will always be run to completion.

@mvdan
Copy link
Contributor

mvdan commented Jan 18, 2022

For anyone stumbling upon this, here's some context on the retry API: https://pkg.go.dev/github.com/rogpeppe/retry

@frankban
Copy link
Owner

frankban commented Jan 19, 2022

This looks great!

It would be nice to try to improve how the strategy is provided.

One option is to make Eventually return a concrete type implementing Checker (rather than Checker itself), this concrete type having a WithStrategy(s retry.Strategy) Checker method.
So, in most cases, we can just write

qt.Assert(f, qt.Eventually(qt.Equals), 42)
qt.Assert(f, qt.Eventually(qt.IsNil))

which reads well. And, when needed, a customized strategy can be provided:

qt.Assert(f, qt.Eventually(qt.Equals).WithStrategy(s), 42)

Similarly, WithStrategies(before, after *retry.Strategy) Checker could be used on EventuallyStable.

What do you think?

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

No branches or pull requests

3 participants