New patch attached which addresses Petr's review comments.
-- John Dennis <jden...@redhat.com> Looking to carve out IT costs? www.redhat.com/carveoutcosts/
>From 20fc57e1995aeab074cbb586187aedda4be5c1b0 Mon Sep 17 00:00:00 2001 From: John Dennis <jden...@redhat.com> Date: Fri, 23 Mar 2012 01:44:04 -0400 Subject: [PATCH 66-1] Replace broken i18n shell test with Python test Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit We had been using shell scripts and sed to test our translations. But trying to edit pot and po files with sed is nearly impossible because the file format can vary significantly and the sed editing was failing and gettext tools were complaining about our test strategy. We had been using a Python script (test_i18n.py) to perform the actual test after using shell, sed, and gettext tools to create the files. There is a Python library (polib) which can read/write/edit pot/po/mo files (used internally by Transifex, our translation portal). The strategy now is to do everything in Python (in test_i18n.py). This is easier, more robust and allows us to do more things. * add python-polib to BuildRequires * Remove the logic for creating the test lang from Makefile.in and replace it with calls to test_i18n.py * add argument parsing, usage, configuration parameters, etc. to test_i18n.py to make it easier to use and configurable. * add function to generate a test po and mo file. It also writes the files and creates the test directory structure. * Took the existing validate code and refactored it into validation function. It used to just pick one string and test it, now it iterates over all strings and all plural forms. * added support for plural forms. * Add pot po file validation for variable substitution --- freeipa.spec.in | 1 + install/po/Makefile.in | 12 +- install/po/test_i18n.py | 515 +++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 465 insertions(+), 63 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index 2db0f83..2259edf 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -72,6 +72,7 @@ BuildRequires: python-kerberos BuildRequires: python-rhsm BuildRequires: pyOpenSSL BuildRequires: pylint +BuildRequires: python-polib BuildRequires: libipa_hbac-python BuildRequires: python-memcached BuildRequires: sssd >= 1.8.0 diff --git a/install/po/Makefile.in b/install/po/Makefile.in index d267bea..65bdb23 100644 --- a/install/po/Makefile.in +++ b/install/po/Makefile.in @@ -192,15 +192,13 @@ maintainer-clean: distclean test_lang: rm -rf test.po test_locale - $(MSGINIT) --no-translator -i $(DOMAIN).pot -l en_US -o test.po - $(SED) -i -r -e 's/Language: en_US/Language: xh_ZA/' test.po - $(SED) -i -r -e 's/^msgstr[ \t]+"(..*)"[ \t]*$$/msgstr "\xe2\x86\x92\1\xe2\x86\x90"/' test.po - $(SED) -i -r -e 's/^msgstr[ \t]+"(.*)(\\n)(.)"[ \t]*$$/msgstr "\1\xe2\x86\x90\2"/' test.po - $(MKDIR_P) test_locale/xh_ZA/LC_MESSAGES - $(MSGFMT) -o test_locale/xh_ZA/LC_MESSAGES/ipa.mo test.po + ./test_i18n.py -c test: test_lang - ./test_i18n.py + ./test_i18n.py -t + +validate: + ./test_i18n.py -s $(DOMAIN).pot $(po_files) debug: @echo Python potfiles: diff --git a/install/po/test_i18n.py b/install/po/test_i18n.py index 0797dd7..e54c4f1 100755 --- a/install/po/test_i18n.py +++ b/install/po/test_i18n.py @@ -19,91 +19,494 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. # +import optparse import sys import gettext import locale import re import os +import traceback +import polib -def get_msgid(po_file): - 'Get the first non-empty msgid from the po file' - msgid_re = re.compile(r'^\s*msgid\s+"(.+)"\s*$') - f = open(po_file) - for line in f.readlines(): - match = msgid_re.search(line) - if match: - msgid = match.group(1) - f.close() - return msgid - f.close() - raise ValueError('No msgid found in %s' % po_file) - -# We test our translations by taking the original untranslated string -# (e.g. msgid) and prepend a prefix character and then append a suffix -# character. The test consists of asserting that the first character in the -# translated string is the prefix, the last character in the translated string -# is the suffix and the everything between the first and last character exactly -# matches the original msgid. -# -# We use unicode characters not in the ascii character set for the prefix and -# suffix to enhance the test. To make reading the translated string easier the -# prefix is the unicode right pointing arrow and the suffix left pointing arrow, -# thus the translated string looks like the original string enclosed in -# arrows. In ASCII art the string "foo" would render as: -# -->foo<-- +''' +We test our translations by taking the original untranslated string +(e.g. msgid) and prepend a prefix character and then append a suffix +character. The test consists of asserting that the first character in the +translated string is the prefix, the last character in the translated string +is the suffix and the everything between the first and last character exactly +matches the original msgid. + +We use unicode characters not in the ascii character set for the prefix and +suffix to enhance the test. To make reading the translated string easier the +prefix is the unicode right pointing arrow and the suffix left pointing arrow, +thus the translated string looks like the original string enclosed in +arrows. In ASCII art the string "foo" would render as: +-->foo<-- +''' + +#------------------------------------------------------------------------------- + +verbose = False +print_traceback = False +pedantic = False, # Unicode right pointing arrow prefix = u'\u2192' # utf-8 == '\xe2\x86\x92' # Unicode left pointing arrow suffix = u'\u2190' # utf-8 == '\xe2\x86\x90' -def main(): +page_width = 80 +section_seperator = '=' * page_width +entry_seperator = '-' * page_width + +#------------------------------------------------------------------------------- +# For efficiency compile these regexps just once +_substitution_regexps = [re.compile(r'%[srduoxf]\b'), # e.g. %s + re.compile(r'%\(\w+\)[srduoxf]\b'), # e.g. %(foo)s + re.compile(r'\$\w+'), # e.g. $foo + re.compile(r'\${\w+}'), # e.g. ${foo} + re.compile(r'\$\(\w+\)') # e.g. $(foo) + ] +# Python style substitution, e.g. %(foo)s +# where foo is the key and s is the format char +# group 1: whitespace between % and ( +# group 2: whitespace between ( and key +# group 3: whitespace between key and ) +# group 4: whitespace between ) and format char +# group 5: format char +_python_substitution_regexp = re.compile(r'%(\s*)\((\s*)\w+(\s*)\)(\s*)([srduoxf]\b)?') + +# Shell style substitution, e.g. $foo $(foo) ${foo} +# where foo is the variable +_shell_substitution_regexp = re.compile(r'\$(\s*)([({]?)(\s*)\w+(\s*)([)}]?)') +# group 1: whitespace between $ and delimiter +# group 2: begining delimiter +# group 3: whitespace between beginning delmiter and variable +# group 4: whitespace between variable and ending delimiter +# group 5: ending delimiter + +def validate_substitutions_match(s1, s2, s1_name='string1', s2_name='string2'): + ''' + Validate both s1 and s2 have the same number of substitution strings. + A substitution string would be something that looked like this: + + * %(foo)s + * $foo + * ${foo} + * $(foo) + + The substitutions may appear in any order in s1 and s2, however their + format must match exactly and the exact same number of each must exist + in both s1 and s2. + + A list of error diagnostics is returned explaining how s1 and s2 failed + the validation check. If the returned error list is empty then the + validation succeeded. + + :param s1: First string to validate + :param s2: First string to validate + :param s1_name: In diagnostic messages the name for s1 + :param s2_name: In diagnostic messages the name for s2 + :return: List of diagnostic error messages, if empty then success + ''' + errors = [] + + def get_subs(s): + ''' + Return a dict whoses keys are each unique substitution and whose + value is the count of how many times that substitution appeared. + ''' + subs = {} + for regexp in _substitution_regexps: + for match in regexp.finditer(s): + matched = match.group(0) + subs[matched] = subs.get(matched, 0) + 1 + return subs + + # Get the substitutions and their occurance counts + subs1 = get_subs(s1) + subs2 = get_subs(s2) + + # Form a set for each strings substitutions and + # do set subtraction and interesection + set1 = set(subs1.keys()) + set2 = set(subs2.keys()) + + missing1 = set2 - set1 + missing2 = set1 - set2 + common = set1 & set2 + + # Test for substitutions which are absent in either string + if missing1: + errors.append("The following substitutions are absent in %s: %s" % + (s1_name, ' '.join(missing1))) + + if missing2: + errors.append("The following substitutions are absent in %s: %s" % + (s2_name, ' '.join(missing2))) + + if pedantic: + # For the substitutions which are shared assure they occur an equal number of times + for sub in common: + if subs1[sub] != subs2[sub]: + errors.append("unequal occurances of '%s', %s has %d occurances, %s has %d occurances" % + (sub, s1_name, subs1[sub], s2_name, subs2[sub])) + + return errors + + +def validate_substitution_syntax(s, s_name='string'): + ''' + If s has one or more substitution variables then validate they + are syntactically correct. + A substitution string would be something that looked like this: + + * %(foo)s + * $foo + * ${foo} + * $(foo) + + A list of error diagnostics is returned explaining how s1 and s2 failed + the validation check. If the returned error list is empty then the + validation succeeded. + + :param s: String to validate + :param s_name: In diagnostic messages the name for s + :return: List of diagnostic error messages, if empty then success + ''' + errors = [] + + # Look for Python style substitutions, e.g. %(foo)s + for match in _python_substitution_regexp.finditer(s): + if match.group(1): + errors.append("%s has whitespace between %% and key in '%s'" % + (s_name, match.group(0))) + if match.group(2) or match.group(3): + errors.append("%s has whitespace next to key in '%s'" % + (s_name, match.group(0))) + if match.group(4): + errors.append("%s has whitespace between key and format character in '%s'" % + (s_name, match.group(0))) + if not match.group(5): + errors.append("%s has no format character in '%s'" % + (s_name, match.group(0))) + + # Look for shell style substitutions, e.g. $foo $(foo) ${foo} + for match in _shell_substitution_regexp.finditer(s): + if match.group(1): + errors.append("%s has whitespace between $ and variable in '%s'" % + (s_name, match.group(0))) + if match.group(3) or (match.group(4) and match.group(5)): + errors.append("%s has whitespace next to variable in '%s'" % + (s_name, match.group(0))) + + beg_delimiter = match.group(2) + end_delimiter = match.group(5) + matched_delimiters = {'' : '', '(' : ')', '{' : '}'} + if beg_delimiter is not None or end_delimiter is not None: + if matched_delimiters[beg_delimiter] != end_delimiter: + errors.append("%s variable delimiters do not match in '%s', begin delimiter='%s' end delimiter='%s'" % + (s_name, match.group(0), beg_delimiter, end_delimiter)) + + return errors + + +def validate_substitutions(file_path): + ''' + Given a pot or po file scan all it's entries looking for problems + with variable substitutions. See the following functions for + details on how the validation is performed. + + * validate_substitutions_match() + * validate_substitution_syntax() + + Returns the number of entries with errors. + ''' + + print section_seperator + print "validating %s" % (file_path) + po = polib.pofile(file_path) + + n_entries_with_errors = 0 + for entry in po: + total_errors = [] + msgid = entry.msgid + msgstr = entry.msgstr + have_msgid = msgid.strip() != '' + have_msgstr = msgstr.strip() != '' + if have_msgid and have_msgstr: + errors = validate_substitutions_match(msgid, msgstr, 'msgid', 'msgstr') + total_errors.extend(errors) + if have_msgid: + errors = validate_substitution_syntax(msgid, 'msgid') + total_errors.extend(errors) + if have_msgstr: + errors = validate_substitution_syntax(msgstr, 'msgstr') + total_errors.extend(errors) + if total_errors: + print entry_seperator + print 'locations: %s' % (', '.join(["%s:%d" % (x[0], int(x[1])) for x in entry.occurrences])) + print '\n'.join(total_errors) + n_entries_with_errors += 1 + print entry_seperator + print "%d entries with errors (%s)" % (n_entries_with_errors, file_path) + return n_entries_with_errors + + +#---------------------------------------------------------------------- +def create_po(pot_file, po_file, mo_file): + po = polib.pofile(pot_file) + + # Update the metadata in the po file header + # It's case insensitive so search the keys in a case insensitive manner + # + # We need to update the Plural-Forms otherwise gettext.py will raise the + # following error: + # + # raise ValueError, 'plural forms expression could be dangerous' + # + # It is demanding the rhs of plural= only contains the identifer 'n' + + for k,v in po.metadata.items(): + if k.lower() == 'plural-forms': + po.metadata[k] = 'nplurals=2; plural=(n != 1)' + break + + + # Iterate over all msgid's and form a the msgstr by prepending + # the prefix and appending the suffix + for entry in po: + if entry.msgid_plural: + entry.msgstr_plural = {0: prefix + entry.msgid + suffix, + 1: prefix + entry.msgid_plural + suffix} + else: + entry.msgstr = prefix + entry.msgid + suffix + + # Write out the po and mo files + po.save(po_file) + print "Wrote: %s" % (po_file) - test_file = 'test.po' + po.save_as_mofile(mo_file) + print "Wrote: %s" % (mo_file) + +#---------------------------------------------------------------------- + +def validate_unicode_edit(msgid, msgstr): + # Verify the first character is the test prefix + if msgstr[0] != prefix: + raise ValueError('First char in translated string "%s" not equal to prefix "%s"' % + (msgstr.encode('utf-8'), prefix.encode('utf-8'))) + + # Verify the last character is the test suffix + if msgstr[-1] != suffix: + raise ValueError('Last char in translated string "%s" not equal to suffix "%s"' % + (msgstr.encode('utf-8'), suffix.encode('utf-8'))) + + # Verify everything between the first and last character is the + # original untranslated string + if msgstr[1:-1] != msgid: + raise ValueError('Translated string "%s" minus the first & last character is not equal to msgid "%s"' % + (msgstr.encode('utf-8'), msgid)) + + if verbose: + msg = 'Success: message string "%s" maps to translated string "%s"' % (msgid, msgstr) + print msg.encode('utf-8') + + +def test_translations(po_file, lang, domain, locale): try: - # The test installs the test message catalog under the xh_ZA (e.g. Zambia Xhosa) - # language. 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'] = 'xh_ZA' + # 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 + + # 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) + + # Iterate over the msgid's + po = polib.pofile(po_file) - # Tell gettext that our domain is 'ipa', that locale_dir is 'test_locale' - # (i.e. where to look for the message catalog) and that we want the translations - # returned as unicode from the _() function - gettext.install('ipa', 'test_locale', unicode=1) + n_entries = 0 + n_translations = 0 + n_valid = 0 + n_fail = 0 + for entry in po: + if entry.msgid_plural: + msgid = entry.msgid + msgid_plural = entry.msgid_plural + msgstr = t.ungettext(msgid, msgid_plural, 1) + msgstr_plural = t.ungettext(msgid, msgid_plural, 2) - # We need a translatable string to test with, read one from the test po file - msgid = get_msgid(test_file) + try: + n_translations += 1 + validate_unicode_edit(msgid, msgstr) + n_valid += 1 + except Exception, e: + n_fail += 1 + if print_traceback: + traceback.print_exc() + print >> sys.stderr, "ERROR: %s" % e - print "Using message string \"%s\" found in file \"%s\"" % (msgid, test_file) + try: + n_translations += 1 + validate_unicode_edit(msgid_plural, msgstr_plural) + n_valid += 1 + except Exception, e: + n_fail += 1 + if print_traceback: + traceback.print_exc() + print >> sys.stderr, "ERROR: %s" % e - # Get the translated version of the msgid string by invoking _() - translated = _(msgid) #pylint: disable=E0602 - # Verify the first character is the test prefix - if translated[0] != prefix: - raise ValueError("First char in translated string \"%s\" not equal to prefix \"%s\"" % \ - (translated.encode('utf-8'), prefix.encode('utf-8'))) + else: + msgid = entry.msgid + msgstr = t.ugettext(msgid) - # Verify the last character is the test suffix - if translated[-1] != suffix: - raise ValueError("Last char in translated string \"%s\" not equal to suffix \"%s\"" % \ - (translated.encode('utf-8'), suffix.encode('utf-8'))) + try: + n_translations += 1 + validate_unicode_edit(msgid, msgstr) + n_valid += 1 + except Exception, e: + n_fail += 1 + if print_traceback: + traceback.print_exc() + print >> sys.stderr, "ERROR: %s" % e - # Verify everything between the first and last character is the - # original untranslated string - if translated[1:-1] != msgid: - raise ValueError("Translated string \"%s\" minus the first & last character is not equal to msgid \"%s\"" % \ - (translated.encode('utf-8'), msgid)) + n_entries += 1 - print "Success: message string \"%s\" maps to translated string \"%s\"" % (msgid, _(msgid).encode('utf-8')) #pylint: disable=E0602 except Exception, e: + if print_traceback: + traceback.print_exc() print >> sys.stderr, "ERROR: %s" % e return 1 + if not n_entries: + print >> sys.stderr, "ERROR: no translations found in %s" % (po_filename) + return 1 + + if n_fail: + print >> sys.stderr, "ERROR: %d failures out of %d translations" % (n_fail, n_entries) + return 1 + + print "%d translations in %d messages successfully tested" % (n_translations, n_entries) return 0 +#---------------------------------------------------------------------- + +usage ='''\ +%prog [hvT] ct +%prog [hvT] s file1 [file2 ...] + +Examples: + +# create test files +%prog -c + +# run test using test files +%prog -t + +# create tests files and run test +%prog -c -t + +# validate variable substitutions in pot file +%prog -s ipa.pot +''' + +def main(): + parser = optparse.OptionParser(usage=usage) + parser.add_option('-c', '--create', action='store_true', dest='create_po', default=False, + help='create the test translation file(s)') + parser.add_option('-t', '--test', action='store_true', dest='test_translations', default=False, + help='perform test using test file(s)') + parser.add_option('-s', '--substitutions', action='store_true', dest='validate_substitutions', default=False, + help='validate variable substitutions in file(s)') + + parser.add_option('-v', '--verbose', action='store_true', dest='verbose', default=False, + help='be informative') + parser.add_option('--traceback', action='store_true', dest='print_traceback', default=False, + help='print the traceback when an exception occurs') + parser.add_option('--pedantic', action='store_true', dest='pedantic', default=False, + help='be aggressive when validating') + + parser.add_option('--test_lang', action='store', dest='test_lang', default='test', + help="test po file uses this as it's basename (default=test)") + parser.add_option('--lang', action='store', dest='lang', default='xh_ZA', + help='lang used for locale, MUST be a valid lang (default=xh_ZA)') + parser.add_option('--domain', action='store', dest='domain', default='ipa', + help='translation domain used during test (default=ipa)') + parser.add_option('--locale', action='store', dest='locale', default='test_locale', + help='locale used during test (default=test_locale)') + parser.add_option('--pot_file', action='store', dest='pot_file', default='ipa.pot', + help='input pot file, used to generate test po and mo files (default=ipa.pot)') + + options, args = parser.parse_args() + + verbose = options.verbose + print_traceback = options.print_traceback + pedantic = options.pedantic + + if options.validate_substitutions: + if options.create_po or options.test_translations: + print >>sys.stderr, "ERROR: you may not select -c or -t when validating substitutions" + return 1 + files = args + total_errors = 0 + for f in files: + n_errors = validate_substitutions(f) + total_errors += n_errors + if total_errors: + print "%d errors in %d files" % (total_errors, len(files)) + return 1 + else: + return 0 + + if not (options.create_po or options.test_translations): + print >>sys.stderr, "ERROR: must select -c or -t or both" + parser.print_usage() + return 1 + + if options.create_po: + po_file = '%s.po' % options.test_lang + pot_file = options.pot_file + + msg_dir = os.path.join(options.locale, options.lang, 'LC_MESSAGES') + if not os.path.exists(msg_dir): + os.makedirs(msg_dir) + + mo_basename = '%s.mo' % options.domain + mo_file = os.path.join(msg_dir, mo_basename) + + create_po(pot_file, po_file, mo_file) + + if options.test_translations: + po_file = '%s.po' % options.test_lang + + # 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 :) + + lang = options.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. + + domain = options.domain + locale = options.locale + + return test_translations(po_file, lang, domain, locale) + if __name__ == "__main__": sys.exit(main()) -- 1.7.7.6
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
