From f4d2f8f99371e8dcd640f2401340b25e5fb2319e Mon Sep 17 00:00:00 2001 From: Miles Yucht Date: Wed, 21 Aug 2024 17:01:19 +0200 Subject: [PATCH] Better test cases, and make properties no longer computed --- catalog/resource_sql_table.go | 4 +- catalog/resource_sql_table_test.go | 123 +++++++++++++++++++++++++++++ qa/testing.go | 6 ++ 3 files changed, 131 insertions(+), 2 deletions(-) diff --git a/catalog/resource_sql_table.go b/catalog/resource_sql_table.go index 9f8101e176..89d3b29697 100644 --- a/catalog/resource_sql_table.go +++ b/catalog/resource_sql_table.go @@ -41,7 +41,7 @@ type SqlTableInfo struct { StorageCredentialName string `json:"storage_credential_name,omitempty" tf:"force_new"` ViewDefinition string `json:"view_definition,omitempty"` Comment string `json:"comment,omitempty"` - Properties map[string]string `json:"properties,omitempty" tf:"computed"` + Properties map[string]string `json:"properties,omitempty"` EffectiveProperties map[string]string `json:"effective_properties,omitempty" tf:"computed"` Options map[string]string `json:"options,omitempty" tf:"force_new"` EffectiveOptions map[string]string `json:"effective_options,omitempty" tf:"computed"` @@ -556,7 +556,7 @@ func ResourceSqlTable() common.Resource { effective := d.Get(effectiveField).(map[string]interface{}) diff := make(map[string]interface{}) for k, userSpecifiedValue := range userSpecified { - if effective[k] != userSpecifiedValue { + if effectiveValue, ok := effective[k]; !ok || effectiveValue != userSpecifiedValue { diff[k] = userSpecifiedValue } } diff --git a/catalog/resource_sql_table_test.go b/catalog/resource_sql_table_test.go index 1e77289716..612c0db2ec 100644 --- a/catalog/resource_sql_table_test.go +++ b/catalog/resource_sql_table_test.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/terraform-provider-databricks/clusters" "github.com/databricks/terraform-provider-databricks/common" "github.com/databricks/terraform-provider-databricks/qa" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/stretchr/testify/assert" "golang.org/x/exp/slices" ) @@ -1398,6 +1399,128 @@ func TestResourceSqlTableCreateTable_OnlyManagedProperties(t *testing.T) { assert.NoError(t, err) } +func TestResourceSqlTable_Diff_ExistingResource(t *testing.T) { + testCases := []struct { + name string + hcl string + instanceState map[string]string + expectedDiff map[string]*terraform.ResourceAttrDiff + }{ + { + "existing resource with no properties or options", + "", + map[string]string{ + "effective_properties.%": "0", + "effective_options.%": "0", + }, + nil, + }, + { + "existing resource with computed properties and options", + "", + map[string]string{ + "effective_properties.%": "1", + "effective_properties.computedprop": "computedvalue", + "effective_options.%": "1", + "effective_options.computedprop": "computedvalue", + }, + nil, + }, + { + "existing resource with property and option", + `properties = { + "myprop" = "myval" + } + options = { + "myopt" = "myval" + }`, + map[string]string{ + "properties.%": "1", + "properties.myprop": "myval", + "options.%": "1", + "options.myopt": "myval", + "effective_properties.%": "1", + "effective_properties.myprop": "myval", + "effective_options.%": "1", + "effective_options.myopt": "myval", + }, + nil, + }, + { + "existing resource with property and option and computed property and computed option", + `properties = { + "myprop" = "myval" + } + options = { + "myopt" = "myval" + }`, + map[string]string{ + "properties.%": "1", + "properties.myprop": "myval", + "options.%": "1", + "options.myopt": "myval", + "effective_properties.%": "2", + "effective_properties.myprop": "myval", + "effective_properties.computedprop": "computedval", + "effective_options.%": "2", + "effective_options.myopt": "myval", + "effective_options.computedopt": "computedval", + }, + nil, + }, + { + "existing resource with conflicting property", + `properties = { + "myprop" = "myval" + } + `, + map[string]string{ + "properties.%": "1", + "properties.myprop": "myval", + "effective_properties.%": "1", + "effective_properties.myprop": "otherval", + "effective_options.%": "0", + }, + map[string]*terraform.ResourceAttrDiff{ + "effective_properties.myprop": {New: "myval", Old: "otherval"}, + }, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + instanceState := testCase.instanceState + if instanceState == nil { + instanceState = make(map[string]string) + } + instanceState["catalog_name"] = "main" + instanceState["schema_name"] = "default" + instanceState["table_type"] = "MANAGED" + instanceState["name"] = "mytable" + instanceState["cluster_id"] = "test-1234" + instanceState["column.#"] = "0" + instanceState["owner"] = "testuser" + + expectedDiff := testCase.expectedDiff + if expectedDiff == nil { + expectedDiff = make(map[string]*terraform.ResourceAttrDiff) + } + qa.ResourceFixture{ + HCL: ` + catalog_name = "main" + schema_name = "default" + table_type = "MANAGED" + name = "mytable" + cluster_id = "test-1234" + owner = "testuser" + ` + testCase.hcl, + InstanceState: instanceState, + ExpectedDiff: expectedDiff, + Resource: ResourceSqlTable(), + }.ApplyNoError(t) + }) + } +} + var baseClusterFixture = []qa.HTTPFixture{ { Method: "GET", diff --git a/qa/testing.go b/qa/testing.go index d14a8a77a1..0fc362c97e 100644 --- a/qa/testing.go +++ b/qa/testing.go @@ -310,6 +310,12 @@ func (f ResourceFixture) Apply(t *testing.T) (*schema.ResourceData, error) { ctx := context.Background() diff, err := resource.Diff(ctx, is, resourceConfig, client) if f.ExpectedDiff != nil { + // Users can specify that there is no diff by setting an empty but initialized map. + // resource.Diff returns nil if there is no diff. + if len(f.ExpectedDiff) == 0 { + assert.Nil(t, diff) + return nil, err + } assert.Equal(t, f.ExpectedDiff, diff.Attributes) return nil, err }