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

[Petr Viktorin]

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³