From 94ddf54c5a8aab7d00d9ab93e1cc5695c28d73e7 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Wed, 10 Jul 2019 22:16:01 -0700 Subject: [PATCH 01/20] Flip equality to use mock calls' __eq__ --- Lib/unittest/mock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index c2802726d75d9c..696a131734da72 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -337,7 +337,7 @@ def __contains__(self, value): for i in range(0, len_self - len_value + 1): sub_list = self[i:i+len_value] - if sub_list == value: + if value == sub_list: return True return False From b4c7d78c8e62c5eb8ec78de44934b1fb6b107826 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 14 Jul 2019 13:53:09 -0700 Subject: [PATCH 02/20] bpo-37555: Regression test demonstrating assert_has_calls not working with ANY and spec_set Co-authored-by: Neal Finne --- Lib/unittest/test/testmock/testhelpers.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Lib/unittest/test/testmock/testhelpers.py b/Lib/unittest/test/testmock/testhelpers.py index 301bca430c131c..49fffa38439fea 100644 --- a/Lib/unittest/test/testmock/testhelpers.py +++ b/Lib/unittest/test/testmock/testhelpers.py @@ -63,7 +63,19 @@ def __ne__(self, other): pass ] self.assertEqual(expected, mock.mock_calls) self.assertEqual(mock.mock_calls, expected) + mock.assert_has_calls(expected) + def test_assert_has_calls_with_any_and_spec_set(self): + # This is a regression test for bpo-37555 + class Foo(object): + def __eq__(self, other): pass + def __ne__(self, other): pass + + mock = Mock(spec_set=Foo) + expected = [call(ANY)] + mock(Foo()) + + mock.assert_has_calls(expected) class CallTest(unittest.TestCase): From ad99a9d4c280c1a6ab163a279c0dc36833d29122 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 14 Jul 2019 13:56:42 -0700 Subject: [PATCH 03/20] Revert "Flip equality to use mock calls' __eq__" This reverts commit 94ddf54c5a8aab7d00d9ab93e1cc5695c28d73e7. --- Lib/unittest/mock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 696a131734da72..c2802726d75d9c 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -337,7 +337,7 @@ def __contains__(self, value): for i in range(0, len_self - len_value + 1): sub_list = self[i:i+len_value] - if value == sub_list: + if sub_list == value: return True return False From 49c5310ad493c4356dd3bc58c03653cd9466c4fa Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 14 Jul 2019 18:51:43 -0700 Subject: [PATCH 04/20] 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 --- Lib/unittest/test/testmock/testhelpers.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Lib/unittest/test/testmock/testhelpers.py b/Lib/unittest/test/testmock/testhelpers.py index 49fffa38439fea..75d4ac54078e17 100644 --- a/Lib/unittest/test/testmock/testhelpers.py +++ b/Lib/unittest/test/testmock/testhelpers.py @@ -322,6 +322,16 @@ def test_call_list(self): kall = call(1).method(2)(3).foo.bar.baz(4)(5).__int__() self.assertEqual(kall.call_list(), mock.mock_calls) + def test_call_list_with_any_comparison_order(self): + class Foo: + def __eq__(self, other): pass + def __ne__(self, other): pass + + mock = MagicMock() + mock(Foo()) + + self.assertEqual(call(ANY).call_list(), mock.mock_calls) + self.assertEqual(mock.mock_calls, call(ANY).call_list()) def test_call_any(self): self.assertEqual(call, ANY) @@ -331,6 +341,16 @@ def test_call_any(self): self.assertEqual(m.mock_calls, [ANY]) self.assertEqual([ANY], m.mock_calls) + def test_call_any_comparison_order(self): + class Foo: + def __eq__(self, other): pass + def __ne__(self, other): pass + + m = MagicMock() + m(Foo()) + + self.assertEqual(m.mock_calls[0], call(ANY)) + self.assertEqual(call(ANY), m.mock_calls[0]) def test_two_args_call(self): args = _Call(((1, 2), {'a': 3}), two=True) From 874fb697b8376fcea130116e56189061f944fde6 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Wed, 17 Jul 2019 23:28:50 -0700 Subject: [PATCH 05/20] 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 --- Lib/unittest/mock.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index c2802726d75d9c..93379dbfb56e68 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -337,13 +337,19 @@ def __contains__(self, value): for i in range(0, len_self - len_value + 1): sub_list = self[i:i+len_value] - if sub_list == value: + if value == sub_list: return True return False def __repr__(self): return pprint.pformat(list(self)) + def __eq__(self, other): + self_list = list(self) + other_list = list(other) + # checking equality both directions is necessary for ANY to work + return self_list.__eq__(other_list) or other_list.__eq__(self_list) + def _check_and_set_parent(parent, value, name, new_name): # function passed to create_autospec will have mock @@ -2293,7 +2299,6 @@ def _format_call_signature(name, args, kwargs): return message % formatted_args - class _Call(tuple): """ A tuple for holding the results of a call to a mock, either in the form @@ -2403,8 +2408,12 @@ def __eq__(self, other): if self_name and other_name != self_name: return False - # this order is important for ANY to work! - return (other_args, other_kwargs) == (self_args, self_kwargs) + self_params = self_args, self_kwargs + other_params = other_args, other_kwargs + return ( + self_params.__eq__(other_params) + or other_params.__eq__(self_params) + ) __ne__ = object.__ne__ From d72d6f50f54330571ea36a1ce20979c95e32e0cf Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Fri, 19 Jul 2019 19:19:00 -0700 Subject: [PATCH 06/20] bpo-37555: Ensure _call_matcher returns _Call object --- Lib/unittest/mock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 93379dbfb56e68..f91836554107b1 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -820,7 +820,7 @@ def _call_matcher(self, _call): else: name, args, kwargs = _call try: - return name, sig.bind(*args, **kwargs) + return call(name, sig.bind(*args, **kwargs)) except TypeError as e: return e.with_traceback(None) else: From f0e841148fb7baf1782a9f855b0593408f3f9622 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Fri, 19 Jul 2019 20:36:24 -0700 Subject: [PATCH 07/20] Adding ACK and news entry --- Misc/ACKS | 2 ++ .../next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst | 4 ++++ 2 files changed, 6 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst diff --git a/Misc/ACKS b/Misc/ACKS index 36fe727c8421c0..2c6a9344156330 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -496,6 +496,7 @@ Tomer Filiba Segev Finer Jeffrey Finkelstein Russell Finn +Neal Finne Dan Finnie Nils Fischbeck Frederik Fix @@ -1699,6 +1700,7 @@ Roger Upole Daniel Urban Michael Urman Hector Urtubia +Elizabeth Uselton Lukas Vacek Ville Vainio Andi Vajda 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 00000000000000..6ab8b920c2e59e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst @@ -0,0 +1,4 @@ +Fix `NonCallableMock._call_matcher` returning tuple instead of `_Call` object +when `self._spec_signature` exists. Additionally fix `__eq__` to be +commutative on `_Call` and `_CallList`, to better account for `ANY`. Patch by +Elizabeth Uselton \ No newline at end of file From f295eaca5bceac6636c0e2b10e6c7d9a8ee8296a Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Fri, 19 Jul 2019 20:45:52 -0700 Subject: [PATCH 08/20] bpo-37555: Replacing __eq__ with == to sidestep NotImplemented bool(NotImplemented) returns True, so it's necessary to use == instead of __eq__ in this comparison. --- Lib/unittest/mock.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index f91836554107b1..aff5e70065a01e 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -348,7 +348,7 @@ def __eq__(self, other): self_list = list(self) other_list = list(other) # checking equality both directions is necessary for ANY to work - return self_list.__eq__(other_list) or other_list.__eq__(self_list) + return self_list == other_list or other_list == self_list def _check_and_set_parent(parent, value, name, new_name): @@ -2411,8 +2411,8 @@ def __eq__(self, other): self_params = self_args, self_kwargs other_params = other_args, other_kwargs return ( - self_params.__eq__(other_params) - or other_params.__eq__(self_params) + self_params == other_params + or other_params == self_params ) From 18e964ba0126d8964d89842cb95534b63c2d326e Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sat, 20 Jul 2019 08:49:18 -0700 Subject: [PATCH 09/20] bpo-37555: cleaning up changes unnecessary to the final product --- Lib/unittest/mock.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index aff5e70065a01e..01925539b349bb 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -337,19 +337,13 @@ def __contains__(self, value): for i in range(0, len_self - len_value + 1): sub_list = self[i:i+len_value] - if value == sub_list: + if sub_list == value: return True return False def __repr__(self): return pprint.pformat(list(self)) - def __eq__(self, other): - self_list = list(self) - other_list = list(other) - # checking equality both directions is necessary for ANY to work - return self_list == other_list or other_list == self_list - def _check_and_set_parent(parent, value, name, new_name): # function passed to create_autospec will have mock From 883841ad3104aa009d28aa1f5feb82d957f965eb Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sat, 20 Jul 2019 08:58:14 -0700 Subject: [PATCH 10/20] bpo-37555: Fixed call on bound arguments to respect args and kwargs --- Lib/unittest/mock.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 01925539b349bb..95c728fa6604ab 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -814,7 +814,8 @@ def _call_matcher(self, _call): else: name, args, kwargs = _call try: - return call(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: From f4844c7a7f49fc07fdfc3f6d056c22cc14283562 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 21 Jul 2019 18:43:13 -0700 Subject: [PATCH 11/20] Revert "bpo-37555: Add regression tests for mock ANY ordering issues" This reverts commit 49c5310ad493c4356dd3bc58c03653cd9466c4fa. --- Lib/unittest/test/testmock/testhelpers.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/Lib/unittest/test/testmock/testhelpers.py b/Lib/unittest/test/testmock/testhelpers.py index 75d4ac54078e17..49fffa38439fea 100644 --- a/Lib/unittest/test/testmock/testhelpers.py +++ b/Lib/unittest/test/testmock/testhelpers.py @@ -322,16 +322,6 @@ def test_call_list(self): kall = call(1).method(2)(3).foo.bar.baz(4)(5).__int__() self.assertEqual(kall.call_list(), mock.mock_calls) - def test_call_list_with_any_comparison_order(self): - class Foo: - def __eq__(self, other): pass - def __ne__(self, other): pass - - mock = MagicMock() - mock(Foo()) - - self.assertEqual(call(ANY).call_list(), mock.mock_calls) - self.assertEqual(mock.mock_calls, call(ANY).call_list()) def test_call_any(self): self.assertEqual(call, ANY) @@ -341,16 +331,6 @@ def test_call_any(self): self.assertEqual(m.mock_calls, [ANY]) self.assertEqual([ANY], m.mock_calls) - def test_call_any_comparison_order(self): - class Foo: - def __eq__(self, other): pass - def __ne__(self, other): pass - - m = MagicMock() - m(Foo()) - - self.assertEqual(m.mock_calls[0], call(ANY)) - self.assertEqual(call(ANY), m.mock_calls[0]) def test_two_args_call(self): args = _Call(((1, 2), {'a': 3}), two=True) From 84489c8489aa7993a7603b668e390e8e0f80708d Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 21 Jul 2019 18:44:37 -0700 Subject: [PATCH 12/20] Revert "bpo-37555: cleaning up changes unnecessary to the final product" This reverts commit 18e964ba0126d8964d89842cb95534b63c2d326e. --- Lib/unittest/mock.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 95c728fa6604ab..a09671a0324869 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -337,13 +337,19 @@ def __contains__(self, value): for i in range(0, len_self - len_value + 1): sub_list = self[i:i+len_value] - if sub_list == value: + if value == sub_list: return True return False def __repr__(self): return pprint.pformat(list(self)) + def __eq__(self, other): + self_list = list(self) + other_list = list(other) + # checking equality both directions is necessary for ANY to work + return self_list == other_list or other_list == self_list + def _check_and_set_parent(parent, value, name, new_name): # function passed to create_autospec will have mock From 344ef173c0daeea3ef7072aabb38aa10b05e0479 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 21 Jul 2019 18:45:24 -0700 Subject: [PATCH 13/20] Revert "bpo-37555: Replacing __eq__ with == to sidestep NotImplemented" This reverts commit f295eaca5bceac6636c0e2b10e6c7d9a8ee8296a. --- Lib/unittest/mock.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index a09671a0324869..c343cf4939ecb4 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -348,7 +348,7 @@ def __eq__(self, other): self_list = list(self) other_list = list(other) # checking equality both directions is necessary for ANY to work - return self_list == other_list or other_list == self_list + return self_list.__eq__(other_list) or other_list.__eq__(self_list) def _check_and_set_parent(parent, value, name, new_name): @@ -2412,8 +2412,8 @@ def __eq__(self, other): self_params = self_args, self_kwargs other_params = other_args, other_kwargs return ( - self_params == other_params - or other_params == self_params + self_params.__eq__(other_params) + or other_params.__eq__(self_params) ) From bdf430de3980c0733188e1b832176395f7bae234 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 21 Jul 2019 18:45:50 -0700 Subject: [PATCH 14/20] Revert "bpo-37555: Fix _CallList and _Call order sensitivity" This reverts commit 874fb697b8376fcea130116e56189061f944fde6. --- Lib/unittest/mock.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index c343cf4939ecb4..47592057e1b1c0 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -337,19 +337,13 @@ def __contains__(self, value): for i in range(0, len_self - len_value + 1): sub_list = self[i:i+len_value] - if value == sub_list: + if sub_list == value: return True return False def __repr__(self): return pprint.pformat(list(self)) - def __eq__(self, other): - self_list = list(self) - other_list = list(other) - # checking equality both directions is necessary for ANY to work - return self_list.__eq__(other_list) or other_list.__eq__(self_list) - def _check_and_set_parent(parent, value, name, new_name): # function passed to create_autospec will have mock @@ -2300,6 +2294,7 @@ def _format_call_signature(name, args, kwargs): return message % formatted_args + class _Call(tuple): """ A tuple for holding the results of a call to a mock, either in the form @@ -2409,12 +2404,8 @@ def __eq__(self, other): if self_name and other_name != self_name: return False - self_params = self_args, self_kwargs - other_params = other_args, other_kwargs - return ( - self_params.__eq__(other_params) - or other_params.__eq__(self_params) - ) + # this order is important for ANY to work! + return (other_args, other_kwargs) == (self_args, self_kwargs) __ne__ = object.__ne__ From d3522b1e170cadca2c2afd72f9e133fe00b45087 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sun, 21 Jul 2019 20:14:05 -0700 Subject: [PATCH 15/20] Updated NEWS.d --- .../next/Library/2019-07-19-20-13-48.bpo-37555.S5am28.rst | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 index 6ab8b920c2e59e..16d1d62d30960a 100644 --- 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 @@ -1,4 +1,2 @@ Fix `NonCallableMock._call_matcher` returning tuple instead of `_Call` object -when `self._spec_signature` exists. Additionally fix `__eq__` to be -commutative on `_Call` and `_CallList`, to better account for `ANY`. Patch by -Elizabeth Uselton \ No newline at end of file +when `self._spec_signature` exists. Patch by Elizabeth Uselton \ No newline at end of file From f47699de12ca9aa7e79ade6ea958708f5b88eab8 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Sat, 3 Aug 2019 18:46:43 -0700 Subject: [PATCH 16/20] bpo-37555: Add tests checking every function using _call_matcher both with and without spec --- Lib/unittest/test/testmock/testasync.py | 28 +++++++++++++++++++++-- Lib/unittest/test/testmock/testhelpers.py | 25 +++++++++++++------- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py index fa906e4f7152f8..ec6809cbd9490d 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(): @@ -599,6 +599,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 49fffa38439fea..8a954095051cde 100644 --- a/Lib/unittest/test/testmock/testhelpers.py +++ b/Lib/unittest/test/testmock/testhelpers.py @@ -63,20 +63,29 @@ def __ne__(self, other): pass ] self.assertEqual(expected, mock.mock_calls) self.assertEqual(mock.mock_calls, expected) - mock.assert_has_calls(expected) - def test_assert_has_calls_with_any_and_spec_set(self): + def test_any_no_spec(self): # This is a regression test for bpo-37555 - class Foo(object): + class Foo: def __eq__(self, other): pass - def __ne__(self, other): pass - mock = Mock(spec_set=Foo) - expected = [call(ANY)] - mock(Foo()) + 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.assert_has_calls(expected) + 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): From 38650c98c6afadf25951fa128deea3cfefd82ef7 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Mon, 5 Aug 2019 00:51:24 -0700 Subject: [PATCH 17/20] bpo-37555: Ensure all assert methods using _call_matcher are actually passing calls --- Lib/unittest/mock.py | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 47592057e1b1c0..f9789e5e85a97c 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -864,9 +864,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))) 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 @@ -926,10 +926,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 @@ -982,6 +982,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: @@ -2133,9 +2149,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 @@ -2154,9 +2170,9 @@ 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)) actual = [self._call_matcher(c) for c in self.await_args_list] - if expected not in actual: + if expected not in _AnyComparer(actual): cause = expected if isinstance(expected, Exception) else None expected_string = self._format_mock_call_signature(args, kwargs) raise AssertionError( From 24973c0b32ce7d796a7f4eeaf259832222aae0f5 Mon Sep 17 00:00:00 2001 From: Xtreak Date: Mon, 19 Aug 2019 15:11:01 +0530 Subject: [PATCH 18/20] Remove AnyCompare and use call objects everywhere. --- Lib/unittest/mock.py | 25 ++++--------------------- Lib/unittest/test/testmock/testasync.py | 4 ++++ 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index f9789e5e85a97c..49d9fc7584180c 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -864,7 +864,7 @@ def assert_called_with(self, /, *args, **kwargs): def _error_message(): msg = self._format_mock_failure_message(args, kwargs) return msg - expected = self._call_matcher(_Call((args, kwargs))) + expected = self._call_matcher(_Call((args, kwargs), two=True)) actual = self._call_matcher(self.call_args) if actual != expected: cause = expected if isinstance(expected, Exception) else None @@ -927,9 +927,9 @@ def assert_any_call(self, /, *args, **kwargs): `assert_called_with` and `assert_called_once_with` that only pass if the call is the most recent one.""" 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 cause or expected not in _AnyComparer(actual): + if expected not in actual: + cause = expected if isinstance(expected, Exception) else None expected_string = self._format_mock_call_signature(args, kwargs) raise AssertionError( '%s call not found' % expected_string @@ -982,23 +982,6 @@ 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: return obj @@ -2172,7 +2155,7 @@ def assert_any_await(self, /, *args, **kwargs): """ expected = self._call_matcher(_Call((args, kwargs), two=True)) actual = [self._call_matcher(c) for c in self.await_args_list] - if expected not in _AnyComparer(actual): + if expected not in actual: cause = expected if isinstance(expected, Exception) else None expected_string = self._format_mock_call_signature(args, kwargs) raise AssertionError( diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py index ec6809cbd9490d..67de767849460e 100644 --- a/Lib/unittest/test/testmock/testasync.py +++ b/Lib/unittest/test/testmock/testasync.py @@ -184,6 +184,10 @@ async def main(): 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(): From 001d708f2fe7fe06d6a20650d26a928d296ab351 Mon Sep 17 00:00:00 2001 From: ElizabethU Date: Fri, 13 Sep 2019 01:03:51 -0700 Subject: [PATCH 19/20] Revert "Remove AnyCompare and use call objects everywhere." This reverts commit 24973c0b32ce7d796a7f4eeaf259832222aae0f5. --- Lib/unittest/mock.py | 25 +++++++++++++++++++++---- Lib/unittest/test/testmock/testasync.py | 4 ---- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 49d9fc7584180c..f9789e5e85a97c 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -864,7 +864,7 @@ def assert_called_with(self, /, *args, **kwargs): def _error_message(): msg = self._format_mock_failure_message(args, kwargs) return msg - expected = self._call_matcher(_Call((args, kwargs), two=True)) + expected = self._call_matcher(_Call((args, kwargs))) actual = self._call_matcher(self.call_args) if actual != expected: cause = expected if isinstance(expected, Exception) else None @@ -927,9 +927,9 @@ def assert_any_call(self, /, *args, **kwargs): `assert_called_with` and `assert_called_once_with` that only pass if the call is the most recent one.""" 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 @@ -982,6 +982,23 @@ 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: return obj @@ -2155,7 +2172,7 @@ def assert_any_await(self, /, *args, **kwargs): """ expected = self._call_matcher(_Call((args, kwargs), two=True)) actual = [self._call_matcher(c) for c in self.await_args_list] - if expected not in actual: + if expected not in _AnyComparer(actual): cause = expected if isinstance(expected, Exception) else None expected_string = self._format_mock_call_signature(args, kwargs) raise AssertionError( diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py index 67de767849460e..ec6809cbd9490d 100644 --- a/Lib/unittest/test/testmock/testasync.py +++ b/Lib/unittest/test/testmock/testasync.py @@ -184,10 +184,6 @@ async def main(): 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(): From 25dec66120f6443b5462b484844cfdc9f73f7697 Mon Sep 17 00:00:00 2001 From: Elizabeth Uselton Date: Fri, 13 Sep 2019 06:02:19 -0700 Subject: [PATCH 20/20] Check for exception in assert_any_await --- Lib/unittest/mock.py | 6 +++--- Lib/unittest/test/testmock/testasync.py | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index f9789e5e85a97c..daa8f08e1576e5 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -864,7 +864,7 @@ def assert_called_with(self, /, *args, **kwargs): def _error_message(): msg = self._format_mock_failure_message(args, kwargs) return msg - expected = self._call_matcher(_Call((args, kwargs))) + expected = self._call_matcher(_Call((args, kwargs), two=True)) actual = self._call_matcher(self.call_args) if actual != expected: cause = expected if isinstance(expected, Exception) else None @@ -2171,9 +2171,9 @@ def assert_any_await(self, /, *args, **kwargs): Assert the mock has ever been awaited with the specified arguments. """ 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 _AnyComparer(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 ec6809cbd9490d..67de767849460e 100644 --- a/Lib/unittest/test/testmock/testasync.py +++ b/Lib/unittest/test/testmock/testasync.py @@ -184,6 +184,10 @@ async def main(): 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():