@serhiy-storchaka would you mind please taking a look at this? :)

@serhiy-storchaka a friendly bump :)

Hi @serhiy-storchaka, sorry for the multiple bumps, but would you possibly have a chance to take a look at this PR? Thanks :)

@jacksonriley Would be good to add yourself to Misc/ACKS :)

Docs also need updating (https://docs.python.org/3/library/unittest.mock.html#magic-mock) when the behaviour has been finalised.

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'>
>>> 

@jacksonriley Would be good to add yourself to Misc/ACKS :)

Docs also need updating (https://docs.python.org/3/library/unittest.mock.html#magic-mock) when the behaviour has been finalised.

Done, thank you for pointing these out :)

Hi @aeros - sorry for the random question but do you know if news items need to be regenerated via blurb? I added one for this PR a year ago, and wasn't sure whether it was collated into news based on the date on the autogenerated file in Misc/NEWS.d/next/Library or based on the date of merge :)

This PR is stale because it has been open for 30 days with no activity.