quicktest icon indicating copy to clipboard operation
quicktest copied to clipboard

proposal: `Eventually` checker

Open rogpeppe opened this issue 4 years ago • 3 comments

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

rogpeppe avatar Jan 18 '22 16:01 rogpeppe

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 avatar Jan 18 '22 16:01 mvdan

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

mvdan avatar Jan 18 '22 17:01 mvdan

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?

frankban avatar Jan 19 '22 09:01 frankban