How to deal with functions that expect a `testing.TB` when using `assert.EventuallyWithT`

[chancez]

I have a bunch of custom assert functions that currently take either *testing.T or testing.TB, and I recently realized I cannot use any of them with assert.EventuallyWithT which we started using more heavily in our e2e tests, because the assert.CollectT is a concrete type that does not implement testing.TB. Obviously for the things we use that currently take a *testing.T we could update to a testing.TB but that doesn't fix the issue since CollectT doesn't implement testing.TB.

I'm wondering what kind of options we have to solve this. My current idea is maybe to just copying the current implementation of both assert.CollectT and assert.EventuallyWithT and adapt the CollectT implementation to implement testing.TB, but the interface is pretty large and I can see it might not play well with the requirements for a CollectT, so it doesn't seem trivial to do that.

I'm open to ideas, and of course I'd prefer if testify could fix this issue, but from my scanning of the GH issues around the assert.TestingT interface, the tHelper() interface and assert.CollectT types, it seems like fixing this directly isn't feasible without a huge rearchitecture of testify itself, so I'm open to writing my own functions to make this work.

[fredbi]

You raised a very valid point imho.
I’ve also been scratching my head about what kind of interface CollectT is supposed to have.

An idea just from the top of my head:

CollectT could be an interface for types that know how to collect. Besides the default implementation, we could provide an alternative implementation that wraps a T or TB that closures called could resolve.

Just a draftish idea for now.

[dolmen]

CollectT was definitely a bad design, but that's what we have now and we can't fix that without a breaking change.

*CollectT implements a small subset of testing.TB that is enough to call assert/require assertions.

Tell us what things you would like to do in the callback function that you can't do now.

[dolmen]

Note that the documentation of testing.T says:

A test ends when its Test function returns or calls any of the methods T.FailNow, T.Fatal, T.Fatalf, T.SkipNow, T.Skip, or T.Skipf.
Those methods, as well as the T.Parallel method, must be called only from the goroutine running the Test function.

So it is not safe to call methods of your testing.T from a func(*CollectT) callback as those are happening in another goroutine.

[chancez]

Note that the documentation of testing.T says:

A test ends when its Test function returns or calls any of the methods T.FailNow, T.Fatal, T.Fatalf, T.SkipNow, T.Skip, or T.Skipf.
Those methods, as well as the T.Parallel method, must be called only from the goroutine running the Test function.

So it is not safe to call methods of your testing.T from a func(*CollectT) callback as those are happening in another goroutine.

Right, well the idea is that the collect function would implement the testing.TB interface and you would be delegating to it to handle this. That's what it does already, but it just doesnt implement the interface. Effectively calling any fail/skip methods would cause the collect func to cancel the goroutine then call the actual fail/skip method after.

[chancez]

Tell us what things you would like to do in the callback function that you can't do now.

A simple example is this helper function:

func AssertProtoEqual(t testing.TB, want, got any, msgAndArgs ...any) {
	t.Helper()
	if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
		assert.Fail(t, fmt.Sprintf("not equal (-want +got):\n%s", diff), msgAndArgs...)
	}
}

I have plenty of other helpers too, but they're basically all custom assert functions of some kind usually.