[3.8] bpo-36871: Handle spec errors in assert_has_calls (GH-16005) (GH-16364)

The fix in PR 13261 handled the underlying issue about the spec for specific methods not being applied correctly, but it didn't fix the issue that was causing the misleading error message.

The code currently grabs a list of responses from _call_matcher (which may include exceptions). But it doesn't reach inside the list when checking if the result is an exception. This results in a misleading error message when one of the provided calls does not match the spec.

https://bugs.python.org/issue36871

Automerge-Triggered-By: @gpshead
(cherry picked from commit b5a7a4f0c20717a4c92c371583b5521b83f40f32)

Co-authored-by: Samuel Freilich <sfreilich@google.com>
https://bugs.python.org/issue36871

Automerge-Triggered-By: @gpshead

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 0c7545bd2b7f4704fb6168e86410892946ec5adb..30f4663e5ef4d794753b656c55862498477c9384 100644 (file)
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -931,13 +931,21 @@ class NonCallableMock(Base):
         If `any_order` is True then the calls can be in any order, but
         they must all appear in `mock_calls`."""
         expected = [self._call_matcher(c) for c in calls]
-        cause = expected if isinstance(expected, Exception) else None
+        cause = next((e for e in expected if isinstance(e, Exception)), None)
         all_calls = _CallList(self._call_matcher(c) for c in self.mock_calls)
         if not any_order:
             if expected not in all_calls:
+                if cause is None:
+                    problem = 'Calls not found.'
+                else:
+                    problem = ('Error processing expected calls.\n'
+                               'Errors: {}').format(
+                                   [e if isinstance(e, Exception) else None
+                                    for e in expected])
                 raise AssertionError(
-                    'Calls not found.\nExpected: %r%s'
-                    % (_CallList(calls), self._calls_repr(prefix="Actual"))
+                    f'{problem}\n'
+                    f'Expected: {_CallList(calls)}'
+                    f'{self._calls_repr(prefix="Actual").rstrip(".")}'
                 ) from cause
             return
 
@@ -2220,12 +2228,20 @@ class AsyncMockMixin(Base):
         they must all appear in :attr:`await_args_list`.
         """
         expected = [self._call_matcher(c) for c in calls]
-        cause = expected if isinstance(expected, Exception) else None
+        cause = next((e for e in expected if isinstance(e, Exception)), None)
         all_awaits = _CallList(self._call_matcher(c) for c in self.await_args_list)
         if not any_order:
             if expected not in all_awaits:
+                if cause is None:
+                    problem = 'Awaits not found.'
+                else:
+                    problem = ('Error processing expected awaits.\n'
+                               'Errors: {}').format(
+                                   [e if isinstance(e, Exception) else None
+                                    for e in expected])
                 raise AssertionError(
-                    f'Awaits not found.\nExpected: {_CallList(calls)}\n'
+                    f'{problem}\n'
+                    f'Expected: {_CallList(calls)}\n'
                     f'Actual: {self.await_args_list}'
                 ) from cause
             return

diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py
index 9b50b6d86bbde43d6ba7fada4fed1c0075a83500..af53210d97b3f1221dcfaae707bbd88b23d34cc9 100644 (file)
--- a/Lib/unittest/test/testmock/testasync.py
+++ b/Lib/unittest/test/testmock/testasync.py
@@ -1,5 +1,6 @@
 import asyncio
 import inspect
+import re
 import unittest
 
 from unittest.mock import (call, AsyncMock, patch, MagicMock, create_autospec,
@@ -698,3 +699,30 @@ class AsyncMockAssert(unittest.TestCase):
         asyncio.run(self._runnable_test())
         with self.assertRaises(AssertionError):
             self.mock.assert_not_awaited()
+
+    def test_assert_has_awaits_not_matching_spec_error(self):
+        async def f(x=None): pass
+
+        self.mock = AsyncMock(spec=f)
+        asyncio.run(self._runnable_test(1))
+
+        with self.assertRaisesRegex(
+                AssertionError,
+                '^{}$'.format(
+                    re.escape('Awaits not found.\n'
+                              'Expected: [call()]\n'
+                              'Actual: [call(1)]'))) as cm:
+            self.mock.assert_has_awaits([call()])
+        self.assertIsNone(cm.exception.__cause__)
+
+        with self.assertRaisesRegex(
+                AssertionError,
+                '^{}$'.format(
+                    re.escape(
+                        'Error processing expected awaits.\n'
+                        "Errors: [None, TypeError('too many positional "
+                        "arguments')]\n"
+                        'Expected: [call(), call(1, 2)]\n'
+                        'Actual: [call(1)]'))) as cm:
+            self.mock.assert_has_awaits([call(), call(1, 2)])
+        self.assertIsInstance(cm.exception.__cause__, TypeError)

diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py
index 2847d650cddced35a1ce525fd923abf18d3ee28f..817c54822630566da1d21c8662c2c501e8bc4ddd 100644 (file)
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -1426,6 +1426,32 @@ class MockTest(unittest.TestCase):
             mock.assert_has_calls(calls[:-1])
         mock.assert_has_calls(calls[:-1], any_order=True)
 
+    def test_assert_has_calls_not_matching_spec_error(self):
+        def f(x=None): pass
+
+        mock = Mock(spec=f)
+        mock(1)
+
+        with self.assertRaisesRegex(
+                AssertionError,
+                '^{}$'.format(
+                    re.escape('Calls not found.\n'
+                              'Expected: [call()]\n'
+                              'Actual: [call(1)]'))) as cm:
+            mock.assert_has_calls([call()])
+        self.assertIsNone(cm.exception.__cause__)
+
+
+        with self.assertRaisesRegex(
+                AssertionError,
+                '^{}$'.format(
+                    re.escape(
+                        'Error processing expected calls.\n'
+                        "Errors: [None, TypeError('too many positional arguments')]\n"
+                        "Expected: [call(), call(1, 2)]\n"
+                        'Actual: [call(1)]'))) as cm:
+            mock.assert_has_calls([call(), call(1, 2)])
+        self.assertIsInstance(cm.exception.__cause__, TypeError)
 
     def test_assert_any_call(self):
         mock = Mock()

diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-09-24-18-45-46.bpo-36871.p47knk.rst b/Misc/NEWS.d/next/Core and Builtins/2019-09-24-18-45-46.bpo-36871.p47knk.rst
new file mode 100644 (file)
index 0000000..6b7b19a
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-09-24-18-45-46.bpo-36871.p47knk.rst
@@ -0,0 +1,3 @@
+Improve error handling for the assert_has_calls and assert_has_awaits methods of
+mocks. Fixed a bug where any errors encountered while binding the expected calls
+to the mock's spec were silently swallowed, leading to misleading error output.