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

unittest mock's reset_mock throws an error when an attribute has been deleted #75360

Closed
hmvp mannequin opened this issue Aug 10, 2017 · 8 comments
Closed

unittest mock's reset_mock throws an error when an attribute has been deleted #75360

hmvp mannequin opened this issue Aug 10, 2017 · 8 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@hmvp
Copy link
Mannequin

hmvp mannequin commented Aug 10, 2017

BPO 31177
Nosy @vstinner, @cjw296, @voidspace, @eli-b, @mariocj89, @miss-islington, @tirkarthi
PRs
  • bpo-31177: Skip deleted attributes while calling reset_mock #9302
  • bpo-31177: Fix reset_mock on mock with deleted children #10807
  • bpo-31177: Fix reset_mock on mock with deleted children #10836
  • [3.6] bpo-31177: Skip deleted attributes while calling reset_mock (GH-9302) #10842
  • [3.7] bpo-31177: Skip deleted attributes while calling reset_mock (GH-9302) #10843
  • 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 = None
    closed_at = <Date 2018-12-01.17:58:42.475>
    created_at = <Date 2017-08-10.14:22:19.659>
    labels = ['3.7', '3.8', 'type-bug', 'library']
    title = "unittest mock's reset_mock throws an error when an attribute has been deleted"
    updated_at = <Date 2018-12-01.17:58:42.473>
    user = 'https://bugs.python.org/hmvp'

    bugs.python.org fields:

    activity = <Date 2018-12-01.17:58:42.473>
    actor = 'xtreak'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-12-01.17:58:42.475>
    closer = 'xtreak'
    components = ['Library (Lib)']
    creation = <Date 2017-08-10.14:22:19.659>
    creator = 'hmvp'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31177
    keywords = ['patch']
    message_count = 8.0
    messages = ['300090', '325340', '326322', '330740', '330846', '330847', '330848', '330857']
    nosy_count = 8.0
    nosy_names = ['vstinner', 'cjw296', 'michael.foord', 'Eli_B', 'mariocj89', 'hmvp', 'miss-islington', 'xtreak']
    pr_nums = ['9302', '10807', '10836', '10842', '10843']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue31177'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @hmvp
    Copy link
    Mannequin Author

    hmvp mannequin commented Aug 10, 2017

    When using a mock and deleting a attribute reset_mock cannot be used anymore since it tries to call reset_mock on the _deleted sentinel value.

    Reproduction path:

    from unittest.mock import MagicMock
    mock = MagicMock()
    mock.a = 'test'
    del mock.a
    mock.reset_mock()
    

    Gives:

    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.5/unittest/mock.py", line 544, in reset_mock
        child.reset_mock(visited)
    AttributeError: '_SentinelObject' object has no attribute 'reset_mock'
    

    Expected result:
    mock is reset without throwing an exception and the 'a' attribute is no longer in a deleted state

    Only checked 3.5 and current master if bug is present

    @hmvp hmvp mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Aug 10, 2017
    @tirkarthi
    Copy link
    Member

    Can confirm this behavior on CPython master as well. It seems that when an attribute is deleted then a deleted flag is set for the attribute at

    self._mock_children[name] = _deleted
    . But when reset_mock is called it doesn't check for the deleted flag at
    for child in self._mock_children.values():

    ➜  cpython git:(master) ./python.exe ../backups/bpo31177.py
    Traceback (most recent call last):
      File "../backups/bpo31177.py", line 5, in <module>
        m.reset_mock()
      File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 546, in reset_mock
        child.reset_mock(visited)
    AttributeError: '_SentinelObject' object has no attribute 'reset_mock'

    A simple patch would be to skip the deleted as below but some of the code in mock module raise an AttributeError. I don't know the correct behavior here. But applying the below patch and running tests with ./python.exe [Lib/unittest/test/](https://github.com/python/cpython/blob/main/Lib/unittest/test) doesn't cause any test failure.

    diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
    index db1e642c00..700e2fb8b9 100644
    --- a/Lib/unittest/mock.py
    +++ b/Lib/unittest/mock.py
    @@ -541,7 +541,7 @@ class NonCallableMock(Base):
                 self._mock_side_effect = None
     
             for child in self._mock_children.values():
    -            if isinstance(child, _SpecState):
    +            if isinstance(child, _SpecState) or child is _deleted:
                     continue
                 child.reset_mock(visited)
     
    I will try to make a PR if it's ok.

    Thanks

    @tirkarthi
    Copy link
    Member

    Adding Michael for thoughts on the fix and desired behavior. Removing 3.5 since only security fixes are accepted and adding 3.8 which is also affected.

    Thanks

    @tirkarthi tirkarthi added the 3.8 only security fixes label Sep 25, 2018
    @eli-b
    Copy link
    Mannequin

    eli-b mannequin commented Nov 30, 2018

    I suggest that after reset_mock(), deleted attributes should be available again. In other words, their deletion is reset.

    I'm opening a PR to this effect.

    I reported this issue to testing-cabal's mock repo in May 2016 (testing-cabal/mock#361), but my original PR there just avoided an exception without reinstating the deleted attribute.

    @vstinner
    Copy link
    Member

    vstinner commented Dec 1, 2018

    New changeset edeca92 by Victor Stinner (Xtreak) in branch 'master':
    bpo-31177: Skip deleted attributes while calling reset_mock (GH-9302)
    edeca92

    @miss-islington
    Copy link
    Contributor

    New changeset c0566e0 by Miss Islington (bot) in branch '3.6':
    bpo-31177: Skip deleted attributes while calling reset_mock (GH-9302)
    c0566e0

    @miss-islington
    Copy link
    Contributor

    New changeset 422c165 by Miss Islington (bot) in branch '3.7':
    bpo-31177: Skip deleted attributes while calling reset_mock (GH-9302)
    422c165

    @tirkarthi
    Copy link
    Member

    I am closing this as fixed since all the PRs were merged. Feel free to reopen this if needed. Thanks @mariocj89 and @vstinner for the review.

    @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
    3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants