Merge lp:~gz/bzr/escape_selftest_console_output_633216 into lp:bzr

[Martin Packman]

Backslash escape selftest output when printing to non-unicode consoles

Simple change to make selftest output escaped rather than breaking the run with a UnicodeEncodeError when on a console that can't encode a given unicode string. Hasn't been much of a problem so far as selftest has tended to print pre-encoded junk bytes, but testtools changes mean that unicode will now be better preserved, and some future fixes to selftest will also help.

The test overrides osutils.get_terminal_encoding but TextTestWriter could switching to look at the encoding attribute of the stream rather than calling that in future.

[Martin Pool (mbp)]

nice, thanks.

news entry please.

review: Approve

[Martin Packman (gz)]

I was trying to write a testcase that didn't require testtools > 0.9.4 but looking at it again this will clearly break older versions. Should I skip the test on older versions, or give in and bump the selftest requirement?

[Vincent Ladeuil (vila)]

>>>>> Martin [gz] <email address hidden> writes:

    > I was trying to write a testcase that didn't require testtools >
    > 0.9.4 but looking at it again this will clearly break older
    > versions. Should I skip the test on older versions, or give in and
    > bump the selftest requirement?

bump

We may need to update pqm for landing it though.

But since we are fixing long standing bugs, I don't see how to avoid
that.

[Martin Packman (gz)]

Added NEWS entry as request, also put up lp:~gz/bzr/require_testtools_0.9.5_for_selftest which should be viewed as a prerequisite to this branch.

[lp:~gz/bzr/escape_selftest_console_output_633216]

5412. By Martin Packman on 2010-09-10

Add NEWS entry, remove some unintended whitespace after the test

[Andrew Bennetts (spiv)]

There's a trivial typo, a missing double-quote at the end of a line:

+ "Text attachment: log\n"

The new test doesn't seem to pass for me (when I cherrypick this change into the 2.2 branch), but on the other hand the selftest run does appear to complete...

The approach seems reasonable, though.

[lp:~gz/bzr/escape_selftest_console_output_633216]

5413. By Martin Packman on 2010-09-23

Fix SyntaxError causing typo in test spotted by spiv in review

[Martin Packman (gz)]

> There's a trivial typo, a missing double-quote at the end of a line:
>
> + "Text attachment: log\n"

Well spotted, I presume I must have added the line wrapping after running the test then not run it again before pushing.

> The new test doesn't seem to pass for me (when I cherrypick this change into
> the 2.2 branch), but on the other hand the selftest run does appear to
> complete...

Can you paste the exact failure? Also, what version of testtools do you have?

[Vincent Ladeuil (vila)]

@spiv: did you sort out why the test was failing for you ?
@gz: Are we still waiting for testtools to be upgraded on pqm for this mp too ?

review: Needs Information

[Martin Packman (gz)]

Yes, this does need a newer testtools on PQM as well. If that's going to be stuck for a long time yet, I could rewrite the test to skip if the version is too low.

[Martin Pool (mbp)]

I think we can upgrade testtools. Is there a release, ideally a packaged release, with the changes needed?

[Martin Packman (gz)]

See lp:~gz/bzr/require_testtools_0.9.5_for_selftest for details Martin, but either of the last two testtools releases would do, and Vincent has filed an rt we're waiting on.

[lp:~gz/bzr/escape_selftest_console_output_633216]

5414. By Martin Packman on 2010-11-19

Merge require_testtools_0.9.5_for_selftest to reflect test requirment and resolve news conflict

[Martin Packman (gz)]

Have merged lp:~gz/bzr/require_testtools_0.9.5_for_selftest into this branch as the test requires it and news was going to conflict anyway. Once that lands, this should also be good to go.

[Martin Packman (gz)]

sent to pqm by email

=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	2010-10-18 17:06:37 +0000
+++ bzrlib/tests/__init__.py	2010-11-19 23:07:43 +0000
@@ -55,8 +55,8 @@
 import testtools
 # nb: check this before importing anything else from within it
 _testtools_version = getattr(testtools, '__version__', ())
-if _testtools_version < (0, 9, 2):
-    raise ImportError("need at least testtools 0.9.2: %s is %r"
+if _testtools_version < (0, 9, 5):
+    raise ImportError("need at least testtools 0.9.5: %s is %r"
         % (testtools.__file__, _testtools_version))
 from testtools import content
 
@@ -651,7 +651,10 @@
             encode = codec[0]
         else:
             encode = codec.encode
-        stream = osutils.UnicodeOrBytesToBytesWriter(encode, stream)
+        # GZ 2010-09-08: Really we don't want to be writing arbitrary bytes,
+        #                so should swap to the plain codecs.StreamWriter
+        stream = osutils.UnicodeOrBytesToBytesWriter(encode, stream,
+            "backslashreplace")
         stream.encoding = new_encoding
         self.stream = stream
         self.descriptions = descriptions

=== modified file 'bzrlib/tests/test_selftest.py'
--- bzrlib/tests/test_selftest.py	2010-10-15 11:20:45 +0000
+++ bzrlib/tests/test_selftest.py	2010-11-19 23:07:43 +0000
@@ -1282,6 +1282,23 @@
         result = self.run_test_runner(runner, test)
         self.assertLength(1, calls)
 
+    def test_unicode_test_output_on_ascii_stream(self):
+        """Showing results should always succeed even on an ascii console"""
+        class FailureWithUnicode(tests.TestCase):
+            def test_log_unicode(self):
+                self.log(u"\u2606")
+                self.fail("Now print that log!")
+        out = StringIO()
+        self.overrideAttr(osutils, "get_terminal_encoding",
+            lambda trace=False: "ascii")
+        result = self.run_test_runner(tests.TextTestRunner(stream=out),
+            FailureWithUnicode("test_log_unicode"))
+        self.assertContainsRe(out.getvalue(),
+            "Text attachment: log\n"
+            "-+\n"
+            "\d+\.\d+  \\\\u2606\n"
+            "-+\n")
+
 
 class SampleTestCase(tests.TestCase):
 

=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- doc/en/release-notes/bzr-2.3.txt	2010-11-18 19:38:28 +0000
+++ doc/en/release-notes/bzr-2.3.txt	2010-11-19 23:07:43 +0000
@@ -103,6 +103,14 @@
   Instead, use '...' as a wildcard if you don't care about the output.
   (Martin Pool, #637830)
 
+* Bump minimum testtools version required to run ``bzr selftest`` from 0.9.2
+  to 0.9.5 which will allow tests that need the fixed unicode handling to be
+  written. (Martin [gz])
+
+* Printing selftest results to a non-UTF-8 console will now escape characters
+  that can't be encoded rather than aborting the test run with an exception.
+  (Martin [gz], #633216)
+
 
 bzr 2.3b3
 #########