Author ElizabethU
Recipients ElizabethU, cjw296, mariocj89, michael.foord, p-ganssle, serhiy.storchaka, xtreak
Date 2019-07-18.07:42:49
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1563435769.37.0.0814746020122.issue37555@roundup.psfhosted.org>
In-reply-to
Content
Just added a fix for the tests I added in the last commit showing the order sensitivity in assertEqual when dealing with ANY and _Call or _CallList objects. _Call and _CallList currently depend on ordering to correctly process that an object being compared to ANY with __eq__ should return True. I put more notes in the commit message.

A note on the two newer tests: I want to be sensitive to the idea that I might be pushing back against desired behavior here. I think there is a case to be made that one should expect mock.assertEqual(a, b) to implement a == b, and not care about b == a, in which case maybe order sensitivity in assertEqual makes sense. That said, today I showed a few of my most Python experience heavy colleagues the tests showing that assertEqual was order sensitive when passed _Call or _CallList objects, and based on anecdotal evidence the existing behavior definitely violates the principle of least astonishment. Additionally, the test Karthikeyan kindly linked me https://github.com/python/cpython/blob/master/Lib/unittest/test/testmock/testhelpers.py#L46 seems to imply that an order agnostic assertEqual is the desired behavior.

A note on the fix: This fix feels like it's not as small as it could be. I am still musing over whether it'd be better if instead of the or statements and swapping the comparison order in _CallList's __contains__ method, I made a wrapper that would swap that comparison order in the case that one of the arguments is a BoundArgument, since that was the original problem. That fix would probably not affect the order sensitivity of assertEqual, so it's partially about what we want there. I'll play around with it and see what I can discover. I'll also see if I can do anything to more explicitly to check for ANY, as Paul suggested.

I'm very curious what you all think about whether the assertEqual behavior is a feature or a bug, and if the other fix idea seems better than this one. If neither of these ideas seem like a good fix, I'd love some guidance on what concerns are and what might be a good approach to try next.
History
Date User Action Args
2019-07-18 07:42:49ElizabethUsetrecipients: + ElizabethU, cjw296, michael.foord, serhiy.storchaka, p-ganssle, mariocj89, xtreak
2019-07-18 07:42:49ElizabethUsetmessageid: <1563435769.37.0.0814746020122.issue37555@roundup.psfhosted.org>
2019-07-18 07:42:49ElizabethUlinkissue37555 messages
2019-07-18 07:42:49ElizabethUcreate