Title: Wrong behavior when using `assert_called_with` with mutable object
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.7
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Jim Jeon, cjw296, lisroach, mariocj89, michael.foord, veky, xtreak
Priority: normal Keywords:

Created on 2019-10-02 08:52 by Jim Jeon, last changed 2019-10-05 11:38 by mariocj89. This issue is now closed.

File name Uploaded Description Edit
Screen Shot 2019-10-02 at 5.22.05 PM.png Jim Jeon, 2019-10-02 08:52 source code of it, I've tested with python 3.7.4
Messages (8)
msg353727 - (view) Author: Jim Jeon (Jim Jeon) Date: 2019-10-02 08:52
When `assert_called_with` is used with mutable object, test fails if the object has changed. I think this is not what it meant to do.

The same situation is explained in this document. But I don't understand why they did not fix and just are proposing some solutions to the users.
msg353730 - (view) Author: Vedran Čačić (veky) * Date: 2019-10-02 11:21
I don't see what the "fix" should be. Python doesn't have value semantics. Functions are called with objects, not with their values.

Imagine a was global variable, and then you say:
    a = mutable_object()

In your view of the world, this should raise??
msg353733 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-10-02 12:03
As explained copy would cause backwards incompatible change on objects that depend on identity comparison. I am not sure if this can be fixed in a backwards compatible manner.

> One possibility would be for mock to copy the arguments you pass in. This could then cause problems if you do assertions that rely on object identity for equality.
msg353734 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-10-02 12:09
See also
msg353874 - (view) Author: Mario Corchero (mariocj89) * (Python triager) Date: 2019-10-03 19:37
This might be painful in certain scenarios, like when using wraps on functions that modify the arguments:

def func(d):
  return d.pop("key")
>>> def func(d):
...   return d.pop("key")
>>> m = Mock(wraps=func)
>>> m({"key": 1})
>>> m.assert_called_with({"key": 1})

But I think "not fixing" this through copy is reasonable, especially when doing copy can also break assertions on objects that cannot be copied, which can happen if they implement their own __copy__ and some other situations. Additionally, copy does not fully capture "the value of the object when it was passed" for custom types.

A copying mock was published under pypi in but doesn't seem to get a lot of attention, if this was interesting by users it could be added as a new type of Mock, or maybe just a mixin that users could add to any existing mock if they wished.
msg353894 - (view) Author: Jim Jeon (Jim Jeon) Date: 2019-10-04 02:02
Thank you all for the kind answers.
I didn't know copying could cause so many problems.

Thank you for the example.
But it seems that the example will actually raise and I think it should.
I am talking f.assert_called_with(b) when `b` has same values of `a` before it is mutated. I thought the function's purpose is to check values of the exact moment the function is called.

Thanks for the related issue. I didn't consider `identity comparison` situations. Definitely that could cause problems.

> copy does not fully capture "the value of the object when it was passed" for custom types.
Thank you for the kind answer, it was really helpful to me. Now I understand why copying is so dangerous.
msg353944 - (view) Author: Vedran Čačić (veky) * Date: 2019-10-04 13:00
> when `b` has same values of `a` before it is mutated.

There might be no such object. Or it might exist, but you still wouldn't want it.

Consider the case when a is a coroutine that has just started (a=coro()). You call f with a, and then advance a to the next yielding point. Is f called with a? You say no. Ok. But imagine you call coro again, and call that b (b=coro()). Is f called with b? Do you really want to say yes to that?
msg354002 - (view) Author: Mario Corchero (mariocj89) * (Python triager) Date: 2019-10-05 11:38
Thanks! We can look at adding a copying mock if we see people needing it, but yeah, adding copy by default would be quite complex if we don't want to break existing users.
Date User Action Args
2019-10-05 11:38:34mariocj89setstatus: open -> closed

messages: + msg354002
stage: resolved
2019-10-04 13:00:08vekysetmessages: + msg353944
2019-10-04 02:02:00Jim Jeonsetmessages: + msg353894
2019-10-03 19:37:12mariocj89setmessages: + msg353874
2019-10-02 12:09:15xtreaksetmessages: + msg353734
2019-10-02 12:03:41xtreaksetnosy: + lisroach, cjw296, xtreak, mariocj89, michael.foord
messages: + msg353733
2019-10-02 11:21:40vekysetnosy: + veky
messages: + msg353730
2019-10-02 08:52:49Jim Jeoncreate