_CallList.__contains__ doesn't always respect ANY.

[ElizabethU]

BPO	37555
Nosy	@cjw296, @voidspace, @serhiy-storchaka, @pganssle, @mariocj89, @tirkarthi, @ElizabethU, @iritkatriel
PRs	bpo-37555: Update _CallList.__contains__ to respect ANY #14700

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-05-26.13:26:55.129>
created_at = <Date 2019-07-11.05:12:04.785>
labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
title = "_CallList.__contains__ doesn't always respect ANY."
updated_at = <Date 2021-05-26.13:26:55.129>
user = 'https://github.com/ElizabethU'

bugs.python.org fields:

activity = <Date 2021-05-26.13:26:55.129>
actor = 'iritkatriel'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-26.13:26:55.129>
closer = 'iritkatriel'
components = ['Library (Lib)']
creation = <Date 2019-07-11.05:12:04.785>
creator = 'ElizabethU'
dependencies = []
files = []
hgrepos = []
issue_num = 37555
keywords = ['patch']
message_count = 26.0
messages = ['347652', '347654', '347698', '347722', '347729', '347730', '347731', '347733', '347737', '347739', '347753', '347818', '347941', '347949', '347961', '348068', '348105', '348121', '348214', '348278', '348453', '352351', '378782', '378800', '378837', '393497']
nosy_count = 8.0
nosy_names = ['cjw296', 'michael.foord', 'serhiy.storchaka', 'p-ganssle', 'mariocj89', 'xtreak', 'ElizabethU', 'iritkatriel']
pr_nums = ['14700']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37555'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[ElizabethU]

I have a test that goes something like:

@patch('a.place.to.patch')
def test_a_thing_calls_what_it_should(self, my_mock):
    # Set up logic here
    my_mock.assert_has_calls([
        call(
            ANY,
            Decimal('20')
        ),
        call(
            ANY,
            Decimal('10')
        )
    ])```

Which fails, where my_mock.call_args_list looks like 

[(, Decimal('20')), (, Decimal('10'))]

This seems like wrong behavior. ANY should be happy to be compared to anything, even a Django object. Doing some digging, I found that on line 340 of cpython/Lib/unittest/mock.py \_CallList is overriding \_\_contains__ and comparing each item in the tuples with what I'd passed in to assert_has_calls on the right, which means that instead of using ANY.\_\_eq__, it's calling the Django model's \_\_eq__ with ANY as an argument. Django first checks if the thing it's comparing to is another Django model, and returns False if not. So, \<DjangoModel\> == ANY is False, but ANY == \<DjangoModel\> is True. I know that this could also be considered a bug with Django, and I plan to file one with them too, but I don't see any downside to improving the mock library to be more defensive in honoring ANY over any other custom class's overridden \_\_eq__ method.

[tirkarthi]

Thanks for the report. Can you please add an example without Django or other dependencies so that I can try reproducing it? I am trying out the below program from the report where Foo has overridden __eq__ to return False if the other object being compared is not a Foo object. In the next statements ANY == Foo() returns True since the left side's object.__eq__ is used which in this case is ANY.__eq__ as you have noted in the original report.

When list of call objects are compared there is also a code comment about this in the tuple comparison of args and kwargs such that ANY is placed on the left side [0] so that ANY.__eq__ is used. A pure python example and traceback if any would help here.

from unittest.mock import call, ANY, Mock

class Foo:

    def __eq__(self, other):
        if not isinstance(other, Foo):
            return False
        return True

m = Mock()
obj = Foo()
m(obj, 1)
m.assert_has_calls([call(ANY, 1)])

print(ANY == Foo()) # ANY.__eq__ is called
print(Foo() == ANY) # Foo().__eq__ is called

Is the report more about the below case where position of call objects returns different values?

print(call(ANY, 1) == call(obj, 1)) # False
print(call(obj, 1) == call(ANY, 1)) # True

[0]

cpython/Lib/unittest/mock.py

Line 2407 in 2a3d4d9

return (other_args, other_kwargs) == (self_args, self_kwargs)

[serhiy-storchaka]

Karthikeyan, I think there is a flaw in your example. The good __eq__ should return NotImplemented instead of False for other types. This wil allow the right operand's __eq__ to play.

[tirkarthi]

Karthikeyan, I think there is a flaw in your example. The good __eq__ should return NotImplemented instead of False for other types. This wil allow the right operand's __eq__ to play.

Serhiy, my example was to try reproducing the original report where Django model tries to check if the other's type and returns False if the type is not a Django model. There could be cases where __eq__ implementation of a class could be just returning False instead of raising a NotImplemented error due to difference in type that affects comparison of ANY.

Django first checks if the thing it's comparing to is another Django model, and returns False if not. So, <DjangoModel> == ANY is False, but ANY == <DjangoModel> is True.

[serhiy-storchaka]

Then I think the problem is with DjangoModel.

I suggest to close this issue as "third party".

[serhiy-storchaka]

Though some __eq__ implementations in the stdlib should be fixed too.

[tirkarthi]

https://docs.python.org/3/library/unittest.mock.html#any

mock.ANY doesn't make any guarantees that this is a generic implementation that can be used to test equality against any object irrespective of the order to return True. It's documented only to be used for mock calls but there are many users using it for other objects too outside of mock assertions. mock.ANY also depends on the order so that mock.ANY.__eq__ is used but I am not sure about documenting the order here.

Would be helpful if Elizabeth presents a simplified example to evaluate if there are any bugs in the call comparison with ANY or to close it as third party as suggested. As noted in my PR comment there should be a test case for this change proposed to make sure someone doesn't change the order by accident in future : #14700 (comment)

Though some __eq__ implementations in the stdlib should be fixed too.

Curious about what problems that this example surface in the stdlib definition of __eq__ . Are you saying that there are Django model like __eq__ definitions where they return False when other's type is different?

[serhiy-storchaka]

Yes, see timedelta.__eq__ for example. datetime.__eq__ looks correct.