Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reset_mock on mock created by mock_open causes infinite recursion #62822

Closed
voidspace opened this issue Aug 1, 2013 · 11 comments
Closed

reset_mock on mock created by mock_open causes infinite recursion #62822

voidspace opened this issue Aug 1, 2013 · 11 comments
Assignees
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@voidspace
Copy link
Contributor

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
    Copy link
    Contributor Author

    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 voidspace self-assigned this Aug 1, 2013
    @voidspace voidspace added stdlib Python modules in the Lib dir easy type-bug An unexpected behavior, bug, or error labels Aug 1, 2013
    @voidspace
    Copy link
    Contributor Author

    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
    Copy link
    Mannequin

    nicolapalumbo mannequin commented Sep 9, 2013

    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
    Copy link
    Mannequin

    LaurentDeBuyst mannequin commented Jan 13, 2014

    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
    Copy link
    Mannequin

    LaurentDeBuyst mannequin commented Jan 14, 2014

    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
    Copy link
    Mannequin

    nicolapalumbo mannequin commented Jan 14, 2014

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

    @LaurentDeBuyst
    Copy link
    Mannequin

    LaurentDeBuyst mannequin commented Jan 14, 2014

    And here's an actual patch with the corrected code

    @florentx
    Copy link
    Mannequin

    florentx mannequin commented May 25, 2014

    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
    Copy link
    Member

    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
    Copy link
    Mannequin

    python-dev mannequin commented Jul 14, 2015

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

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 14, 2015

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

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

    @rbtcollins rbtcollins assigned rbtcollins and unassigned voidspace Jul 15, 2015
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants