stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
23.44k stars 1.6k forks source link

Unexpected matches by Argument.Diff #752

Open mikenicholson opened 5 years ago

mikenicholson commented 5 years ago

I'm seeing unexpected behavior in mock call matching that is resulting from the way Arguments.Diff() compares objects. If I have something like:

   snake := &mocks.Animal{}
   honeyBadger := &mocks.Animal{}

   mockDetector := &mocks.CareDetector{}
   mockDetector.On("DoesItCare", snake).Return(true)
   mockDetector.On("DoesItCare", honeyBadger).Return(false)

   //  ... Somewhere later, probably buried in the system under test
  doesItCare := mockDetector.DoesItCare(honeyBadger)

  assert.False(t, doesItCare)  // Causes a test failure

I expect mockDetector to return false when passed honeyBadger and true when passed snake. Instead, I see true for both. If I switched the order I set up the expectations it would return false for both. It's clearly matching the first expectation, regardless of the arguments.

I can trace this down to an issue with Arguments.Diff:

args := mocks.Arguments{honeyBadger}
output, difference := args.Diff([]interface{}{snake}) // differences is 0

The underlying issue is caused by the fact that Arguments.Diff uses assert.EqualObjects() at line 710, which performs a deep equality check rather than comparing the pointers.

Coming from other languages and mocking frameworks this has always worked. Maybe I'm still new to Go, but the fact that the fallback is a deep equality check rather than a simple '==' was surprising. this might be more clear if it was made more explicit by implementing a matcher like mockDetector.On("DoesItCare", mock.SameObject(snake)) or mock.DeeplyEqual(snake) depending on which behavior should be default. A mention of this behavior in the documentation would help if it was intended.

I'm also a little surprised by the fact that two different mocks end up being deeply equal.

mikenicholson commented 5 years ago

More unexpected behavior related to the same root cause:

snake := &mocks.Animal{}
honeyBadger := &mocks.Animal{}

assert.False(t, snake == honeyBadger) // Passes
assert.NotContains(t, []*mocks.Animal{snake}, honeyBadger) // Fails

In this case, the fact that underlying equality checks default to something other than Go's definition of equality isn't called out in the documentation for more complex methods like assert.Contains.

The documentation is clear for assert.Equal() and an alternative to get the standard golang behavior (assert.Same()) is present, so maybe these more complex assertions deserve a similar treatment e.g. via a ContainsSame() ?