Better test cases, and make properties no longer computed

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
 					}
 				}

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",

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
 	}