Remove/Replace mitchellh/go-testing-interface Dependency #254
Labels
dependencies
Pull requests that update a dependency file
Milestone
Comments
bflad
added a commit
that referenced
this issue
Mar 19, 2024
…sting helpers Reference: #254 Reference: #303 _Note: This change is easiest to view with whitespace changes ignored._ As part of upcoming efforts to consider prerelease versioning more pragmatically in the `tfversion` package checks, the need to properly unit test `(*testing.T).Skip()` and friends behaviors is more apparent. Similar to issues with testing `(*testing.T).Fatal()` behaviors, the Go standard library in general is not the most friendly to capture test helper behaviors since the `testing.T` implementation is not intended to be extensible. This change was initially intended to extend our current usage of `github.com/mitchellh/go-testing-interface` as a stopgap, however while attempting to implement skip assertion testing, the `RuntimeT` implementation did not call `runtime.Goexit()` and would cause tests to continue to execute test logic after skipping even though in reality they would not. While the tests could be fixed to ensure they still passed due to that behavior change, it felt like a good time as any to replace the now-unsuitable and archived `github.com/mitchellh/go-testing-interface` with our own internal implementation. The new `internal/testingiface.T` interface is a wholesale duplicate interface to the prior `github.com/mitchellh/go-testing-interface.T` interface. Swapping this interface in exported functions, such as `helper/resource.Test()` is only a compatibility issue for consumers if by chance they were implementing a wrapping function that also depended on `github.com/mitchellh/go-testing-interface.T` and passed that implementation detail directly to function calls of this Go module. This is a gray compatibility area as the intention of using `github.com/mitchellh/go-testing-interface.T` was for consumers to use real `*testing.T` implementations, which will work the same after this change, but it was not appropriately documented that extensions of exported functionality using `github.com/mitchellh/go-testing-interface.T` should not rely on that explicitly. Changing this detail is therefore a pragmatic compromise that it is more important to prioritize and document the intended usage while removing the archived dependency, which is almost exclusively how the ecosystem interacts with the functionality, over the potential of compatibility issues in a very small subset of the ecosystem. Since `(*testing.T).Skip()` is considered a passing test, there are now assertions whether skipping was called or not, as expected, in the existing `tfversion` package tests. The new `internal/testingiface` package also includes similar assertion helpers for `(*testing.T).Fatal()` and `(*testing.T).Parallel()` for consistency and cleans up some existing logic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
terraform-plugin-testing version
Use cases
https://github.com/mitchellh/go-testing-interface was archived earlier this year.
Proposal
Consider replacing
testing.Tusage everywhere with error/skip response structures except for the "front door"resource.Test(),resource.UnitTest(), andresource.ParallelTest()functions.The text was updated successfully, but these errors were encountered: