
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2019-03-19 15:57 bysfreilich, last changed2022-04-11 14:59 byadmin. This issue is nowclosed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 12472 | merged | xtreak,2019-03-20 12:18 | |
| Messages (7) | |||
|---|---|---|---|
| msg338371 -(view) | Author: Samuel Freilich (sfreilich)* | Date: 2019-03-19 15:57 | |
Currently, it's an error to call the stop() method of a patcher object created by mock.patch repeatedly:>>> patch = mock.patch.object(Foo, 'BAR', 'x')>>> patch.start()'x'>>> patch.stop()>>> patch.stop()RuntimeError: stop called on unstarted patcherThis causes unnecessary problems when test classes using mock.patch.stopall for cleanup are mixed with those cleaning up patches individually:class TestA(unittest.TestCase): def setUp(): patcher = mock.patch.object(...) self.mock_a = patcher.start() self.addCleanup(patcher.stop) ...class TestB(TestA): def setUp(): super().setUp() self.addCleanup(mock.patch.stopall) self.mock_b = mock.patch.object(...).start() ...This fails because mock.patch.stopall stops the patch set up in TestA.setUp(), then that raises an exception when it's stopped again.But why does patcher.stop() enforce that precondition? Wouldn't it be sufficient for it to just enforce the postcondition, that after stop() is called the patch is stopped()? That would make it easier to write test code which makes use of mock.patch.stopall, which allows the proper cleanup of patches to be configured much more concisely. | |||
| msg338372 -(view) | Author: Stéphane Wirtel (matrixise)*![]() | Date: 2019-03-19 15:59 | |
@Samuel,1. What's the version of Python?2. Could you provide a script with an example?Thank you | |||
| msg338379 -(view) | Author: Karthikeyan Singaravelan (xtreak)*![]() | Date: 2019-03-19 16:32 | |
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 mockclass Foo: bar = Nonepatch = mock.patch.object(Foo, 'bar', 'x')patch.start()patch.stop()patch.stop() | |||
| msg338395 -(view) | Author: Michael Foord (michael.foord)*![]() | Date: 2019-03-19 18:16 | |
It's almost certainly an oversight rather than a design decision. I'd be happy with the change you suggest Karthikeyan. | |||
| msg338436 -(view) | Author: Karthikeyan Singaravelan (xtreak)*![]() | Date: 2019-03-20 02:45 | |
Thanks for the confirmation. I will raise a PR for this. | |||
| msg339075 -(view) | Author: miss-islington (miss-islington) | Date: 2019-03-28 21:08 | |
New changeset02b84cb1b4f5407309c81c8b1ae0397355d6e568 by Miss Islington (bot) (Xtreak) in branch 'master':bpo-36366: Return None on stopping unstarted patch object (GH-12472)https://github.com/python/cpython/commit/02b84cb1b4f5407309c81c8b1ae0397355d6e568 | |||
| msg339076 -(view) | Author: Brett Cannon (brett.cannon)*![]() | Date: 2019-03-28 21:10 | |
Thanks, everyone! | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:12 | admin | set | github: 80547 |
| 2019-03-28 21:10:27 | brett.cannon | set | status: open -> closed nosy: +brett.cannon messages: +msg339076 resolution: fixed stage: patch review -> resolved |
| 2019-03-28 21:08:47 | miss-islington | set | nosy: +miss-islington messages: +msg339075 |
| 2019-03-20 12:18:12 | xtreak | set | keywords: +patch stage: patch review pull_requests: +pull_request12424 |
| 2019-03-20 02:45:01 | xtreak | set | messages: +msg338436 |
| 2019-03-19 18:16:53 | michael.foord | set | messages: +msg338395 |
| 2019-03-19 16:32:39 | xtreak | set | versions: + Python 3.8 nosy: +xtreak messages: +msg338379 type: behavior |
| 2019-03-19 16:08:15 | xtreak | set | nosy: +cjw296,michael.foord,mariocj89 |
| 2019-03-19 15:59:39 | matrixise | set | nosy: +matrixise messages: +msg338372 |
| 2019-03-19 15:57:41 | sfreilich | create | |