[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Change by Chris Withers :


--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Chris Withers  added the comment:


New changeset e8f9e4785caeef8a68bb7859280e91a4cb424b79 by Chris Withers (Miss 
Islington (bot)) in branch '3.7':
bpo-35226: Fix equality for nested unittest.mock.call objects. (GH-10555)
https://github.com/python/cpython/commit/e8f9e4785caeef8a68bb7859280e91a4cb424b79


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Chris Withers  added the comment:


New changeset 67e6136a6d5c07141d4dba820c450a70db7aedd5 by Chris Withers (Miss 
Islington (bot)) in branch '3.6':
bpo-35226: Fix equality for nested unittest.mock.call objects. (GH-10555)
https://github.com/python/cpython/commit/67e6136a6d5c07141d4dba820c450a70db7aedd5


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[miss-islington]

Change by miss-islington :


--
pull_requests: +10118

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Chris Withers  added the comment:


New changeset 8ca0fa9d2f4de6e69f0902790432e0ab2f37ba68 by Chris Withers in 
branch 'master':
bpo-35226: Fix equality for nested unittest.mock.call objects. (#10555)
https://github.com/python/cpython/commit/8ca0fa9d2f4de6e69f0902790432e0ab2f37ba68


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[miss-islington]

Change by miss-islington :


--
pull_requests: +10117

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Daniel Fortunov]

Change by Daniel Fortunov :


--
nosy: +dfortunov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Chris Withers  added the comment:

Éric, doesn't look like I can add labels, what needs to happen for me to do so? 
(I was/am a core developer, just an extremely inactive one ;-), but happy to 
jump through whatever hoops necessary... )

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Éric Araujo]

Éric Araujo  added the comment:

Add the right «needs backport to X.Y» labels to the PR and a bot will take care 
of it.

(This info does not seem to be in the devguide!)

--
nosy: +eric.araujo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Chris Withers  added the comment:

Assuming the PR is merged, what do I need to do for 3.7 and 3.6 backports?
There's no doubt a doc for this somewhere, so please just point me at it :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Roundup Robot]

Change by Roundup Robot :


--
keywords: +patch
pull_requests: +9798
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Michael Foord]

Michael Foord  added the comment:

Parents comparing upwards sounds like the right (and simple) fix. Not breaking 
the tuple tests would be good. I'm happy for Chris to produce the patch.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Chris Withers  added the comment:

I'm chatting with Michael already (on Facebook of all places), and I use this 
feature heavily and deeply, being one of the people who originally requested it.

I'm both happy and capable of seeing this through, so no need for anyone else 
to dive in :-)

Hopefully Michael will follow up when he gets a chance...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Mario Corchero]

Mario Corchero  added the comment:

If this is to be done we should not change those tests, I am sure there is code 
validating calls relying on its "tupleness". Example:

```

>>> import unittest.mock
>>> m = unittest.mock.Mock()
>>> m(1)
>>> m(2, a=1) 
>>> m.assert_has_calls([
...   ((1,), {}),
...   ((2,), {'a':1}),
... ])

```

This is documented here: 
https://github.com/python/cpython/blob/0d12672b30b8c6c992bef7564581117ae83e11ad/Lib/unittest/mock.py#L1993

On the addition in general,
I cannot really comment on the "call(x=1).foo == call(x=2).foo" or how that is 
supposed to work, I've used those "nesting calls" minimally. I would try to get 
a hold of Michael Ford.

As a note, I think nesting calls in unittest.mock.call is supposed to be used 
only via the 
https://docs.python.org/3/library/unittest.mock.html#unittest.mock.call.call_list
 method and for calls only.

See:

```
>>> call(x=1).foo(z=1).call_list() == call(x=1).foo(z=1).call_list()
True
>>> call(x=2).foo(z=1).call_list() == call(x=1).foo(z=1).call_list()
False
```

which "works as expected".

Having support for:

```call(x=1).foo == call(x=2).foo``` is kind of trying to validate an attribute 
has been accessed, which is a different thing from what mock.calls seems to be 
doing at the moment. It could be added I guess, but I don't see how do can you 
use that in `mock.assert_has_calls` for example. What is the "real life" issue 
that you faced with this issue? Because creating and comparing calls yourself 
is not something that is usually done, I guess you compared it to a mock call.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

Yes, for call objects it's always there but some of the tests use a tuple 
during self.assertEquals and hence during equality check other fails with the 
attribute error for parent like below. Also getattr(self, 'parent', None) != 
getattr(other, 'parent', None) will skip the attribute error but still fail for 
tuples in tests I guess with tuple.parent returned as None and checked against 
a valid parent. Hence the patch with both the checks of getattr and self.parent 
that passes the test suite. I will wait for input from maintainers if my 
approach is correct since I am little new to mock :)
 

==
ERROR: test_explicit_mock (unittest.test.testmock.testwith.TestMockOpen)
--
Traceback (most recent call last):
  File 
"/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/test/testmock/testwith.py",
 line 177, in test_explicit_mock
mock.assert_called_once_with('foo')
  File 
"/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", 
line 838, in assert_called_once_with
return self.assert_called_with(*args, **kwargs)
  File 
"/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", 
line 823, in assert_called_with
if expected != actual:
  File 
"/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", 
line 2057, in __eq__
if self.parent != other.parent:
AttributeError: 'tuple' object has no attribute 'parent'

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

Chris Withers  added the comment:

I don't think the getattr(self, 'parent', None) is necessary, the attribute is 
always there and None == None ;-)

I reckon this will work:

if self.parent != other.parent):
return True

...but obviously, we'd add some more tests to the suite to make sure this keeps 
working.

If this is the way to go, lemme know and I'll work up a PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +mariocj89

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

I think this is due to the fact that when an attribute of a call object is 
accessed a new call object is returned with the parent as self.parent [0] but 
the original information regarding the parent is lost during representation and 
no check for self.parent equality while the call objects are compared [1]

./python.exe
Python 3.8.0a0 (heads/master:0dc1e45dfd, Nov 13 2018, 11:19:49)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest.mock import call
>>> call(x=2).foo
call().foo
>>> call(x=1).foo
call().foo
>>> # Comparison returns true since it's similar to call().foo == call().foo 
>>> though parents are set
>>> call(x=1).foo == call(x=2).foo
True
>>> call(x=1).foo.parent
call(x=1)
>>> call(x=2).foo.parent
call(x=2)
>>> call(x=2).foo(x=2).bar
call().foo().bar
>>> call(x=4).foo(x=3).bar
call().foo().bar
>>> # Comparison returns true since it's similar to call().foo().bar == 
>>> call().foo().bar
>>> call(x=2).foo(x=2).bar == call(x=4).foo(x=3).bar 
True
>>> call(x=4).foo(x=3).bar.parent
call().foo(x=3)

Maybe we can add a check for parent to be equal if present? 

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index a9c82dcb5d..a7565d5f60 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -2054,6 +2054,10 @@ class _Call(tuple):
 else:
 self_name, self_args, self_kwargs = self

+if (getattr(self, 'parent', None) and getattr(other, 'parent', None)
+and self.parent != other.parent):
+return False
+
 other_name = ''
 if len_other == 0:
 other_args, other_kwargs = (), {}

With patch the checks return False and there is no test suite failure but I am 
not sure if I need to put this inside a while loop to check for nested parents 
and there maybe other cases that my code doesn't handle.

➜  cpython git:(master) ✗ ./python.exe -c 'from unittest.mock import call; 
print(call(x=2).foo.bar == call(x=1).foo.bar)'
False
➜  cpython git:(master) ✗ ./python.exe -c 'from unittest.mock import call; 
print(call(x=2).foo == call(x=1).foo)'
False


[0] 
https://github.com/python/cpython/blob/0d12672b30b8c6c992bef7564581117ae83e11ad/Lib/unittest/mock.py#L2109
[1] 
https://github.com/python/cpython/blob/0d12672b30b8c6c992bef7564581117ae83e11ad/Lib/unittest/mock.py#L2043

--
nosy: +xtreak

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35226] mock.call equality surprisingly broken

[Chris Withers]

New submission from Chris Withers :

$ python3
Python 3.7.0 (v3.7.0:1bf9cc5093, Jun 26 2018, 23:26:24) 
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest.mock import call
>>> call(x=1) == call(x=2)
False

Good so far?

>>> call(x=1).foo == call(x=2).foo
True

Eep, I think a lot of people might find that quite surprising!

--
assignee: michael.foord
components: Tests
messages: 329813
nosy: cjw296, michael.foord
priority: normal
severity: normal
status: open
title: mock.call equality surprisingly broken
type: behavior
versions: Python 2.7, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com