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 

```
[(<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 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.

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] https://github.com/python/cpython/blob/2a3d4d9c53dd4831c3ecf56bc7c4a289c33030d6/Lib/unittest/mock.py#L2407

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.

> 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.

Then I think the problem is with DjangoModel.

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

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

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 : https://github.com/python/cpython/pull/14700#issuecomment-510342012

> 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?

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

Good catch, commenting out the c implementation of datetime in setup.py I can see the following difference.

➜  cpython git:(master) ✗ ./python.exe
Python 3.9.0a0 (heads/master-dirty:c8e7146de2, Jul 12 2019, 15:51:00)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime, sys, unittest.mock
>>> datetime.timedelta(seconds=1) == unittest.mock.ANY
False

➜  cpython git:(master) ✗ python3.7
Python 3.7.3 (v3.7.3:ef4ec6ed12, Mar 25 2019, 16:52:21)
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime, sys, unittest.mock
>>> datetime.timedelta(seconds=1) == unittest.mock.ANY
True

The C implementation and Python are different for timedelta__eq__ as mentioned with datetime.__eq__ being same and correct in NotImplementedError.

https://github.com/python/cpython/blob/c8e7146de257930ea8d0d4aa74b3a64fcaa79d4b/Modules/_datetimemodule.c#L2152

static PyObject *
delta_richcompare(PyObject *self, PyObject *other, int op)
{
    if (PyDelta_Check(other)) {
        int diff = delta_cmp(self, other);
        return diff_to_bool(diff, op);
    }
    else {
        Py_RETURN_NOTIMPLEMENTED;
    }
}


https://github.com/python/cpython/blob/c8e7146de257930ea8d0d4aa74b3a64fcaa79d4b/Lib/datetime.py#L732

def __eq__(self, other):
    if isinstance(other, timedelta):
        return self._cmp(other) == 0
    else:
        return False

I can see difference in time.__eq__ and timedelta.__eq__ C and Python implementations. Serhiy, are you planning to fix them or shall I raise an issue with PR for these?

You can take it, please.

Would be nice to report a bug in Django.

Hi there, I completely missed that this had caused so much interesting discussion. I've added a regression test that shows the bug I was encountering, which seems to be related to 
 spec_set bypassing _Call's overwritten __eq__ and so not respecting ANY https://github.com/python/cpython/pull/14700

I agree, this is definitely also a third party bug with Django, but I think it's a problem here too, and am working on a fix.

Thanks Elizabeth for the test. The regression test seems to be same as the case noted by Serhiy that Foo.__eq__ is not returning NotImplemented so that ANY.__eq__ can be executed. Below would be the correct implementation that passes. 

The actual comparison is done at [0]. If Foo.__eq__ returned NotImplemented due to type difference it would have called other.arguments. So there is no chance for ANY.__eq__ to be executed. I feel it's more about the third party class that needs to be fixed rather than the stdlib code here and changing the order of arguments for ANY's __eq__ precedence might introduce other subtle bugs.

self.arguments = OrderedDict([('args', (<__main__.Foo object at 0x10a54b500>,))])
other.arguments = OrderedDict([('args', (<ANY>,))])

def __eq__(self, other):
    if self is other:
        return True
    if not isinstance(other, BoundArguments):
        return NotImplemented
    return (self.signature == other.signature and
            self.arguments == other.arguments)

# Better implementation

from unittest.mock import Mock, call, ANY

class Foo(object):
     def __eq__(self, other):
          if not isinstance(other, self.__class__):
               return NotImplemented
          return True
     def __ne__(self, other): pass

mock = Mock(spec_set=Foo)
expected = [call(ANY)]
mock(Foo())

mock.assert_has_calls(expected)

[0] https://github.com/python/cpython/blob/cd6e83b4810549c308ab2d7315dbab526e35ccf6/Lib/inspect.py#L2708


3.5 and 3.6 are in security fixes only mode. If this is considered to be a bug it can go in master, 3.8 and 3.7. The tests section is for CPython test suite and not for the unittest related bugs. So triaging it back.

Maybe I am missing something, but while it is true that DjangoModel is doing the wrong thing by returning False instead of NotImplemented, the `ANY` sentinel is supposed to match *anything*, not just things that compare equal to it, right? I would expect this to work, for example:

  class EqualsNothing:
    def __eq__(self, other):
        return False

  m = Mock(spec_set=EqualsNothing)
  obj = EqualsNothing()
  m(obj)
  m.assert_has_calls([call(ANY)])

In that example, it is deliberate that EqualsNothing returns False for any type, but ANY should still match it. I think maybe the solution here is to special-case the matching with ANY so the criterion for a call matching would be `y is ANY or x == y`.

I feel like I agree with Paul here, unsurprising behavior from ANY is it matching anything, with no expectation that third party objects have to correctly have a return path for NotImplemented on their __eq__ method. ANY doesn't currently do that.

I've added a commit with couple other failing tests showing how using assertEqual is currently order sensitive when used with _Call or _CallList objects. I've got a work in progress fixing all three tests that just needs a little clean up, but think I might be able to lessen the complexity of it by adding Paul's idea of just special casing ANY directly, which may be smaller than the fix I have.

Thank you all for providing such great feedback so far, this is really interesting stuff to me, and a great first time submitting to Python experience so far.

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.

Giving this a reread with fresh eyes this morning, I realized I wasn't explicit enough in saying that I see that this fix is about the same as the one you mentioned, Karthikeyan, but said you were concerned might cause more subtle bugs. I hear that, and will try some more specific fixes, I was just trying to gather whether the tests showing 

self.assertEqual(call(ANY).call_list(), mock.mock_calls)
self.assertEqual(mock.mock_calls, call(ANY).call_list())

results in one passing and the other not (and the same for calls) changed the math on how brittle the current logic is, so just leaving it seemed like less of an option. I'll keep working on some other ideas though, whether we can check for ANY directly, or just wrap the BoundArguments that are causing problems when the mock has spec set.

This issue is a part of more general issue. I have opened a thread on Python-Dev for discussing it: https://mail.python.org/archives/list/python-dev@python.org/thread/VSV4K4AOKM4CBQMOELPFV5VMYALPH464/.

Serhiy, thanks for pointing that out. I generally agree with everything in that thread, and learned some new things (I had no idea count(), index() and remove() used needle on the left side!) However, I'm not trying to spearhead a rewrite of everything. I think what I'm advocating for on the two assertEqual order tests is that as Guido mentioned "people should fix their __eq__ implementation to properly return NotImplemented". _Call's __eq__ currently has no path to NotImplemented, and if it did, those tests would pass. Perhaps I should change it from the or statement to make it more clear that's what's effectively happening.

I'd like to talk more about what a good solution would be for the _Call __eq__ 
issue, since people (including myself!) have concerns, but I think it would be best to open another bug for that, and not scope creep myself. I've reverted all the commits having to do with that part so hopefully the issue with _call_matcher can get sorted separately, and then we can talk about this as it's own issue.

See issue37685.

New changeset d6a9d17d8b6c68073931dd8ffa213b4ac351a4ab by Paul Ganssle (Elizabeth Uselton) in branch 'master':
bpo-37555: Update _CallList.__contains__ to respect ANY (#14700)
https://github.com/python/cpython/commit/d6a9d17d8b6c68073931dd8ffa213b4ac351a4ab

Does this need a backport? It's in 3.9 but not 3.8.

I would prefer not backporting this change given the discussion in https://github.com/python/cpython/pull/14700#issuecomment-531791029

I believe it should work in both 3.8 and 3.9, the difference is that someone (Serhiy if I'm remembering correctly?) did the work in 3.9 of standardizing which side of the = the needle was on. So, this change works on 3.9 without the _AnyComparer class I added, but I believe unless _AnyComparer has been removed, this change would work in 3.8. Xtreak, it seems like most of your concerns about backporting it are about how complicated this part of the code is, and wanting it to get more testing via the mock backport? Is there any way I can help mitigate those concerns, or keep up with how much use it's gotten from the mock backport so we can determine when (if ever) it would be a good time to backport this?

3.8 is in security fix mode now, so I think this issue can be closed.