Issue18622
Created on 2013-08-01 21:04 by michael.foord, last changed 2015-07-15 00:08 by rbcollins. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| issue18622.patch | nicola.palumbo, 2013-09-09 09:58 | review | ||
| issue18622_2.patch | Laurent.De.Buyst, 2014-01-14 13:23 | review | ||
| Messages (11) | |||
|---|---|---|---|
| msg194119 - (view) | Author: Michael Foord (michael.foord) * | Date: 2013-08-01 21:04 | |
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() ... |
|||
| msg194166 - (view) | Author: Michael Foord (michael.foord) * | Date: 2013-08-02 08:59 | |
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. |
|||
| msg197357 - (view) | Author: Nicola Palumbo (nicola.palumbo) * | Date: 2013-09-09 09:58 | |
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'> |
|||
| msg208029 - (view) | Author: Laurent De Buyst (Laurent.De.Buyst) | Date: 2014-01-13 15:52 | |
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... |
|||
| msg208092 - (view) | Author: Laurent De Buyst (Laurent.De.Buyst) | Date: 2014-01-14 12:56 | |
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
|
|||
| msg208093 - (view) | Author: Nicola Palumbo (nicola.palumbo) * | Date: 2014-01-14 13:21 | |
I should have read more carefully, too! Thanks to both. |
|||
| msg208095 - (view) | Author: Laurent De Buyst (Laurent.De.Buyst) | Date: 2014-01-14 13:23 | |
And here's an actual patch with the corrected code |
|||
| msg219105 - (view) | Author: Florent Xicluna (flox) * | Date: 2014-05-25 21:26 | |
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 :-) |
|||
| msg246743 - (view) | Author: Robert Collins (rbcollins) * | Date: 2015-07-14 23:21 | |
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. |
|||
| msg246745 - (view) | Author: Roundup Robot (python-dev) | Date: 2015-07-14 23:42 | |
New changeset 4c8cb603ab42 by Robert Collins in branch '3.4': - Issue #18622: unittest.mock.mock_open().reset_mock would recurse infinitely. https://hg.python.org/cpython/rev/4c8cb603ab42 |
|||
| msg246746 - (view) | Author: Roundup Robot (python-dev) | Date: 2015-07-14 23:51 | |
New changeset c0ec61cf5a7d by Robert Collins in branch '3.5': - Issue #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 #18622: unittest.mock.mock_open().reset_mock would recurse infinitely. https://hg.python.org/cpython/rev/a4fe32477df6 |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2015-07-15 00:08:57 | rbcollins | set | status: open -> closed assignee: michael.foord -> rbcollins resolution: fixed |
| 2015-07-14 23:51:34 | python-dev | set | messages: + msg246746 |
| 2015-07-14 23:42:41 | python-dev | set | nosy:
+ python-dev messages: + msg246745 |
| 2015-07-14 23:21:13 | rbcollins | set | messages: + msg246743 |
| 2015-07-14 20:20:25 | rbcollins | set | nosy:
+ rbcollins versions: + Python 3.6 |
| 2014-05-25 21:26:32 | flox | set | nosy:
+ flox messages: + msg219105 |
| 2014-04-16 20:50:16 | michael.foord | set | nosy:
+ kushal.das versions: + Python 3.5, - Python 3.3 |
| 2014-02-01 09:22:23 | berker.peksag | set | nosy:
+ berker.peksag |
| 2014-01-14 13:23:25 | Laurent.De.Buyst | set | files:
+ issue18622_2.patch messages: + msg208095 |
| 2014-01-14 13:21:46 | nicola.palumbo | set | messages: + msg208093 |
| 2014-01-14 12:56:48 | Laurent.De.Buyst | set | messages: + msg208092 |
| 2014-01-13 15:52:23 | Laurent.De.Buyst | set | nosy:
+ Laurent.De.Buyst messages: + msg208029 |
| 2013-09-09 09:58:09 | nicola.palumbo | set | files:
+ issue18622.patch nosy:
+ nicola.palumbo keywords: + patch |
| 2013-08-02 08:59:19 | michael.foord | set | messages: + msg194166 |
| 2013-08-01 21:04:17 | michael.foord | create | |