Improve testing of diagnostic comparer #36455
Conversation
SarahFrench
added
the
no-changelog-needed
internal/tfdiags/compare.go
Outdated
| // Example usage: | ||
| // | ||
| // cmp.Diff(diag1, diag2, tfdiags.DiagnosticComparerWithSource) | ||
| var DiagnosticComparerWithSource cmp.Option = cmp.Comparer(diagnosticComparerWithSource) |
There was a problem hiding this comment.
Question for reviewer - do you know of any tests that would need/want to assert about the source data in a diagnostic? If we can't find a use for this then it should be removed
There was a problem hiding this comment.
I'd like to know too. Otherwise, I wonder if we should wait for such use case before building such a comparer
There was a problem hiding this comment.
Pretty much anything in the configs and stackconfig packages should care about the source data, as it is important in those cases that the errors point to the part of the configuration that is invalid. An example of this would be the tests here: https://github.com/hashicorp/terraform/blob/main/internal/configs/config_test.go#L542-L582. But, there should be lots of examples in those packages.
There was a problem hiding this comment.
It looks like the tests in configs use hcl.Diagnostic/hcl.Diagnostics types and in stackconfig the diags comparison uses sourceless diags in assertions. So overall it looks like this comparer isn't needed. I'll slim down the PR to just the edits to the existing comparer and then open up the stacked PR for review too.
SarahFrench
changed the title
SarahFrench
changed the title
|
Thanks! |
This PR is part of recognising that tests compare diagnostics in different ways in different places. It's unclear if this is due to a need or just due to varying approaches being used in different parts of the code base.
This PR updates the tests for the 'comparer' in ./internal/tfdiags/compare.go to make it clearer about what to expect from the comparer:
cmpraises diffs if the concrete type is different.I'm opening a related PR that explores refactoring existing tests to use a new set of test helpers in the
tfdiagspackage: #36456 . Those test helpers will allow users to convert diagnostics to RPC-friendly diagnostic struct types and compare them using the comparers in this PR.Target Release
N/A
CHANGELOG entry