Skip to content

reset_mock resets MagicMock's magic methods in an unexpected way #123934

Description

@mentalAdventurer

Bug report

Bug description:

The reset_mock(return_value=True) method behaves in a wrong/inconsistent way.
When used with MagicMock, the method reset_mock(return_value=True) does not reset the return values of the magic methods. Only if you call for example __str__ and then call the reset_mock function, the return value will be reset, but not to the default value.

from unittest import mock

mm = mock.MagicMock()
print(type(mm.__str__()))
mm.reset_mock(return_value=True)
print(type(mm.__str__()))
print(type(mm.__hash__()))
mm.reset_mock(return_value=True)
print(type(mm.__hash__()))

Output

<class 'str'>
<class 'unittest.mock.MagicMock'>
<class 'int'>
<class 'unittest.mock.MagicMock'>

Since Python 3.9 PR reset_mock now also resets child mocks. This explains the behaviour. Calling the __str__ method creates a child MagicMock with a set return value. Since this child mock now exists, its return value is reset when reset_mock(return_value=True) is called.
Although this can be logically explained, it's counter-intuitive and annoying as I'm never sure which values are being reset.

I would expect the same behaviour as Mock. The return value of __str__ and other magic methods should not be effected.

from unittest import mock

m = mock.Mock()
print(type(m.__str__()))
m.reset_mock(return_value=True)
print(type(m.__str__()))
print(type(m.__hash__()))
m.reset_mock(return_value=True)
print(type(m.__hash__()))

Output

<class 'str'>
<class 'str'>
<class 'int'>
<class 'int'>

CPython versions tested on:

3.10

Operating systems tested on:

Linux

Linked PRs

Activity

  1. changed the title [-]reset_mock resets MagicMock's magic methods in an unexpected way[/-] [+]`reset_mock` resets `MagicMock`'s magic methods in an unexpected way[/+] on Sep 11, 2024
  2. self-assigned this
    on Sep 11, 2024
  3. sobolevn commented on Sep 11, 2024

    @sobolevn
    Member

    Thank you for the report. I will take a look soon :)

  4. added
    stdlibStandard Library Python modules in the Lib/ directory
    on Sep 11, 2024
  5. added a commit that references this issue on Sep 13, 2024
  6. sobolevn commented on Sep 13, 2024

    @sobolevn
    Member

    Since this child mock now exists, its return value is reset when reset_mock(return_value=True) is called.

    Yes, this is correct.

    I would expect the same behaviour as Mock. The return value of str and other magic methods should not be effected.

    Mock does not mock magic methods at all, so they are unaffected at any case. Compare:

    print(type(mock.Mock().__str__))
    # <class 'method-wrapper'>
    
    print(type(mock.MagicMock().__str__))
    # <class 'unittest.mock.MagicMock'>

    But, I agree that retuning <class 'unittest.mock.MagicMock'> from __str__ is not correct.
    So, my proposed solution is to ignore cases when:

    1. child is a MagicMock instance
    2. key is a magic method
    3. return_value is True
  7. moved this from Todo to In Progress in Unittest & doctest issueson Sep 13, 2024
  8. added a commit that references this issue on Sep 19, 2024
  9. added 2 commits that reference this issue on Sep 19, 2024
  10. added a commit that references this issue on Sep 19, 2024
  11. added a commit that references this issue on Sep 22, 2024
  12. added a commit that references this issue on Sep 30, 2024
  13. hroncok commented on Nov 18, 2024

    @hroncok
    Contributor

    I see that this test in mu-editor fails with infinite RecursionError:

    https://github.com/mu-editor/mu/blob/7641d7ada71a986f895ebd8a26d2ee2f52c3f1fe/tests/modes/test_python3.py#L486-L490

        with mock.patch("builtins.super") as mock_super:
            pm = PythonMode(editor, view)
            pm.set_buttons = mock.MagicMock()
            mock_super.reset_mock()

    I believe this is because it is calling reset_mock on a mock of super and after this PR reset_mock calls super.

    I think that mocking super is probably dangerous (and always has been), but I thought I should mention this here.

  14. sobolevn commented on Nov 18, 2024

    @sobolevn
    Member

    Hm, I think that we can not use super() for this feature technically. But, I think that mocking super() is dangerous indeed.

    What do others think? Should we fix this?

  15. hroncok commented on Nov 18, 2024

    @hroncok
    Contributor

    FWIW this particular use case was simple enough to fix: mu-editor/mu#2528

  16. hugovk commented on Feb 5, 2025

    @hugovk
    Member

    Triage: PR merged and backported.

    The output on 3.12 and 3.14 is now as expected:

    <class 'str'>
    <class 'str'>
    <class 'int'>
    <class 'int'>

    Please re-open if there's more to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions