bpo-24959: fix unittest.assertRaises bug where traceback entries are dropped from chained exceptions

[iritkatriel]

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]

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

[iritkatriel]

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]

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

[iritkatriel]

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.

[iritkatriel]

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]

Ah I've rebased against main as well.

[serhiy-storchaka]

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]

See testStackFrameTrimming in test_result.

[iritkatriel]

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

[yashk2810]

Hello,

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

[serhiy-storchaka]

LGTM!

[miss-islington]

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

[miss-islington]

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]

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

[iritkatriel]

Thank you @serhiy-storchaka !

[bedevere-bot]

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

[yashk2810]

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

[iritkatriel]

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