classification
Title: Align expected and actual calls on mock.assert_called_with error message
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Adnan Umer, cheryl.sabella, cjw296, lisroach, mariocj89, michael.foord, suhearsawho, taleinat, terry.reedy, xtreak
Priority: normal Keywords: patch, patch, patch, patch

Created on 2018-12-14 18:45 by xtreak, last changed 2019-02-17 07:59 by taleinat. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11804 merged python-dev, 2019-02-09 23:41
Messages (16)
msg331849 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2018-12-14 18:45
Currently, assert_called_with has expected calls list in the same line with AssertionError that causes the visualizing the difference to be hard. It will be great if Expected call occurs on the next line so that the diff is improved. The change has to be made at https://github.com/python/cpython/blob/f8e9bd568adf85c1e4aea1dda542a96b027797e2/Lib/unittest/mock.py#L749 .

from unittest import mock

m = mock.Mock()
m(1, 2)
m.assert_called_with(2, 3)

Current output : 

Traceback (most recent call last):
  File "/tmp/bar.py", line 5, in <module>
    m.assert_called_with(2, 3)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/mock.py", line 820, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: Expected call: mock(2, 3)
Actual call: mock(1, 2)

Proposed output : 

Traceback (most recent call last):
  File "/tmp/bar.py", line 5, in <module>
    m.assert_called_with(2, 3)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 827, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError:
Expected call: mock(2, 3)
Actual call: mock(1, 2)

Some more alignment with the call list starting in the same column

AssertionError:
Expected call: mock(2, 3)
Actual call:   mock(1, 2)


Originally reported in the GitHub repo at https://github.com/testing-cabal/mock/issues/424 . PR for this was closed since GitHub is used only for backporting (https://github.com/testing-cabal/mock/pull/425). I thought to report it here for discussion. Currently call list output is as per proposed output.

AssertionError: Calls not found.
Expected: [call(1, 2, 3)]
Actual: [call(1, 2)].
msg331909 - (view) Author: Mario Corchero (mariocj89) * (Python triager) Date: 2018-12-15 22:27
Makes sense!

I'd not align them though but that might be my view as I generally don't like aligning text like that.

Also if you feel that the exceptions read "weird" with the first sentence is empty, an option might be to say the calls don't match, to make it symmetric with the assert_calls message. Example:


Traceback (most recent call last):
  File "/tmp/bar.py", line 5, in <module>
    m.assert_called_with(2, 3)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 827, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: Expected call not found.
Expected call: mock(2, 3)
Actual call: mock(1, 2)

This way all error reports give you the issue in the first line of the message and further details in the next lines.
msg331916 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2018-12-16 05:05
Thanks Mario for the feedback. The alignment was just a personal preference of mine. I agree with you on adding "Expected call not found" next to AssertionError. I will wait if others have more feedback on this before proceeding on the PR.
msg332302 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-12-21 21:41
We don't usually wrap error messages, but this is plausible.  With the addition to the first line, we don't need to repeat 'call'.  We can line things up without violating the PEP 8 recommendation against doing so with spaces.  Also, the convention is to not capitolize the phrase after ':'.

AssertionError: expected call not found.
Expect: mock(2, 3)
Actual: mock(1, 2)
msg332303 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2018-12-21 22:00
Thanks for the feedback. I personally prefer 'expected' than 'expect' though it comes at the cost that this cannot be aligned with 'actual'. Other places use 'expected' and it reads more natural in test case scenarios.
msg332893 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-01-02 20:34
Perhaps "expected" and "observed" or "detected"?
msg332900 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-01-02 23:43
"Expected" and "Observed" seem good.  I like "Received" slightly better, but would not argue with PR author.  It depends on whether one anthropomorphizes the assert function (or test machinery) as saying 'I expected to see' or 'I expected to get'.
msg334516 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2019-01-29 13:48
@xtreak, were you going to submit a pull request for this or do you think this would be a 'good first issue' for a new contributor?
msg334521 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-01-29 14:14
Cheryl, this is a good first issue for new contributor. I was waiting for suggestion and I am not sure if there is a consensus over the format. I would like to go with the below format as per suggestion from Mario and Terry.

AssertionError: expected call not found.
Expected: mock(2, 3)
Actual: mock(1, 2)

test_assert_called_with_failure_message has to be fixed as a result of this change. Feel free to mark it as an easy issue.

Thanks
msg335142 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-02-10 07:16
Karthikeyan, what do you think of the suggestions by Terry and myself to achieve alignment by replacing the word "Actual" with a longer alternative, such as "Observed" or "Received"?
msg335143 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-02-10 07:33
I think "Actual" is more consistent with other error messages like assert_has_calls. I haven't seen usage of observed or received in error messages and hence my opinion on using expected and actual though they are not visually aligned. I don't have a strong opinion on this. I will leave it to the maintainer who wants to merge this change and I am fine as long as the calls are represented on separate lines which is a good improvement over current format.

>>> m.assert_has_calls(mock.call(1))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 857, in assert_has_calls
    raise AssertionError(
AssertionError: Calls not found.
Expected: ['', (1,), {}]
Actual: [call(1), call(2)].
msg335145 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-02-10 08:17
After more thought, I agree that it isn't worth changing the current wording from "Actual" to something else.
msg335391 - (view) Author: Susan Su (suhearsawho) * Date: 2019-02-13 07:43
After taking a look at the assert_called_with function, I noticed that the formatting is inconsistent for the following case:
from unittest import mock

m = mock.Mock()
a = [1, 2, 3, 4]
m.assert_called_with(*a)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/vagrant/cpython/Lib/unittest/mock.py", line 817, in assert_called_with
    raise AssertionError('Expected call: %s\nNot called' % (expected,))
AssertionError: Expected call: mock(1, 2, 3)
Not called


If you believe it would be appropriate, I would like to change the format of the code above to the following:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/vagrant/cpython/Lib/unittest/mock.py", line 817, in assert_called_with
    raise AssertionError(
AssertionError: expected call not found.
Expected: mock(1, 2, 3, 4)
Not called

This way, we would create consistency in our output.
msg335394 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-02-13 09:03
Susan, I agree that similarly improving the failure message for assert_called_with would be good.

I find the final "Not called" line unclear, though. Perhaps something like the following:

AssertionError: expected call not found.
Expected: mock(1, 2, 3, 4)
Actual: Not called
msg335491 - (view) Author: Lisa Roach (lisroach) * (Python committer) Date: 2019-02-14 02:22
New changeset 2bdd5858e3f89555c8de73a0f307d63536129dbd by Lisa Roach (Susan Su) in branch 'master':
bpo-35500: align expected and actual calls on mock.assert_called_with error message. (GH-11804)
https://github.com/python/cpython/commit/2bdd5858e3f89555c8de73a0f307d63536129dbd
msg335759 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-02-17 07:59
Thanks everyone for seeing this through!
History
Date User Action Args
2019-02-17 07:59:22taleinatsetstatus: open -> closed
messages: + msg335759

keywords: patch, patch, patch, patch
resolution: fixed
stage: patch review -> resolved
2019-02-14 02:22:36lisroachsetnosy: + lisroach
messages: + msg335491
2019-02-13 09:03:00taleinatsetkeywords: patch, patch, patch, patch

messages: + msg335394
2019-02-13 07:43:39suhearsawhosetnosy: + suhearsawho
messages: + msg335391
2019-02-10 08:17:30taleinatsetkeywords: patch, patch, patch, patch

messages: + msg335145
2019-02-10 07:33:39xtreaksetkeywords: patch, patch, patch, patch

messages: + msg335143
2019-02-10 07:16:30taleinatsetkeywords: patch, patch, patch, patch

messages: + msg335142
2019-02-10 07:00:16taleinatsetpull_requests: - pull_request11818
2019-02-10 07:00:01taleinatsetpull_requests: - pull_request11817
2019-02-10 06:59:47taleinatsetpull_requests: - pull_request11816
2019-02-09 23:42:14python-devsetkeywords: + patch
stage: test needed -> patch review
pull_requests: + pull_request11818
2019-02-09 23:42:00python-devsetkeywords: + patch
stage: test needed -> test needed
pull_requests: + pull_request11817
2019-02-09 23:41:45python-devsetkeywords: + patch
stage: test needed -> test needed
pull_requests: + pull_request11816
2019-02-09 23:41:31python-devsetkeywords: + patch
stage: test needed -> test needed
pull_requests: + pull_request11815
2019-01-29 14:14:24xtreaksetmessages: + msg334521
2019-01-29 13:48:01cheryl.sabellasetnosy: + cheryl.sabella
messages: + msg334516
2019-01-02 23:43:08terry.reedysetstage: test needed
messages: + msg332900
versions: + Python 3.8
2019-01-02 20:34:43taleinatsetnosy: + taleinat
messages: + msg332893
2018-12-24 15:31:02cjw296setmessages: - msg332464
2018-12-24 15:30:42cjw296setmessages: - msg332462
2018-12-24 15:20:18Adnan Umersetmessages: + msg332464
2018-12-24 15:15:39Adnan Umersetnosy: + Adnan Umer

messages: + msg332462
versions: - Python 3.8
2018-12-21 22:00:07xtreaksetmessages: + msg332303
2018-12-21 21:41:47terry.reedysetnosy: + terry.reedy
messages: + msg332302
2018-12-16 05:05:01xtreaksetmessages: + msg331916
2018-12-15 22:27:29mariocj89setmessages: + msg331909
2018-12-14 18:45:46xtreakcreate