Skip to content

Commit

Permalink
Improve testing of diagnostic comparer (#36455)
Browse files Browse the repository at this point in the history
* 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`
  • Loading branch information
SarahFrench authored Feb 11, 2025
1 parent 17f4dcf commit dcce20f
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 10 deletions.
9 changes: 6 additions & 3 deletions internal/tfdiags/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ 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
}
if l.Description() != r.Description() {
return false
}

// Do the diagnostics originate from the same attribute name, if any?
lp := GetAttribute(l)
rp := GetAttribute(r)
if len(lp) != len(rp) {
Expand Down
56 changes: 49 additions & 7 deletions internal/tfdiags/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
},
}

Expand All @@ -94,5 +137,4 @@ func TestDiagnosticComparer(t *testing.T) {
}
})
}

}

0 comments on commit dcce20f

Please sign in to comment.