mirror of
https://github.com/opentffoundation/opentf.git
synced 2025-12-19 17:59:05 -05:00
My original intention was just to reduce our number of dependencies by standardizing on a single comparison library, but in the process of doing so I found various examples of the kinds of problems that caused this codebase to begin adopting go-cmp instead of go-test/deep in the first place, which make it easy to accidentally write a false-positive test that doesn't actually check what the author thinks is being checked: - deep.Equal silently ignores unexported fields, so comparing two values that differ only in data in unexported fields succeeds even when it ought not to. TestContext2Apply_multiVarComprehensive in package tofu was an excellent example of this problem: it had various test assertions that were actually checking absolutely nothing, despite appearing to compare pairs of cty.Value. - deep.Equal also silently ignores anything below a certain level of nesting, and so comparison of deep data structures can appear to succeed even though they don't actually match. There were a few examples where that problem had already been found and fixed by temporarily overriding the package deep global settings, but with go-cmp the default behavior already visits everything, or panics if it cannot. This does mean that in a few cases this needed some more elaborate options to cmp.Diff to align with the previous behavior, which is a little annoying but overall I think better to be explicit about what each test is relying on. Perhaps we can rework these tests to need fewer unusual cmp options in future, but for this commit I want to keep focused on the smallest possible changes to remove our dependency on github.com/go-test/deep . Signed-off-by: Martin Atkins <mart@degeneration.co.uk>