Skip to content

Conversation

mitioshi
Copy link

@mitioshi mitioshi commented Feb 22, 2024

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 trivial

Related issues

Split from #1480 as per @dolmen's suggestion

@mitioshi mitioshi force-pushed the switch-diff-library-to-go-diff branch from 31ad2a7 to bc848ba Compare February 22, 2024 07:09
@dolmen dolmen added the dependencies Pull requests that update a dependency file label Mar 7, 2024
@Softianix
Copy link

Why its not merged yet?

@stefanpenner
Copy link

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.

@Softianix
Copy link

Merge it at all costs

@dolmen
Copy link
Collaborator

dolmen commented May 25, 2025

From #1546 (comment):

Merge it at all costs

@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.

Copy link
Collaborator

@dolmen dolmen left a 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"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this empty line.

Suggested change

//
// This function does no assertion of any kind.
//
// Deprecated: Use [EqualExportedValues] instead.
Copy link
Collaborator

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?

@dolmen
Copy link
Collaborator

dolmen commented Jun 2, 2025

Our preference goes to #1708 which imports the frozen difflib into this module: there is a lower risk of regressions.

However, an alternative way would be to provide a mean for the user to plug his chosen alternate difflib, without having that dependency linked (via go.mod) in Testify itself. See how this is done for YAML in #1579.

@dolmen dolmen added the enhancement: output format Enhancement related to formatting of messages label Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement: output format Enhancement related to formatting of messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants