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

[hmvp]

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]

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

[tirkarthi]

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

cpython/Lib/unittest/mock.py

Line 737 in 73820a6

self._mock_children[name] = _deleted

. But when reset_mock is called it doesn't check for the deleted flag at

cpython/Lib/unittest/mock.py

Line 543 in 73820a6

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]

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

[eli-b]

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]

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

[miss-islington]

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]

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

[tirkarthi]

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.