Add tests & overwrite less of diff

CHANGELOG_PENDING.md

@@ -8,8 +8,8 @@
 
 ### Bug Fixes
 
-- [cli/engine] - Accuretly computes the fields changed when diffing with unhelpful providers. This
-  allows the replaceOnChanges feature to be respected for all providers.
+- [cli/engine] - Accurately computes the fields changed when diffing with unhelpful providers. This
+  allows the `replaceOnChanges` feature to be respected for all providers.
   [#8488](https://github.com/pulumi/pulumi/pull/8488)
 
 - [codegen/go] - Respect default values in Pulumi object types.

pkg/resource/deploy/step_generator.go

@@ -1132,7 +1132,13 @@ func diffResource(urn resource.URN, id resource.ID, oldInputs, oldOutputs,
 		return diff, err
 	}
 	if diff.Changes == plugin.DiffUnknown {
-		diff = plugin.NewDiffResultFromObjectDiff(oldInputs.Diff(newInputs), ignoreChanges...)
+		tmp := oldInputs.Diff(newInputs)
+		if tmp.AnyChanges() {
+			diff.Changes = plugin.DiffSome
+			diff.ChangedKeys = tmp.ChangedKeys()
+		} else {
+			diff.Changes = plugin.DiffNone
+		}
 	}
 	return diff, nil
 }

pkg/resource/deploy/step_generator_test.go

@@ -3,6 +3,7 @@ package deploy
 import (
 	"testing"
 
+	"github.com/pulumi/pulumi/pkg/v3/resource/deploy/deploytest"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
 	"github.com/stretchr/testify/assert"
@@ -194,3 +195,62 @@ func TestApplyReplaceOnChangesEmptyDetailedDiff(t *testing.T) {
 	}
 
 }
+
+func TestEngineDiffResource(t *testing.T) {
+	cases := []struct {
+		name                 string
+		oldInputs, newInputs resource.PropertyMap
+		expected             []resource.PropertyKey
+	}{
+		{
+			name: "Empty diff",
+			oldInputs: resource.NewPropertyMapFromMap(map[string]interface{}{
+				"val1": resource.NewPropertyValue(8),
+				"val2": resource.NewPropertyValue("hello"),
+			}),
+			newInputs: resource.NewPropertyMapFromMap(map[string]interface{}{
+				"val1": resource.NewPropertyValue(8),
+				"val2": resource.NewPropertyValue("hello"),
+			}),
+			expected: nil,
+		},
+		{
+			name: "All changes",
+			oldInputs: resource.NewPropertyMapFromMap(map[string]interface{}{
+				"val0": resource.NewPropertyValue(3.14),
+			}),
+			newInputs: resource.NewPropertyMapFromMap(map[string]interface{}{
+				"val1": resource.NewNumberProperty(42),
+				"val2": resource.NewPropertyValue("world"),
+			}),
+			expected: []resource.PropertyKey{"val0", "val1", "val2"},
+		},
+		{
+			name: "Some changes",
+			oldInputs: resource.NewPropertyMapFromMap(map[string]interface{}{
+				"val1": resource.NewPropertyValue(42),
+			}),
+			newInputs: resource.NewPropertyMapFromMap(map[string]interface{}{
+				"val1": resource.NewNumberProperty(42),
+				"val2": resource.NewPropertyValue("world"),
+			}),
+			expected: []resource.PropertyKey{"val2"},
+		},
+	}
+	urn := resource.URN("urn:pulumi:dev::website-and-lambda::aws:s3/bucket:Bucket::my-bucket")
+	id := resource.ID("someid")
+	var oldOutputs resource.PropertyMap
+	allowUnknowns := false
+	ignoreChanges := []string{}
+	provider := deploytest.Provider{}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			diff, err := diffResource(urn, id, c.oldInputs, oldOutputs, c.newInputs, &provider, allowUnknowns, ignoreChanges)
+			t.Logf("diff.ChangedKeys = %v", diff.ChangedKeys)
+			t.Logf("diff.StableKeys = %v", diff.StableKeys)
+			t.Logf("diff.ReplaceKeys = %v", diff.ReplaceKeys)
+			assert.NoError(t, err)
+			assert.EqualValues(t, c.expected, diff.ChangedKeys)
+		})
+	}
+}

sdk/go/common/resource/plugin/provider.go

@@ -217,34 +217,6 @@ type DiffResult struct {
 	DeleteBeforeReplace bool                    // if true, this resource must be deleted before recreating it.
 }
 
-// Transforms a `ObjectDiff` into an equivalent `DiffResult`. Because ObjectDiff has no way of
-// determining what should be changed vs replaced, all operations are added to the ChangedKeys set.
-//
-// Any key in ignoreChanged is treated as stable.
-func NewDiffResultFromObjectDiff(obj *resource.ObjectDiff, ignoreChanged ...string) DiffResult {
-	result := DiffResult{}
-	if !obj.AnyChanges() {
-		result.Changes = DiffNone
-		return result
-	}
-
-	ignoreSet := make(map[string]bool, len(ignoreChanged))
-	for _, v := range ignoreChanged {
-		ignoreSet[v] = true
-	}
-
-	result.Changes = DiffSome
-	for _, k := range obj.Keys() {
-		if !obj.Changed(k) || ignoreSet[string(k)] {
-			result.StableKeys = append(result.StableKeys, k)
-		} else {
-			result.ChangedKeys = append(result.ChangedKeys, k)
-		}
-	}
-
-	return result
-}
-
 // Replace returns true if this diff represents a replacement.
 func (r DiffResult) Replace() bool {
 	for _, v := range r.DetailedDiff {

sdk/go/common/resource/properties_diff.go

@@ -80,6 +80,19 @@ func (diff *ObjectDiff) Keys() []PropertyKey {
 	return ks
 }
 
+// All keys where Changed(k) = true.
+func (diff *ObjectDiff) ChangedKeys() []PropertyKey {
+	var ks []PropertyKey
+	if diff != nil {
+		for _, k := range diff.Keys() {
+			if diff.Changed(k) {
+				ks = append(ks, k)
+			}
+		}
+	}
+	return ks
+}
+
 // ValueDiff holds the results of diffing two property values.
 type ValueDiff struct {
 	Old    PropertyValue // the old value.