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

John Dennis jdennis at redhat.com
Mon Apr 9 15:24:23 UTC 2012


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.


-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jdennis-0068-3-text-unit-test-should-validate-using-installed-mo-fi.patch
Type: text/x-patch
Size: 65725 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120409/b1b9c44e/attachment.bin>


More information about the Freeipa-devel mailing list