[Piglit] [PATCH] junit.py: Fix handling of special test names.

[Jose Fonseca]

On 21/02/15 02:00, Dylan Baker wrote:
> This looks fine to me, and fixes our problem.
>
> Jose, is this going to break your setup?
>
> Assuming Jose says that is good:
> Reviewed-by: Dylan Baker <baker.dylan.c at gmail.com>
>
> On Fri, Feb 20, 2015 at 05:51:34PM -0800, Mark Janes wrote:
>> In junit.py, the testname variable is used by a closure within the
>> write_test() scope.  Modifying it breaks the filtering of expected
>> failures.

That's subtle. I did search whether testname were used elsewhere, but 
missed this.

>> ---
>>   framework/backends/junit.py | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>> index ddaf826..d4b5041 100644
>> --- a/framework/backends/junit.py
>> +++ b/framework/backends/junit.py
>> @@ -166,16 +166,17 @@ class JUnitBackend(FileBackend):
>>           # set different root names.
>>           classname = 'piglit.' + classname
>>
>> -        testname += self._test_suffix
>> -
>>           # Jenkins will display special pages when the test has certain names.
>>           # https://urldefense.proofpoint.com/v2/url?u=https-3A__jenkins-2Dci.org_issue_18062&d=AwIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=seHXT-vNhxoMsHbvPnVAJ13Oa_dx281dugC1MaN1uew&s=1af7XzZZFlcd2-04Wal262nqO5o9N3URGyOSLdDI7mA&e=
>>           # https://urldefense.proofpoint.com/v2/url?u=https-3A__jenkins-2Dci.org_issue_19810&d=AwIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=seHXT-vNhxoMsHbvPnVAJ13Oa_dx281dugC1MaN1uew&s=jtIsqFvKdOW8zjnk3uXyHSfW0EG-Jk40y45JjTb7EBU&e=
>> -        if testname in ('api', 'search'):
>> +        # If there is a suffix, then the test link will not be one of
>> +        # the reserved names.  Don't modify testname, as it is used in
>> +        # the calculate_result closure.
>> +        if not self._test_suffix and testname in ('api', 'search'):

Maybe

   if (testname + self._test_suffix) in  ('api', 'search'):

or use a different variable for the `testname + self._test_suffix` 
sub-expression.

Either way,

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>




>>               testname += '_'
>>
>>           # Create the root element
>> -        element = etree.Element('testcase', name=testname,
>> +        element = etree.Element('testcase', name=testname + self._test_suffix,
>>                                   classname=classname,
>>                                   time=str(data['time']),
>>                                   status=str(data['result']))
>> --
>> 2.1.4
>>