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

Accept interfaces instead of concrete *testing.T #50

Closed
mirandaconrado opened this issue Feb 12, 2023 · 5 comments
Closed

Accept interfaces instead of concrete *testing.T #50

mirandaconrado opened this issue Feb 12, 2023 · 5 comments

Comments

@mirandaconrado
Copy link

mirandaconrado commented Feb 12, 2023

I've noticed that the concrete type *testing.T is used as argument in many places, like rapid.Check. But in the implementation, it almost immediately calls checkTB which converts it to the interface tb, which is a mirror of TB.

We would like to be able to pass a rapid.TB instead of *testing.T so that we can wrap the testing structure with additional information and behavior. Would you be opposed to changing the signature of the functions to accept rapid.TB instead of *testing.T? I believe this would be a transparent change to the user given that *testing.T satisfies rapid.TB. We'd be willing to do the work if approved.

@flyingmutant
Copy link
Owner

The proposal sounds logical, but I am a bit vary of making the Check signature less obvious. What additional information/behavior do you want to attach?

@worr
Copy link

worr commented Aug 2, 2023

One big benefit of doing this is that it allows rapid to be more easily used when using a framework like ginkgo. The easiest way to do this in Ginkgo is to use GinkgoT() to get a struct that isn't exactly a *testing.T, but implements all of the methods (and some extras) of *testing.T.

See:
https://onsi.github.io/ginkgo/#third-party-integrations
https://pkg.go.dev/github.com/onsi/ginkgo/v2#GinkgoT

flyingmutant added a commit that referenced this issue Aug 3, 2023
@flyingmutant
Copy link
Owner

flyingmutant commented Aug 3, 2023

Does f4756c7 look like a proper fix, or is anything still missing?

@worr
Copy link

worr commented Aug 6, 2023

Just checked it out on a test project, and this solution works perfectly (at least for my purposes using it with Ginkgo). Thanks a ton.

@conradoverta
Copy link

Awesome, thanks!

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

No branches or pull requests

4 participants