bpo-40864: Fetch instance variables for spec

[efagerberg]

My main driver for this change is I wanted the security of spec_set with the knowledge of the attributes created in init.

https://bugs.python.org/issue40864

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@efagerberg

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

[github-actions]

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

[efagerberg]

@cjw296 this should be good to review I believe. Please let me know if I missed anything. I think this feature would improve dev UX when using spec_set tremendously as users will not need to work around missing attributes that should be present in mocks.

This does not set the spec of the attributes as they are found though (they will end up just being regular mocks). Figure that is a good enough chunk of work to be it's own PR.

[MaxwellDupre]

Your example works but running the testmock produces:
.....................................................................F.F...................................................................

FAIL: test_finds_attr_set_in__init__ (main.MockTest)

Traceback (most recent call last):
File "/home/me/Documents/cpython/Lib/unittest/test/testmock/testmock.py", line 908, in test_finds_attr_set_in__init__
self.assertTrue(hasattr(mock, 'z'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: False is not true

======================================================================
FAIL: test_finds_underscore_attr_set_in__init__ (main.MockTest)

Traceback (most recent call last):
File "/home/me/Documents/cpython/Lib/unittest/test/testmock/testmock.py", line 919, in test_finds_underscore_attr_set_in__init__
self.assertTrue(hasattr(mock, '_z'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: False is not true

---

Ran 139 tests in 0.402s

FAILED (failures=2)

Ran with checkout of this branch. Could you fix please?

[efagerberg]

Hey @MaxwellDupre, I just ran this after updating the branch to be in sync with remote and then rebuilding the python executable, things all passed.

./configure --with-pydebug
make -s -j2
./python Lib/unittest/test/testmock/__main__.py 
.................................................................s...................................................................................................................................................................................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 501 tests in 2.537s

OK (skipped=1)

[MaxwellDupre]

I see my error: I had not done an make altinstall, sorry.
All tests passed.

[efagerberg]

Hello, is there any update on this PR? Anything I need to do? I would love to get this in as this is an issue that hits me pretty regularly and forces me to use non spec'd mocks.

cc @cjw296

[cjw296]

@tirkarthi - what're your thoughts on this?

[tirkarthi]

https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock

spec: This can be either a list of strings or an existing object (a class or instance) that acts as the specification for the mock object. If you pass in an object then a list of strings is formed by calling dir on the object (excluding unsupported magic attributes and methods). Accessing any attribute not in this list will raise an AttributeError.
If spec is an object (rather than a list of strings) then class returns the class of the spec object. This allows mocks to pass isinstance() tests.

The docs state that the dir method is used to retrieve the attributes and the PR changes this to include attributes part of __init__ that are not part of dir when a class is used. The attribute gets listed in dir output though when the object is used instead of class. In below case with current behavior m_class which uses a class to spec will not have name but with the patch this introduces a behavior change where name is accessible. This can cause confusion where we don't know if name is a class variable or instance variable. Also objects in __init__ might dynamically add or remove attributes. Though object construction to use it as a spec (e.g. database connection object) might defeat the purpose in some cases I guess it's good to have a difference between class and instance in specs.. Though our tests don't fail I am not sure if this can cause some unexpected behavior changes in the real world with spec.

import dataclasses
from unittest.mock import Mock

class Person:

    def __init__(self, name):
        self.name = name
    
m_instance = Mock(spec=Person("Jim"))
print(m_instance.name)


m_class = Mock(spec=Person)
print(m_class.name)

Current behavior

python /tmp/foo.py
<Mock name='mock.name' id='139643658080832'>
Traceback (most recent call last):
  File "/tmp/foo.py", line 16, in <module>
    print(m_class.name)
  File "/usr/lib/python3.10/unittest/mock.py", line 634, in __getattr__
    raise AttributeError("Mock object has no attribute %r" % name)
AttributeError: Mock object has no attribute 'name'

With patch

./python /tmp/foo.py
<Mock name='mock.name' id='140172872405264'>
<Mock name='mock.name' id='140172866729872'>

[tirkarthi]

See also https://bugs.python.org/issue36580

[tirkarthi]

I am not experienced with code object. This also seems to end up including the functions and methods as part of __init__ as per breakpoint here below where startswith is represented as a valid attribute. In case you are trying to get signature of the __init__ you can use inspect module : https://docs.python.org/3/library/inspect.html#introspecting-callables-with-the-signature-object . But not all the methods in signature become attributes as they could be flags to set another set of attributes dynamically too.

class Person:

    def __init__(self, name):
        self.name = name
        foo = 1

        if name.startswith("Test"):
            print("Test user")


m_instance = Mock(spec=Person("John"))
print(m_instance.name)
print(m_instance.startswith)

./python /tmp/foo.py
> /home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py(522)_mock_add_spec()
-> spec.extend(member_v.__code__.co_names)
(Pdb) member_v.__code__.co_names
('name', 'startswith', 'print')
(Pdb) c
<Mock name='mock.name' id='140293864592656'>
<Mock name='mock.startswith' id='140293862696912'>

[efagerberg]

Thanks for the feedback. Those are good points. When I have some free time I will see if I can find a more surgical solution and add in some test cases for the examples you folks gave.

[iritkatriel]

See #22572 (comment).

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[ricardodns]

Any updates on this? I really like this change.