On 11/6/20 6:13 PM, Steven D'Aprano wrote:
For the benefit of others, the problem is that
`unittest.mock.call.__wrapped__` generates a new object, which in turn
has a dynamic `__wrapped__` attribute, which does the same, thus
generating an infinite chain of *distinct* proxies.
Being distinct proxy objects defeats the loop detection algorithm of
`inspect.unwrap`, which raises ValueError when the recursion limit is
reached, and that breaks doctest.
(I am explicitly stating that here because I spent an embarassingly long
time misunderstanding the nature of the bug, then more time digging into
the PR and bug track issues to understand what it was, rather than what
it isn't. Maybe I can save anyone else from my misunderstanding.)
I'm not convinced that this should be fixed by catching the ValueError
inside doctest. Or at least, not *just* by doing so. There's a deeper
problem that should be fixed, outside of doctest. Looking at the
similar issue here:
https://bugs.python.org/issue25532
`mock.call` has broken other functions in the past, and will probably
continue to do so in the future.
I don't think this infinite chain is intentional, I think it just
happens by accident, which makes this a bug in `call`. I think.
Michael Foord (creator of mock, if I recall correctly) suggested
blacklisting `__wrapped__` from the proxying:
https://bugs.python.org/issue25532#msg254726
which I think is the right solution, rather than touching doctest.
Michael also said he wasn't happy with an arbitrary limit on the depth
of proxies, but I would say that limiting the depth to
sys.getrecursionlimit() is not arbitrary and should avoid or at least
mitigate the risk of infinite loops and/or memory exhaustion in the
general case of arbitrary attribute lookups:
py> a = unittest.mock.call
py> for i in range(5): # for arbitrary large values of 5
... a = a.wibble
...
py> a
wibble.wibble.wibble.wibble.wibble
I'm not a mock expert, but I guess such mock dynamic lookups should be
limited to the recursion limit. Currently they will loop forever or
until you run out of memory.
Setting `call.__wrapped__` to None seems to directly fix the problem
with doctest:
[steve@susan test]$ cat demo2.py
"""
Run doctest on this module.
"""
from unittest.mock import call
call.__wrapped__ = None
[steve@susan test]$ python3.9 -m doctest -v demo2.py
1 items had no tests:
demo2
0 tests in 1 items.
0 passed and 0 failed.
Test passed.
but I don't know if that will break any uses of `mock.call`.
Another fix (untested) would be to explicitly test for mocked call:
inspect.unwrap(val, stop=lambda obj: isinstance(obj, unittest.mock._Call))
but I don't like that much.
I agree. It would be nice to fix mock.call, however that would leave a
gap in doctest which it can fail for strange objects that behave
similarly for no good reason.
Source: https://bugs.python.org/issue25532#msg273272
> Nick Coghlan (ncoghlan)
> unittest.mock.Mock is the only case of that in the standard library,
but it's far from the only Python mocking library out there, and we
should give a clear exception in such cases, rather than eating up all
the memory on the machine.
Doctest existed for a very long time ~2001 through 2014 without the
"vulnerable" (sorry, just calling it that) call to "unwrap" in place,
hence the unwrap itself is not really key to doctest working or not.
Adding unwrap without the check for unwrap failing is a bug on par with
the unwrap problem with mock.call itself.
Based on defensive coding and that doctest should not fail in difficult
to understand ways I think it makes sense to me to fix both mock.call
AND doctest, not only one.
-Alfred
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at
https://mail.python.org/archives/list/python-dev@python.org/message/T6ZS2YMOIAJIZCU443MQPEC2UHNY26W6/
Code of Conduct: http://python.org/psf/codeofconduct/
