Code review comment for lp:~mbp/bzr/test-errors

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 6/14/2011 5:51 AM, Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/test-errors into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> For more details, see:
> https://code.launchpad.net/~mbp/bzr/test-errors/+merge/64485
>
> A few selftest annoyances fixed:
>
> * Errors are printed only once, as they occur, not again at the end.
> If someone actually really likes this, the code is there and you can
> add a configuration to turn it on. (See bug 408192, bug 625597, though
> this is not exactly what was discussed.)
>
> * UnicodeOrBytesToBytesWriter had the somewhat confusing behavior of
> printing the repr of the wrapped object.
>
> * The indentation of test failures did not fit well with testtools' behavior
> of returning the whole traceback as part of the error string.
>
> * We shouldn't print the whole traceback for known failures. Just
> the reason string is enough. (Bug 499713)

Yay!

>
> * To let the unicode-escaping code work, we need to pass it the error
> as a unicode string.
>
> * Don't attach empty log files; as a consequence the log is not attached
> until the test completes. (However, it can still be read from
> TestCase.get_log()).

This also seems nice.

>
> * Bugs in old subunits do not deserve a 'known failure' in bzr. Only
> things we could fix (without time travel) should get that.

"We know this fails under these conditions", that seems like known
failure to me. However, I guess it could just be skipped.

Anyway, the code all looks good to me.

 merge: approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk33eXgACgkQJdeBCYSNAAMXzgCaAik/3rhrICt85tsgBYVf584Z
hGUAoNC7d66TJ+xGIQYtT2GU85REikDc
=hh8p
-----END PGP SIGNATURE-----