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

remove resource.TestT #397

Closed
wants to merge 1 commit into from
Closed

remove resource.TestT #397

wants to merge 1 commit into from

Conversation

appilon
Copy link
Contributor

@appilon appilon commented Apr 20, 2020

The interface and associated tests are not useful for V2

@appilon appilon requested review from a team and paultyng April 20, 2020 23:14
@paultyng
Copy link
Contributor

I think you may want to keep this so you don't need to import the testing package in live code?

@appilon
Copy link
Contributor Author

appilon commented Apr 21, 2020

Probably, except we currently do in RunNewTest which probably was an oversight in V1 (so testing is exposed in the V1 API as it is). We type asserted to testing.T to satisfy a testing interface in terraform-plugin-test https://github.com/hashicorp/terraform-plugin-test/blob/master/guard.go#L79.

We will have to harmonize the interfaces if we don't want to directly import testing in V2

@paultyng
Copy link
Contributor

We should probably just push them all to https://github.com/mitchellh/go-testing-interface ? Not sure, would maybe want to ask internally if there is a consensus here.

@appilon
Copy link
Contributor Author

appilon commented Apr 21, 2020

@paultyng We should make them both the same I like that idea but then we are exposing an interface owned by mitchell to the surface. I can try type asserting to terraform-plugin-test's interface and seeing if that works (then testing.T appears nowhere).

@appilon
Copy link
Contributor Author

appilon commented Apr 21, 2020

#396 opts to simulate fatal on mockT with panic/recover, cherry-picking that to V2 will remove the need for this PR.

cleaning up the interface usage and casting in V2 can be done in another PR

@appilon appilon closed this Apr 21, 2020
@appilon appilon deleted the remove-TestingT branch April 21, 2020 18:20
@ghost
Copy link

ghost commented May 22, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants