This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: contextlib.suppress not tested for nested usage
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: martin.panter Nosy List: Nan Wu, RazerM, berker.peksag, martin.panter, ncoghlan, python-dev, r.david.murray, rhettinger, yselivanov
Priority: normal Keywords: patch

Created on 2015-10-06 09:21 by RazerM, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fix_reentrant_cm_test.patch Nan Wu, 2015-10-07 20:53 review
Messages (11)
msg252382 - (view) Author: Frazer McLean (RazerM) * Date: 2015-10-06 09:21
In Lib/test/test_contextlib.py, there is a bug in the nested usage part of the following function:

def test_cm_is_reentrant(self): 
    ignore_exceptions = suppress(Exception) 
    with ignore_exceptions: 
        pass 
    with ignore_exceptions: 
        len(5) 
    with ignore_exceptions: 
        1/0 
        with ignore_exceptions: # Check nested usage 
            len(5)

Specifically, the final 2 lines aren't reached since the exception raised by 1/0 exits the context manager.
msg252390 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-06 13:04
Hah.  I predicted this thing was going to attract bug reports :)

The behavior you observe is how context managers in general and this one in particular work.  I'll let Nick explain it, since he wrote it.  Maybe there is an doc improvement that would help.
msg252391 - (view) Author: Frazer McLean (RazerM) * Date: 2015-10-06 13:08
I may not have been clear—I understand how context managers work with exceptions.

I'm just pointing out that the nested context manager isn't called, so those final two lines aren't testing anything, right?
msg252393 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-06 13:26
Oh, I didn't read your message carefully enough. But that makes this bug report even more ironic.
msg252438 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-06 23:21
My suggested fix would look like:

with ignore_exceptions:
    with ignore_exceptions:  # Check nested usage 
        len(5)
    ignored = True
    1/0
self.assertTrue(ignored)
msg252442 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-07 01:04
Looks good to me.  I hope I never see anything like that in real code, though :)
msg252448 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-10-07 02:58
That patch looks correct.
msg252484 - (view) Author: Nan Wu (Nan Wu) * Date: 2015-10-07 20:53
Added a patch with Martina's idea. Isn't ignored better set as false instead ? Since interpreter did not ignore it.
msg252704 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-10 10:43
Okay maybe “ignored” wasn’t the best choice of variable name. Perhaps I will write “outer_continued = True” instead.
msg252708 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-10 11:10
New changeset 452f76cbebdd by Martin Panter in branch '3.4':
Issue #25322: Fix test for nested contextlib.suppress
https://hg.python.org/cpython/rev/452f76cbebdd

New changeset 836ac579e179 by Martin Panter in branch '3.5':
Issue #25322: Merge contextlib.suppress test fix from 3.4 into 3.5
https://hg.python.org/cpython/rev/836ac579e179

New changeset c601496c2829 by Martin Panter in branch 'default':
Issue #25322: Merge contextlib.suppress test fix from 3.5
https://hg.python.org/cpython/rev/c601496c2829
msg252715 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-10 14:30
I had no problem understanding that ignored meant the exception had been ignored, but making it clearer certainly doesn't hurt.
History
Date User Action Args
2022-04-11 14:58:22adminsetgithub: 69509
2015-10-10 14:30:34r.david.murraysetmessages: + msg252715
2015-10-10 11:43:20martin.pantersetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2015-10-10 11:10:39python-devsetnosy: + python-dev
messages: + msg252708
2015-10-10 10:43:38martin.pantersetversions: + Python 3.6
nosy: + berker.peksag

messages: + msg252704

assignee: martin.panter
stage: commit review
2015-10-07 20:53:54Nan Wusetfiles: + fix_reentrant_cm_test.patch

nosy: + Nan Wu
messages: + msg252484

keywords: + patch
2015-10-07 02:58:47rhettingersetnosy: + rhettinger
messages: + msg252448
2015-10-07 01:05:00r.david.murraysetmessages: + msg252442
2015-10-06 23:21:43martin.pantersetnosy: + martin.panter
messages: + msg252438
2015-10-06 13:26:51r.david.murraysetmessages: + msg252393
2015-10-06 13:08:31RazerMsetmessages: + msg252391
2015-10-06 13:04:37r.david.murraysetnosy: + r.david.murray
messages: + msg252390
2015-10-06 09:21:46RazerMcreate