hashicorp / terraform-plugin-testing

Module for testing Terraform providers
Mozilla Public License 2.0
44 stars 11 forks source link

tfversion: Treat Terraform CLI prerelease versions as equal to patch versions in SkipBelow #316

Closed bflad closed 3 months ago

bflad commented 3 months ago

Reference: https://github.com/hashicorp/terraform-plugin-testing/issues/303 Reference: https://github.com/hashicorp/terraform-plugin-testing/pull/308

This change is mainly due to the internals of how github.com/hashicorp/go-version treats version comparisons when there is prerelease data. When the running Terraform CLI is a prerelease version and the given version is a patch version, SkipBelow will currently skip the test. However, Terraform CLI prerelease versions are semantically treated as candidates for the associated patch version and therefore should be tested. This adjusts SkipBelow for that intended behavior. In the unexpected use case that prerelease version checking is still needed, callers can (still) opt into giving a prerelease version, which will perform the check including prerelease data.

The unit testing for the tfversion package skip functionality is still manual because github.com/mitchellh/go-testing-interface will not immediately stop the test logic Goroutine when the equivalent of (*testing.T).Skip() is called nor does it provide helpful troubleshooting information should a test fail as it raises a panic with a generic error message. Future changes could switch to using a different testing interface, however they would require a breaking change to the exported API of this Go module, so that design and effort is being done separately.

With the addition of the new tests, but no logic changes:

=== RUN   Test_SkipBelow_SkipTest
    skip_below_test.go:23: Terraform CLI version 1.0.7 is below minimum version 1.1.0: skipping test
--- SKIP: Test_SkipBelow_SkipTest (3.91s)
=== RUN   Test_SkipBelow_RunTest
--- PASS: Test_SkipBelow_RunTest (3.32s)
=== RUN   Test_SkipBelow_Prerelease_EqualCoreVersion
    skip_below_test.go:77: Terraform CLI version 1.8.0-rc1 is below minimum version 1.8.0: skipping test
--- SKIP: Test_SkipBelow_Prerelease_EqualCoreVersion (4.23s)
=== RUN   Test_SkipBelow_Prerelease_HigherCoreVersion
    skip_below_test.go:101: Terraform CLI version 1.7.0-rc1 is below minimum version 1.8.0: skipping test
--- SKIP: Test_SkipBelow_Prerelease_HigherCoreVersion (4.40s)
=== RUN   Test_SkipBelow_Prerelease_HigherPrerelease
    skip_below_test.go:122: Terraform CLI version 1.7.0-rc1 is below minimum version 1.7.0-rc2: skipping test
--- SKIP: Test_SkipBelow_Prerelease_HigherPrerelease (3.32s)
=== RUN   Test_SkipBelow_Prerelease_LowerCoreVersion
--- PASS: Test_SkipBelow_Prerelease_LowerCoreVersion (3.93s)
=== RUN   Test_SkipBelow_Prerelease_LowerPrerelease
--- PASS: Test_SkipBelow_Prerelease_LowerPrerelease (3.38s)

After logic changes (note only difference is Test_SkipBelow_Prerelease_EqualCoreVersion):

=== RUN   Test_SkipBelow_SkipTest
    skip_below_test.go:22: Terraform CLI version 1.0.7 is below minimum version 1.1.0: skipping test
--- SKIP: Test_SkipBelow_SkipTest (4.17s)
=== RUN   Test_SkipBelow_RunTest
--- PASS: Test_SkipBelow_RunTest (3.21s)
=== RUN   Test_SkipBelow_Prerelease_EqualCoreVersion
--- PASS: Test_SkipBelow_Prerelease_EqualCoreVersion (3.21s)
=== RUN   Test_SkipBelow_Prerelease_HigherCoreVersion
    skip_below_test.go:99: Terraform CLI version 1.7.0-rc1 is below minimum version 1.8.0: skipping test
--- SKIP: Test_SkipBelow_Prerelease_HigherCoreVersion (3.18s)
=== RUN   Test_SkipBelow_Prerelease_HigherPrerelease
    skip_below_test.go:120: Terraform CLI version 1.7.0-rc1 is below minimum version 1.7.0-rc2: skipping test
--- SKIP: Test_SkipBelow_Prerelease_HigherPrerelease (3.21s)
=== RUN   Test_SkipBelow_Prerelease_LowerCoreVersion
--- PASS: Test_SkipBelow_Prerelease_LowerCoreVersion (3.04s)
=== RUN   Test_SkipBelow_Prerelease_LowerPrerelease
--- PASS: Test_SkipBelow_Prerelease_LowerPrerelease (3.41s)
bflad commented 3 months ago

This type of change should probably also occur on the other functions, but I wanted to first gauge feelings on the highest impact function, since there is expected to be a number of provider-facing core features in the coming releases.

bflad commented 3 months ago

Great idea, @SBGoods! I'm going to merge this in since I'm not sure when I'm going to be able to do the rest, but will keep that in mind. 👍

bflad commented 3 months ago

As promised, @SBGoods 😉 https://github.com/hashicorp/terraform-plugin-testing/pull/317/files#diff-ad363bec2802c6c3fbeec150f0940b53650e0db5d190b3224de952f847de97c5

github-actions[bot] commented 2 months ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.