[Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file

[Petr Viktorin]

On 03/27/2012 04:58 PM, John Dennis wrote:
[...]

>   def test_create_translation():
>       f = text.create_translation
>       key = ('foo', None)
> @@ -129,6 +45,84 @@ def test_create_translation():
>       assert context.__dict__[key] is t
>
>
> +class test_TestLang(object):
> +    def setUp(self):
> +        self.tmp_dir = None
> +        self.saved_lang  = None
> +
> +        self.lang = 'xh_ZA'
> +        self.domain = 'ipa'
> +
> +        self.ipa_i18n_dir = os.path.join(os.path.dirname(__file__), '../../install/po')
> +        sys.path.insert(0, self.ipa_i18n_dir)

To keep the test isolated, I would save sys.path before modifying it, 
wrap the insert and import in a try block, and restore the path in finally.
Or even better, put the common code in an importable package: (most of) 
test_i18n.py could very well live under tests/.

> +        try:
> +            import test_i18n
> +        except ImportError, e:
> +            raise nose.SkipTest('cannot import test_i18n: %s')

Why would test_i18n not be importable?

> +        self.pot_basename = '%s.pot' % self.domain
> +        self.po_basename = '%s.po' % self.lang
> +        self.mo_basename = '%s.mo' % self.domain
> +
[...]
> +
> +        result = test_i18n.create_po(self.pot_file, self.po_file, self.mo_file)
> +        if result:
> +            raise nose.SkipTest('Unable to create po file "%s"&  mo file "%s" from pot file "%s"' %
> +                                (self.po_file, self.mo_file, self.pot_file))
> +
> +        if not file_exists(self.po_file):
> +            raise nose.SkipTest('Test po file unavailable, run "make test" in install/po')
> +
> +        if not file_exists(self.mo_file):
> +            raise nose.SkipTest('Test mo file unavailable, run "make test" in install/po')

Maybe it would be better to skip only if the pot file doesn't exist?
Is there a reason the PO generation should fail?

> +        self.po_file_iterate = test_i18n.po_file_iterate

Seeing this, I definitely recommend putting po_file_iterate in an 
importable package.

> +    def tearDown(self):
> +        if self.saved_lang is not None:
> +            os.environ['LANG'] = self.saved_lang
> +
> +        if self.tmp_dir is not None:
> +            shutil.rmtree(self.tmp_dir)
> +
[...]

One thing I missed in the previous patch: Makefile.in has a big comment 
near the end that's obsolete now.

-- 
Petr³