reset_mock on mock created by mock_open causes infinite recursion

[voidspace]

BPO	18622
Nosy	@rbtcollins, @voidspace, @florentx, @berkerpeksag, @kushaldas
Files	issue18622.patch issue18622_2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/rbtcollins'
closed_at = <Date 2015-07-15.00:08:57.714>
created_at = <Date 2013-08-01.21:04:17.258>
labels = ['easy', 'type-bug', 'library']
title = 'reset_mock on mock created by mock_open causes infinite recursion'
updated_at = <Date 2015-07-15.00:08:57.714>
user = 'https://github.com/voidspace'

bugs.python.org fields:

activity = <Date 2015-07-15.00:08:57.714>
actor = 'rbcollins'
assignee = 'rbcollins'
closed = True
closed_date = <Date 2015-07-15.00:08:57.714>
closer = 'rbcollins'
components = ['Library (Lib)']
creation = <Date 2013-08-01.21:04:17.258>
creator = 'michael.foord'
dependencies = []
files = ['31694', '33463']
hgrepos = []
issue_num = 18622
keywords = ['patch', 'easy']
message_count = 11.0
messages = ['194119', '194166', '197357', '208029', '208092', '208093', '208095', '219105', '246743', '246745', '246746']
nosy_count = 8.0
nosy_names = ['rbcollins', 'michael.foord', 'flox', 'python-dev', 'berker.peksag', 'kushal.das', 'nicola.palumbo', 'Laurent.De.Buyst']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue18622'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

[voidspace]

As reported at: http://code.google.com/p/mock/issues/detail?id=209

>>> from unittest import mock
[107971 refs]
>>> mock.mock_open
<function mock_open at 0x10cff9d20>
[107974 refs]
>>> a = mock.mock_open()
[109965 refs]
>>> a.reset_mock()
...

[voidspace]

The best way to solve this seems to be to track a set of visited ids (mocks we've reset) and not recurse into mocks we've already done. This is similar to the patch proposed on the google code issue - but not identical as that uses a list and has it as the default argument to reset_mock.

[nicolapalumbo]

Hi all,

I've fixed the infinite recursion in reset_mock(). It has been solved by tracking a set of visited ids as suggested.

>>> from unittest import mock
>>> a = mock.mock_open()
>>> a.reset_mock()
>>> a
<MagicMock name='open' spec='builtin_function_or_method' id='4449688192'>

[LaurentDeBuyst]

The proposed patch does solve the infinite recursion bug, but a different problem appears when resetting the same mock multiple times: it only works the first time.

Using the patch as it stands:

>>> from unittest.mock import mock_open
>>> mo = mock_open()
>>> a = mo()
>>> mo.call_count
1
>>> mo.reset_mock()
>>> mo.call_count
0
>>> b = mo()
>>> mo.call_count
1
>>> mo.reset_mock()
>>> mo.call_count
1

And here from a version with an added print(visited) statement:

>>> from unittest.mock import mock_open
>>> mo = mock_open()
>>> a = mo()
>>> mo.call_count
1
>>> mo.reset_mock()
[]
[139803191795152]
[139803191795152, 139803181189008]
[139803191795152, 139803181189008, 139803213598416]
[139803191795152, 139803181189008, 139803213598416, 139803213652048]
[139803191795152, 139803181189008, 139803213598416, 139803213652048]
>>> mo.call_count
0
>>> b = mo()
>>> mo.call_count
1
>>> mo.reset_mock()
[139803191795152, 139803181189008, 139803213598416, 139803213652048, 139803213598288]
>>> mo.call_count
1
>>> mo.reset_mock(visited=[])
[]
[139803191795152]
[139803191795152, 139803181189008]
[139803191795152, 139803181189008, 139803213598416]
[139803191795152, 139803181189008, 139803213598416, 139803213652048]
[139803191795152, 139803181189008, 139803213598416, 139803213652048]
>>> mo.call_count
0

As you can see, for some reason I don't quite grasp, the 'visited' parameter persists across calls to reset_mock(), meaning that the very first call does indeed reset it but subsequent calls do not.

As the last two calls show, one can force a reset by explicitly providing an empty list, but this is starting to become a change in API and not just a bugfix...

[LaurentDeBuyst]

Sorry Michael, I should have read your second comment more closely since you already pointed out that using a list as default argument is bad.

It is, however, easily fixed by changing to this:

def reset_mock(self, visited=None):
    "Restore the mock object to its initial state."
    if visited is None:
        visited = []
    if id(self) in visited:
        return

[nicolapalumbo]

I should have read more carefully, too! Thanks to both.

[LaurentDeBuyst]

And here's an actual patch with the corrected code

[florentx]

I've been bitten by this issue with a custom psycopg2 mock.

>>> cur = mock.Mock()
>>> 
>>> cur.connection.cursor.return_value = cur
>>> cur.reset_mock()
RuntimeError: maximum recursion depth exceeded

the patch looks ok, except the mix of tab and spaces :-)

[rbtcollins]

Applying this to 3.4 and up with a test.

Laurent, it would be good to sign the CLA - since your change here is minimal and Nicola has, I'm just going ahead.

[python-dev]

New changeset 4c8cb603ab42 by Robert Collins in branch '3.4':

• Issue bpo-18622: unittest.mock.mock_open().reset_mock would recurse infinitely.
  https://hg.python.org/cpython/rev/4c8cb603ab42

[python-dev]

New changeset c0ec61cf5a7d by Robert Collins in branch '3.5':

• Issue bpo-18622: unittest.mock.mock_open().reset_mock would recurse infinitely.
  https://hg.python.org/cpython/rev/c0ec61cf5a7d

New changeset a4fe32477df6 by Robert Collins in branch 'default':

• Issue bpo-18622: unittest.mock.mock_open().reset_mock would recurse infinitely.
  https://hg.python.org/cpython/rev/a4fe32477df6