bpo-24959: fix unittest.assertRaises bug where traceback entries are dropped from chained exceptions by iritkatriel · Pull Request #23688 · python/cpython

iritkatriel · 2020-12-08T01:43:13Z

Unittest applies a hack that uses the limit parameter of TracebackException to hide traceback frames from the unittest code itself. The problem with this is that the limit applies to the chained (context) exception too, so we lose frames we want to keep.

This patch replaces the hack by code that modifies the traceback to remove the frames we don't want.

https://bugs.python.org/issue24959

github-actions · 2021-01-23T01:00:52Z

This PR is stale because it has been open for 30 days with no activity.

iritkatriel · 2021-04-09T13:28:34Z

See also issue24959

serhiy-storchaka

LGTM in general.

I think that tracebacks in __context__ (and maybe __cause__) chains should be cleaned recursively too. For the case like:

try:
    self.assertFoo()
finally:
    self.assertBar()

serhiy-storchaka · 2021-09-05T20:58:13Z

Is it possible to write tests which do not rely on private methods?

iritkatriel · 2021-09-05T21:04:34Z

Thanks. I've marked this as draft because I wanted consider a different approach to this.

We recently had some changes in the traceback module that allow you to customize how frames are rendered, and also to exclude frames from the output. I think it might be better to use that approach here. I'll make another PR soon.

… dropped from the chained exception

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

iritkatriel · 2021-09-06T10:57:01Z

I rewrote the test to not rely on private methods.

The traceback utility we added recently doesn't help because it allows to filter FrameSummary objects (not frames) so we no longer have access to the globals.

Regarding filtering chained tracebacks - I think this is another problem. The current filtering assumes that the traceback starts in user code. With chained tracebacks I'm not sure it is (in the contrived test I tried this wasn't the case). I think we should define the use case and desired behaviour before implementing.

iritkatriel · 2021-09-06T10:58:17Z

Ah I've rebased against main as well.

serhiy-storchaka · 2021-09-06T12:50:31Z

The use case is:

prepare()
try:
    test()
finally:
    cleanup()

We want to acquire some resource or make some environment changes for the part of the test and release the resource or restore environment after testing, so we use the try/finally block. Now, if an assertion failed inside the block, and an error occurred in the clean up code, the new exception will override the original AssertionError which is now accessible via the __context__ link. The traceback output will contain lines from the unittest code. Note that tracebecks for __context__ should be cleaned up not only if exctype is test.failureException, but if type(value.__context__) is test.failureException, type(value.__context__.__context__) is test.failureException, etc. But it may be a different issue.

I do not know the use case for __cause__, but I would clean up them too.

serhiy-storchaka

LGTM, but I'll try to write more tests. Surprisingly, there are no tests for removing unittest frames at all.

iritkatriel · 2021-09-06T13:54:14Z

See testStackFrameTrimming in test_result.

iritkatriel · 2021-09-06T15:43:59Z

I added the chained exceptions. I avoided recursion because it can be just a bit too deep for certain constructs involving recursion.

yashk2810 · 2022-02-25T16:47:41Z

Hello,

Any updates on the PR? Is it possible to get this in soon? :)

serhiy-storchaka

LGTM!

miss-islington · 2022-03-08T21:43:52Z

Thanks @iritkatriel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

miss-islington · 2022-03-08T21:44:01Z

Sorry, @iritkatriel, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 88b7d86a73da9388aa65c96401c2984c8c16f8db 3.9

bedevere-bot · 2022-03-08T21:44:04Z

GH-31775 is a backport of this pull request to the 3.10 branch.

iritkatriel · 2022-03-08T21:44:11Z

Thank you @serhiy-storchaka !

…dropped from chained exceptions (pythonGH-23688) (cherry picked from commit 88b7d86) Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

…dropped from chained exceptions (GH-23688) (cherry picked from commit 88b7d86)

bedevere-bot · 2022-03-08T21:56:39Z

GH-31776 is a backport of this pull request to the 3.9 branch.

…dropped from chained exceptions (GH-23688) (cherry picked from commit 88b7d86) Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

…dropped from chained exceptions (GH-23688) (GH-31776) (cherry picked from commit 88b7d86)

yashk2810 · 2022-03-09T01:25:07Z

Just to clarify: Will this work with python 3.7 or it would need to be backported?

iritkatriel · 2022-03-09T07:04:51Z

It will not be backport Ed to 3.7.
https://devguide.python.org/devcycle/

…dropped from chained exceptions (pythonGH-23688) (pythonGH-31776) (cherry picked from commit 88b7d86)

the-knights-who-say-ni added the CLA signed label Dec 8, 2020

bedevere-bot added the awaiting review label Dec 8, 2020

iritkatriel changed the title ~~bpo-25573: fix unittest.assertRaises bug where traceback entries were…~~ bpo-42247: fix unittest.assertRaises bug where traceback entries were… Dec 8, 2020

iritkatriel force-pushed the bpo-42247 branch from 10ace29 to 3a01233 Compare December 8, 2020 01:50

iritkatriel requested a review from voidspace December 8, 2020 10:04

iritkatriel added the type-bug An unexpected behavior, bug, or error label Dec 23, 2020

github-actions Bot added the stale Stale PR or inactive for long period of time. label Jan 23, 2021

iritkatriel marked this pull request as draft July 3, 2021 16:33

serhiy-storchaka added needs backport to 3.9 needs backport to 3.10 only security fixes and removed stale Stale PR or inactive for long period of time. labels Sep 5, 2021

serhiy-storchaka reviewed Sep 5, 2021

View reviewed changes

Comment thread Lib/unittest/test/test_result.py Outdated

Comment thread Lib/unittest/test/test_result.py Outdated

Comment thread Lib/unittest/test/test_result.py Outdated

iritkatriel and others added 5 commits September 5, 2021 22:43

bpo-42247: fix unittest.assertRaises bug where traceback entries were…

f642752

… dropped from the chained exception

Update Lib/unittest/test/test_result.py

e0c8019

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Update Lib/unittest/test/test_result.py

6bca38a

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Update Lib/unittest/test/test_result.py

b60a829

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

rewrite test

7274a04

iritkatriel force-pushed the bpo-42247 branch from 9c9acf5 to 7274a04 Compare September 6, 2021 10:54

iritkatriel marked this pull request as ready for review September 6, 2021 10:57

iritkatriel changed the title ~~bpo-42247: fix unittest.assertRaises bug where traceback entries were…~~ bpo-42959: fix unittest.assertRaises bug where traceback entries were… Sep 6, 2021

iritkatriel changed the title ~~bpo-42959: fix unittest.assertRaises bug where traceback entries were…~~ bpo-24959: fix unittest.assertRaises bug where traceback entries were… Sep 6, 2021

serhiy-storchaka reviewed Sep 6, 2021

View reviewed changes

clean tracebacks for chained exceptions as well

d067f50

📜🤖 Added by blurb_it.

4ae0bf0

serhiy-storchaka approved these changes Mar 5, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Mar 5, 2022

iritkatriel and others added 2 commits March 8, 2022 20:31

Merge remote-tracking branch 'upstream/main' into bpo-42247

d51e954

newline

9290967

iritkatriel changed the title ~~bpo-24959: fix unittest.assertRaises bug where traceback entries were…~~ bpo-24959: fix unittest.assertRaises bug where traceback entries are dropped from chained exceptions Mar 8, 2022

iritkatriel merged commit 88b7d86 into python:main Mar 8, 2022

bedevere-bot removed the awaiting merge label Mar 8, 2022

miss-islington assigned iritkatriel Mar 8, 2022

bedevere-bot removed the needs backport to 3.10 only security fixes label Mar 8, 2022

iritkatriel added a commit that referenced this pull request Mar 8, 2022

bpo-24959: fix unittest.assertRaises bug where traceback entries are …

ca229a5

…dropped from chained exceptions (GH-23688) (cherry picked from commit 88b7d86)

bedevere-bot removed the needs backport to 3.9 label Mar 8, 2022

iritkatriel added a commit that referenced this pull request Mar 8, 2022

bpo-24959: fix unittest.assertRaises bug where traceback entries are …

f3ea249

…dropped from chained exceptions (GH-23688) (GH-31776) (cherry picked from commit 88b7d86)

hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022

bpo-24959: fix unittest.assertRaises bug where traceback entries are …

cfbf720

…dropped from chained exceptions (pythonGH-23688) (pythonGH-31776) (cherry picked from commit 88b7d86)

iritkatriel deleted the bpo-42247 branch October 18, 2022 14:25

Uh oh!

Conversation

iritkatriel commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jan 23, 2021

Uh oh!

iritkatriel commented Apr 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka commented Sep 5, 2021

Uh oh!

iritkatriel commented Sep 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iritkatriel commented Sep 6, 2021

Uh oh!

iritkatriel commented Sep 6, 2021

Uh oh!

serhiy-storchaka commented Sep 6, 2021

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

iritkatriel commented Sep 6, 2021

Uh oh!

iritkatriel commented Sep 6, 2021

Uh oh!

yashk2810 commented Feb 25, 2022

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Mar 8, 2022

Uh oh!

miss-islington commented Mar 8, 2022

Uh oh!

bedevere-bot commented Mar 8, 2022

Uh oh!

iritkatriel commented Mar 8, 2022

Uh oh!

bedevere-bot commented Mar 8, 2022

Uh oh!

yashk2810 commented Mar 9, 2022

Uh oh!

iritkatriel commented Mar 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

iritkatriel commented Dec 8, 2020 •

edited

Loading

iritkatriel commented Apr 9, 2021 •

edited

Loading

iritkatriel commented Sep 5, 2021 •

edited

Loading