-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
mock.call equality surprisingly broken #79407
Comments
$ 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! |
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 + if (getattr(self, 'parent', None) and getattr(other, 'parent', None) 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)' [0] Line 2109 in 0d12672
[1] Line 2043 in 0d12672
|
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. |
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 :) ====================================================================== 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' |
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:
This is documented here: Line 1993 in 0d12672
On the addition in general, 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:
which "works as expected". Having support for:
|
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... |
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. |
Assuming the PR is merged, what do I need to do for 3.7 and 3.6 backports? |
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!) |
É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... ) |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: