Add reusable Diagnostic comparer to tfdiags package (#36385)

* Add `DiagnosticComparer` to standardize how diagnostics are compared for equality. This uses code originally in the s3 backend. Co-authored-by: Graham Davison <[email protected]> * Refactor `test` related code to use new comparer * Refactor `s3` related code to use new comparer * Replace use of reflect and go-spew with new diagnostics comparison approach * Fix whitespace * Make `tfdiags.DiagnosticComparer` a var * Fix DiagnosticComparer test --------- Co-authored-by: Graham Davison <[email protected]>
hashicorp · Jan 22, 2025 · b16a509 · b16a509
1 parent 405fbf4
commit b16a509
Show file tree

Hide file tree

Showing 12 changed files with 172 additions and 91 deletions.
diff --git a/internal/backend/remote-state/s3/backend_complete_test.go b/internal/backend/remote-state/s3/backend_complete_test.go
@@ -47,7 +47,7 @@ func ExpectDiagsEqual(expected tfdiags.Diagnostics) DiagsValidator {
 	return func(t *testing.T, diags tfdiags.Diagnostics) {
 		t.Helper()
 
-		if diff := cmp.Diff(diags, expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+		if diff := cmp.Diff(diags, expected, tfdiags.DiagnosticComparer); diff != "" {
 			t.Fatalf("unexpected diagnostics difference: %s", diff)
 		}
 	}
@@ -1610,7 +1610,7 @@ func (d testCaseDriver) Apply(ctx context.Context, t *testing.T) (context.Contex
 
 	var expected tfdiags.Diagnostics
 
-	if diff := cmp.Diff(diags, expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+	if diff := cmp.Diff(diags, expected, tfdiags.DiagnosticComparer); diff != "" {
 		t.Errorf("unexpected diagnostics difference: %s", diff)
 	}
 

diff --git a/internal/backend/remote-state/s3/backend_test.go b/internal/backend/remote-state/s3/backend_test.go
@@ -362,7 +362,7 @@ func TestBackendConfig_InvalidRegion(t *testing.T) {
 			confDiags := b.Configure(configSchema)
 			diags = diags.Append(confDiags)
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -528,7 +528,7 @@ func TestBackendConfig_DynamoDBEndpoint(t *testing.T) {
 			raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config))
 			b := raw.(*Backend)
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 
@@ -660,7 +660,7 @@ func TestBackendConfig_IAMEndpoint(t *testing.T) {
 
 			_, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config))
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -783,7 +783,7 @@ func TestBackendConfig_S3Endpoint(t *testing.T) {
 			raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config))
 			b := raw.(*Backend)
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 
@@ -943,7 +943,7 @@ func TestBackendConfig_EC2MetadataEndpoint(t *testing.T) {
 			raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config))
 			b := raw.(*Backend)
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 
@@ -1435,7 +1435,7 @@ func TestBackendConfig_PrepareConfigValidation(t *testing.T) {
 
 			_, valDiags := b.PrepareConfig(populateSchema(t, b.ConfigSchema(), tc.config))
 
-			if diff := cmp.Diff(valDiags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(valDiags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -1935,7 +1935,7 @@ func TestBackendConfig_Proxy(t *testing.T) {
 			raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config))
 			b := raw.(*Backend)
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 
@@ -2499,7 +2499,7 @@ func TestBackendConfigKmsKeyId(t *testing.T) {
 				diags = diags.Append(confDiags)
 			}
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Fatalf("unexpected diagnostics difference: %s", diff)
 			}
 
@@ -2629,7 +2629,7 @@ func TestBackendSSECustomerKey(t *testing.T) {
 				diags = diags.Append(confDiags)
 			}
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Fatalf("unexpected diagnostics difference: %s", diff)
 			}
 
@@ -3283,7 +3283,7 @@ func TestAssumeRole_PrepareConfigValidation(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateNestedAttribute(assumeRoleSchema, config, path, &diags)
 
-			if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})

diff --git a/internal/backend/remote-state/s3/testing_test.go b/internal/backend/remote-state/s3/testing_test.go
diff --git a/internal/backend/remote-state/s3/validate_test.go b/internal/backend/remote-state/s3/validate_test.go
@@ -73,7 +73,7 @@ func TestValidateKMSKey(t *testing.T) {
 
 			diags := validateKMSKey(path, testcase.in)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -137,7 +137,7 @@ func TestValidateKeyARN(t *testing.T) {
 
 			diags := validateKMSKeyARN(path, testcase.in)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -189,7 +189,7 @@ func TestValidateStringLenBetween(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateStringLenBetween(min, max)(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -232,7 +232,7 @@ func TestValidateStringMatches(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateStringMatches(testcase.re, "Value must be like ok")(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -298,7 +298,7 @@ func TestValidateARN(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateARN(validators...)(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -386,7 +386,7 @@ The string content was valid JSON, your policy document may have been double-enc
 			var diags tfdiags.Diagnostics
 			validateIAMPolicyDocument(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -449,7 +449,7 @@ func TestValidateSetStringElements(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateSetStringElements(validators...)(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -512,7 +512,7 @@ func TestValidateSetStringElements(t *testing.T) {
 // 			var diags tfdiags.Diagnostics
 // 			validateStringSetValues(validators...)(testcase.val, path, &diags)
 
-// 			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+// 			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 // 				t.Errorf("unexpected diagnostics difference: %s", diff)
 // 			}
 // 		})
@@ -578,7 +578,7 @@ func TestValidateDuration(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateDuration(validators...)(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -635,7 +635,7 @@ func TestValidateDurationBetween(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateDurationBetween(min, max)(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -712,7 +712,7 @@ func TestValidateStringLegacyURL(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateStringLegacyURL(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -789,7 +789,7 @@ func TestValidateStringValidURL(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateStringValidURL(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})
@@ -832,7 +832,7 @@ func Test_validateStringDoesNotContain(t *testing.T) {
 			var diags tfdiags.Diagnostics
 			validateStringDoesNotContain(testcase.s)(testcase.val, path, &diags)
 
-			if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" {
+			if diff := cmp.Diff(diags, testcase.expected, tfdiags.DiagnosticComparer); diff != "" {
 				t.Errorf("unexpected diagnostics difference: %s", diff)
 			}
 		})

diff --git a/internal/command/arguments/modules_test.go b/internal/command/arguments/modules_test.go
@@ -4,10 +4,9 @@
 package arguments
 
 import (
-	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
@@ -83,8 +82,8 @@ func TestParseModules_invalid(t *testing.T) {
 			if *got != *tc.want {
 				t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want)
 			}
-			if !reflect.DeepEqual(gotDiags, tc.wantDiags) {
-				t.Fatalf("wrong result\ngot: %s\nwant: %s", spew.Sdump(gotDiags), spew.Sdump(tc.wantDiags))
+			if diff := cmp.Diff(gotDiags, tc.wantDiags, tfdiags.DiagnosticComparer); diff != "" {
+				t.Fatalf("unexpected diff in diags:\n%s", diff)
 			}
 		})
 	}

diff --git a/internal/command/arguments/output_test.go b/internal/command/arguments/output_test.go
@@ -4,10 +4,9 @@
 package arguments
 
 import (
-	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
@@ -137,8 +136,8 @@ func TestParseOutput_invalid(t *testing.T) {
 			if *got != *tc.want {
 				t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want)
 			}
-			if !reflect.DeepEqual(gotDiags, tc.wantDiags) {
-				t.Errorf("wrong result\ngot: %s\nwant: %s", spew.Sdump(gotDiags), spew.Sdump(tc.wantDiags))
+			if diff := cmp.Diff(gotDiags, tc.wantDiags, tfdiags.DiagnosticComparer); diff != "" {
+				t.Fatalf("unexpected diff in diags:\n%s", diff)
 			}
 		})
 	}

diff --git a/internal/command/arguments/show_test.go b/internal/command/arguments/show_test.go
@@ -4,10 +4,9 @@
 package arguments
 
 import (
-	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
@@ -94,8 +93,8 @@ func TestParseShow_invalid(t *testing.T) {
 			if *got != *tc.want {
 				t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want)
 			}
-			if !reflect.DeepEqual(gotDiags, tc.wantDiags) {
-				t.Errorf("wrong result\ngot: %s\nwant: %s", spew.Sdump(gotDiags), spew.Sdump(tc.wantDiags))
+			if diff := cmp.Diff(gotDiags, tc.wantDiags, tfdiags.DiagnosticComparer); diff != "" {
+				t.Fatalf("unexpected diff in diags:\n%s", diff)
 			}
 		})
 	}

diff --git a/internal/command/arguments/test_test.go b/internal/command/arguments/test_test.go
@@ -4,10 +4,8 @@
 package arguments
 
 import (
-	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 
@@ -167,8 +165,8 @@ func TestParseTest(t *testing.T) {
 				t.Errorf("diff:\n%s", diff)
 			}
 
-			if !reflect.DeepEqual(diags, tc.wantDiags) {
-				t.Errorf("wrong result\ngot: %s\nwant: %s", spew.Sdump(diags), spew.Sdump(tc.wantDiags))
+			if diff := cmp.Diff(diags, tc.wantDiags, tfdiags.DiagnosticComparer); diff != "" {
+				t.Errorf("unexpected diff in diags:\n%s", diff)
 			}
 		})
 	}

diff --git a/internal/command/arguments/validate_test.go b/internal/command/arguments/validate_test.go
@@ -4,11 +4,9 @@
 package arguments
 
 import (
-	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
-
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
@@ -117,8 +115,8 @@ func TestParseValidate_invalid(t *testing.T) {
 			if *got != *tc.want {
 				t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want)
 			}
-			if !reflect.DeepEqual(gotDiags, tc.wantDiags) {
-				t.Errorf("wrong result\ngot: %s\nwant: %s", spew.Sdump(gotDiags), spew.Sdump(tc.wantDiags))
+			if diff := cmp.Diff(gotDiags, tc.wantDiags, tfdiags.DiagnosticComparer); diff != "" {
+				t.Fatalf("unexpected diff in diags:\n%s", diff)
 			}
 		})
 	}