bpo-34716: Change MagicMock().divmod to return a pair of MagicMock instances by jacksonriley · Pull Request #17291 · python/cpython (original) (raw)

I've had a play around with these changes and they seem to work as expected - returning type(self)() appears to be standard and the correct approach. Also noted divmod and rdivmod seem to be the only special cases of dunder methods expected to return a fixed number of multiple values.

At a higher level there are some considerations for making this change, e.g. it's not strictly speaking backwards compatible (although I don't expect the change to cause many problems).

Previously:

>>> m = MagicMock()
>>> divmod(m, 1)
<MagicMock name='mock.__divmod__()' id='140147753274096'>
>>> divmod(m, 1).return_value = 1
>>> divmod(m, 1)()
1

Now:

>>> m = MagicMock()
>>> divmod(m, 1)
(<MagicMock id='139643052028928'>, <MagicMock id='139643052032640'>)
>>> divmod(m, 1).return_value = 1
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
AttributeError: 'tuple' object has no attribute 'return_value'

Another potential concern is the following behaviour:

>>> m = MagicMock()
>>> m.__divmod__.return_value
<MagicMock name='mock.__divmod__()' id='139643052068720'>
>>> m.__divmod__.return_value[0]
<MagicMock name='mock.__divmod__().__getitem__()' id='139643049016480'>
>>> m.__divmod__.return_value[0] = 1
>>> m.__divmod__.return_value[0]
<MagicMock name='mock.__divmod__().__getitem__()' id='139643049016480'>
>>> divmod(m, 1)
<MagicMock name='mock.__divmod__()' id='139643052068720'>

I think what's happening here is that m.__divmod__.return_value initially holds sentinal.DEFAULT rather than what will actually be returned (the tuple of mocks), and when you change something on it then it becomes a concrete mock and the tuple is no longer returned (when what I expected to happen was to change the first element of the tuple).

Perhaps it would be better to add to _return_values rather than _side_effect_methods to fix this?

With the most recent commit (6bb2e07), the behaviour is (I think) more correct:

>>> m = MagicMock()
>>> m.__divmod__.return_value
(<MagicMock id='140534571186688'>, <MagicMock id='140534525562832'>)
>>> m.__divmod__.return_value[0]
<MagicMock id='140534571186688'>
>>> m.__divmod__.return_value[0] = 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'tuple' object does not support item assignment
>>> divmod(m, 1)
(<MagicMock id='140534571186688'>, <MagicMock id='140534525562832'>)
>>> m.__divmod__.return_value = 1
>>> divmod(m, 1)
1

It should be noted that with this setup, the same pair of mocks is returned every time the magic method is called:

>>> divmod(m, 1)
(<MagicMock id='140715446614240'>, <MagicMock id='140715401200640'>)
>>> divmod(m, 1)
(<MagicMock id='140715446614240'>, <MagicMock id='140715401200640'>)
>>> 

However, as this behaviour is the same as other magic methods without specific implementations, I think this isn't a problem. For example, __add__:

>>> m + 1
<MagicMock name='mock.__add__()' id='140207358300704'>
>>> m + 2
<MagicMock name='mock.__add__()' id='140207358300704'>
>>>