-
Notifications
You must be signed in to change notification settings - Fork 1.7k
assert: switch the diff library to go-diff #1546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
31ad2a7
to
bc848ba
Compare
Why its not merged yet? |
late to the party but @mitioshi thanks for going down this path, complex diffs in tests in golang assertion libs could use some of TLC. |
Merge it at all costs |
From #1546 (comment):
@Devourian I consider this to be abusive because not respectful of the benevolent work of maintainers who will have to endure the consequences of a such change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch is incomplete: difflib is not replaced in package mock
. The result is that it adds a direct dependency (and many indirect dependencies, including circular dependency on older Testify), but doesn't remove the original dependency.
"unicode/utf8" | ||
|
||
"github.com/sergi/go-diff/diffmatchpatch" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this empty line.
// | ||
// This function does no assertion of any kind. | ||
// | ||
// Deprecated: Use [EqualExportedValues] instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you removing this?
Our preference goes to #1708 which imports the frozen However, an alternative way would be to provide a mean for the user to plug his chosen alternate |
Summary
This PR changes the diff library used in the assert package to go-diff.
Changes
the library used for diffing is replaced to
go-diff/diffmatchpatch
which is actively maintained and enables us to access structured diffs.Motivation
See the parent PR for more context
difflib
is no longer maintained and cannot be extended to modify the diff structure. We end up outputting whatever the library spews without being able to modify it easily. Having the access to the diff structure gives us a more fine-grained control over what we show to users.The initial motivation for this PR was to add colorized diffs to
assert.Equal
. Accessing the diff structure would make this task trivialRelated issues
Split from #1480 as per @dolmen's suggestion