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.

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

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 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