testify icon indicating copy to clipboard operation
testify copied to clipboard

Feature: assert.Consistently

Open shaneutt opened this issue 4 years ago • 14 comments

I've been using testify for a long time now, I appreciate you building and maintaining this for us all :bow:

I'm looking for functionality that is inspired by other testing frameworks that would be similar to assert.Eventually and require.Eventually, but instead would be:

stagger := time.Millisecond * 200
tries := 15
assert.Consistently(t, func() bool {
   fmt.Println("my logic here")
   return result()
}, stagger, tries)

The point of this would be to ensure that a result is consistently occurring over a period of time. In the above example, that the fake result() function is returning true consistently every 200ms for 3 seconds total.

Does testify already have something like this and I've just missed it?

If not, are you cool with me contributing something like this?

Thanks!

shaneutt avatar Jun 16 '21 15:06 shaneutt

Would Consistently() be more readable over say this code:

for i := 1 ; i <= 15 ; i++ {
    t.Logf("attempt %d", i)
    time.Sleep(200*time.Millisecond)
    got := myCode()
    assert.Equal(t, expectation, got)
}

Another concern I have is what if someone calls t.Error() inside the function? The testify framework will have no way to know they did that, so the Consistently() assertion will pass but the test will fail and you wont be told on which attempt it failed.

Edit: Actually testify could call t.Failed() after each iteration to see if it passed. Perhaps using t and regular assertions in the function is a better way to indicate failures than having a return value because then all assertions are available to the user. Calls to t.FailNow() would still escape though.

brackendawson avatar Jul 16 '21 17:07 brackendawson

Readability is subjective: I'm more interested in the prescriptiveness it would bring: setting a clear standard in tests that use testify for how to achieve this and leaving less room for interpretation for contributors. As far as your concerns about t.Error does that problem also exist for Eventually or no?

shaneutt avatar Jul 16 '21 17:07 shaneutt

Eventually probably does have the same quirk with use of t inside it's function. And it does set a precedent for your suggested function signature.

I think I wouldn't object to adding this.

brackendawson avatar Jul 16 '21 17:07 brackendawson

Maybe it should take two durations, a tick and a testFor, a bit like Eventually does?

If the function takes longer than the tick do you run them concurrently or fail the assertion?

brackendawson avatar Jul 16 '21 17:07 brackendawson

Maybe it should take two durations, a tick and a testFor, a bit like Eventually does?

Right, in the original post I had mocked it up that way but apparently messed up the var names so it's a little confusing, here's a redux:

assert.Consistently(t, func() bool {
   fmt.Println("my logic here")
   return result()
}, testFor, tick)

If the function takes longer than the tick do you run them concurrently or fail the assertion?

Good question, I think the caller should have to instruct the test mechanism in regards to how long the actual test takes.

Perhaps this would be a better signature:

assert.Consistently(t, func() bool {
   fmt.Println("my logic here")
   return result()
}, totalTestDuration, requiredSuccesses, tick)
  • totalTestDuration: how long overall the test will run
  • requiredSuccesses: how many success results are required during totalTestDuration

This fits well with my use case, because the current tests I would use these in test things like HTTP responses to a local API and this adds a slight performance testing edge to the mix which I think adds to the utility, and fits with the theme of "consistency" I think.

shaneutt avatar Jul 16 '21 18:07 shaneutt

Maybe, specifying a total duration and a number of successful executions feel a little over-constrained though? You can define a Consistently call which can never pass. It might just be that it needs a better name, but it sounds like requiredSuccesses means you can have some failures so long as you get that many passes, which isn't consistent.

It makes me lean towards something similar to how you wrote it in your first post here actually, even though that wasn't what you intended:

func Consistently(t TestingT, f func() bool, every time.Duration, times int) bool

Run f times times at least every times apart. So if f is slower than every the test takes longer but still runs times times. I know it's not a performance test any more, is it an approach that fits your use case?

brackendawson avatar Jul 19 '21 15:07 brackendawson

Maybe, specifying a total duration and a number of successful executions feel a little over-constrained though? You can define a Consistently call which can never pass. It might just be that it needs a better name, but it sounds like requiredSuccesses means you can have some failures so long as you get that many passes, which isn't consistent.

It makes me lean towards something similar to how you wrote it in your first post here actually, even though that wasn't what you intended:

func Consistently(t TestingT, f func() bool, every time.Duration, times int) bool

Run f times times at least every times apart. So if f is slower than every the test takes longer but still runs times times. I know it's not a performance test any more, is it an approach that fits your use case?

Yes, this definition works too. The performance style stuff can be a consideration for later.

shaneutt avatar Jul 21 '21 12:07 shaneutt

Hey! I would love to implement that feature. As far as I am aware the way to do this is:

  1. Fork the Repository
  2. Implement the feature
  3. Create PR to merge my work into the original repository
  4. "include a complete test function that demonstrates the issue"

Did I understand the process correctly? Thanks a lot!

MatthiasReumann avatar May 17 '22 12:05 MatthiasReumann

It looks like it could be called Repeat as well:

func Repeat(t TestingT, f func() bool, every time.Duration, times int) bool

However you can use the -count parameter of go test to repeat a test. Also, you could just wrap your test in a loop with time.Sleep. I don't think this is worth having to learn the testify API to be able to read the test code.

In addition, I see nothing directly related to assert that would require it to be in the same package.

So I'm not convinced that this is worth extending the API surface of testify for this feature.

dolmen avatar Jul 07 '23 09:07 dolmen

In my mind this isn't intended as a utility to repeat a test, but instead it serves as the inverse of Eventually when testing asynchronous code.

Consistently allows someone to write an assertion that something will hold true over the acceptable period of time. Obviously this should be used within reason as it will increase the duration of the test by the specified interval, but from an assertion perspective:

require.Consistently(t, func bool() {
  // do something
}, duration, interval, message);

Reads nicer than:

for i := 1 ; i <= 15 ; i++ {
    t.Logf("attempt %d", i)
    time.Sleep(200*time.Millisecond)
    got := myCode()
    assert.Equal(t, expectation, got)
}

To provide a concrete example: https://book.kubebuilder.io/cronjob-tutorial/writing-tests#testing-your-controllers-behavior

You can see how Kubebuilder uses a similar assertion provided by Gomega when testing Kubernetes Operators:

            By("By checking the CronJob has zero active Jobs")
            Consistently(func() (int, error) {
                err := k8sClient.Get(ctx, cronjobLookupKey, createdCronjob)
                if err != nil {
                    return -1, err
                }
                return len(createdCronjob.Status.Active), nil
            }, duration, interval).Should(Equal(0))

jfmyers9 avatar Aug 09 '23 18:08 jfmyers9