[issue36366] Patcher stop method should be idempotent

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan <tir.karthi at gmail.com> added the comment:

When mock.patch is creates a patch object and patch.start calls __enter__ that sets is_local. On stop __exit__ is called where a check is done is to make sure is_local attribute is present and then cleanup is done along with deleting calling del self.is_local so calling stop second time causes the attribute check to fail. There is no specific reason I could find with git history. 

It seems that calling patch.stop without patch.start makes cleanup to happen on unpatched objects and raises errors which I hope is avoided by always setting is_local on start and removing it on stop like a flag. That being said I am not sure why a early return couldn't be made when is_local is absent instead of proceeding with cleanup logic or raising a runtime error. I see no tests failing on early return except a test where RuntimeError is intentionally tested by calling stop on unstarted patch. I have added mock module devs for some context.

A sample script would be as below : 

from unittest import mock

class Foo:
    bar = None

patch = mock.patch.object(Foo, 'bar', 'x')
patch.start()
patch.stop()
patch.stop()

----------
nosy: +xtreak
type:  -> behavior
versions: +Python 3.8

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue36366>
_______________________________________