Skip to content

Commit

Permalink
chore: Detect changes in lists and sets (#3147)
Browse files Browse the repository at this point in the history
Test cases for changes in lists and sets. From the given time, I only
went through essential cases that consisted of:
- Ignoring the order of list items after creation.
- Having the ability to update an item while ignoring the order.
For the testing, I created a test resource because currently, we don't
have anything to test more complex examples of certain resource
behaviors (temporary providers we create for custom diff testing are not
sufficient in this case). The resource is only added to the resource
list whenever a special env is set (we could remove it entirely and
leave some documentation in the resource file (and acc test file) on how
to prepare for the tests). The imitation of external storage was done by
creating a struct and its global instance (some of the things needed to
be exported since external changes tested in acceptance tests needed to
access those). Certain resource fields were researched to test different
approaches, each is described in the implementation file.

Also added an assert on lists/sets that is able to assert items in order
independent manner (it was needed for the tests of the proposals).

> Note: Only lists were tested, because there was no major issue with
them in current resources. For the lists the following issues were
addressed: #420, #753

## Next pr
- Apply (parameterized tests in object renaming test cases)
#3130 (comment)
  • Loading branch information
sfc-gh-jcieslak authored Nov 5, 2024
1 parent 9fd8f88 commit c3edb79
Show file tree
Hide file tree
Showing 7 changed files with 2,035 additions and 17 deletions.
1 change: 1 addition & 0 deletions pkg/acceptance/bettertestspoc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,5 @@ func (w *WarehouseDatasourceShowOutputAssert) IsEmpty() {
1. Lists of objects are partially generated, and only parameter name is generated in some functions (the type has to be added manually).
2. `testing` is a package name that makes Go think that we want to have unnamed parameter there, but we just didn't generate the type for that field in the function argument.
- generate assertions checking that time is not empty - we often do not compare time fields by value, but check if they are set
- utilize `ContainsExactlyInAnyOrder` function in `pkg/acceptance/bettertestspoc/assert/commons.go` to create asserts on collections that are order independent
- support generating provider config and use generated configs in `pkg/provider/provider_acceptance_test.go`
83 changes: 83 additions & 0 deletions pkg/acceptance/bettertestspoc/assert/commons.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ package assert
import (
"errors"
"fmt"
"slices"
"strconv"
"strings"
"testing"

"golang.org/x/exp/maps"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/terraform"
)
Expand Down Expand Up @@ -101,3 +106,81 @@ func AssertThatObject(t *testing.T, objectAssert InPlaceAssertionVerifier) {
t.Helper()
objectAssert.VerifyAll(t)
}

func ContainsExactlyInAnyOrder(resourceKey string, attributePath string, expectedItems []map[string]string) resource.TestCheckFunc {
return func(state *terraform.State) error {
var actualItems []map[string]string
var resourceValue *terraform.ResourceState

if value, ok := state.RootModule().Resources[resourceKey]; ok {
resourceValue = value
} else {
return fmt.Errorf("resource %s not found", resourceKey)
}

// Allocate space for actualItems and assert length
for attrKey, attrValue := range resourceValue.Primary.Attributes {
if strings.HasPrefix(attrKey, attributePath) {
attr := strings.TrimPrefix(attrKey, attributePath+".")

if attr == "#" {
attrValueLen, err := strconv.Atoi(attrValue)
if err != nil {
return fmt.Errorf("failed to convert length of the attribute %s: %w", attrKey, err)
}
if len(expectedItems) != attrValueLen {
return fmt.Errorf("expected to find %d items in %s, but found %d", len(expectedItems), attributePath, attrValueLen)
}

actualItems = make([]map[string]string, attrValueLen)
for i := range actualItems {
actualItems[i] = make(map[string]string)
}
}
}
}

// Gather all actual items
for attrKey, attrValue := range resourceValue.Primary.Attributes {
if strings.HasPrefix(attrKey, attributePath) {
attr := strings.TrimPrefix(attrKey, attributePath+".")

if strings.HasSuffix(attr, "%") || strings.HasSuffix(attr, "#") {
continue
}

attrParts := strings.SplitN(attr, ".", 2)
index, indexErr := strconv.Atoi(attrParts[0])
isIndex := indexErr == nil

if len(attrParts) > 1 && isIndex {
itemKey := attrParts[1]
actualItems[index][itemKey] = attrValue
}
}
}

errs := make([]error, 0)
for _, actualItem := range actualItems {
found := false
if slices.ContainsFunc(expectedItems, func(expected map[string]string) bool { return maps.Equal(expected, actualItem) }) {
found = true
}
if !found {
errs = append(errs, fmt.Errorf("unexpected item found: %s", actualItem))
}
}

for _, expectedItem := range expectedItems {
found := false
if slices.ContainsFunc(actualItems, func(actual map[string]string) bool { return maps.Equal(actual, expectedItem) }) {
found = true
}
if !found {
errs = append(errs, fmt.Errorf("expected item to be found, but it wasn't: %s", expectedItem))
}
}

return errors.Join(errs...)
}
}
16 changes: 8 additions & 8 deletions pkg/acceptance/helpers/database_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,6 @@ func (c *DatabaseClient) CreateDatabaseWithOptions(t *testing.T, id sdk.AccountO
return database, c.DropDatabaseFunc(t, id)
}

func (c *DatabaseClient) Alter(t *testing.T, id sdk.AccountObjectIdentifier, opts *sdk.AlterDatabaseOptions) {
t.Helper()
ctx := context.Background()

err := c.client().Alter(ctx, id, opts)
require.NoError(t, err)
}

func (c *DatabaseClient) DropDatabaseFunc(t *testing.T, id sdk.AccountObjectIdentifier) func() {
t.Helper()
return func() { require.NoError(t, c.DropDatabase(t, id)) }
Expand Down Expand Up @@ -192,3 +184,11 @@ func (c *DatabaseClient) ShowAllReplicationDatabases(t *testing.T) ([]sdk.Replic

return c.context.client.ReplicationFunctions.ShowReplicationDatabases(ctx, nil)
}

func (c *DatabaseClient) Alter(t *testing.T, id sdk.AccountObjectIdentifier, opts *sdk.AlterDatabaseOptions) {
t.Helper()
ctx := context.Background()

err := c.client().Alter(ctx, id, opts)
require.NoError(t, err)
}
16 changes: 8 additions & 8 deletions pkg/acceptance/helpers/schema_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,6 @@ func (c *SchemaClient) CreateSchemaWithOpts(t *testing.T, id sdk.DatabaseObjectI
return schema, c.DropSchemaFunc(t, id)
}

func (c *SchemaClient) Alter(t *testing.T, id sdk.DatabaseObjectIdentifier, opts *sdk.AlterSchemaOptions) {
t.Helper()
ctx := context.Background()

err := c.client().Alter(ctx, id, opts)
require.NoError(t, err)
}

func (c *SchemaClient) DropSchemaFunc(t *testing.T, id sdk.DatabaseObjectIdentifier) func() {
t.Helper()
ctx := context.Background()
Expand Down Expand Up @@ -108,3 +100,11 @@ func (c *SchemaClient) ShowWithOptions(t *testing.T, opts *sdk.ShowSchemaOptions
require.NoError(t, err)
return schemas
}

func (c *SchemaClient) Alter(t *testing.T, id sdk.DatabaseObjectIdentifier, opts *sdk.AlterSchemaOptions) {
t.Helper()
ctx := context.Background()

err := c.client().Alter(ctx, id, opts)
require.NoError(t, err)
}
10 changes: 9 additions & 1 deletion pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"strings"
"time"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/datasources"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider/docs"
Expand Down Expand Up @@ -417,7 +419,7 @@ func Provider() *schema.Provider {
}

func getResources() map[string]*schema.Resource {
return map[string]*schema.Resource{
resourceList := map[string]*schema.Resource{
"snowflake_account": resources.Account(),
"snowflake_account_authentication_policy_attachment": resources.AccountAuthenticationPolicyAttachment(),
"snowflake_account_role": resources.AccountRole(),
Expand Down Expand Up @@ -504,6 +506,12 @@ func getResources() map[string]*schema.Resource {
"snowflake_view": resources.View(),
"snowflake_warehouse": resources.Warehouse(),
}

if os.Getenv(string(testenvs.EnableObjectRenamingTest)) != "" {
resourceList["snowflake_object_renaming"] = resources.ObjectRenamingListsAndSets()
}

return resourceList
}

func getDataSources() map[string]*schema.Resource {
Expand Down
Loading

0 comments on commit c3edb79

Please sign in to comment.