bpo-35022: Add __fspath__ support to unittest.mock.MagicMock
#9960
Conversation
The `MagicMock` class supports many magic methods, but not `__fspath__`. To ease testing with modules such as `os.path`, this function is now supported by default.
|
@mariocj89: Would you mind to review this change? |
There was a problem hiding this comment.
LGTM. But I would prefer to have a second review, @mariocj89?
Lib/unittest/mock.py
Outdated
| @@ -1760,6 +1761,7 @@ def method(self, *args, **kw): | |||
| '__hash__': lambda self: object.__hash__(self), | |||
| '__str__': lambda self: object.__str__(self), | |||
| '__sizeof__': lambda self: object.__sizeof__(self), | |||
| '__fspath__': lambda self: "%s/%s" % (type(self).__name__, id(self)) | |||
There was a problem hiding this comment.
This will elide all naming information. See:
>>> os.fspath(m.absolute)
'MagicMock/140083495922656'
>>> str(m.absolute)
"<MagicMock name='mock.absolute' id='140083495922656'>"
Or when a mock is given a name. The id is unique per attribute and probably quite unuseful for the user when debugging issues.
Have you considered:
lambda self: f"{self._extract_mock_name()}/{id(self)}"
It would look nicer in some scenarios like:
>>> os.fspath(mock.MagicMock(**{"__fspath__.return_value": "abc"}).absolute)
'mock.absolute/140476946856592'
But maybe weird for some others like:
>>> os.fspath(mock.MagicMock() / 'other' / 'path')
'mock.__truediv__().__truediv__()/140476946879824'
There was a problem hiding this comment.
Agree that naming information should be preserved, what would you think of:
lambda self: f"{type(self).__name__}/{self._extract_mock_name()}/{id(self)}"
The __truediv__ case is a bit strange but it is correct, though.
|
@mariocj89: does it look better to you? |
|
Sure! I think it will be easier to debug. :) |
I mean: can you please review the updated PR? Like approving it if you consider that it now looks good to you? |
This is a naive implementation of what has been working for us internally.
https://bugs.python.org/issue35022