Merge lp://staging/~mars/zope.testing/fix-subunit-utf8-traceback-reporting into lp://staging/zope.testing

Looks good to me. +1!

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

Māris Fogels wrote:
> Māris Fogels has proposed merging lp:~mars/zope.testing/fix-subunit-utf8-traceback-reporting into lp:zope.testing.
>
> Requested reviews:
> ZTK steering group (ztk-steering-group)
> Related bugs:
> #591309 Crash when subunit reports test failures containing UTF8-encoded data
> https://bugs.launchpad.net/bugs/591309
>
>
> Hello,
>
> This branch fixes a problem in zope.testing's subunit support.
> Instead of raising an Exception when the traceback contains
> UTF8-encoded characters we try to handle the text in a robust way.
> See bug 591309 for the details.
>
> We assume the traceback is in utf-8, as that is the encoding most
> people would use for a doctest or Python module, but just in case, I
> included a test for handling other codecs.
>
> This entire patch would be unnecessary if the traceback were
> guaranteed to be a unicode object. Unfortunately, I do not know the
> entry points from which a bytestring can be passed into the
> formatter, so this defensive code feels safer than risking a
> testrunner to crash. The unicode-type test makes the code
> forward-compatible.
>
> I exposed the SubunitOutputFormatter's 'stream' attribute to
> facilitate testing.

I'm -1 on doing any further work on the subunit stuff on the trunk until
we clean up the testing story: at the moment, the features are
completely untested in 95% of the developement environments where folks
work on zope.testing / zope.testrunner, because subunit cannot be
installed in them.

I have a branch merge pending to add distutils support to subunit for
the Python libraries and scripts, which would make releasing them to
PyPI as an sdist feasible. Once that occurs, we can make the
'subunit-python' egg a testing dependency for zope.testing /
zope.testrunner, and ensure that the features don't bitrot.

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkwPrYQACgkQ+gerLs4ltQ7RUACfc/7DzxUKL8m0eVOPgY70vRdf
AzgAn3vm4VNvVyTlaEvqI/LWdEAl9GT0
=9Lzj
-----END PGP SIGNATURE-----

> I'm -1 on doing any further work on the subunit stuff on the trunk until
> we clean up the testing story: at the moment, the features are
> completely untested in 95% of the developement environments where folks
> work on zope.testing / zope.testrunner, because subunit cannot be
> installed in them.
>
> I have a branch merge pending to add distutils support to subunit for
> the Python libraries and scripts, which would make releasing them to
> PyPI as an sdist feasible. Once that occurs, we can make the
> 'subunit-python' egg a testing dependency for zope.testing /
> zope.testrunner, and ensure that the features don't bitrot.
>
>
> Tres.

Tres, I agree, but does "any further work" include bugfixes to the subunit code? I have spent weeks hunting these bugs down. It would be a pity to lose them. (I may have more fixes coming, too.)

Maris

I don't think it makes sense to hold back things that devs with a working environment can validate simply because other devs *might* cause regressions. Yes, we should get the zope.testing buildbot testing subunit, but the lack of that isn't a reason to stop fixing code that someone can run a test on.

I'm working on a fix to make the subunit support conditional.

I used virtualenv to compare the results of running the suite with and without subunit installed. The suite skips the test_subunit module when subunit is missing, and includes it when subunit is present. This is the proper behaviour.

The branch should be good to land as-is. If desired, I can include a comment in the docstring of the test_subunit module that states that module is run conditionally.

Tres, now that subunit has a release on PyPI (see http://pypi.python.org/pypi/python-subunit), can this patch please be landed?

The copyright line in the new module (src/zope/testing/testrunner/test_subunit.py) should have "2010" as the date. Other than that this change looks good.

On Mon, Jul 19, 2010 at 9:27 AM, Benji York <email address hidden> wrote:
> Review: Approve
> The copyright line in the new module (src/zope/testing/testrunner/test_subunit.py) should have "2010" as the date.  Other than that this change looks good.

Fixed.

I merged this in in r114856.

I added at test dependency for python-subunit on the principal that testing more is better than less as long as the test dependencies aren't too evil, and python-subunit doesn't seem evil.