unittest.mock does not wrap dunder methods (__getitem__ etc)

[8671e77b-46c5-4678-9050-b2b49b1d6e73]

BPO	25597
Nosy	@rbtcollins, @cjw296, @bitdancer, @voidspace, @phmc, @lisroach, @tonybaloney, @mariocj89, @tirkarthi, @akulakov
PRs	python/cpython#16029 python/cpython#19734
Files	test-mock-wraps-dict.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library', '3.9'] title = 'unittest.mock does not wrap dunder methods (__getitem__ etc)' updated_at = user = 'https://bugs.python.org/DarraghBailey' ``` bugs.python.org fields: ```python activity = actor = 'andrei.avk' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'Darragh Bailey' dependencies = [] files = ['41001'] hgrepos = [] issue_num = 25597 keywords = ['patch'] message_count = 12.0 messages = ['254453', '254466', '261835', '341615', '351469', '351824', '352124', '352125', '360739', '367559', '367623', '400457'] nosy_count = 11.0 nosy_names = ['rbcollins', 'cjw296', 'r.david.murray', 'michael.foord', 'pconnell', 'Darragh Bailey', 'lisroach', 'anthonypjshaw', 'mariocj89', 'xtreak', 'andrei.avk'] pr_nums = ['16029', '19734'] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue25597' versions = ['Python 3.9'] ```

[8671e77b-46c5-4678-9050-b2b49b1d6e73]

Both unittest.mock and the backported release for earlier pythons don't appear to support mocking of dictionary objects.

Specifically I'm expecting that any of the methods used to test for membership, or get items from a mock object wrapping a dictionary should succeed. However it appears that MagicMock doesn't appear to support this.

Attached file shows an attempt to use different methods with a wrapped dictionary object where only the '.get()' method appears to work as expected.

[bitdancer]

Looking at the source, it's not clear that wraps is supported for __ methods, despite what the documentation implies. That is, MagicProxy doesn't seem to look at the wraps information. I suspect it is doable, but it may be an enhancement request rather than a bug fix, I'm not sure.

[rbtcollins]

I think this is a valid mock bug; it likely needs some thoughtful exhaustive testing, and obviously support added for it.

[9bdb45ff-41f1-405d-999a-c9a9eb99e027]

The assertions in the attached test still fail on master (3.8a3), so this still applies.

Michael, are you able to look at this, the code hasn't changed since the original PEP-417 implementation, which doesn't specify if this behaviour should be supported.

The documentation does not specify that this is supported also, so i suspect this is an enhancement request.

    elif result is None:
        wraps = None
        if self._mock_wraps is not None:
            # XXXX should we get the attribute without triggering code
            # execution?
            wraps = getattr(self._mock_wraps, name)

            result = self._get_child_mock(
                parent=self, name=name, wraps=wraps, _new_name=name,
                _new_parent=self
            )

[tirkarthi]

This seems to a reasonable change to me since dict.get returns the value then making a contains check dict.__contains__ should return True instead of the fixed return value of False. Below is a patch where the mock_wraps attribute is set with the relevant method that would make sure in the report dict.get would used.

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 298b41e0d7..077d22d08e 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1935,6 +1935,12 @@ _side_effect_methods = {

 def _set_return_value(mock, method, name):
+    # If _mock_wraps is present then attach it so that it's return
+    # value is used when called.
+    if mock._mock_wraps is not None:
+        method._mock_wraps = getattr(mock._mock_wraps, name)
+        return
+
     fixed = _return_values.get(name, DEFAULT)
     if fixed is not DEFAULT:
         method.return_value = fixed

[tirkarthi]

Michael, any thoughts on this? This more feels like an enhancement to me and I have marked it as 3.9 since I am not sure someone might be depending on the behavior where they have used wraps but still expect default values for magicmethods as they do now.

[voidspace]

As discussed with Karthik, I think this is a nice feature enhancement for the wraps functionality and worth fixing. It has the great advantage that the fix is nice and isolated and simple.

[voidspace]

The previous behaviour was unspecified and clearly due to missing functionality, so the advantages of fixing it outweigh any potential compatibility issues. But I'd see it as a feature enhancement for 3.9.

[cjw296]

New changeset 72b1004657e60c900e4cd031b2635b587f4b280e by Chris Withers (Karthikeyan Singaravelan) in branch 'master': bpo-25597: Ensure wraps' return value is used for magic methods in MagicMock (bpo-16029) https://github.com/python/cpython/commit/72b1004657e60c900e4cd031b2635b587f4b280e

[cjw296]

New changeset 521c8d6806adf0305c158d280ec00cca48e8ab22 by Karthikeyan Singaravelan in branch 'master': bpo-39966: Revert "bpo-25597: Ensure wraps' return value is used for magic methods in MagicMock" (GH-19734) https://github.com/python/cpython/commit/521c8d6806adf0305c158d280ec00cca48e8ab22

[tirkarthi]

The change has been reverted as per bpo-39966. I am reopening this for further discussion.

[akulakov]

I went through dunder methods to check if any other operators or builtins work on objects without respective dunder methods:

• del x works even though there is no object.__del__

• operator.lengthhint() => 0 when there is no object.\_length_hint__

So in addition to __bool__, these dunder methods would have to be special cased.