From 4042e1afd252858de53e2b79d946a51a0182d1ba Mon Sep 17 00:00:00 2001
From: "Gregory P. Smith" <greg@krypto.org>
Date: Tue, 24 Sep 2019 22:29:17 -0700
Subject: [PATCH] [3.7] bpo-36871: Handle spec errors in assert_has_calls
 (GH-16364) (GH-16374)

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

Co-authored-by: Samuel Freilich <sfreilich@google.com>

(cherry picked from commit 1a17a054f6314ce29cd2632c28aeed317a615360)
---
 Lib/unittest/mock.py                          | 13 ++++++---
 Lib/unittest/test/testmock/testmock.py        | 27 +++++++++++++++++++
 .../2019-09-24-18-45-46.bpo-36871.p47knk.rst  |  3 +++
 3 files changed, 40 insertions(+), 3 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-09-24-18-45-46.bpo-36871.p47knk.rst

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index b1ab8631a422ef..2b9e7f14a75635 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -895,13 +895,20 @@ def assert_has_calls(self, calls, any_order=False):
         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\n'
-                    'Actual: %r' % (_CallList(calls), self.mock_calls)
+                    '%s\nExpected: %r\nActual: %r' % (
+                        problem, _CallList(calls), self.mock_calls)
                 ) from cause
             return
 
diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py
index 4e0f4694b77115..3046d4cfe63938 100644
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -1,4 +1,5 @@
 import copy
+import re
 import sys
 import tempfile
 
@@ -1394,6 +1395,32 @@ def f(a, b, c, d=None):
             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
index 00000000000000..56ee0f43f5d4c4
--- /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 method 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.