testtools should pass the real exc_info to TestResult methods

Bug #501169 reported by Martin Packman
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
testtools
Triaged
Wishlist
Unassigned

Bug Description

TestResult methods designed for overriding such as addFailure and report_failure get a (nearly useless) _StringException instance and no traceback object from the err parameter, rather than the actual exc_info from the test. Have not been able to work out what exactly is going on here yet, but it seems the current behaviour has nothing to recommend it.

Revision history for this message
Martin Packman (gz) wrote :
Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 501169] [NEW] testtools should pass the real exc_info to TestResult methods

On Mon, 2009-12-28 at 23:50 +0000, Martin [gz] wrote:
> Public bug reported:
>
> TestResult methods designed for overriding such as addFailure and
> report_failure get a (nearly useless) _StringException instance and no
> traceback object from the err parameter, rather than the actual exc_info
> from the test. Have not been able to work out what exactly is going on
> here yet, but it seems the current behaviour has nothing to recommend
> it.

There is no requirement that the exc info get an AssertionError: the
exc_info protocol is fairly well understood, and the standard
stringification logic in the standard library works on the objects
passed in.

report_failure is a bzrlibism - note that bzrlib is now using the
extended details API that testtools offers: the 'real exc_info' is
discarded many stages earlier and the exception is being reconstitued
Just-In-Time for whatever down-level reporter you are using. (Note that
some methods in bzrlib haven't been upgraded yet).

We could put some effort into passing the real exc_info across but I
would consider this rather pointless: for bzrlib, which I believe you
are working on, this won't help with subunit (using in bzr selftest
--parallel), as subunit is in a different process, so a 'real exc_info'
is never available.

In short, I don't consider this behaviour a bug, but if you wanted to
put a patch forward I could review it for you.

 status triaged
 importance wishlist

Changed in testtools:
importance: Undecided → Wishlist
status: New → Triaged
Revision history for this message
Martin Packman (gz) wrote :

I think there is (and should be) be a requirement that just after a test finishes, the test framework has access to the actual exception raised and the traceback object. It might be there's some way of achieving this with testtools currently, but it's not obvious to me how if so.

This isn't just about the current stringification in testtools being broken compared to plain unittest or the old bzrlib test code. Access to the real traceback object, not just a (lossy) text representation, allows a bunch of extra things you can do by grovelling around the stack.

Revision history for this message
Jonathan Lange (jml) wrote : Re: [Bug 501169] Re: testtools should pass the real exc_info to TestResult methods

On Tue, Dec 29, 2009 at 7:24 PM, Martin [gz] <email address hidden> wrote:
> I think there is (and should be) be a requirement that just after a test
> finishes, the test framework has access to the actual exception raised
> and the traceback object.

How's this done in stdlib unittest?

Also, fwiw, testtools does have a way of getting at the raw exc_info
-- the onException / addOnException hooks.

jml

Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 501169] Re: testtools should pass the real exc_info to TestResult methods

On Tue, 2009-12-29 at 08:24 +0000, Martin [gz] wrote:
> I think there is (and should be) be a requirement that just after a test
> finishes, the test framework has access to the actual exception raised
> and the traceback object. It might be there's some way of achieving this
> with testtools currently, but it's not obvious to me how if so.

There isn't really such a requirement in unittest proper. Can you
enlarge on why you feel strongly about this - what it will do for you,
and how you see that working with remoted tests (those in different
languages, on different machines, or simply in different processes (e.g.
via the multiprocessing module).

> This isn't just about the current stringification in testtools being
> broken compared to plain unittest or the old bzrlib test code. Access to
> the real traceback object, not just a (lossy) text representation,
> allows a bunch of extra things you can do by grovelling around the
> stack.

I've had a fairly serious look around other test frameworks (in other
languages too) and not seen any doing that other than py.test, which
does it during assert, not during reporting of the activity.

As I've said though, I'm open to making it possible *when possible*, but
I feel very strongly that it must be an optional protocol not a required
behaviour.

-Rob

Revision history for this message
Martin Packman (gz) wrote :

> How's this done in stdlib unittest?

As explained already, unittest.TestResult.addError gets the exc_info passed to it as the third argument. With testtools you get a useless string wrapper instead.

> There isn't really such a requirement in unittest proper. Can you
> enlarge on why you feel strongly about this...

You broke my code. I had to write some ridiculous bullshit to get it working again.

A change I needed to make to support testtools:

- def addError(self, test, err):
+ def addError(self, test, err=None, details=None):
+ if err is None:
+ err = sys.exc_info()

And even worse (and the stack is barely useful most of the time here 'cos it's from the wrong place):

- def addExpectedFailure(self, test, err):
+ def addExpectedFailure(self, test, err=None, details=None):
+ if err is None:
+ olderr = sys.exc_info()[1].args[0]
+ if isinstance(olderr, tuple) and len(olderr) == 3:
+ # This is the knock-on ExpectedFailure, but it's passed the
+ # original exc_info for some reason, so get that back
+ err = olderr
+ else:
+ # The ExpectedFailure was manually raised
+ err = sys.exc_info()

Let me revert that and I'll be happy again.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.