[Freeipa-devel] [PATCH 66] Replace broken i18n shell test with Python test

[John Dennis]

Attached is is a new patch addressing Petr's review comment as well as 
some sample output for illustration purposes (I suggest you review the 
example.txt).

Rather than wading through a log of dialog in the previous emails let me 
address Petr's issues and how I addressed them.

The option parsing is totally reworked (see example.txt). Not only is 
optparse being used, but the options have been simplified and appear in 
groups. The script can do a variety of different things so you must 
specify the mode you want it to run in. I don't see an obvious candidate 
for a default mode, plus most users will run it as a make target anyway 
obviating the need to specify any command line options.

I logically divided the validation checks, one for checking the English 
source strings (pot file) and one for checking the translations (po 
files). These represent two of the three possible modes of operations.

Petr's has said several times that he would prefer the English sources 
strings be checked by other tests. I presume he is referring to the unit 
tests, but we do not have any such unit test nor are we likely to (at 
least not for string validation). The reason is you have to have 
available the set of strings from the sources which have been marked for 
translation. It's a somewhat complicated process to produce that set of 
strings and that logic are part of the install/po area, namely the 
generation of the pot file. This is the only logical place I can see for 
performing tests and/or validation on (translatable) source strings. I 
do not envision the validation ever being part of a unit test, it is 
functionally very different. There is a good argument for generating a 
new pot file and validating it on every code check-in. This patch does 
not do that, instead it's done when the pot file is generated manually, 
but it would be trival to add that feature once this framework is in place.

However a good case could be made for the gettext test currently 
implemented in test_i18n.py being invoked as a unit test but there would 
be some structural work needed to make that happen (e.g. assuring the 
test mo file is generated and pointed to and calling things through IPA 
Gettext classes and having the framework run in the context of that 
locale). But we've never had a failure in that area yet so I'd be 
inclined to defer that at the moment, open a new ticket to integrate the 
gettext test in test_i18n.py with the test_text.py in the unit tests and 
consider that a separate work item so as not let it interfere with 
getting this committed. Being able to run the gettext test in the 
translation directory is valuable none-the-less. It would be better if 
it were fully integrated with our Gettext classes in text.py but that is 
best left as another task.

By dividing the validation checks between the pot file and po files I 
think I addressed the majority of Petr's concerns over the validation of 
source strings. Petr also alluded Transifex errors as if Transifex would 
somehow catch the translation problems, but no such error checking 
exists (actually I'm considering giving this code to upstream Transifex, 
they're entirely Python based and they might find parts of it useful)

But wait there's more ...

Some of the validations were producing false positives because the 
validator could not determine whether some uses of $ and % were valid or 
mangled syntax. So I added a pedantic flag. It's off by default and 
skips checking for errors which are unlikely to be present and are 
difficult to check. You can run the validation with --pedantic as a 
safeguard and interpret the results. I thought this was preferred to 
removing the ability to perform the checks whatsoever.

I also added a --show-strings flag so along with a error message and 
location it will output the offending string(s). This if off by default 
for brevity purposes but is useful for those having to make fixes or 
just to better understand the nature of the error.

I tried to make the error output a bit less verbose with better formatting.

I updated .gitignore to ignore the generated test files.

-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: example.txt
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120323/ad7f155a/attachment.txt>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jdennis-0066-2-Replace-broken-i18n-shell-test-with-Python-test.patch
Type: text/x-patch
Size: 31792 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120323/ad7f155a/attachment.bin>