Re: [Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
Petr Viktorin wrote: On 04/09/2012 05:24 PM, John Dennis wrote: On 03/30/2012 08:57 AM, Petr Viktorin wrote: On 03/30/2012 02:41 AM, John Dennis wrote: On 03/28/2012 04:40 AM, Petr Viktorin wrote: Can install/po/Makefile just call test_i18n.py from the tests/ tree? It doesn't import any IPA code so there's no need to set sys.path in this case (though there'd have to be a comment saying we depend on this). In the other case, unit tests, the path is already set by Nose. Also the file would have to be renamed so nose doesn't pick it up as a test module. Good idea. I moved test_i18n.py to tests/i18n.py. I was reluctant about moving the file, but that was without merit, it works better this way. The downside is that the file now looks like a test utility module. It could use a comment near the imports saying that it's also called as a script, and that it shouldn't import IPA code (this could silently use the system-installed version of IPA, or crash if it's not there). Alternatively, set PYTHONPATH in the Makefile. I also removed the superfluous comment in Makefile.in you pointed out. When I was exercising the code I noticed the validation code was not treating msgid's from C code correctly (we do have some C code in the client area). That required a much more nuanced parsing the format conversion specifiers to correctly identify what was a positional format specifier vs. an indexed format specifier. The new version of the i18n.py includes the function parse_printf_fmt() and get_prog_langs() to identify the source programming language. More non-trivial code without tests. This makes me worry. But, tests for this can be added later I guess. Two more patches will follow shortly, one which adds validation when make lint is run and a patch to correct the problems it found in the C code strings which did not used indexed format specifiers. revised patch with comment about imports attached. ACK pushed to master and ipa-2-2 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
On 04/09/2012 05:24 PM, John Dennis wrote: On 03/30/2012 08:57 AM, Petr Viktorin wrote: On 03/30/2012 02:41 AM, John Dennis wrote: On 03/28/2012 04:40 AM, Petr Viktorin wrote: Can install/po/Makefile just call test_i18n.py from the tests/ tree? It doesn't import any IPA code so there's no need to set sys.path in this case (though there'd have to be a comment saying we depend on this). In the other case, unit tests, the path is already set by Nose. Also the file would have to be renamed so nose doesn't pick it up as a test module. Good idea. I moved test_i18n.py to tests/i18n.py. I was reluctant about moving the file, but that was without merit, it works better this way. The downside is that the file now looks like a test utility module. It could use a comment near the imports saying that it's also called as a script, and that it shouldn't import IPA code (this could silently use the system-installed version of IPA, or crash if it's not there). Alternatively, set PYTHONPATH in the Makefile. I also removed the superfluous comment in Makefile.in you pointed out. When I was exercising the code I noticed the validation code was not treating msgid's from C code correctly (we do have some C code in the client area). That required a much more nuanced parsing the format conversion specifiers to correctly identify what was a positional format specifier vs. an indexed format specifier. The new version of the i18n.py includes the function parse_printf_fmt() and get_prog_langs() to identify the source programming language. More non-trivial code without tests. This makes me worry. But, tests for this can be added later I guess. Two more patches will follow shortly, one which adds validation when make lint is run and a patch to correct the problems it found in the C code strings which did not used indexed format specifiers. revised patch with comment about imports attached. ACK -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
On 03/30/2012 02:41 AM, John Dennis wrote: On 03/28/2012 04:40 AM, Petr Viktorin wrote: Can install/po/Makefile just call test_i18n.py from the tests/ tree? It doesn't import any IPA code so there's no need to set sys.path in this case (though there'd have to be a comment saying we depend on this). In the other case, unit tests, the path is already set by Nose. Also the file would have to be renamed so nose doesn't pick it up as a test module. Good idea. I moved test_i18n.py to tests/i18n.py. I was reluctant about moving the file, but that was without merit, it works better this way. The downside is that the file now looks like a test utility module. It could use a comment near the imports saying that it's also called as a script, and that it shouldn't import IPA code (this could silently use the system-installed version of IPA, or crash if it's not there). Alternatively, set PYTHONPATH in the Makefile. I also removed the superfluous comment in Makefile.in you pointed out. When I was exercising the code I noticed the validation code was not treating msgid's from C code correctly (we do have some C code in the client area). That required a much more nuanced parsing the format conversion specifiers to correctly identify what was a positional format specifier vs. an indexed format specifier. The new version of the i18n.py includes the function parse_printf_fmt() and get_prog_langs() to identify the source programming language. More non-trivial code without tests. This makes me worry. But, tests for this can be added later I guess. Two more patches will follow shortly, one which adds validation when make lint is run and a patch to correct the problems it found in the C code strings which did not used indexed format specifiers. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
On 03/27/2012 10:31 PM, John Dennis wrote: On 03/27/2012 01:57 PM, Petr Viktorin wrote: Seeing this, I definitely recommend putting po_file_iterate in an importable package. Of course I considered that. Clearly not any existing top level directory in the tree, those are reserved for what we install and what is import visible after installation. Test utility code should not be installed with our normal modules and packages. It has to be importable from both the install/po area and the test area. It can't depend on nosetests setting the import path prior to execution (because only the unit tests are run via nose). So we could create a directory tests/util which hosts utilities used for test code and locate it there. I did consider that, it would be (somewhat) cleaner. But unless I'm missing something someone is going to have to modify the include path prior to importing any test utility code. It just becomes a question of where the file is located. I'd be happy to move the bulk of the logic into tests/util/i18n.py, but to import it the importing code is going to have to add tests/util to the import path, which puts you pretty much back into the original situation, just with a different path (albeit perhaps a more logical cleaner path). Can install/po/Makefile just call test_i18n.py from the tests/ tree? It doesn't import any IPA code so there's no need to set sys.path in this case (though there'd have to be a comment saying we depend on this). In the other case, unit tests, the path is already set by Nose. Also the file would have to be renamed so nose doesn't pick it up as a test module. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
This patch addresses ticket 2582, see comments in commit message.
--
John Dennis jden...@redhat.com
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
From fff7d0a42f7643039987d49c4de69b248f9baa9f Mon Sep 17 00:00:00 2001
From: John Dennis jden...@redhat.com
Date: Tue, 27 Mar 2012 10:18:24 -0400
Subject: [PATCH 68] text unit test should validate using installed mo file
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
We use custom gettext classes (e.g. GettextFactory
NGettextFactory). We should exercise those classes with an installed
binary mo file to demonstrate we are actually returning the expected
translated strings for all strings defined as being translatable.
The test logic in install/po/test_i18n.py was recently enhanced to
make this type of testing easier and more complete.
tests/test_ipalib/test_text.py should import the new i18n test support
and run it.
Previously tests/test_ipalib/test_text.py made a feeble but incomplete
attempt to do the above but even that was often not run because the
test would skip because the necessary test files were not available
unless they had been manually created in the install/po subdir. It is
now possible to correct those deficiencies in the test.
This patch does the following:
* Imports test_i18n.py from the install/po directory in the tree
* Creates a tmp directory for the test localedir
* Parses the current ipa.pot file in install/po and generates
a test po and mo file with special unicode markers. It installs
the test mo file in the tmp localedir. This is accomplished by
calling create_po() from the test_i18n.py file.
* If any of the above does not work it raises nose.SkipTest with
the reason, and skips the test.
* It sets up functions to get a translation and a plural translation
via our text.GettextFactory class and text.NGettextFactory class
respectively. This are the functions we use intenally to get
translations. It set the localdir and lang which are used by those
classes to match our test configuration. It then runs a validation
test on every translation and it's plural found in the test.po file
by calling po_file_iterate and passed it the function pointers to
our internal routines.
* At the conclusion of the test it cleans up after itself.
Note: extraneous files are not created in the tree, only a tmp
directory is utilized.
---
tests/test_ipalib/test_text.py | 103 +---
1 files changed, 64 insertions(+), 39 deletions(-)
diff --git a/tests/test_ipalib/test_text.py b/tests/test_ipalib/test_text.py
index f46b78e..45a6ca5 100644
--- a/tests/test_ipalib/test_text.py
+++ b/tests/test_ipalib/test_text.py
@@ -23,6 +23,8 @@ Test the `ipalib.text` module.
import os
import sys
+import shutil
+import tempfile
import re
import nose
import locale
@@ -45,58 +47,81 @@ def test_create_translation():
class test_TestLang(object):
def setUp(self):
+self.tmp_dir = None
+self.saved_lang = None
+
self.lang = 'xh_ZA'
self.domain = 'ipa'
-self.po_dir = os.path.join(os.path.dirname(__file__), '../../install/po')
-self.locale_dir = os.path.join(self.po_dir, 'test_locale')
+self.ipa_i18n_dir = os.path.join(os.path.dirname(__file__), '../../install/po')
+sys.path.insert(0, self.ipa_i18n_dir)
+try:
+import test_i18n
+except ImportError, e:
+raise nose.SkipTest('cannot import test_i18n: %s')
+
+self.pot_basename = '%s.pot' % self.domain
+self.po_basename = '%s.po' % self.lang
+self.mo_basename = '%s.mo' % self.domain
+
+self.tmp_dir = tempfile.mkdtemp()
+self.saved_lang = os.environ['LANG']
+
+self.locale_dir = os.path.join(self.tmp_dir, 'test_locale')
+self.msg_dir = os.path.join(self.locale_dir, self.lang, 'LC_MESSAGES')
+
+if not os.path.exists(self.msg_dir):
+os.makedirs(self.msg_dir)
+self.pot_file = os.path.join(self.ipa_i18n_dir, self.pot_basename)
+self.mo_file = os.path.join(self.msg_dir, self.mo_basename)
+self.po_file = os.path.join(self.tmp_dir, self.po_basename)
+
+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))
-self.po_file = os.path.join(self.po_dir, 'test.po')
if not file_exists(self.po_file):
raise nose.SkipTest('Test po file unavailable, run make test in install/po')
-mo_file = os.path.join(self.locale_dir, self.lang, 'LC_MESSAGES', '%s.mo' % self.domain)
-if not file_exists(mo_file):
+if not file_exists(self.mo_file):
raise nose.SkipTest('Test mo file unavailable, run make test in install/po')
-sys.path.insert(0, self.po_dir)
-
Re: [Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
On 03/27/2012 10:34 AM, John Dennis wrote: This patch addresses ticket 2582, see comments in commit message. Sorry, the patch I originally posted was incorrect, the work had been done in several commits but I forgot to squash them into one commit and patch before posting it. This new patch fixes that mistake. -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ From 03ffa8e2b1419de80c4769d8391ca56ffb4e2695 Mon Sep 17 00:00:00 2001 From: John Dennis jden...@redhat.com Date: Mon, 26 Mar 2012 22:26:35 -0400 Subject: [PATCH 68-1] text unit test should validate using installed mo file Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit We use custom gettext classes (e.g. GettextFactory NGettextFactory). We should exercise those classes with an installed binary mo file to demonstrate we are actually returning the expected translated strings for all strings defined as being translatable. The test logic in install/po/test_i18n.py was recently enhanced to make this type of testing easier and more complete. tests/test_ipalib/test_text.py should import the new i18n test support and run it. Previously tests/test_ipalib/test_text.py made a feeble but incomplete attempt to do the above but even that was often not run because the test would skip because the necessary test files were not available unless they had been manually created in the install/po subdir. It is now possible to correct those deficiencies in the test. This patch does the following: * Modfies test function in test_i18n.py to accept function pointers for retreiving a translation. * Imports test_i18n.py from the install/po directory in the tree * Creates a tmp directory for the test localedir * Parses the current ipa.pot file in install/po and generates a test po and mo file with special unicode markers. It installs the test mo file in the tmp localedir. This is accomplished by calling create_po() from the test_i18n.py file. * If any of the above does not work it raises nose.SkipTest with the reason, and skips the test. * It sets up functions to get a translation and a plural translation via our text.GettextFactory class and text.NGettextFactory class respectively. This are the functions we use intenally to get translations. It set the localdir and lang which are used by those classes to match our test configuration. It then runs a validation test on every translation and it's plural found in the test.po file by calling po_file_iterate and passed it the function pointers to our internal routines. * At the conclusion of the test it cleans up after itself. Note: extraneous files are not created in the tree, only a tmp directory is utilized. --- install/po/test_i18n.py| 35 + tests/test_ipalib/test_text.py | 168 +++- 2 files changed, 101 insertions(+), 102 deletions(-) diff --git a/install/po/test_i18n.py b/install/po/test_i18n.py index beb43cc..c97874f 100755 --- a/install/po/test_i18n.py +++ b/install/po/test_i18n.py @@ -387,23 +387,28 @@ def validate_unicode_edit(msgid, msgstr): def test_translations(po_file, lang, domain, locale_dir): -try: +# The test installs the test message catalog under the xh_ZA +# (e.g. Zambia Xhosa) language by default. It would be nice to +# use a dummy language not associated with any real language, +# but the setlocale function demands the locale be a valid +# known locale, Zambia Xhosa is a reasonable choice :) -# The test installs the test message catalog under the xh_ZA -# (e.g. Zambia Xhosa) language by default. It would be nice to -# use a dummy language not associated with any real language, -# but the setlocale function demands the locale be a valid -# known locale, Zambia Xhosa is a reasonable choice :) +os.environ['LANG'] = lang -os.environ['LANG'] = lang +# Create a gettext translation object specifying our domain as +# 'ipa' and the locale_dir as 'test_locale' (i.e. where to +# look for the message catalog). Then use that translation +# object to obtain the translation functions. -# Create a gettext translation object specifying our domain as -# 'ipa' and the locale_dir as 'test_locale' (i.e. where to -# look for the message catalog). Then use that translation -# object to obtain the translation functions. +t = gettext.translation(domain, locale_dir) + +get_msgstr = t.ugettext +get_msgstr_plural = t.ungettext -t = gettext.translation(domain, locale_dir) +return po_file_iterate(po_file, get_msgstr, get_msgstr_plural) +def po_file_iterate(po_file, get_msgstr, get_msgstr_plural): +try: # Iterate over the msgid's if not os.path.isfile(po_file): print sys.stderr, 'file does not exist %s' % (po_file) @@ -422,8 +427,8 @@ def test_translations(po_file, lang,
Re: [Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
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³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 68] text unit test should validate using installed mo file
On 03/27/2012 01:57 PM, Petr Viktorin wrote: Seeing this, I definitely recommend putting po_file_iterate in an importable package. Of course I considered that. Clearly not any existing top level directory in the tree, those are reserved for what we install and what is import visible after installation. Test utility code should not be installed with our normal modules and packages. It has to be importable from both the install/po area and the test area. It can't depend on nosetests setting the import path prior to execution (because only the unit tests are run via nose). So we could create a directory tests/util which hosts utilities used for test code and locate it there. I did consider that, it would be (somewhat) cleaner. But unless I'm missing something someone is going to have to modify the include path prior to importing any test utility code. It just becomes a question of where the file is located. I'd be happy to move the bulk of the logic into tests/util/i18n.py, but to import it the importing code is going to have to add tests/util to the import path, which puts you pretty much back into the original situation, just with a different path (albeit perhaps a more logical cleaner path). -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
