chore: migrate acctest in its own package and split tests in their ow…

…n package (#2458)

* ensure that tests are in their own package

* fix tfproviderlint

.github/contributing/acceptance_test.md

@@ -99,7 +99,7 @@ The definition of a complete test looks like this:
 
 ```go
 func TestAccScalewayInstanceServerImport(t *testing.T) {
-	tt := NewTestTools(t)
+	tt := acctest.NewTestTools(t)
 	defer tt.Cleanup()
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -152,7 +152,7 @@ When executing the test, the following steps are taken for each `TestStep`:
     			return fmt.Errorf("resource not found: %s", n)
     		}
 
-    		instanceAPI, zone, ID, err := instanceAPIWithZoneAndID(tt.Meta, rs.Primary.ID)
+    		instanceAPI, zone, ID, err := scaleway.InstanceAPIWithZoneAndID(tt.Meta, rs.Primary.ID)
     		if err != nil {
     			return err
     		}
@@ -190,7 +190,7 @@ When executing the test, the following steps are taken for each `TestStep`:
                     continue
                 }
 
-                instanceAPI, zone, ID, err := instanceAPIWithZoneAndID(tt.Meta, rs.Primary.ID)
+                instanceAPI, zone, ID, err := scaleway.InstanceAPIWithZoneAndID(tt.Meta, rs.Primary.ID)
                 if err != nil {
                     return err
                 }

.golangci.yml

@@ -62,6 +62,7 @@ linters:
     - tagliatelle # Checks the struct tags. [fast: true, auto-fix: false]
     - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 [fast: false, auto-fix: false]
     - testifylint # Checks usage of github.com/stretchr/testify. [fast: false, auto-fix: false]
+    - testpackage # linter that makes you use a separate _test package [fast: true, auto-fix: false]
     - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers [fast: false, auto-fix: false]
     - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes [fast: false, auto-fix: false]
     - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code [fast: false, auto-fix: false]

cmd/tftemplate/datasource_test.go.tmpl

@@ -8,7 +8,7 @@ import (
 )
 
 func TestAccScalewayDataSource{{.Resource}}_Basic(t *testing.T) {
-	tt := NewTestTools(t)
+	tt := acctest.NewTestTools(t)
 	defer tt.Cleanup()
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:          func() { testAccPreCheck(t) },

cmd/tftemplate/resource_test.go.tmpl

@@ -46,7 +46,7 @@ func testSweep{{ .Resource }}(_ string) error {
 }
 
 func TestAccScaleway{{.Resource}}_Basic(t *testing.T) {
-	tt := NewTestTools(t)
+	tt := acctest.NewTestTools(t)
 	defer tt.Cleanup()
 
 	resource.ParallelTest(t, resource.TestCase{

internal/acctest/acctest.go

@@ -0,0 +1,152 @@
+package acctest
+
+import (
+	"context"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/scaleway/terraform-provider-scaleway/v2/internal/meta"
+	"github.com/scaleway/terraform-provider-scaleway/v2/internal/transport"
+	"github.com/scaleway/terraform-provider-scaleway/v2/scaleway"
+	"github.com/stretchr/testify/require"
+)
+
+func PreCheck(_ *testing.T) {}
+
+type TestTools struct {
+	T                 *testing.T
+	Meta              *meta.Meta
+	ProviderFactories map[string]func() (*schema.Provider, error)
+	Cleanup           func()
+}
+
+func NewTestTools(t *testing.T) *TestTools {
+	t.Helper()
+	ctx := context.Background()
+	// Create a http client with recording capabilities
+	httpClient, cleanup, err := getHTTPRecoder(t, *UpdateCassettes)
+	require.NoError(t, err)
+
+	// Create meta that will be passed in the provider config
+	m, err := meta.NewMeta(ctx, &meta.Config{
+		ProviderSchema:   nil,
+		TerraformVersion: "terraform-tests",
+		HTTPClient:       httpClient,
+	})
+	require.NoError(t, err)
+
+	if !*UpdateCassettes {
+		tmp := 0 * time.Second
+		transport.DefaultWaitRetryInterval = &tmp
+	}
+
+	return &TestTools{
+		T:    t,
+		Meta: m,
+		ProviderFactories: map[string]func() (*schema.Provider, error){
+			"scaleway": func() (*schema.Provider, error) {
+				return scaleway.Provider(&scaleway.ProviderConfig{Meta: m})(), nil
+			},
+		},
+		Cleanup: cleanup,
+	}
+}
+
+// Test Generated name has format: "{prefix}-{generated_number}
+// example: test-acc-scaleway-project-3723338038624371236
+func extractTestGeneratedNamePrefix(name string) string {
+	// {prefix}-{generated}
+	//         ^
+	dashIndex := strings.LastIndex(name, "-")
+
+	generated := name[dashIndex+1:]
+	_, generatedToIntErr := strconv.ParseInt(generated, 10, 64)
+
+	if dashIndex == -1 || generatedToIntErr != nil {
+		// some are only {name}
+		return name
+	}
+
+	// {prefix}
+	return name[:dashIndex]
+}
+
+// Generated names have format: "tf-{prefix}-{generated1}-{generated2}"
+// example: tf-sg-gifted-yonath
+func extractGeneratedNamePrefix(name string) string {
+	if strings.Count(name, "-") < 3 {
+		return name
+	}
+	// tf-{prefix}-gifted-yonath
+	name = strings.TrimPrefix(name, "tf-")
+
+	// {prefix}-gifted-yonath
+	//                ^
+	dashIndex := strings.LastIndex(name, "-")
+	name = name[:dashIndex]
+	// {prefix}-gifted
+	//         ^
+	dashIndex = strings.LastIndex(name, "-")
+	name = name[:dashIndex]
+	return name
+}
+
+// compareJSONFieldsStrings compare two strings from request JSON bodies
+// has special case when string are terraform generated names
+func compareJSONFieldsStrings(expected, actual string) bool {
+	expectedHandled := expected
+	actualHandled := actual
+
+	// Remove s3 url suffix to allow comparison
+	if strings.HasSuffix(actual, ".s3-website.fr-par.scw.cloud") {
+		actual = strings.TrimSuffix(actual, ".s3-website.fr-par.scw.cloud")
+		expected = strings.TrimSuffix(expected, ".s3-website.fr-par.scw.cloud")
+	}
+
+	// Try to parse test generated name
+	if strings.Contains(actual, "-") {
+		expectedHandled = extractTestGeneratedNamePrefix(expected)
+		actualHandled = extractTestGeneratedNamePrefix(actual)
+	}
+
+	// Try provider generated name
+	if actualHandled == actual && strings.HasPrefix(actual, "tf-") {
+		expectedHandled = extractGeneratedNamePrefix(expected)
+		actualHandled = extractGeneratedNamePrefix(actual)
+	}
+
+	return expectedHandled == actualHandled
+}
+
+// compareJSONBodies compare two given maps that represent json bodies
+// returns true if both json are equivalent
+func compareJSONBodies(expected, actual map[string]interface{}) bool {
+	// Check for each key in actual requests
+	// Compare its value to cassette content if marshal-able to string
+	for key := range actual {
+		expectedValue, exists := expected[key]
+		if !exists {
+			// Actual request may contain a field that does not exist in cassette
+			// New fields can appear in requests with new api features
+			// We do not want to generate new cassettes for each new features
+			continue
+		}
+		if !compareJSONFields(expectedValue, actual[key]) {
+			return false
+		}
+	}
+
+	for key := range expected {
+		_, exists := actual[key]
+		if !exists && expected[key] != nil {
+			// Fails match if cassettes contains a field not in actual requests
+			// Fields should not disappear from requests unless a sdk breaking change
+			// We ignore if field is nil in cassette as it could be an old deprecated and unused field
+			return false
+		}
+	}
+	return true
+}

internal/acctest/checks.go

@@ -0,0 +1,69 @@
+package acctest
+
+import (
+	"errors"
+	"fmt"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+	"github.com/scaleway/terraform-provider-scaleway/v2/internal/locality"
+)
+
+// CheckResourceIDChanged checks that the ID of the resource has indeed changed, in case of ForceNew for example.
+// It will fail if resourceID is empty so be sure to use acctest.CheckResourceIDPersisted first in a test suite.
+func CheckResourceIDChanged(resourceName string, resourceID *string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if resourceID == nil || *resourceID == "" {
+			return errors.New("resourceID was not set")
+		}
+		rs, ok := s.RootModule().Resources[resourceName]
+		if !ok {
+			return fmt.Errorf("resource was not found: %s", resourceName)
+		}
+		if *resourceID == rs.Primary.ID {
+			return errors.New("resource ID persisted when it should have changed")
+		}
+		*resourceID = rs.Primary.ID
+		return nil
+	}
+}
+
+// CheckResourceIDPersisted checks that the ID of the resource is the same throughout tests of migration or mutation
+// It can be used to check that no ForceNew has been done
+func CheckResourceIDPersisted(resourceName string, resourceID *string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[resourceName]
+		if !ok {
+			return fmt.Errorf("resource was not found: %s", resourceName)
+		}
+		if *resourceID != "" && *resourceID != rs.Primary.ID {
+			return errors.New("resource ID changed when it should have persisted")
+		}
+		*resourceID = rs.Primary.ID
+		return nil
+	}
+}
+
+// CheckResourceRawIDMatches asserts the equality of IDs from two specified attributes of two Scaleway resources.
+func CheckResourceRawIDMatches(res1, attr1, res2, attr2 string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs1, ok1 := s.RootModule().Resources[res1]
+		if !ok1 {
+			return fmt.Errorf("not found: %s", res1)
+		}
+
+		rs2, ok2 := s.RootModule().Resources[res2]
+		if !ok2 {
+			return fmt.Errorf("not found: %s", res2)
+		}
+
+		id1 := locality.ExpandID(rs1.Primary.Attributes[attr1])
+		id2 := locality.ExpandID(rs2.Primary.Attributes[attr2])
+
+		if id1 != id2 {
+			return fmt.Errorf("ID mismatch: %s from resource %s does not match ID %s from resource %s", id1, res1, id2, res2)
+		}
+
+		return nil
+	}
+}