Merge lp:~jr/bzr/i18n-unoverride-no-i18n-tests into lp:bzr

[Jonathan Riddell]

re-enable i18n for tests which need it
add i18n info to testing documentation
add some API docs to functions which miss it

re-enable i18n for tests which need it
add i18n info to testing documentation
add some API docs to functions which miss it

[Vincent Ladeuil (vila)]

Thanks for following up on this !

70 + self.enableI18n()
71 from bzrlib import help
72 i18n.install()
73 self.overrideAttr(i18n, '_translations', ZzzTranslations())

Since these tests are already installing a specific translation, do they really need enable18n() ? Isn't there some possible simplification across:

- i18n.installed (disable multiple installs)
- TestCase.enable18n() (restore the ability to install, err, really?)
- i18n._translations (backdoor to install anyway)

In fact, I think the tests do not care about i18n.installed no ?

review: Needs Information

[Jonathan Riddell (jr)]

Yes well spotted, updated.

[Vincent Ladeuil (vila)]

After our IRC chat, I think you're good to land !

Thanks for your work here, it's more involved than it appears and it's good to know you care about it.

review: Approve

[Jonathan Riddell (jr)]

sent to pqm by email

=== modified file 'bzrlib/i18n.py'
--- bzrlib/i18n.py	2011-09-13 15:34:45 +0000
+++ bzrlib/i18n.py	2011-09-14 17:21:43 +0000
@@ -26,7 +26,7 @@
 import os
 import sys
 
-_translations = _gettext.NullTranslations()
+_translations = None
 
 
 def gettext(message):
@@ -61,6 +61,7 @@
 
     :returns: concatenated translated message as unicode.
     """
+    install()
     paragraphs = message.split(u'\n\n')
     ugettext = _translations.ugettext
     # Be careful not to translate the empty string -- it holds the
@@ -71,15 +72,17 @@
 def disable_i18n():
     """Do not allow i18n to be enabled.  Useful for third party users
     of bzrlib."""
-    global installed
-    installed = lambda: True
+    global _translations
+    _translations = _gettext.NullTranslations()
 
 
 def installed():
-    return not isinstance(_translations, _gettext.NullTranslations)
+    """Returns whether translations are in use or not."""
+    return _translations is not None
 
 
 def install(lang=None):
+    """Enables gettext translations."""
     global _translations
     if installed():
         return
@@ -97,8 +100,9 @@
 
 
 def uninstall():
+    """Disables gettext translations."""
     global _translations
-    _translations = _gettext.NullTranslations()
+    _translations = None
 
 
 def _get_locale_dir():

=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	2011-09-12 18:40:02 +0000
+++ bzrlib/tests/__init__.py	2011-09-14 17:21:43 +0000
@@ -1007,7 +1007,7 @@
         if 'config_stats' in selftest_debug_flags:
             self._install_config_stats_hooks()
         # Do not use i18n for tests (unless the test reverses this)
-        self.overrideAttr(i18n, 'installed', lambda: True)
+        i18n.disable_i18n()
 
     def debug(self):
         # debug a frame up.

=== modified file 'bzrlib/tests/test_i18n.py'
--- bzrlib/tests/test_i18n.py	2011-09-13 15:34:53 +0000
+++ bzrlib/tests/test_i18n.py	2011-09-14 17:21:43 +0000
@@ -97,22 +97,36 @@
 
 class TestInstall(tests.TestCase):
 
+    def setUp(self):
+        super(TestInstall, self).setUp()
+        # Restore a proper env to test translation installation
+        self.overrideAttr(i18n, '_translations', None)
+
     def test_custom_languages(self):
-        self.addCleanup(i18n.install)
         i18n.install('nl:fy')
+        # Whether we found a valid tranlsation or not doesn't matter, we got
+        # one and _translations is not None anymore.
+        self.assertIsInstance(i18n._translations,
+                              i18n._gettext.NullTranslations)
 
     def test_no_env_variables(self):
-        self.addCleanup(i18n.install)
         self.overrideEnv('LANGUAGE', None)
         self.overrideEnv('LC_ALL', None)
         self.overrideEnv('LC_MESSAGES', None)
         self.overrideEnv('LANG', None)
         i18n.install()
+        # Whether we found a valid tranlsation or not doesn't matter, we got
+        # one and _translations is not None anymore.
+        self.assertIsInstance(i18n._translations,
+                              i18n._gettext.NullTranslations)
 
     def test_disable_i18n(self):
         i18n.disable_i18n()
         i18n.install()
-        self.assertTrue(isinstance(i18n._translations, i18n._gettext.NullTranslations))
+        # It's disabled, you can't install anything and we fallback to null
+        self.assertIsInstance(i18n._translations,
+                              i18n._gettext.NullTranslations)
+
 
 class TestTranslate(tests.TestCaseWithTransport):
 
@@ -124,7 +138,6 @@
         """do errors get translated?"""
         err = None
         tree = self.make_branch_and_tree('.')
-        self.overrideAttr(i18n, '_translations', ZzzTranslations())
         try:
             workingtree.WorkingTree.open('./foo')
         except errors.NotBranchError,e:
@@ -135,8 +148,6 @@
     def test_topic_help_translation(self):
         """does topic help get translated?"""
         from bzrlib import help
-        i18n.install()
-        self.overrideAttr(i18n, '_translations', ZzzTranslations())
         from StringIO import StringIO
         out = StringIO()
         help.help("authentication", out)

=== modified file 'doc/developers/testing.txt'
--- doc/developers/testing.txt	2011-08-04 00:28:09 +0000
+++ doc/developers/testing.txt	2011-09-14 17:21:43 +0000
@@ -745,6 +745,22 @@
         _test_needs_features = [features.apport]
 
 
+Testing translations
+-----------------------
+
+Translations are disabled by default in tests.  If you want to test
+that code is translated you can use the ``ZzzTranslations`` class from
+``test_i18n``::
+
+    self.overrideAttr(i18n, '_translations', ZzzTranslations())
+
+And check the output strings look like ``u"zz\xe5{{output}}"``.
+
+To test the gettext setup and usage you override i18n.installed back
+to self.i18nInstalled and _translations to None, see
+test_i18n.TestInstall.
+
+
 Testing deprecated code
 -----------------------