This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Add docs for Mock name and parent args and deprecation warning when wrong args are passed
Type: enhancement Stage: patch review
Components: Tests Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: andrei.avk, lukasz.langa
Priority: normal Keywords: patch

Created on 2021-09-16 02:13 by andrei.avk, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 28378 open andrei.avk, 2021-09-16 02:39
Messages (4)
msg401913 - (view) Author: Andrei Kulakov (andrei.avk) * (Python triager) Date: 2021-09-16 02:13
Currently using *name* and *parent* args in Mock and MagicMock is problematic in a few ways:

*name*

 - any value can be passed silently but at a later time, any value except for an str will cause
   an exception when repr() or str() on the mock object is done.

 - a string name can be passed but will not be equal to the respective attr value:
   Mock(name='foo').name != 'foo', as users would expect. (this should be documented).

*parent*

 - any value can be passed but, similarly to *name*, will cause an exception when str() or
   repr() is done on the object.

 - this arg is not documented so users will expect it to be set as an attr, but instead the
   attribute is going to be a Mock instance. [1]


I propose to fix these issues by:

 - checking the types that are passed in and display a DeprecationWarning if types are wrong.
 (note that this check should be fast because at first value can be compared to None default,
 which is what it's going to be in vast majority of cases, and isinstance() check is only done
 after that.) (in 3.11)

 - in 3.12, convert warnings into TypeError.

 - Document that *name* attribute will be a Mock instance.

 - Document that *name* argument needs to be a string.

 - Document *parent* argument.

 - In the docs for the two args, point to `configure_mock()` method for setting them to
   arbitrary values.

(Note that other args for Mock() have more specialized names and are much less likely to cause
similar issues.)

[1] https://bugs.python.org/issue39222
msg401990 - (view) Author: Andrei Kulakov (andrei.avk) * (Python triager) Date: 2021-09-16 20:39
I forgot to include example of the breakage:

>>> m=Mock(name=1)
>>> m
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ak/opensource/cpython2/Lib/unittest/mock.py", line 735, in __repr__
    name = self._extract_mock_name()
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ak/opensource/cpython2/Lib/unittest/mock.py", line 732, in _extract_mock_name
    return ''.join(_name_list)
           ^^^^^^^^^^^^^^^^^^^
TypeError: sequence item 0: expected str instance, int found

>>> m=Mock(parent='foo')
>>> m
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ak/opensource/cpython2/Lib/unittest/mock.py", line 735, in __repr__
    name = self._extract_mock_name()
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ak/opensource/cpython2/Lib/unittest/mock.py", line 719, in _extract_mock_name
    _name_list.append(_parent._mock_new_name + dot)
                      ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute '_mock_new_name'
msg402369 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-09-21 21:39
Have you encountered misuse of Mock's `name` and `parent` in the wild? As you're saying, since `str()` and `repr()` will error out anyway, it's unlikely users were doing this in the first place.

I'm a little hesitant to accept the deprecation since there are very many places in the stdlib where we accept an equivalent of, say, `name=`  during some object's `__init__()` and just assign it to a field. If that happens not to be printable and crash `repr()` then too bad... but we won't be adding checks for each such case, right?
msg402404 - (view) Author: Andrei Kulakov (andrei.avk) * (Python triager) Date: 2021-09-22 02:09
Łukasz:

For parent, there's this issue: #39222 (which is why I started looking into this).

For name, I couldn't find anything here or on SO, but it's hard to search for this because keywords 'mock argument name' on SO finds over 700 results but they're referring to argument names, rather than literal argument `name`. I didn't look through all 700 though :)

The scenario that I was thinking about that could cause issues is that one developer creates a bunch of tests with Mock(name=1) for example, that doesn't cause any issues if the mocks aren't printed (for example if they're attributes). Some time later another dev is expanding or debugging the tests and needs to print the mocks, which causes errors. Now the 2nd dev will need to figure out why the first dev used these names and if they can be safely changed to strings - quite annoying.

It seems like the instances in the std lib of name args are of the form:

init(name)
 self.name = name

repr()
 return '<... %s ...>' % self.name

or equivalent; I've also did a grep for `return self.name` and it was a fairly short list so i was able to check all of them, and all of the hits either internal use classes or functions, or check the type of `name` in __init__, or the return is not from __repr__ but from some other function.

I didn't check all instances of where name arg is passed to init, because it's a much longer list and I figured the cases where it could be a problem would be ones with `__repr__(): return self.name`, and those are all fine.

The case with Mock is also different because users are not passing it as a known argument but as a "mock accepts all arguments and sets them as attrs" type of usage.

On the other hand, I don't think this is a high priority issue, users are probably not using name and parent args that often, and when they do, most of them probably try to print them as they work on the test; and if they do run into this issue later, it's not very hard to fix.

So it might be worth just leaving this open and seeing if more users run into it.

Let me know what you think!
History
Date User Action Args
2022-04-11 14:59:50adminsetgithub: 89378
2021-09-22 02:09:52andrei.avksetmessages: + msg402404
2021-09-21 21:39:28lukasz.langasetnosy: + lukasz.langa
messages: + msg402369
2021-09-16 20:39:22andrei.avksetmessages: + msg401990
2021-09-16 02:39:05andrei.avksetkeywords: + patch
stage: patch review
pull_requests: + pull_request26793
2021-09-16 02:13:10andrei.avkcreate