Conversation
| slices.Sort(actualNeedles) | ||
| assert.DeepEqual(t, actualNeedles, needles) | ||
| if diff := cmp.Diff(gotNeedles, wantNeedles, sortSlices); diff != "" { | ||
| t.Errorf("replacer.Needles() diff (-got +want):\n%s", diff) |
There was a problem hiding this comment.
Do you use an editor snippet or something for this?
It's quite fiddly to type out many times 😅
There was a problem hiding this comment.
I see https://google.github.io/styleguide/go/decisions#print-diffs doesn't have a strong opinion on the order of (got, want) vs (want, got).
Here's an example from another test I'm trying this style on, where I used the same (got, want) order you've used here:
env_test.go:44: env.Get() diff (-got +want):
string(
- "intrestng",
+ "interesting",
)
To me that diff reads backwards… I'd expect the + to be showing me what it got, as a delta against what it wanted. (Perhaps there's also an argument for the other direction.) I guess I think of the - side of a diff as the older/control side, and the + side as the newer/experiment/work-in-progress side.
I do note that the first (primary?) example in that style guide does use diff (-want +got) which matches what makes sense to me.
Any thoughts?
There was a problem hiding this comment.
Do you use an editor snippet or something for this?
It's quite fiddly to type out many times 😅
I don't, but I should look into using some IDE feature for this. As fiddly as it is to write, I think it pays for itself in time spent writing out.
opinion on the order of (got, want) vs (want, got)
I've seen a few discussions on this as the print-diffs guidance was written 😆 Before it became common, I already had "got before want" drilled into my brain. Someone (I forget who) raised the idea that (-got, +want) is a diff describing "here's what would need to change in the output to match the test", and the original stance (which was to prefer (-want +got)) was weakened to "make the diff direction clear".
Description
Arranging a test I looked at recently to be closer to personal preference.1 I promise I won't let my feelings get hurt if this PR goes nowhere.
Context
Noticed during recent work.
Changes
Remove
assertin favour of stdlibtestingplusgo-cmp.Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go fmt ./...)Footnotes
https://google.github.io/styleguide/go/decisions#assertion-libraries ↩