Code review comment for lp:~lifeless/testtools/bug-812793

Revision history for this message
Jonathan Lange (jml) wrote :

The code change is indeed self-evident and good. Thanks.

This might have unintended consequences. _details_to_str treats the 'traceback' detail specially, rendering it not so much as a detail but as a traditional test traceback. With this change, if a fixture has a detail named 'traceback', that will be rendered as the main error. This could be confusing to users.

On balance, I'm OK with this patch landing, but would be much happier if we could resolve (or at least document) that potential source of confusion.

(Note: I think the _details_to_str special casing of 'traceback' is a bit of a kludge and would be glad to see it go.)

review: Approve

« Back to merge proposal