feat: add assert.Consistently

[jfmyers9]

Summary

This changeset adds the assert.Consistently and it's associated functions to assert that a condition is true over the entire period of waitFor. This is useful when testing the behavior of asynchronous functions.

Changes

• assert.Consistently checks that a provided function is true for the entirety of waitFor
• assert.Consistentlyf checks that a provided function is true for the entirety of waitFor and allows for formatting the error message
• assert.ConsistentlyWithT checks that a provided function that accepts a testing.T is true for the entirety of waitFor
• assert.ConsistentlyWithTf checks that a provided function that accepts a testing.T is true for the entirety of waitFor and allows for formatting the error message

Motivation

See: #1087 (comment)

Related issues

Closes #1087

[dolmen]

I don't want more uses of CollectT (which should never have been exposed as a concrete type in the first place). So let's remove all the WithT functions for now.

[dolmen]

We already have Never and Neverf which match the use case: it just need a reversed condition.

[z0marlin]

I don't want more uses of CollectT (which should never have been exposed as a concrete type in the first place). So let's remove all the WithT functions for now.

@dolmen, curious, what are the issues with CollectT? Also, I cannot say much about the CollectT type but I have found WithT quite useful in cases when I already have a certain set of assertions and want to assert that they succeed eventually (testing k8s controllers).

[dolmen]

CollectT should have been an interface to keep its implementation opaque.

[ccoVeille]

We already have Never and Neverf which match the use case: it just need a reversed condition.

Make sense, what would be the reversed condition asserter ? NotNever ? The name would be strange. But using another name that is not the opposite of Never would be strange no?

[jfmyers9]

In my opinion the opposite of Never would be Always.

Consistently works better as a name when paired with Eventually.

I don't have a strong preference on naming here.

[z0marlin]

nit: not sure if this is the right error message. To me, "Condition never satisfied" makes sense in the case of Eventually. The term never is not applicable in the context of Consistently.

[z0marlin]

add the errors to t first before calling Fail. Reference: https://github.com/stretchr/testify/blob/master/assert/assertions.go#L2017

[jfmyers9]

If we are happy with Always, then this just becomes:

func Always(t TestingT, condition func() bool, waitFor time.Duration, tick time.Duration, msgAndArgs ...interface{}) bool {
	return Never(t, func() bool { return !condition() }, waitFor, tick, msgAndArgs)
}

I missed the fact that Never exists when implementing this. Thoughts on moving to that approach?

[z0marlin]

CollectT should have been an interface to keep its implementation opaque.

Got it. So you would like to get rid of CollectT but not the WithT feature?

[z0marlin]

I missed the fact that Never exists when implementing this. Thoughts on moving to that approach?

My only concern here is that using Never will not allow having something like ConsistentlyWithT or AlwaysWithT (not really concerned about the naming). Not sure about how big of a use-case it is though.

[dolmen]

Never isn't a great name as it isn't what users expect. Always is in the same bucket, or even worse. I think that Consistently is a better name as it is consistent (pun intended) with Eventually, even if inconsistent with Never.

I was reluctant to add Consistently (I want to limit the increase the API surface), but as Never already exists it would be consistent (pun intended) to have it also. I understand also that ConsistentlyWithT would be convenient as it would allow to use require. NeverWithT would not make sense.

About the implementation, I think it's a bit early because we are not yet done with Eventually's bugs. I would like every eyes to focus on #1611.