unittest.mock spec calls class properties

[melwitt]

BPO	41768
Nosy	@terryjreedy, @cjw296, @voidspace, @ambv, @lisroach, @mariocj89, @tirkarthi, @sobolevn, @melwitt
PRs	gh-85934: Use getattr_static when adding mock spec #22209

^{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:

assignee = None
closed_at = None
created_at = <Date 2020-09-12.01:10:24.232>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = 'unittest.mock spec calls class properties'
updated_at = <Date 2021-12-03.08:45:33.635>
user = 'https://github.com/melwitt'

bugs.python.org fields:

activity = <Date 2021-12-03.08:45:33.635>
actor = 'AlexWaygood'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-09-12.01:10:24.232>
creator = 'melwitt'
dependencies = []
files = []
hgrepos = []
issue_num = 41768
keywords = ['patch']
message_count = 3.0
messages = ['376757', '377140', '407569']
nosy_count = 9.0
nosy_names = ['terry.reedy', 'cjw296', 'michael.foord', 'lukasz.langa', 'lisroach', 'mariocj89', 'xtreak', 'sobolevn', 'melwitt']
pr_nums = ['22209']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue41768'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[melwitt]

When async magic method support was added to unittest.mock.Mock to address issue bpo-26467, it introduced a getattr call [1] that causes class properties to be called when the class is used as a mock spec.

This caused a problem for a test in my project when running with Python 3.8 where previously the test worked OK with Python 3.6.

The test aims to verify that a class instance is not created if the called code path does not access the class property and thus the class will not create a heavy object unless it's needed (lazy create on access via @Property).

As of Python 3.8, the @Property is always called and is called by the mock spec process itself, even though the code path being tested does not access the class @Property.

Here is a code snippet that illustrates the @Property calling from the mock spec alone:

class SomethingElse(object):
    def __init__(self):
        self._instance = None

    @property
    def instance(self):
        if not self._instance:
            self._instance = 'object'

...

    def test_property_not_called_with_spec_mock(self):
        obj = SomethingElse()
        self.assertIsNone(obj._instance)
        mock = Mock(spec=obj)
        self.assertIsNone(obj._instance)

$ ./python -m unittest -v unittest.test.testmock.testmock.MockTest.test_property_not_called_with_spec_mock
test_property_not_called_with_spec_mock (unittest.test.testmock.testmock.MockTest) ... FAIL


======================================================================
FAIL: test_property_not_called_with_spec_mock (unittest.test.testmock.testmock.MockTest)
\----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/cpython/Lib/unittest/test/testmock/testmock.py", line 2173, in test_property_not_called_with_spec_mock
    self.assertIsNone(obj._instance)
AssertionError: 'object' is not None

[1]

cpython/Lib/unittest/mock.py

Line 492 in fb27187

if iscoroutinefunction(getattr(spec, attr, None)):

[terryjreedy]

Without lines numbers, I cannot test which of the two identical asserts failed. Either comments or msg arguments will differentiate. Nor can I run snippets from two different files. Here is a minimal reproducible self-contained code that demonstrates the claim (which I verified on 3.9 and current master).

import unittest
from unittest.mock import Mock

class SomethingElse(object):
    def __init__(self):
        self._instance = None

    @property
    def instance(self):
        if not self._instance:
            self._instance = 'object'

class Test(unittest.TestCase):

    def test_property_not_called_with_spec_mock(self):
        obj = SomethingElse()
        self.assertIsNone(obj._instance, msg='before') # before
        mock = Mock(spec=obj)
        self.assertIsNone(obj._instance, msg='after') # after

unittest.main()

[sobolevn]

Related PR: #29901

[f0k]

I'm affected by the same problem: The iscoroutinefunction(getattr(spec, attr, None)) call to search for coroutines triggers properties that were not meant to be triggered (and were not triggered in earlier versions of Python). This issue is also reported in #89917 and #98301. In my use case, I am mocking a base class that raises NotImplementedError on some properties which are not involved in the unit test anyway. I'd need to mock a derived class instead that has all properties implemented, but that feels wrong given that I want to test aspects of the base class.

There are three PRs and one issue proposing different solutions:

1. gh-85934: Use getattr_static when adding mock spec #22209: uses inspect.getattr_static instead of getattr. It was not merged because the sole reviewer was not confident that they could judge whether the change was correct.
2. bpo-45756: do not execute @property descrs while creating mock autospecs #29901: uses inspect.getattr_static to check if the attribute is a property, and skips the iscoroutinefunction(getattr(...)) call in this case. It was not merged because it missed a core review. Raymond Hettinger left a comment asking whether some use cases might rely on all properties being called.
3. bpo-45756: Fix mock triggers dynamic lookup via the descriptor protocol. #31348: iterates over inspect.getmembers_static(spec, iscoroutinefunction) instead of iterating over all attributes. It was asked whether async classmethods would still be caught.
4. unittest.mock.create_autospec() can crash when accessing properties from objects #98301: proposes to wrap getattr in a try/except block to silence any occurring exceptions. It did not receive any feedback.

All solutions would work for the case depicted here. There is a corner case that is treated differently between the current code and the first three proposals:

class X(object):
    @property
    def foo():
        async def bar():
            pass
        return bar

If we spec an instance of this class, X(), the current code will add foo to the _spec_asyncs list, since it returns a coroutine function. The first three proposals will not add foo because they get to see the property instance instead of the result of the property's getter. I'm not sure if this is relevant.

The fourth solution will handle this corner case as the current code, but does not fix the issue discussed in this post, which was about avoiding expensive lazy initialization of objects triggered by a property getter.

I would suggest to go for the fix in #22209, and include the additional test case from #31348.