Re: [Piglit] [PATCH] framework/backends/json: support non-piglit junit files

2016-10-26 Thread Dylan Baker
Quoting Mark Janes (2016-10-26 17:04:49)
> Dylan Baker  writes:
> 
> > Quoting Mark Janes (2016-10-26 16:30:12)
> >> The junit loader is unnecessarily strict with the input that it
> >> accepts.  It expects input generated by piglit, but can be made to
> >> handle junit from other test suites like crucible.
> >> ---
> >>  framework/backends/junit.py | 28 
> >>  1 file changed, 20 insertions(+), 8 deletions(-)
> >> 
> >> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> >> index 4c9b7af..ab14074 100644
> >> --- a/framework/backends/junit.py
> >> +++ b/framework/backends/junit.py
> >> @@ -361,14 +361,17 @@ def _load(results_file):
> >>  else:
> >>  run_result.name = 'junit result'
> >>  
> >> -tree = 
> >> etree.parse(results_file).getroot().find('.//testsuite[@name="piglit"]')
> >> +tree = etree.parse(results_file).getroot().find('.//testsuite')
> >>  for test in tree.iterfind('testcase'):
> >>  result = results.TestResult()
> >>  # Take the class name minus the 'piglit.' element, replace 
> >> junit's '.'
> >>  # separator with piglit's separator, and join the group and test 
> >> names
> >> -name = test.attrib['classname'].split('.', 1)[1]
> >> +name = test.attrib['name']
> >> +if 'classname' in test.attrib:
> >> +name = grouptools.join(test.attrib['classname'], name)
> >>  name = name.replace('.', grouptools.SEPARATOR)
> >> -name = grouptools.join(name, test.attrib['name'])
> >> +if name.startswith("piglit"):
> >> +name = name.split(grouptools.SEPARATOR, 1)[1]
> >
> > I assumed that grouptools.split could do this, but apparently not.
> >
> >>  
> >>  # Remove the trailing _ if they were added (such as to api and 
> >> search)
> >>  if name.endswith('_'):
> >> @@ -378,14 +381,23 @@ def _load(results_file):
> >>  
> >>  # This is the fallback path, we'll try to overwrite this with the 
> >> value
> >>  # in stderr
> >> -result.time = 
> >> results.TimeAttribute(end=float(test.attrib['time']))
> >> -result.err = test.find('system-err').text
> >> +result.time = results.TimeAttribute()
> >> +if 'time' in test.attrib:
> >> +result.time = 
> >> results.TimeAttribute(end=float(test.attrib['time']))
> >> +result.err = ""
> >
> > result.err is initialized to "", so no need to set it.
> >
> >> +syserr = test.find('system-err')
> >> +if syserr is not None:
> >> +result.err = syserr.text
> > 
> >>  
> >>  # The command is prepended to system-out, so we need to separate 
> >> those
> >>  # into two separate elements
> >> -out = test.find('system-out').text.split('\n')
> >> -result.command = out[0]
> >> -result.out = '\n'.join(out[1:])
> >> +out_tag = test.find('system-out')
> >
> >> +result.out = ""
> >> +result.command = ""
> >
> > These are initialized to "" as well.
> >
> >> +if out_tag is not None:
> >> +out = out_tag.text.split('\n')
> >> +result.command = out[0]
> >
> > This isn't right, if the suite generates stdout it is assumed that the 
> > command
> > will be the first line, but that isn't true unless piglit generates it. We
> > probably need to do like we do we do with stderr and have a "command: ..." 
> > and
> > search for that.
> 
> You are right.  We could use the earlier check for piglit as the first
> component in the name to conditionally take the first line as the
> command.  Is that acceptable?
> 
> Other suites will have an empty command in the field, which is still
> usable.

Yeah, that's probably fine.

> 
> >> +result.out = '\n'.join(out[1:])
> >>  
> >>  # Try to get the values in stderr for time and pid
> >>  for line in result.err.split('\n'):
> >> -- 
> >> 2.9.3
> >> 
> >
> > Dylan


signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] framework/backends/json: support non-piglit junit files

2016-10-26 Thread Mark Janes
Dylan Baker  writes:

> Quoting Mark Janes (2016-10-26 16:30:12)
>> The junit loader is unnecessarily strict with the input that it
>> accepts.  It expects input generated by piglit, but can be made to
>> handle junit from other test suites like crucible.
>> ---
>>  framework/backends/junit.py | 28 
>>  1 file changed, 20 insertions(+), 8 deletions(-)
>> 
>> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>> index 4c9b7af..ab14074 100644
>> --- a/framework/backends/junit.py
>> +++ b/framework/backends/junit.py
>> @@ -361,14 +361,17 @@ def _load(results_file):
>>  else:
>>  run_result.name = 'junit result'
>>  
>> -tree = 
>> etree.parse(results_file).getroot().find('.//testsuite[@name="piglit"]')
>> +tree = etree.parse(results_file).getroot().find('.//testsuite')
>>  for test in tree.iterfind('testcase'):
>>  result = results.TestResult()
>>  # Take the class name minus the 'piglit.' element, replace junit's 
>> '.'
>>  # separator with piglit's separator, and join the group and test 
>> names
>> -name = test.attrib['classname'].split('.', 1)[1]
>> +name = test.attrib['name']
>> +if 'classname' in test.attrib:
>> +name = grouptools.join(test.attrib['classname'], name)
>>  name = name.replace('.', grouptools.SEPARATOR)
>> -name = grouptools.join(name, test.attrib['name'])
>> +if name.startswith("piglit"):
>> +name = name.split(grouptools.SEPARATOR, 1)[1]
>
> I assumed that grouptools.split could do this, but apparently not.
>
>>  
>>  # Remove the trailing _ if they were added (such as to api and 
>> search)
>>  if name.endswith('_'):
>> @@ -378,14 +381,23 @@ def _load(results_file):
>>  
>>  # This is the fallback path, we'll try to overwrite this with the 
>> value
>>  # in stderr
>> -result.time = results.TimeAttribute(end=float(test.attrib['time']))
>> -result.err = test.find('system-err').text
>> +result.time = results.TimeAttribute()
>> +if 'time' in test.attrib:
>> +result.time = 
>> results.TimeAttribute(end=float(test.attrib['time']))
>> +result.err = ""
>
> result.err is initialized to "", so no need to set it.
>
>> +syserr = test.find('system-err')
>> +if syserr is not None:
>> +result.err = syserr.text
> 
>>  
>>  # The command is prepended to system-out, so we need to separate 
>> those
>>  # into two separate elements
>> -out = test.find('system-out').text.split('\n')
>> -result.command = out[0]
>> -result.out = '\n'.join(out[1:])
>> +out_tag = test.find('system-out')
>
>> +result.out = ""
>> +result.command = ""
>
> These are initialized to "" as well.
>
>> +if out_tag is not None:
>> +out = out_tag.text.split('\n')
>> +result.command = out[0]
>
> This isn't right, if the suite generates stdout it is assumed that the command
> will be the first line, but that isn't true unless piglit generates it. We
> probably need to do like we do we do with stderr and have a "command: ..." and
> search for that.

You are right.  We could use the earlier check for piglit as the first
component in the name to conditionally take the first line as the
command.  Is that acceptable?

Other suites will have an empty command in the field, which is still
usable.

>> +result.out = '\n'.join(out[1:])
>>  
>>  # Try to get the values in stderr for time and pid
>>  for line in result.err.split('\n'):
>> -- 
>> 2.9.3
>> 
>
> Dylan
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] framework/backends/json: support non-piglit junit files

2016-10-26 Thread Dylan Baker
Quoting Mark Janes (2016-10-26 16:30:12)
> The junit loader is unnecessarily strict with the input that it
> accepts.  It expects input generated by piglit, but can be made to
> handle junit from other test suites like crucible.
> ---
>  framework/backends/junit.py | 28 
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index 4c9b7af..ab14074 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -361,14 +361,17 @@ def _load(results_file):
>  else:
>  run_result.name = 'junit result'
>  
> -tree = 
> etree.parse(results_file).getroot().find('.//testsuite[@name="piglit"]')
> +tree = etree.parse(results_file).getroot().find('.//testsuite')
>  for test in tree.iterfind('testcase'):
>  result = results.TestResult()
>  # Take the class name minus the 'piglit.' element, replace junit's 
> '.'
>  # separator with piglit's separator, and join the group and test 
> names
> -name = test.attrib['classname'].split('.', 1)[1]
> +name = test.attrib['name']
> +if 'classname' in test.attrib:
> +name = grouptools.join(test.attrib['classname'], name)
>  name = name.replace('.', grouptools.SEPARATOR)
> -name = grouptools.join(name, test.attrib['name'])
> +if name.startswith("piglit"):
> +name = name.split(grouptools.SEPARATOR, 1)[1]

I assumed that grouptools.split could do this, but apparently not.

>  
>  # Remove the trailing _ if they were added (such as to api and 
> search)
>  if name.endswith('_'):
> @@ -378,14 +381,23 @@ def _load(results_file):
>  
>  # This is the fallback path, we'll try to overwrite this with the 
> value
>  # in stderr
> -result.time = results.TimeAttribute(end=float(test.attrib['time']))
> -result.err = test.find('system-err').text
> +result.time = results.TimeAttribute()
> +if 'time' in test.attrib:
> +result.time = 
> results.TimeAttribute(end=float(test.attrib['time']))
> +result.err = ""

result.err is initialized to "", so no need to set it.

> +syserr = test.find('system-err')
> +if syserr is not None:
> +result.err = syserr.text

>  
>  # The command is prepended to system-out, so we need to separate 
> those
>  # into two separate elements
> -out = test.find('system-out').text.split('\n')
> -result.command = out[0]
> -result.out = '\n'.join(out[1:])
> +out_tag = test.find('system-out')

> +result.out = ""
> +result.command = ""

These are initialized to "" as well.

> +if out_tag is not None:
> +out = out_tag.text.split('\n')
> +result.command = out[0]

This isn't right, if the suite generates stdout it is assumed that the command
will be the first line, but that isn't true unless piglit generates it. We
probably need to do like we do we do with stderr and have a "command: ..." and
search for that.

> +result.out = '\n'.join(out[1:])
>  
>  # Try to get the values in stderr for time and pid
>  for line in result.err.split('\n'):
> -- 
> 2.9.3
> 

Dylan


signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] framework/backends/json: support non-piglit junit files

2016-10-26 Thread Mark Janes
The junit loader is unnecessarily strict with the input that it
accepts.  It expects input generated by piglit, but can be made to
handle junit from other test suites like crucible.
---
 framework/backends/junit.py | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/framework/backends/junit.py b/framework/backends/junit.py
index 4c9b7af..ab14074 100644
--- a/framework/backends/junit.py
+++ b/framework/backends/junit.py
@@ -361,14 +361,17 @@ def _load(results_file):
 else:
 run_result.name = 'junit result'
 
-tree = 
etree.parse(results_file).getroot().find('.//testsuite[@name="piglit"]')
+tree = etree.parse(results_file).getroot().find('.//testsuite')
 for test in tree.iterfind('testcase'):
 result = results.TestResult()
 # Take the class name minus the 'piglit.' element, replace junit's '.'
 # separator with piglit's separator, and join the group and test names
-name = test.attrib['classname'].split('.', 1)[1]
+name = test.attrib['name']
+if 'classname' in test.attrib:
+name = grouptools.join(test.attrib['classname'], name)
 name = name.replace('.', grouptools.SEPARATOR)
-name = grouptools.join(name, test.attrib['name'])
+if name.startswith("piglit"):
+name = name.split(grouptools.SEPARATOR, 1)[1]
 
 # Remove the trailing _ if they were added (such as to api and search)
 if name.endswith('_'):
@@ -378,14 +381,23 @@ def _load(results_file):
 
 # This is the fallback path, we'll try to overwrite this with the value
 # in stderr
-result.time = results.TimeAttribute(end=float(test.attrib['time']))
-result.err = test.find('system-err').text
+result.time = results.TimeAttribute()
+if 'time' in test.attrib:
+result.time = results.TimeAttribute(end=float(test.attrib['time']))
+result.err = ""
+syserr = test.find('system-err')
+if syserr is not None:
+result.err = syserr.text
 
 # The command is prepended to system-out, so we need to separate those
 # into two separate elements
-out = test.find('system-out').text.split('\n')
-result.command = out[0]
-result.out = '\n'.join(out[1:])
+out_tag = test.find('system-out')
+result.out = ""
+result.command = ""
+if out_tag is not None:
+out = out_tag.text.split('\n')
+result.command = out[0]
+result.out = '\n'.join(out[1:])
 
 # Try to get the values in stderr for time and pid
 for line in result.err.split('\n'):
-- 
2.9.3

___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit