classification
Title: Mock.assert_has_calls([]) is surprising for users
Type: Stage: resolved
Components: Documentation Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: berker.peksag, docs@python, eric.araujo, inada.naoki, kushal.das, michael.foord, rbcollins, serhiy.storchaka, xtreak, ztane
Priority: normal Keywords: patch

Created on 2015-07-17 09:40 by rbcollins, last changed 2019-05-22 12:46 by serhiy.storchaka.

Pull Requests
URL Status Linked Edit
PR 9882 closed thatiparthy, 2018-10-14 22:11
Messages (11)
msg246849 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-17 09:40
From https://github.com/testing-cabal/mock/issues/243

from unittest import mock
mmock = mock.MagicMock()
mmock.foobar("baz")
mmock.assert_has_calls([])  # No exception raised. Why?mmock.assert_has_calls(['x'])  # Exception raised as expected.

---

Traceback (most recent call last):
  File "tt.py", line 7, in <module>
    mmock.assert_has_calls(['x'])  # Exception raised as expected.
  File "/home/robertc/work/cpython/Lib/unittest/mock.py", line 824, in assert_has_calls
    ) from cause
AssertionError: Calls not found.
Expected: ['x']
Actual: [call.foobar('baz')]
msg246850 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-17 09:41
This might go back further, haven't checked 3.3, but IIRC we're only doing fixes on 3.4 up anyhow.
msg246988 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2015-07-20 13:20
assert_has_calls checks that the calls you specified were made. So if you specify an empty list it *should* pass (or be disallowed as it has no meaning).

If you want to check that these calls and *only* those calls were made you should use something like:

    self.assertEqual(mock.mock_calls, [])

http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock.assert_has_calls

Maybe a documentation improvement instead.
msg247044 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-21 16:30
Ok, so as a doc bug this should still be tracked here - I'm going to reopen it to reflect that, hope thats ok.
msg330000 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-11-16 14:05
Why this case should be explicitly mentioned? Is it an exception from some rules? Is the empty list special? Does existing documentation say that assert_has_calls([]) should raise an exception?
msg342887 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2019-05-20 03:36
I concur with Serhiy.  Current document and example describe clearly
that this API is for checking inclusion, not equality.

Current example:

"""
>>> mock = Mock(return_value=None)
>>> mock(1)
>>> mock(2)
>>> mock(3)
>>> mock(4)
>>> calls = [call(2), call(3)]
>>> mock.assert_has_calls(calls)
>>> calls = [call(4), call(2), call(3)]
>>> mock.assert_has_calls(calls, any_order=True)
"""

Empty list is allowed by same rule.  There are no exception.

And when people understand this API checks inclusion, calling with
empty list doesn't make sense at all.  So I don't think it is worth enough.
msg343080 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2019-05-21 19:01
> And when people understand this API checks inclusion, calling with
empty list doesn't make sense at all. 

I can see a thought process being «all my other tests use thing.assert_has_calls([call0, call1]), here let’s check there are no calls with thing2.assert_has_calls([])».

Are the correct methods advertised in the right place?  (I don’t use mock so I forgot if there is a method to assert not called or if it’s assertEqual(mock calls count, 0) or some False property)
msg343083 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-05-21 19:05
> Are the correct methods advertised in the right place?  (I don’t use mock so I forgot if there is a method to assert not called or if it’s assertEqual(mock calls count, 0) or some False property)

There is assert_not_called https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock.assert_not_called

>>> from unittest.mock import Mock
>>> m = Mock()
>>> m.assert_not_called()
msg343163 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2019-05-22 09:54
I'm reopening this because I don't agree.

I opened the bug because we have evidence that users find the current documentation confusing. Saying that its not confusing to us doesn't fix the confusion.

Why should we mention the special case of an empty set? Because its the only special case.

A simple single sentence: "Note:  to assert that no calls were made see `assert_not_called`" would probably both cover the special case and direct users to the right place for that use case.
msg343164 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2019-05-22 10:00
> I opened the bug because we have evidence that users find the current documentation confusing. Saying that its not confusing to us doesn't fix the confusion.

Is there evidence people get confused by the document?

I suppose people get confused because they guessed behavior from existing assert_has_calls usages, without reading docs.
If they guess without reading doc, adding anything in doc doesn't help them.

The document describe well it tests inclusion, not equality.  There are no confusion about it.
msg343189 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-05-22 12:46
alist.extend([]) is also a special case, but it is not explicitly documented, because it is not exceptional.
History
Date User Action Args
2019-05-22 12:46:46serhiy.storchakasetmessages: + msg343189
2019-05-22 10:00:21inada.naokisetmessages: + msg343164
2019-05-22 09:54:07rbcollinssetstatus: closed -> open
resolution: wont fix ->
messages: + msg343163
2019-05-21 19:05:34xtreaksetmessages: + msg343083
2019-05-21 19:01:59eric.araujosetnosy: + eric.araujo
messages: + msg343080
2019-05-20 03:36:04inada.naokisetstatus: open -> closed

nosy: + inada.naoki
messages: + msg342887

resolution: wont fix
stage: patch review -> resolved
2018-11-16 14:05:11serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg330000
2018-10-14 22:11:20thatiparthysetkeywords: + patch
stage: patch review
pull_requests: + pull_request9245
2018-09-23 16:15:22xtreaksetnosy: + xtreak
2015-07-21 20:01:28ztanesetnosy: + ztane
2015-07-21 16:30:36rbcollinssetstatus: closed -> open

assignee: docs@python
components: + Documentation
title: Mock.assert_has_calls([]) incorrectly passes -> Mock.assert_has_calls([]) is surprising for users
nosy: + docs@python

messages: + msg247044
resolution: not a bug -> (no value)
2015-07-20 13:20:09michael.foordsetstatus: open -> closed
resolution: not a bug
messages: + msg246988
2015-07-17 09:41:07rbcollinssetnosy: + michael.foord, berker.peksag, kushal.das

messages: + msg246850
versions: + Python 3.4, Python 3.5, Python 3.6
2015-07-17 09:40:04rbcollinscreate