Opened 9 years ago
Closed 9 years ago
#6282 release blocker: regression closed fixed (fixed)
Trial tracebacks are not properly trimmed since 12.3.0
Reported by: | Ralph Meijer | Owned by: | therve |
---|---|---|---|
Priority: | normal | Milestone: | Python-3.x |
Component: | trial | Keywords: | |
Cc: | Jonathan Lange | Branch: |
branches/async-trim-frames-6282
branch-diff, diff-cov, branch-cov, buildbot |
Author: | therve |
Description
Due to the porting effort of trial to Python 3, tracebacks in the trial reporter are no longer properly trimming tracebacks:
from twisted.trial import unittest class Test(unittest.TestCase): def test_it(self): raise Exception()
yields:
[..] [ERROR] Traceback (most recent call last): File "/usr/lib/python2.7/dist-packages/twisted/internet/defer.py", line 138, in maybeDeferred result = f(*args, **kw) File "/usr/lib/python2.7/dist-packages/twisted/internet/_utilspy3.py", line 41, in runWithWarningsSuppressed reraise(exc_info[1], exc_info[2]) File "/usr/lib/python2.7/dist-packages/twisted/internet/_utilspy3.py", line 37, in runWithWarningsSuppressed result = f(*a, **kw) File "/home/ralphm/work/wokkel/trunk/q.py", line 6, in test_it raise Exception() exceptions.Exception: [..]
This is due to importing runWithWarningsSuppressed
from _utilspy3
instead of util
and an additional frame there. This makes the frames not being trimmed by Reporter._trimFrames
.
Change History (15)
comment:1 Changed 9 years ago by
Cc: | Jonathan Lange added |
---|
comment:2 Changed 9 years ago by
Owner: | set to therve |
---|
comment:3 Changed 9 years ago by
Author: | → therve |
---|---|
Branch: | → branches/async-trim-frames-6282 |
comment:4 Changed 9 years ago by
Keywords: | review added |
---|---|
Owner: | therve deleted |
Nice catch, fix ready to review.
comment:5 Changed 9 years ago by
Milestone: | → Python-3.x |
---|
comment:6 Changed 9 years ago by
Owner: | set to Jean-Paul Calderone |
---|---|
Status: | new → assigned |
comment:7 Changed 9 years ago by
Can you comment on why the branch changes the assertions made in test_hiddenException
? It seems like that's a general improvement (?) in behavior, rather than a fix for a regression.
comment:8 Changed 9 years ago by
Owner: | Jean-Paul Calderone deleted |
---|---|
Status: | assigned → new |
comment:9 Changed 9 years ago by
Sure: in r35861 which introduced the regression by creating utilspy3, the test was changed in a wrong way to comply to the new behavior, instead of fixing it.
comment:10 Changed 9 years ago by
Sure: in r35861 which introduced the regression by creating utilspy3, the test was changed in a wrong way to comply to the new behavior, instead of fixing it.
Crap. :( Thanks for catching and fixing that.
comment:11 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to therve |
- There are a number of missing docstrings.
TestAsynchronousFail
. (It would be nice to add them to the other methods ofTracebackHandling
that got touched as well. - The docstring for
test_exception
isn't clear. The test it is running is synchronous (just raises an exception). I think you mean a test usingtwisted.trial.unittest.TestCase
which *can* handle asynchronous tests. - The comment in
_trimFrames
needs to be updated and turned into a proper docstring. - What is going to happen in
_trimFrames
is the traceback doesn't look as expected? We should detect this and do something, as well as test this case. (Doing this may have caught the regression.
Please address the above issues and re-submit.
Note for archaeologists:
SynchronousTestCase
usestwisted.python.util.runWithWarningsSuppressed
(which doesn't support deferreds), andTestCase
usestwisted.internet.utils.runWithWarningsSuppresed
which does).
comment:12 Changed 9 years ago by
Keywords: | review added |
---|---|
Owner: | therve deleted |
I've fixed 1-3. I don't think we can do anything for 4, as there are too many different cases that can happen. We already had a test which ought to have caught the regression.
comment:13 Changed 9 years ago by
Owner: | set to therve |
---|
A couple of last things:
- As discussed on IRC, we should document that
_trimFrames
gets called with tracebacks other than the ones we expect (primarly when using testcase not from trial). _trimFrames
should useC{}
andL{}
for code and references, and have@param
and@return
getErrorFrames
should have@param
and@return
.
Please address the above issues and merge.
comment:14 Changed 9 years ago by
Keywords: | review removed |
---|
comment:15 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [36988]) Branching to 'async-trim-frames-6282'