refactor and improvements

[puellanivis]

Addresses:

• https://github.com/go-test/deep/issues/3 (diffs are reported from their reflect.Value which will use fmt.Stringer if implemented
• https://github.com/go-test/deep/issues/22 (TimePrecision will use Truncate on time.Duration and time.Time if non-zero)
• https://github.com/go-test/deep/issues/32 (Equal(bType) bool will be called for all kinds of types, not just structs)
• https://github.com/go-test/deep/issues/45 (check CanInterface before calling Error() string on errors)
• https://github.com/go-test/deep/issues/46 (If CompareFunctions is true, deep.Equal will compare functions the same as reflect.DeepEqual)
• https://github.com/go-test/deep/issues/47 (Before recursing on pointers, we check if either a.Pointer or b.Pointer have already been seen.)

Additional edge-cases https://github.com/go-test/deep/issues/48

• Stronger checks on Equal(bType) (…) to ensure it returns a bool kind, before executing it and panicking on it being non-Bool
• Consider negative zero as equal to zero. (IEEE 754 defines that negative zero and positive zero should normally compare equal)
• Part of checking pointers and interfaces before using Elem(), we check to see if they are nil first, and allows more clarity between getting a zero reflect.Value vs having a typed-pointer that is nil. (This means differences are reported as <nil pointer != *bType rather than <nil pointer> != bType.)
• Comparing an untyped nil, (for instance a nil value assigned into an interface) the difference will be reported as <untyped nil> != b
• signigifcantly overhauled all of the tests to focus on some functions shouldBeEqual vs shouldBeDiffs, and using deterministic values for time.Time comparisons rather than time.Now() so we can test the actual output of the tests, rather than focusing only on “differences were found”.

[daniel-nichter]

Hello 👋 Thanks for the PR and fixes. I'd like to include this work, but the blocker is the changes to the test suite. Given the complexity of this pkg, the test suite is critical to ensure that no regressions are introduced. Since the test suite was essentially rewritten top to bottom, I can't tell if the original test suite still passed with the code changes and additions. The new test suite passes, but without a painstaking review of the 1,300+ lines changed, we can't know with certainty if that's coincidence or not.

So to merge this, please revert the test suite and only append new tests for the new functionality. Please also keep the existing code style, i.e. tests are in-place, not factored out to functions like shouldBeEqual which make line numbers reported by t.Error() meaningless.

Overall, these changes might be better as separate PRs. The 400+ line changes to the main code are difficult to compare. It'd be a lot easier to take it one by one because, again, the code is complex and tiny details matter. After all, this code is 4 years of fine tweaks and edits by 10 other engineers, so I'm trying to preserve their work and fixes, too.

[puellanivis]

I had already considered the line number issue, which is why the helper functions call t.Helper(), so that they are skipped when printing file:lineno in the test results.

I’ll consider breaking things out into separate PRs 🤔 and limiting the test changes to an absolute minimum for each.

[gaby]

@puellanivis @daniel-nichter Any updates on this?

[puellanivis]

My company pivoted away from Go, so I haven’t had the time on their behalf to work on breaking things up properly.

[daniel-nichter]

My company pivoted away from Go

Sorry to hear that. Perhaps I'll try to implement/fix some items from your list in issue #48.

For now, I'll close this PR but keep that issue open.