Incorrect error handling in unittest.mock

[bpmcl]

BPO	40126
Nosy	@ncoghlan, @cjw296, @voidspace, @serhiy-storchaka, @tirkarthi, @bpmcl
PRs	[3.7] bpo-40126: Fix attribute out of scope during error handling in mock #19338 bpo-40126: Fix reverting multiple patches in unittest.mock. #19351 [3.8] bpo-40126: Fix reverting multiple patches in unittest.mock. (GH-19351) #19483 [3.7] bpo-40126: Fix reverting multiple patches in unittest.mock. (GH-19351) #19484

^{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 = None
created_at = <Date 2020-03-31.16:07:24.415>
labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
title = 'Incorrect error handling in unittest.mock'
updated_at = <Date 2020-04-12.11:57:36.107>
user = 'https://github.com/bpmcl'

bugs.python.org fields:

activity = <Date 2020-04-12.11:57:36.107>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-03-31.16:07:24.415>
creator = 'bmclarnon'
dependencies = []
files = []
hgrepos = []
issue_num = 40126
keywords = ['patch']
message_count = 8.0
messages = ['365395', '365707', '365728', '366195', '366222', '366238', '366239', '366241']
nosy_count = 6.0
nosy_names = ['ncoghlan', 'cjw296', 'michael.foord', 'serhiy.storchaka', 'xtreak', 'bmclarnon']
pr_nums = ['19338', '19351', '19483', '19484']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue40126'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[bpmcl]

The error handling in mock.decorate_callable (3.5-3.7) and mock.decoration_helper (3.8-3.9) is incorrectly implemented. If the error handler is triggered in the loop, the patching variable is out of scope and raises an unhandled UnboundLocalError instead.

This happened as a result of a 3rd-party library that attempts to clear the patchings list of a decorated function. The below code shows a recreation of the incorrect error handling:

import functools
from unittest import mock


def is_valid():
    return True


def mock_is_valid():
    return False


def decorate(f):
    @functools.wraps(f)
    def decorate_wrapper(*args, **kwargs):
        # This happens in a 3rd-party library
        f.patchings = []
        return f(*args, **kwargs)

    return decorate_wrapper


@decorate
@mock.patch('test.is_valid', new=mock_is_valid)
def test_patch():
    raise Exception()

[bpmcl]

After further investigation, it seems this was fixed in 436c2b0

However, this fix was as part of an unrelated changeset and in a different function in 3.8+, and was never rolled back to 3.7 and below. PR opened to add the missing attribute instantiation to 3.7.

[serhiy-storchaka]

I think that the current code is not correct. __exit__ should not be called if __enter__ is failed. If some __enter__ implementation calls other __enter__s it should manually call corresponding __exit__s.

[serhiy-storchaka]

New changeset 4b222c9 by Serhiy Storchaka in branch 'master':
bpo-40126: Fix reverting multiple patches in unittest.mock. (GH-19351)
4b222c9