Allow callable mocks to make a perfect snapshot of their inputs

[yanivshimoni89]

Bug report

Bug description:

Hi,
I'm experiencing an unexpected behaviour with unittest. Code example:

import unittest
from unittest.mock import MagicMock

class TestCallMutability(unittest.TestCase):
    def test_call_mutability(self):
        def side_effect(x: dict):
            x['a'] = 6

        mock_function = MagicMock()
        mock_function.side_effect = side_effect

        input_dict = {'b': 5}
        mock_function(input_dict)

        self.assertEqual(input_dict, {'b': 5, 'a': 6})
        mock_function.assert_called_once_with({'b': 5})

if __name__ == "__main__":
    unittest.main()

Expected behavior:
This test should pass.

Actual behaviour:
The test fails with the following traceback:

F
======================================================================
FAIL: test_call_mutability (tests.TestCallMutability)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pytest_bug_report/tests/__init__.py", line 16, in test_call_mutability
    mock_function.assert_called_once_with({'b': 5})
  File "/usr/lib/python3.10/unittest/mock.py", line 941, in assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
  File "/usr/lib/python3.10/unittest/mock.py", line 929, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: mock({'b': 5})
Actual: mock({'b': 5, 'a': 6})

----------------------------------------------------------------------
Ran 1 test in 0.001s

FAILED (failures=1)

Basically, even though the call to the function for done using {'b': 5}, the call_args_list contains the value of the call after the change was done. My expected behavior is that the test would make a copy of the input, since the function was indeed called with {'b': 5}.

Thanks!

CPython versions tested on:

3.10

Operating systems tested on:

Linux

[picnixz]

Unfortunately, I'm not sure we can make a perfect copy of the original argument for various reasons, especially because it might not be deep-copiable. In addition, if we make a fresh deepcopy, depending on the object, if two copies are not considered == for various reasons, then we cannot use self.assertEqual(called_with[0], arg0) or something like that.

I would advise you to remember the called arguments using some other variable instead of mocking the function with side-effects =/ (maybe others have ideas on how to fix this, but I don't see a universal and easy solution)

[picnixz]

@sobolevn @Eclips4 @ZeroIntensity do you have some ideas on how to circumvent this? or alternatives other than using another variable for holding the calls?

[sobolevn]

For me - this works rather expected.

[picnixz]

Yes, if we consider that we check that something was called with some object X, I agree that it works as intended. But if we assume that assert_called_with only checks that the object which was called with equals to some other Y, then it's a bug (I also think it's the first behaviour that we assume, but I wanted a second opinion)

[yanivshimoni89]

Personally, I expected the second behavior especially since if I understand correctly, the assert_called_with check that the call that was actually done has the same args and kwargs as given in the assertion (so the expected behavior should maybe depend on how the args/kwargs implement eq, so I thought the example should run since for dictionaries, equality only depends on the keys and values and not on the actual object). I agree that it can be hard to implement in a way that does not introduce more edge cases.
For my case, manually saving the input in a variable worked.

[picnixz]

One possibility is to possibly make a deepcopy of the arguments being called but that would only work if the argument is deepcopiable. And something like this wouldn't work anymore:

def f(x):
	x['a'] = 1

mock = MagicMock()
mock.side_effect = f

x = {}
mock(x)

# before:
self.assertEqual(mock.call_args[0][0], {})  # not ok now
mock.assert_called_once_with({})			# not ok now
self.assertIs(mock.call_args[0][0], x)	  	# currently ok

# after:
mock.assert_called_once_with({})					# would now be ok
self.assertEqual(mock.call_args[0][0], {})  		# would now be ok

self.assertEqual(mock.call_args[0][0], x)			# existing tests would now fail (not good)
self.assertIs(mock.call_args[0][0], x)				# existing tests would now fail (not good)

So we would break existing tests that don't use assert_called but directly access call_args. I'm afraid I will need to close this issue as wont fix (but we can improve the docs if this is not precise enough, although it wouldn't fix your issue)

[asvetlov]

Agree with wont fix decision; the ship has sailed decades ago.
The changing to deep-copy is not backward compatible.

[picnixz]

Considering my fellow members' opinion, I will therefore close this one as wont fix. Sorry that we can't help you (note that I myself encountered such an issue and was a bit sad at that time). I note https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock.assert_called_with only says "This method is a convenient way of asserting that the last call has been made in a particular way" which may incorrectly lead readers to assume that we make a snapshot. A doc PR can be opened if you want (I personally don't have time for this)