[Python-checkins] bpo-37555: Update _CallList.__contains__ to respect ANY (#14700)
Paul Ganssle
webhook-mailer at python.org
Fri Sep 13 11:54:36 EDT 2019
https://github.com/python/cpython/commit/d6a9d17d8b6c68073931dd8ffa213b4ac351a4ab
commit: d6a9d17d8b6c68073931dd8ffa213b4ac351a4ab
branch: master
author: Elizabeth Uselton <elizabeth.uselton at gmail.com>
committer: Paul Ganssle <paul at ganssle.io>
date: 2019-09-13T16:54:32+01:00
summary:
bpo-37555: Update _CallList.__contains__ to respect ANY (#14700)
* Flip equality to use mock calls' __eq__
* bpo-37555: Regression test demonstrating assert_has_calls not working with ANY and spec_set
Co-authored-by: Neal Finne <neal at nealfinne.com>
* Revert "Flip equality to use mock calls' __eq__"
This reverts commit 94ddf54c5a8aab7d00d9ab93e1cc5695c28d73e7.
* bpo-37555: Add regression tests for mock ANY ordering issues
Add regression tests for whether __eq__ is order agnostic on _Call and _CallList, which is useful for comparisons involving ANY, especially if the ANY comparison is to a class not defaulting __eq__ to NotImplemented.
Co-authored-by: Neal Finne <neal at nealfinne.com>
* bpo-37555: Fix _CallList and _Call order sensitivity
_Call and _CallList depend on ordering to correctly process that an object being compared to ANY with __eq__ should return True. This fix updates the comparison to check both a == b and b == a and return True if either condition is met, fixing situations from the tests in the previous two commits where assertEqual would not be commutative if checking _Call or _CallList objects. This seems like a reasonable fix considering that the Python data model specifies that if an object doesn't know how to compare itself to another object it should return NotImplemented, and that on getting NotImplemented from a == b, it should try b == a, implying that good behavior for __eq__ is commutative. This also flips the order of comparison in _CallList's __contains__ method, guaranteeing ANY will be on the left and have it's __eq__ called for equality checking, fixing the interaction between assert_has_calls and ANY.
Co-author: Neal Finne <neal at neal.finne.com>
* bpo-37555: Ensure _call_matcher returns _Call object
* Adding ACK and news entry
* bpo-37555: Replacing __eq__ with == to sidestep NotImplemented
bool(NotImplemented) returns True, so it's necessary to use ==
instead of __eq__ in this comparison.
* bpo-37555: cleaning up changes unnecessary to the final product
* bpo-37555: Fixed call on bound arguments to respect args and kwargs
* Revert "bpo-37555: Add regression tests for mock ANY ordering issues"
This reverts commit 49c5310ad493c4356dd3bc58c03653cd9466c4fa.
* Revert "bpo-37555: cleaning up changes unnecessary to the final product"
This reverts commit 18e964ba0126d8964d89842cb95534b63c2d326e.
* Revert "bpo-37555: Replacing __eq__ with == to sidestep NotImplemented"
This reverts commit f295eaca5bceac6636c0e2b10e6c7d9a8ee8296a.
* Revert "bpo-37555: Fix _CallList and _Call order sensitivity"
This reverts commit 874fb697b8376fcea130116e56189061f944fde6.
* Updated NEWS.d
* bpo-37555: Add tests checking every function using _call_matcher both with and without spec
* bpo-37555: Ensure all assert methods using _call_matcher are actually passing calls
* Remove AnyCompare and use call objects everywhere.
* Revert "Remove AnyCompare and use call objects everywhere."
This reverts commit 24973c0b32ce7d796a7f4eeaf259832222aae0f5.
* Check for exception in assert_any_await
files:
A Misc/NEWS.d/next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst
M Lib/unittest/mock.py
M Lib/unittest/test/testmock/testasync.py
M Lib/unittest/test/testmock/testhelpers.py
M Misc/ACKS
diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index a98decc2c9f0..513cd5c924d4 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -852,7 +852,8 @@ def _call_matcher(self, _call):
else:
name, args, kwargs = _call
try:
- return name, sig.bind(*args, **kwargs)
+ bound_call = sig.bind(*args, **kwargs)
+ return call(name, bound_call.args, bound_call.kwargs)
except TypeError as e:
return e.with_traceback(None)
else:
@@ -901,9 +902,9 @@ def assert_called_with(self, /, *args, **kwargs):
def _error_message():
msg = self._format_mock_failure_message(args, kwargs)
return msg
- expected = self._call_matcher((args, kwargs))
+ expected = self._call_matcher(_Call((args, kwargs), two=True))
actual = self._call_matcher(self.call_args)
- if expected != actual:
+ if actual != expected:
cause = expected if isinstance(expected, Exception) else None
raise AssertionError(_error_message()) from cause
@@ -963,10 +964,10 @@ def assert_any_call(self, /, *args, **kwargs):
The assert passes if the mock has *ever* been called, unlike
`assert_called_with` and `assert_called_once_with` that only pass if
the call is the most recent one."""
- expected = self._call_matcher((args, kwargs))
+ expected = self._call_matcher(_Call((args, kwargs), two=True))
+ cause = expected if isinstance(expected, Exception) else None
actual = [self._call_matcher(c) for c in self.call_args_list]
- if expected not in actual:
- cause = expected if isinstance(expected, Exception) else None
+ if cause or expected not in _AnyComparer(actual):
expected_string = self._format_mock_call_signature(args, kwargs)
raise AssertionError(
'%s call not found' % expected_string
@@ -1019,6 +1020,22 @@ def _calls_repr(self, prefix="Calls"):
return f"\n{prefix}: {safe_repr(self.mock_calls)}."
+class _AnyComparer(list):
+ """A list which checks if it contains a call which may have an
+ argument of ANY, flipping the components of item and self from
+ their traditional locations so that ANY is guaranteed to be on
+ the left."""
+ def __contains__(self, item):
+ for _call in self:
+ if len(item) != len(_call):
+ continue
+ if all([
+ expected == actual
+ for expected, actual in zip(item, _call)
+ ]):
+ return True
+ return False
+
def _try_iter(obj):
if obj is None:
@@ -2171,9 +2188,9 @@ def _error_message():
msg = self._format_mock_failure_message(args, kwargs, action='await')
return msg
- expected = self._call_matcher((args, kwargs))
+ expected = self._call_matcher(_Call((args, kwargs), two=True))
actual = self._call_matcher(self.await_args)
- if expected != actual:
+ if actual != expected:
cause = expected if isinstance(expected, Exception) else None
raise AssertionError(_error_message()) from cause
@@ -2192,10 +2209,10 @@ def assert_any_await(self, /, *args, **kwargs):
"""
Assert the mock has ever been awaited with the specified arguments.
"""
- expected = self._call_matcher((args, kwargs))
+ expected = self._call_matcher(_Call((args, kwargs), two=True))
+ cause = expected if isinstance(expected, Exception) else None
actual = [self._call_matcher(c) for c in self.await_args_list]
- if expected not in actual:
- cause = expected if isinstance(expected, Exception) else None
+ if cause or expected not in _AnyComparer(actual):
expected_string = self._format_mock_call_signature(args, kwargs)
raise AssertionError(
'%s await not found' % expected_string
diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py
index 550234223cc3..87f3cfc8fa47 100644
--- a/Lib/unittest/test/testmock/testasync.py
+++ b/Lib/unittest/test/testmock/testasync.py
@@ -2,8 +2,8 @@
import inspect
import unittest
-from unittest.mock import (call, AsyncMock, patch, MagicMock, create_autospec,
- _AwaitEvent)
+from unittest.mock import (ANY, call, AsyncMock, patch, MagicMock,
+ create_autospec, _AwaitEvent)
def tearDownModule():
@@ -192,6 +192,10 @@ def test_create_autospec(self):
spec.assert_awaited_with(1, 2, c=3)
spec.assert_awaited()
+ with self.assertRaises(AssertionError):
+ spec.assert_any_await(e=1)
+
+
def test_patch_with_autospec(self):
async def test_async():
@@ -607,6 +611,30 @@ def test_assert_has_awaits_no_order(self):
asyncio.run(self._runnable_test('SomethingElse'))
self.mock.assert_has_awaits(calls)
+ def test_awaits_asserts_with_any(self):
+ class Foo:
+ def __eq__(self, other): pass
+
+ asyncio.run(self._runnable_test(Foo(), 1))
+
+ self.mock.assert_has_awaits([call(ANY, 1)])
+ self.mock.assert_awaited_with(ANY, 1)
+ self.mock.assert_any_await(ANY, 1)
+
+ def test_awaits_asserts_with_spec_and_any(self):
+ class Foo:
+ def __eq__(self, other): pass
+
+ mock_with_spec = AsyncMock(spec=Foo)
+
+ async def _custom_mock_runnable_test(*args):
+ await mock_with_spec(*args)
+
+ asyncio.run(_custom_mock_runnable_test(Foo(), 1))
+ mock_with_spec.assert_has_awaits([call(ANY, 1)])
+ mock_with_spec.assert_awaited_with(ANY, 1)
+ mock_with_spec.assert_any_await(ANY, 1)
+
def test_assert_has_awaits_ordered(self):
calls = [call('NormalFoo'), call('baz')]
with self.assertRaises(AssertionError):
diff --git a/Lib/unittest/test/testmock/testhelpers.py b/Lib/unittest/test/testmock/testhelpers.py
index f3c7acb98c21..9e7ec5d62d5d 100644
--- a/Lib/unittest/test/testmock/testhelpers.py
+++ b/Lib/unittest/test/testmock/testhelpers.py
@@ -64,7 +64,28 @@ def __ne__(self, other): pass
self.assertEqual(expected, mock.mock_calls)
self.assertEqual(mock.mock_calls, expected)
+ def test_any_no_spec(self):
+ # This is a regression test for bpo-37555
+ class Foo:
+ def __eq__(self, other): pass
+
+ mock = Mock()
+ mock(Foo(), 1)
+ mock.assert_has_calls([call(ANY, 1)])
+ mock.assert_called_with(ANY, 1)
+ mock.assert_any_call(ANY, 1)
+
+ def test_any_and_spec_set(self):
+ # This is a regression test for bpo-37555
+ class Foo:
+ def __eq__(self, other): pass
+
+ mock = Mock(spec=Foo)
+ mock(Foo(), 1)
+ mock.assert_has_calls([call(ANY, 1)])
+ mock.assert_called_with(ANY, 1)
+ mock.assert_any_call(ANY, 1)
class CallTest(unittest.TestCase):
diff --git a/Misc/ACKS b/Misc/ACKS
index f321131b2ab4..71e61c3db386 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -503,6 +503,7 @@ Tomer Filiba
Segev Finer
Jeffrey Finkelstein
Russell Finn
+Neal Finne
Dan Finnie
Nils Fischbeck
Frederik Fix
@@ -1714,6 +1715,7 @@ Roger Upole
Daniel Urban
Michael Urman
Hector Urtubia
+Elizabeth Uselton
Lukas Vacek
Ville Vainio
Yann Vaginay
diff --git a/Misc/NEWS.d/next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst b/Misc/NEWS.d/next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst
new file mode 100644
index 000000000000..16d1d62d3096
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst
@@ -0,0 +1,2 @@
+Fix `NonCallableMock._call_matcher` returning tuple instead of `_Call` object
+when `self._spec_signature` exists. Patch by Elizabeth Uselton
\ No newline at end of file
More information about the Python-checkins
mailing list