Skip to content
Snippets Groups Projects

all: Replace common usage of cmp.Diff with testutil.DeepCompare

Closed Administrator requested to merge testutil-deepcompare into master

Created by: ryanslade

It's pretty common in our codebase to use `cmp.Diff':

rg cmp.Diff | wc -l
473

Should we replace it with a helper function?

I've made the change to the simple case where we check for a non empty diff and then call t.Fatal

To cover all cases we'd also need to handle uses of t.Fatalf and t.Error/f

We could add testutil.DeepCompareFatal/f that takes a format string.

WDYT?

Merge request reports

Approval is optional

Closed by avatar (Jul 5, 2025 4:31am UTC)

Merge details

  • The changes were not merged into master.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: keegancsmith

    I don't think this is worth it for one main reason. It is useful to use this sort of pattern:

    if d := cmp.Diff(want, have); d != "" {
      t.Fatalf("mismatch (-want, +have):\n%s", d)
    }

    ie the -want and +have are very useful. You can't enforce the param order for this. Maybe if you come up with a reasonable API that could do that? eg testutil.T(t).want(a).have(b). But my personal preference is that I don't really like those sort of APIs.

  • Created by: ryanslade

    If we named the parameters want and have instead of x and y then we could make it a convention.

    Agreed, I also don't like those "fluent" apis.

Please register or sign in to reply
Loading