Skip to content

bpo-36871: Handle spec errors in assert_has_calls #16005

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

Merged
merged 6 commits into from
Sep 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions Lib/unittest/mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -931,13 +931,21 @@ 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%s'
% (_CallList(calls), self._calls_repr(prefix="Actual"))
f'{problem}\n'
f'Expected: {_CallList(calls)}\n'
f'Actual: {self._calls_repr(prefix="Actual")}'
Copy link
Member

@tirkarthi tirkarthi Sep 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes a difference in error message with Actual line printed twice. Using self._calls_repr alone takes care of it. self._calls_repr already prepends a newline so please remove the "\n" in f'Expected: {_CallList(calls)}\n' to avoid an extra new line.

from unittest.mock import *

def f(): pass

m = Mock(spec=f)
m()
m.assert_has_calls([call(), call('wrong')])
➜  cpython git:(master) ✗ ./python.exe /tmp/foo.py
TypeError: too many positional arguments

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/tmp/foo.py", line 7, in <module>
    m.assert_has_calls([call(), call('wrong')])
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 945, in assert_has_calls
    raise AssertionError(
AssertionError: Error processing expected calls.
Errors: [None, TypeError('too many positional arguments')]
Expected: [call(), call('wrong')]
Actual:
Actual: [call()].
Suggested change
f'Actual: {self._calls_repr(prefix="Actual")}'
f'{self._calls_repr(prefix="Actual")}'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whooops. thanks! i missed that. :) follow up with a new PR with that little change. looks like we need to do the backport manually and can lump these two changes together for that anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, sorry. I was making this more similar to cleaner logic in assert_has_awaits, and I missed that repetition.

) from cause
return

Expand Down Expand Up @@ -2214,12 +2222,20 @@ def assert_has_awaits(self, calls, any_order=False):
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
Expand Down
21 changes: 21 additions & 0 deletions Lib/unittest/test/testmock/testasync.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import asyncio
import inspect
import re
import unittest

from unittest.mock import (call, AsyncMock, patch, MagicMock, create_autospec,
Expand Down Expand Up @@ -632,3 +633,23 @@ def test_assert_not_awaited(self):
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(): pass

mock = AsyncMock(spec=f)

with self.assertRaisesRegex(
AssertionError,
re.escape('Awaits not found.\nExpected:')) as cm:
mock.assert_has_awaits([call()])
self.assertIsNone(cm.exception.__cause__)

with self.assertRaisesRegex(
AssertionError,
re.escape('Error processing expected awaits.\n'
"Errors: [None, TypeError('too many positional "
"arguments')]\n"
'Expected:')) as cm:
mock.assert_has_awaits([call(), call('wrong')])
self.assertIsInstance(cm.exception.__cause__, TypeError)
19 changes: 19 additions & 0 deletions Lib/unittest/test/testmock/testmock.py
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,25 @@ def f(a, b, c, d=None): pass
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(): pass

mock = Mock(spec=f)

with self.assertRaisesRegex(
AssertionError,
re.escape('Calls not found.\nExpected:')) as cm:
mock.assert_has_calls([call()])
self.assertIsNone(cm.exception.__cause__)

with self.assertRaisesRegex(
AssertionError,
re.escape('Error processing expected calls.\n'
"Errors: [None, TypeError('too many positional "
"arguments')]\n"
'Expected:')) as cm:
mock.assert_has_calls([call(), call('wrong')])
self.assertIsInstance(cm.exception.__cause__, TypeError)

def test_assert_any_call(self):
mock = Mock()
Expand Down
Original file line number Diff line number Diff line change
@@ -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.