Merge lp:~jtv/launchpad/bug-533668 into lp:launchpad

[Jeroen T. Vermeulen]

Hande non-ascii exceptions in translations-to-bzr exports.

== Bug 533668 ==

When a project name has non-ascii characters, and translations export to a branch fails for that project, the attempt to log an error message breaks. This happened with µBlogger the other day.

Actually, even before we get to logging a message, it's the attempt to produce a sensible error message out of the exception that breaks.

So this branch replaces the logic with a built-in one: Exception.__repr__. In the duck-typing tradition, this takes the question of str-vs-unicode out of our code and also does the right thing with exceptions without messages, as well as extra arguments describing the exception.

To test:
{{{
./bin/test -vv -t exportToBranches_handles_nonascii
}}}

No lint.

To Q/A, have this run on the staging codehosting server, against the staging db. The export for µBlogger should fail and produce a sensible error message, after which the script should recover and continue to run rather than bail out.

Jeroen

[Aaron Bentley (abentley)]

As discussed on IRC, please use repr(e) rather than e.__repr__(). Aside from that, looks good.

review: Approve

=== modified file 'lib/lp/translations/doc/translations-export-to-branch.txt'
--- lib/lp/translations/doc/translations-export-to-branch.txt	2009-11-18 13:26:03 +0000
+++ lib/lp/translations/doc/translations-export-to-branch.txt	2010-03-09 04:39:30 +0000
@@ -242,7 +242,7 @@
     <BLANKLINE>
     DEBUG ...
     INFO Unlock.
-    ERROR Failure: Simulated race condition.
+    ERROR Failure: ConcurrentUpdateError(...Simulated race condition...)
     INFO Processed 1 item(s); 1 failure(s).
 
 
@@ -261,8 +261,9 @@
     >>> script.main()
     INFO Exporting to translations branches.
     INFO Exporting Gazblachko trunk series.
-    ERROR Failure: Translations branch for Gazblachko trunk series
-    has pending translations changes.  Not committing.
+    ERROR Failure: ConcurrentUpdateError(...Translations branch for
+    Gazblachko trunk series has pending translations changes.
+    Not committing...)
     INFO Processed 1 item(s); 1 failure(s).
 
 There is one problem with detecting this race condition.  Jobs are never

=== modified file 'lib/lp/translations/scripts/tests/test_translations_to_branch.py'
--- lib/lp/translations/scripts/tests/test_translations_to_branch.py	2009-10-19 10:09:25 +0000
+++ lib/lp/translations/scripts/tests/test_translations_to_branch.py	2010-03-09 04:39:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Acceptance test for the translations-export-to-branch script."""
@@ -11,15 +11,21 @@
 
 from zope.security.proxy import removeSecurityProxy
 
+from canonical.launchpad.scripts.logger import QuietFakeLogger
 from canonical.launchpad.scripts.tests import run_script
 from canonical.testing import ZopelessAppServerLayer
 
 from lp.testing import map_branch_contents, TestCaseWithFactory
+from lp.testing.fakemethod import FakeMethod
 
 from lp.translations.scripts.translations_to_branch import (
     ExportTranslationsToBranch)
 
 
+class GruesomeException(Exception):
+    """CPU on fire.  Or some other kind of failure, like."""
+
+
 class TestExportTranslationsToBranch(TestCaseWithFactory):
 
     layer = ZopelessAppServerLayer
@@ -122,6 +128,23 @@
         self.assertEqual(
             None, re.search("INFO\s+Committed [0-9]+ file", stderr))
 
+    def test_exportToBranches_handles_nonascii_exceptions(self):
+        # There's an exception handler in _exportToBranches that must
+        # cope well with non-ASCII exception strings.
+        exporter = ExportTranslationsToBranch(test_args=[])
+        exporter.logger = QuietFakeLogger()
+        boom = u'\u2639'
+        exporter._exportToBranch = FakeMethod(failure=GruesomeException(boom))
+
+        exporter._exportToBranches([self.factory.makeProductSeries()])
+
+        self.assertEqual(1, exporter._exportToBranch.call_count)
+
+        exporter.logger.output_file.seek(0)
+        message = exporter.logger.output_file.read()
+        self.assertTrue(message.startswith("ERROR"))
+        self.assertTrue("GruesomeException" in message)
+
 
 class TestExportToStackedBranch(TestCaseWithFactory):
     """Test workaround for bzr bug 375013."""

=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
--- lib/lp/translations/scripts/translations_to_branch.py	2010-01-21 00:46:48 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py	2010-03-09 04:39:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Export translation snapshots to bzr branches where requested."""
@@ -238,10 +238,7 @@
                 raise
             except Exception, e:
                 items_failed += 1
-                message = unicode(e)
-                if message == u'':
-                    message = e.__class__.__name__
-                self.logger.error("Failure: %s" % message)
+                self.logger.error("Failure: %s" % repr(e))
                 if self.txn:
                     self.txn.abort()