Skip to content
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

Support optional diff formatter fn for eq #221

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

orls
Copy link

@orls orls commented Oct 27, 2024

I'm keen to see progress on more useful output when expectations fail – e.g. #130 and various historical tickets on the original repo.

PR #154 proposes directly using go-cmp's cmp.Diff internally for this purpose. That would be great from my POV. But maintainers have given feedback about change scope and extra dependency.

So this is an alternative approach, which tries to assuage those concerns... or at least revive the conversation.


This implements a new Controller option,WithDiffFormatter. It takes a function that is called with the expected and actual values, only when the Eq matcher fails. I think this would cover the vast majority of use-cases.

This gives users freedom to do what they like, including passing to cmp.Diff or a similar lib.

I think this is a significant improvement over the status quo:

  • because true diff-style output, for deep/complex objects, isn't possible with the current GotFormatter and WantFormatter.
  • because it avoids the need for every expectation to be set up with custom formatting wrappers.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants