From dcce20fdfb978412ef9edcbf736e31aa3c7ab23f Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Tue, 11 Feb 2025 18:05:28 +0000 Subject: [PATCH] Improve testing of diagnostic comparer (#36455) * Update simple comparer tests to show that sources (Subject or Context) don't impact matching diags * Add comparer that uses sources (Subject or Context) when comparing diags * Add test cases that show concrete types do affect comparison * Fix test case names * Remove `DiagnosticComparerWithSource` --- internal/tfdiags/compare.go | 9 +++-- internal/tfdiags/compare_test.go | 56 ++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/internal/tfdiags/compare.go b/internal/tfdiags/compare.go index 00a5b76e9580..dfb6f541441a 100644 --- a/internal/tfdiags/compare.go +++ b/internal/tfdiags/compare.go @@ -14,10 +14,12 @@ import "github.com/google/go-cmp/cmp" // // Example usage: // -// cmp.Diff(diag1, diag2, tfdiags.DiagnosticComparer()) -var DiagnosticComparer cmp.Option = cmp.Comparer(diagnosticComparer) +// cmp.Diff(diag1, diag2, tfdiags.DiagnosticComparer) +var DiagnosticComparer cmp.Option = cmp.Comparer(diagnosticComparerSimple) -func diagnosticComparer(l, r Diagnostic) bool { +// diagnosticComparerSimple returns false when a difference is identified between +// the two Diagnostic arguments. +func diagnosticComparerSimple(l, r Diagnostic) bool { if l.Severity() != r.Severity() { return false } @@ -25,6 +27,7 @@ func diagnosticComparer(l, r Diagnostic) bool { return false } + // Do the diagnostics originate from the same attribute name, if any? lp := GetAttribute(l) rp := GetAttribute(r) if len(lp) != len(rp) { diff --git a/internal/tfdiags/compare_test.go b/internal/tfdiags/compare_test.go index 97f14281b9a0..f050387ebf84 100644 --- a/internal/tfdiags/compare_test.go +++ b/internal/tfdiags/compare_test.go @@ -24,11 +24,18 @@ func TestDiagnosticComparer(t *testing.T) { diag2 Diagnostic expectDiff bool }{ + // Correctly identifying things that match "reports that identical diagnostics match": { diag1: hclDiagnostic{&baseError}, diag2: hclDiagnostic{&baseError}, expectDiff: false, }, + // Correctly identifies when things don't match + "reports that diagnostics don't match if the concrete type differs": { + diag1: hclDiagnostic{&baseError}, + diag2: makeRPCFriendlyDiag(hclDiagnostic{&baseError}), + expectDiff: true, + }, "reports that diagnostics don't match if severity differs": { diag1: hclDiagnostic{&baseError}, diag2: func() Diagnostic { @@ -65,19 +72,55 @@ func TestDiagnosticComparer(t *testing.T) { }(), expectDiff: true, }, - "reports that diagnostics don't match if attribute path missing from one differs": { + "reports that diagnostics don't match if attribute path is missing from one": { diag1: func() Diagnostic { return AttributeValue(Error, "summary here", "detail here", cty.Path{cty.GetAttrStep{Name: "foobar1"}}) }(), diag2: func() Diagnostic { - d := hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "summary here", - Detail: "detail here", + return AttributeValue(Error, "summary here", "detail here", cty.Path{}) + }(), + expectDiff: true, + }, + // Scenarios where diagnostics will be considered equavalent, even if they aren't fully the same + "reports that diagnostics match even if sources (Subject) are different; ignored in simple comparison": { + diag1: hclDiagnostic{&baseError}, + diag2: func() Diagnostic { + d := baseError + d.Subject = &hcl.Range{ + Filename: "foobar.tf", + Start: hcl.Pos{ + Line: 0, + Column: 0, + Byte: 0, + }, + End: hcl.Pos{ + Line: 1, + Column: 1, + Byte: 1, + }, + } + return hclDiagnostic{&d} + }(), + }, + "reports that diagnostics match even if sources (Context) are different; ignored in simple comparison": { + diag1: hclDiagnostic{&baseError}, + diag2: func() Diagnostic { + d := baseError + d.Context = &hcl.Range{ + Filename: "foobar.tf", + Start: hcl.Pos{ + Line: 0, + Column: 0, + Byte: 0, + }, + End: hcl.Pos{ + Line: 1, + Column: 1, + Byte: 1, + }, } return hclDiagnostic{&d} }(), - expectDiff: true, }, } @@ -94,5 +137,4 @@ func TestDiagnosticComparer(t *testing.T) { } }) } - }