[Piglit] [PATCH] junit.py: Escape `api`/`search` in test's classname too.

[Jose Fonseca]

Thanks for the reviews. I fixed and pushed it.

Jose

On 28/04/15 18:53, Dylan Baker wrote:
> I have a few style comments, but otherwise this looks good, with the
> below addressed:
> Reviewed-by: Dylan Baker <baker.dylan.c at gmail.com>
>
> On Mon, Apr 27, 2015 at 03:44:19PM +0100, Jose Fonseca wrote:
>> If `api` appears in the middle of the test classname, like it happens
>> with all `spec/glsl-1.20/api/getactiveattrib *` tests, then Jenkins will
>> intercept it making it impossible to see individual tests results.
>> Therefore, just like the testname, it must be escaped.
>>
>> This escaping will affect the expected failure/crash test matching.  I
>> can't think of another solution though...
>> ---
>>   framework/backends/junit.py | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>> index 3602f9e..c6219e2 100644
>> --- a/framework/backends/junit.py
>> +++ b/framework/backends/junit.py
>> @@ -40,6 +40,15 @@ __all__ = [
>>   ]
>>
>>
>> +junitSpecialNames = ('api', 'search')
>
> If this is a constant it should be all caps, per PEP8; also, please add
> an additional newline here. Since this is an internal constant you might
> consider calling it '_JUNIT_SPECIAL_NAMES', which will hide it from
> export.
>
>> +
>> +def junitEscape(name):
>
> Per PEP8, function names should be all lower case with underscores to
> separate words. Could we change this to 'junit_escape(name)'?
>
>> +    name = name.replace('.', '_')
>> +    if name in junitSpecialNames:
>> +        name += '_'
>> +    return name
>> +
>> +
>>   class JUnitBackend(FileBackend):
>>       """ Backend that produces ANT JUnit XML
>>
>> @@ -161,8 +170,9 @@ class JUnitBackend(FileBackend):
>>           # classname), and replace piglits '/' separated groups with '.', after
>>           # replacing any '.' with '_' (so we don't get false groups).
>>           classname, testname = grouptools.splitname(name)
>> -        classname = classname.replace('.', '_')
>> -        classname = classname.replace(grouptools.SEPARATOR, '.')
>> +        classname = classname.split(grouptools.SEPARATOR)
>> +        classname = map(junitEscape, classname)
>
> map is discouraged in python, '[junit_escape(e) for e in classname]', is
> the more standard way.
>
>> +        classname = '.'.join(classname)
>>
>>           # Add the test to the piglit group rather than directly to the root
>>           # group, this allows piglit junit to be used in conjunction with other
>> @@ -177,7 +187,7 @@ class JUnitBackend(FileBackend):
>>           # The testname variable is used in the calculate_result
>>           # closure, and must not have the suffix appended.
>>           full_test_name = testname + self._test_suffix
>> -        if full_test_name in ('api', 'search'):
>> +        if full_test_name in junitSpecialNames:
>>               testname += '_'
>>               full_test_name = testname + self._test_suffix
>>
>> --
>> 2.1.0
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_piglit&d=AwIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=Qr8cKCgUdWrda1BTfc1iBpeV8cKt63at2MYaueURCyI&s=CTbF0f-r9lnLs5jFmucx4gM12P1kvPXJa6d1FJ5kMd4&e=