From a8247f636588a4373e40a150b44c75b76ee8dd08 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 28 Mar 2024 09:49:26 -0400 Subject: [PATCH] tfversion: Treat Terraform CLI prerelease versions as equal to patch versions in SkipBelow (#316) 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) ``` --- .../ENHANCEMENTS-20240327-171628.yaml | 6 + tfversion/skip_below.go | 21 +++- tfversion/skip_below_test.go | 111 ++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/ENHANCEMENTS-20240327-171628.yaml diff --git a/.changes/unreleased/ENHANCEMENTS-20240327-171628.yaml b/.changes/unreleased/ENHANCEMENTS-20240327-171628.yaml new file mode 100644 index 000000000..ea6f6ebe8 --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-20240327-171628.yaml @@ -0,0 +1,6 @@ +kind: ENHANCEMENTS +body: 'tfversion: Ensured semantically equivalent Terraform CLI prerelease versions + are considered equal to patch versions in `SkipBelow`' +time: 2024-03-27T17:16:28.49466-04:00 +custom: + Issue: "303" diff --git a/tfversion/skip_below.go b/tfversion/skip_below.go index 0b3dffddc..4c6bc708f 100644 --- a/tfversion/skip_below.go +++ b/tfversion/skip_below.go @@ -14,6 +14,16 @@ import ( // version is below the given version. For example, if given // version.Must(version.NewVersion("0.15.0")), then 0.14.x or // any other prior minor versions will skip the test. +// +// Prereleases of Terraform CLI (whether alpha, beta, or rc) are considered +// equal to a given patch version. For example, if given +// version.Must(version.NewVersion("1.8.0")), then 1.8.0-rc1 will run, not skip, +// the test. Terraform prereleases are considered as potential candidates for +// the upcoming version and therefore are treated as important for testing to +// run. If skipping prereleases of the same patch release is desired, give a +// higher prerelease version. For example, if given +// version.Must(version.NewVersion("1.8.0-rc2")), then 1.8.0-rc1 will skip the +// test. func SkipBelow(minimumVersion *version.Version) TerraformVersionCheck { return skipBelowCheck{ minimumVersion: minimumVersion, @@ -27,8 +37,17 @@ type skipBelowCheck struct { // CheckTerraformVersion satisfies the TerraformVersionCheck interface. func (s skipBelowCheck) CheckTerraformVersion(ctx context.Context, req CheckTerraformVersionRequest, resp *CheckTerraformVersionResponse) { + var terraformVersion *version.Version - if req.TerraformVersion.LessThan(s.minimumVersion) { + // If given a prerelease version, check the Terraform CLI version directly, + // otherwise use the core version so that prereleases are treated as equal. + if s.minimumVersion.Prerelease() != "" { + terraformVersion = req.TerraformVersion + } else { + terraformVersion = req.TerraformVersion.Core() + } + + if terraformVersion.LessThan(s.minimumVersion) { resp.Skip = fmt.Sprintf("Terraform CLI version %s is below minimum version %s: skipping test", req.TerraformVersion, s.minimumVersion) } diff --git a/tfversion/skip_below_test.go b/tfversion/skip_below_test.go index 39ed9411c..c9da1b4f9 100644 --- a/tfversion/skip_below_test.go +++ b/tfversion/skip_below_test.go @@ -62,3 +62,114 @@ func Test_SkipBelow_RunTest(t *testing.T) { //nolint:paralleltest }, }) } + +func Test_SkipBelow_Prerelease_EqualCoreVersion(t *testing.T) { //nolint:paralleltest + t.Setenv("TF_ACC_TERRAFORM_PATH", "") + t.Setenv("TF_ACC_TERRAFORM_VERSION", "1.8.0-rc1") + + // Pragmatic compromise that 1.8.0-rc1 prerelease is considered to + // be equivalent to the 1.8.0 core release. This enables developers + // to assert that prerelease versions are compatible with upcoming + // core versions. + // + // Reference: https://github.com/hashicorp/terraform-plugin-testing/issues/303 + r.UnitTest(t, r.TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": providerserver.NewProviderServer(testprovider.Provider{}), + }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(version.Must(version.NewVersion("1.8.0"))), + }, + Steps: []r.TestStep{ + { + Config: `//non-empty config`, + }, + }, + }) +} + +func Test_SkipBelow_Prerelease_HigherCoreVersion(t *testing.T) { //nolint:paralleltest + t.Setenv("TF_ACC_TERRAFORM_PATH", "") + t.Setenv("TF_ACC_TERRAFORM_VERSION", "1.7.0-rc1") + + // The 1.7.0-rc1 prerelease should always be considered to be below the + // 1.8.0 core version. This intentionally verifies that the logic does not + // ignore the core version of the prerelease version when compared against + // the core version of the check. + r.UnitTest(t, r.TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": providerserver.NewProviderServer(testprovider.Provider{}), + }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(version.Must(version.NewVersion("1.8.0"))), + }, + Steps: []r.TestStep{ + { + Config: `//non-empty config`, + }, + }, + }) +} + +func Test_SkipBelow_Prerelease_HigherPrerelease(t *testing.T) { //nolint:paralleltest + t.Setenv("TF_ACC_TERRAFORM_PATH", "") + t.Setenv("TF_ACC_TERRAFORM_VERSION", "1.7.0-rc1") + + // The 1.7.0-rc1 prerelease should always be considered to be + // below the 1.7.0-rc2 prerelease. + r.UnitTest(t, r.TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": providerserver.NewProviderServer(testprovider.Provider{}), + }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(version.Must(version.NewVersion("1.7.0-rc2"))), + }, + Steps: []r.TestStep{ + { + Config: `//non-empty config`, + }, + }, + }) +} + +func Test_SkipBelow_Prerelease_LowerCoreVersion(t *testing.T) { //nolint:paralleltest + t.Setenv("TF_ACC_TERRAFORM_PATH", "") + t.Setenv("TF_ACC_TERRAFORM_VERSION", "1.8.0-rc1") + + // The 1.8.0-rc1 prerelease should always be considered to be + // above the 1.7.0 core version. + r.UnitTest(t, r.TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": providerserver.NewProviderServer(testprovider.Provider{}), + }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(version.Must(version.NewVersion("1.7.0"))), + }, + Steps: []r.TestStep{ + { + Config: `//non-empty config`, + }, + }, + }) +} + +func Test_SkipBelow_Prerelease_LowerPrerelease(t *testing.T) { //nolint:paralleltest + t.Setenv("TF_ACC_TERRAFORM_PATH", "") + t.Setenv("TF_ACC_TERRAFORM_VERSION", "1.8.0-rc1") + + // The 1.8.0-rc1 prerelease should always be considered to be + // above the 1.8.0-beta1 prerelease. + r.UnitTest(t, r.TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": providerserver.NewProviderServer(testprovider.Provider{}), + }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(version.Must(version.NewVersion("1.8.0-beta1"))), + }, + Steps: []r.TestStep{ + { + Config: `//non-empty config`, + }, + }, + }) +}