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

Can we replace *testing T with an interface? #105

Closed
anthonyfong100 opened this issue Feb 22, 2021 · 5 comments
Closed

Can we replace *testing T with an interface? #105

anthonyfong100 opened this issue Feb 22, 2021 · 5 comments

Comments

@anthonyfong100
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When using apitest to do testing of api, the Expect method takes in a *testing T pointer which is a concrete type. This makes interacting with other third party testing library like Ginkgo difficult.

Describe the solution you'd like
Replace the reliance on *testing.T with an interface.

From the developer of ginkg:


It looks like testify uses (*testing.T) directly instead of an interface (which gomock does). Ginkgo can provide such frameworks with a Fail function that one can wrap in such a way to satisfy most interfaces (this is how we support go mock), but *testing.T refers to a concrete type and passing in the testing.T for the test that Ginkgo runs in would cause the entire suite to fail should one of your mock assertions fail...


Describe alternatives you've considered
As per the section above another way would to just pass in the testing.T but would cause the entire suite to fail if one of the assertions fail.

Additional context
Other libraries such as testify have changed this to support ginkgo integration

I would be happy to create a PR if this is possible

@steinfletcher
Copy link
Owner

steinfletcher commented Feb 27, 2021

Hi @anthonyfong100. This sounds very useful. Would be happy to take a PR for this. Changes would need to be backwards compatible. For info, apitest uses testify under the hood. You can already override the Verifier to implement a custom assertions instead of using testify - see the Verifier interface in assert.go. I am planning to remove testify in a future commit, but it's really an implementation detail that the consumer shouldn't care about.

Cheers

@anthonyfong100
Copy link
Contributor Author

Hey @steinfletcher i was thinking more along the lines of passing in an interface to replace the *testing.T and continue using testify to do the assertions. As of now, testify already supports accepting an interface testingT as its argument in place of the concrete class *testing.T. This solution is likely to be better as it allows us to leverage on numerous other assertion methods defined in testify. Do let me know your thoughts on this

@steinfletcher
Copy link
Owner

steinfletcher commented Feb 28, 2021

Hello @anthonyfong100. Sorry I wasn't clear. I also agree that there should be an interface TestingT. I was just adding some info on the existing design around assertions, which is completely compatible with the approach you suggest. You'll need to update the Verifier interface to use TestingT. Other than that I think it's a simple change.

I'll be removing testify in a future commit for a like-for-like solution, but that doesn't really affect your changes :)

@anthonyfong100
Copy link
Contributor Author

okay @steinfletcher that sounds wonderful i will go ahead with writing the PR then

@steinfletcher
Copy link
Owner

Looks great @anthonyfong100. Many 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

2 participants