classification
Title: Clarify the behaviour of assert_called_once_with
Type: behavior Stage: resolved
Components: Documentation, Tests Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: berker.peksag, docs@python, lisroach, michael.foord, vstinner
Priority: normal Keywords: patch

Created on 2016-12-08 21:48 by 153957, last changed 2017-03-24 23:51 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
assert_once_with_doc.patch 153957, 2016-12-08 21:48 review
Pull Requests
URL Status Linked Edit
PR 251 merged 153957, 2017-02-23 13:42
PR 252 merged 153957, 2017-02-23 15:12
PR 254 merged 153957, 2017-02-23 15:44
Messages (9)
msg282740 - (view) Author: Arne de Laat (153957) * Date: 2016-12-08 21:48
The name assert_called_once_with and the current method documentation can be interpreted to mean that it asserts that there is precisely one call matching the given signature, regardless of the total number of calls. However, the method first checks that there is precisely one call, and then checks if that call has a signature matching the provided signature.

Additionally, the assert_any_call method documentation references assert_called_once_with in a way that enforces the possible misinterpretation: "… assert_called_with and assert_called_once_with that only pass if the call is the most recent one". This may lead to the interpretation that there must be only one call with the given signature and  that it must be the most recent one, it must in fact be the only one.

In the mock examples documentation there is an important sentence that clarifies the actual functionality: "you can use the assert_called_once_with method that also asserts that the call_count is one".

The example provided in the method documentation also does not satisfactorily address the ambiguity, because it only calls the mock with one call signature. By changing the call signature of the second call (and in the assert) in the example the purpose of the method becomes clearer.
msg285433 - (view) Author: Lisa Roach (lisroach) * (Python committer) Date: 2017-01-13 18:17
It took me a little while to wrap my brain around this, but you are definitely right that the documentation is not sufficient, your changes are an improvement. 

My wonder is, should we change the documentation or be looking at the code itself? I have always interpreted the method as asserting that only one call was made with the matching signature. If I want to test for call count I can just assert mock.call_count == 1.

The code could instead look like this: 

def assert_called_once_with(_mock_self, *args, **kwargs):
    self = _mock_self
    if not self.mock_calls.count(call(*args, **kwargs)) == 1:
        msg = ("Expected '%s' to be called once with %r %r. Called %s times."          % (self._mock_name or 'mock', args, kwargs, self.mock_calls.count(call(*args,  **kwargs))))
        raise AssertionError(msg)


Then again, if users have been using this to assert that the call_count is one (which is likely since it is in the examples documentation), this change would break backwards functionality.

Thoughts?
msg285569 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2017-01-16 16:03
I like the documentation improvement, thank you.

(The purpose of the method is to combine an assert about the arguments the method was called with and an assertion that it was only called once. To change the semantics would be both undesirable and backwards incompatible.)
msg285787 - (view) Author: Arne de Laat (153957) * Date: 2017-01-19 13:04
Unfortunately there cant be commas in the method name, that would clarify the name. It should be read as 'assert called once, with ...' not 'assert called once with ...'.

I have often used the method to test something was called only once, and with the specified arguments.

It seems that it is more difficult (i.e. no single method) to check if only one call was made with the specified arguments (allowing for more calls with other arguments), perhaps that could be added. Something like 'assert_once_called_with' or 'assert_one_call_with'.
msg288356 - (view) Author: Arne de Laat (153957) * Date: 2017-02-22 12:48
*ping*
Is this patch acceptable as is?

Perhaps a new issue can be made for a method to check at most one call with a specific signature i.e. `assert_one_call_with`.
msg288366 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-02-22 15:21
Can you please try to write a pull request?
https://docs.python.org/devguide/pullrequest.html
msg290423 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-03-24 23:50
New changeset fa30453568ae71861aa1928373bd76da4f3a33f6 by Victor Stinner (Arne de Laat) in branch '3.5':
bpo-28911: Clarify the behaviour of assert_called_once_with. (#254)
https://github.com/python/cpython/commit/fa30453568ae71861aa1928373bd76da4f3a33f6
msg290426 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-03-24 23:51
New changeset 55b82e10dc6b75b30a72fa56beb60eaf54a008d4 by Victor Stinner (Arne de Laat) in branch '3.6':
bpo-28911: Clarify the behaviour of assert_called_once_with. (#252)
https://github.com/python/cpython/commit/55b82e10dc6b75b30a72fa56beb60eaf54a008d4
msg290427 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-03-24 23:51
New changeset 324c5d8ca6ed1c964d3b20e5762139ec65c7827c by Victor Stinner (Arne de Laat) in branch 'master':
bpo-28911: Clarify the behaviour of assert_called_once_with. (#251)
https://github.com/python/cpython/commit/324c5d8ca6ed1c964d3b20e5762139ec65c7827c
History
Date User Action Args
2017-03-24 23:51:15vstinnersetmessages: + msg290427
2017-03-24 23:51:07vstinnersetmessages: + msg290426
2017-03-24 23:50:25vstinnersetmessages: + msg290423
2017-02-27 12:06:56berker.peksagsetstatus: open -> closed
nosy: - 153957

resolution: fixed
stage: patch review -> resolved
2017-02-23 15:44:32153957setpull_requests: + pull_request222
2017-02-23 15:12:14153957setpull_requests: + pull_request220
2017-02-23 13:42:44153957setpull_requests: + pull_request219
2017-02-22 15:21:42vstinnersetmessages: + msg288366
2017-02-22 12:48:44153957setnosy: vstinner, michael.foord, docs@python, berker.peksag, lisroach, 153957
messages: + msg288356
2017-01-19 13:04:56153957setnosy: + 153957
messages: + msg285787
2017-01-16 16:03:05michael.foordsetmessages: + msg285569
2017-01-13 18:17:54lisroachsetnosy: + lisroach
messages: + msg285433
2017-01-03 12:49:51vstinnersetnosy: + vstinner
2017-01-01 03:19:52berker.peksagsetnosy: + michael.foord, berker.peksag, - 153957
stage: patch review
type: enhancement -> behavior

versions: - Python 3.3, Python 3.4
2016-12-08 21:48:20153957create