Merge lp:~jameinel/tarmac/non_ascii-750930 into lp:tarmac

[John A Meinel]

Bug #750930, handle non-ascii characters in test output.

This should be a reasonable fix for bug #750930.

I was trying to use Tarmac for lp:meliae, and it seems that gcc likes to put non-ascii characters into the output. (It looks like it uses fancy quotes.) I'm not 100% sure what characters are bad, but I do know that when the test suite failed, Tarmac just dies because of a UnicodeDecodeError.

The test case shows the failure, and the workaround is assuming that stdout/stderr are encoded in UTF-8. It is a slight hack, because the encoding could be something else, but it is unlikely.
At a minimum, it is better to decode w/ UTF-8 than to do u''.join(8-bit-strings).

This fix was enough for Meliae, though arguably any command that fails should cause Tarmac to say 'could not merge this branch because of FAILURE' rather than dying completely.

[dobey (dobey)]

I'm not sure how I feel about this. GCC is using the fancy quotes because the locale you're compiling under is en_US.UTF-8 presumably (as you are presumably running this as a live logged-in user). If GCC runs under the C locale, it uses pure ASCII for error/warning messages. And, as you pointed out, this will still fail if under a UTF-8-incompatible locale, such as Russian KOI8-R. I wonder if a better solution might be to force the locale to be C inside tarmac.

[John A Meinel (jameinel)]

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

On 09/07/2011 03:15 PM, Rodney Dawes wrote:
> I'm not sure how I feel about this. GCC is using the fancy quotes because the locale you're compiling under is en_US.UTF-8 presumably (as you are presumably running this as a live logged-in user). If GCC runs under the C locale, it uses pure ASCII for error/warning messages. And, as you pointed out, this will still fail if under a UTF-8-incompatible locale, such as Russian KOI8-R. I wonder if a better solution might be to force the locale to be C inside tarmac.
>

If it is a C locale, then .decode('UTF-8') will still work.

Yes, it will fail for other locales, and yes there are ways to detect it.

This still solves 90% of the cases without having to do more work.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5nv2sACgkQJdeBCYSNAAOu7QCaA9H+qhXRjrUx7YPSJhdMPgp6
3sEAoM9jNkmeKk6QR3duFbMArr5bafFR
=oLNa
-----END PGP SIGNATURE-----

[dobey (dobey)]

This needs to be re-submitted afresh, without Paul as the reviewer (but with ~tarmac or myself as the reviewer instead).

review: Needs Resubmitting

Unmerged revisions

=== modified file 'tarmac/plugins/command.py'
--- tarmac/plugins/command.py	2011-09-02 04:06:25 +0000
+++ tarmac/plugins/command.py	2011-09-02 14:55:29 +0000
@@ -179,6 +179,8 @@
         exception is then raised to prevent the commit from happening.
         '''
         message = u'Test command "%s" failed.' % self.verify_command
+        stdout_value = stdout_value.decode('UTF-8')
+        stderr_value = stderr_value.decode('UTF-8')
         comment = (u'The attempt to merge %(source)s into %(target)s failed. '
                    u'Below is the output from the failed tests.\n\n'
                    u'%(output)s') % {

=== modified file 'tarmac/plugins/tests/test_command.py'
--- tarmac/plugins/tests/test_command.py	2010-09-21 18:26:57 +0000
+++ tarmac/plugins/tests/test_command.py	2011-09-02 14:55:29 +0000
@@ -57,3 +57,20 @@
                           self.plugin.run,
                           command=self.command, target=target, source=None,
                           proposal=self.proposal)
+
+    def test_run_nonascii_failure(self):
+        self.config.debug = False
+        target = Thing(config=Thing(
+                verify_command="python -c 'import sys;"
+                           " sys.stdout.write(\"f\\xc3\\xa5\\xc3\\xafl\"\n);"
+                           " sys.exit(1)'"),
+            tree=Thing(abspath=os.path.abspath))
+        e = self.assertRaises(command.VerifyCommandFailed,
+                              self.plugin.run,
+                              command=self.command, target=target, source=None,
+                              proposal=self.proposal)
+        self.assertEqual(u'The attempt to merge lp:project/source'
+                         u' into lp:project failed.'
+                         u' Below is the output from the failed tests.'
+                         u'\n\nf\xe5\xefl\n',
+                         e.comment)