python / cpython

The Python programming language
https://www.python.org
Other
62.52k stars 30.01k forks source link

`reset_mock` resets `MagicMock`'s magic methods in an unexpected way #123934

Open mentalAdventurer opened 2 weeks ago

mentalAdventurer commented 2 weeks ago

Bug report

Bug description:

The reset_mock(return_value=True) method behaves in a wrong/inconsistent way. When used with MagicMock, the method reset_mock(return_value=True) does not reset the return values of the magic methods. Only if you call for example __str__ and then call the reset_mock function, the return value will be reset, but not to the default value.

from unittest import mock

mm = mock.MagicMock()
print(type(mm.__str__()))
mm.reset_mock(return_value=True)
print(type(mm.__str__()))
print(type(mm.__hash__()))
mm.reset_mock(return_value=True)
print(type(mm.__hash__()))

Output

<class 'str'>
<class 'unittest.mock.MagicMock'>
<class 'int'>
<class 'unittest.mock.MagicMock'>

Since Python 3.9 PR reset_mock now also resets child mocks. This explains the behaviour. Calling the __str__ method creates a child MagicMock with a set return value. Since this child mock now exists, its return value is reset when reset_mock(return_value=True) is called. Although this can be logically explained, it's counter-intuitive and annoying as I'm never sure which values are being reset.

I would expect the same behaviour as Mock. The return value of __str__ and other magic methods should not be effected.

from unittest import mock

m = mock.Mock()
print(type(m.__str__()))
m.reset_mock(return_value=True)
print(type(m.__str__()))
print(type(m.__hash__()))
m.reset_mock(return_value=True)
print(type(m.__hash__()))

Output

<class 'str'>
<class 'str'>
<class 'int'>
<class 'int'>

CPython versions tested on:

3.10

Operating systems tested on:

Linux

Linked PRs

sobolevn commented 2 weeks ago

Thank you for the report. I will take a look soon :)

sobolevn commented 1 week ago

Since this child mock now exists, its return value is reset when reset_mock(return_value=True) is called.

Yes, this is correct.

I would expect the same behaviour as Mock. The return value of str and other magic methods should not be effected.

Mock does not mock magic methods at all, so they are unaffected at any case. Compare:

print(type(mock.Mock().__str__))
# <class 'method-wrapper'>

print(type(mock.MagicMock().__str__))
# <class 'unittest.mock.MagicMock'>

But, I agree that retuning <class 'unittest.mock.MagicMock'> from __str__ is not correct. So, my proposed solution is to ignore cases when:

  1. child is a MagicMock instance
  2. key is a magic method
  3. return_value is True