Skip to content

bpo-37555: Update _CallList.__contains__ to respect ANY - #14700

Merged
pganssle merged 20 commits into
python:masterfrom
ElizabethU:fix-issue-37555
Sep 13, 2019
Merged

bpo-37555: Update _CallList.__contains__ to respect ANY#14700
pganssle merged 20 commits into
python:masterfrom
ElizabethU:fix-issue-37555

Conversation

@ElizabethU

@ElizabethU ElizabethU commented Jul 11, 2019

Copy link
Copy Markdown
Contributor

Greetings! Long time fan of the language, first time submitting a pull request to it.

I have a test on another project 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

[(<A Django Model>, Decimal('20')), (<A Django Model>, Decimal('10'))]

This seems like wrong behavior. ANY should be happy to be compared to anything, even a random object. I've added a test showing the behavior that fails on master.

Doing some digging, I found that in 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.

More digging and I realized the reason this was only a problem on a mock with a spec. Specifically, when a mock is instantiated with a spec, the _spec_signature attribute gets assigned, and when _spec_signature is not none, _call_matcher, which is used to prep the calls for comparison, returns a tuple of a string and a BoundArguments object instead of a _Call (link here). When _call_matcher returns _Call objects, they use _Call's __eq__ method, which flips the compared calls around, putting ANY on the left. When _call_matcher returns tuples, it doesn't use this logic, and ANY remains on the right, not being used for comparison. The simplest fix seemed to be to ensure _call_matcher always returned a _Call object.

I have verified that there are several tests covering the use of spec signature, which make sure that things continue to match regardless of if they are args or kwargs and break when that logic is broken, and none of them break with my change.

https://bugs.python.org/issue37555

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants