[Piglit] [PATCH 2/2] framework/backends/junit.py: Handle tests with subtests as testsuite elements
Nanley Chery
nanleychery at gmail.com
Fri Dec 4 16:37:17 PST 2015
On Fri, Dec 04, 2015 at 04:21:00PM -0800, baker.dylan.c at gmail.com wrote:
> From: Dylan Baker <baker.dylan.c at gmail.com>
>
> Currently the JUnit backend has no way to represent subtests in such a
> way that they can be understood by jenkins and by the summary tools.
>
> Mark, Nanley and myself consulted and came up with several approaches,
> each with serious drawbacks:
> 1. Print the subtest statuses into stdout or stderr nodes in the JUnit.
> This has the advantage of being simple, but has a problem with being
> shadowed by an expected-<status>. If subtest A fails, an expected
> fail can be entered. If subtest B also starts failing, no one will
> notice. This wont work
> 2. Treat each subtest as a full test, and the test as a group. I have
> two reservations about this approach. It's different than the JSON
> for one, and there isn't a good way to turn the JUnit back into the
> piglit internal representation using this approach, which would make
> running JUnit results through the piglit status tools difficult. This
> would also massively inflate the size of the JSON results, and that's
> already becoming a problem for us.
> 3. Create a main test entry, and then subtest entries as well, which
> pointed back to the original test as the parent in their stdout. This
> also has shadowing problems, and would still make the XML very large.
>
> The final approach taken was suggested by Nanely, to turn tests with
s/Nanely/Nanley/
Thanks for working on this!
> subtests into a testsuite element, which could represent the shared
> values (stdout, stderr), and could hold individual testcases elements
> for each subtest. This solves the shadowing issue, and introduces less
> file size increase than the other ideas floated.
>
> Also adds test for the new feature.
>
> cc: mark.a.janes at intel.com
> cc: jfonseca at vmware.com
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
> framework/backends/junit.py | 174 +++++++++++++++++++++------
> framework/tests/junit_backends_tests.py | 206 +++++++++++++++++++++++++++++++-
> 2 files changed, 343 insertions(+), 37 deletions(-)
>
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index 34df300..329fc4b 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -117,8 +117,6 @@ class JUnitBackend(FileBackend):
>
> shutil.rmtree(os.path.join(self._dest, 'tests'))
>
> -
> -
> def _write(self, f, name, data):
> # Split the name of the test and the group (what junit refers to as
> # classname), and replace piglits '/' separated groups with '.', after
> @@ -135,11 +133,41 @@ class JUnitBackend(FileBackend):
> # set different root names.
> classname = 'piglit.' + classname
>
> - element = self.__make_case(testname, classname, data)
> + if data.subtests:
> + # If there are subtests treat the test as a suite instead of a
> + # test, set system-out, system-err, and time on the suite rather
> + # than on the testcase
> + name='{}.{}'.format(classname, testname)
> + element = etree.Element(
> + 'testsuite',
> + name=name,
> + time=str(data.time.total))
> +
> + out = etree.SubElement(element, 'system-out')
> + out.text = data.command + '\n' + data.out
> + err = etree.SubElement(element, 'system-err')
> + err.text = data.err
> + err.text += '\n\nstart time: {}\nend time: {}\n'.format(
> + data.time.start, data.time.end)
> +
> + for name, result in data.subtests.iteritems():
> + sub = self.__make_subcase(name, result, err)
> + out = etree.SubElement(sub, 'system-out')
> + out.text = 'I am a subtest of {}'.format(name)
> + element.append(sub)
> +
> + for attrib, xpath in [('failures', './/testcase/failure'),
> + ('errors', './/testcase/error'),
> + ('skipped', './/testcase/skipped'),
> + ('tests', './/testcase')]:
> + element.attrib[attrib] = str(len(element.findall(xpath)))
> +
> + else:
> + element = self.__make_case(testname, classname, data)
> +
> f.write(etree.tostring(element))
>
> - def __make_case(self, testname, classname, data):
> - """Create a test case element and return it."""
> + def __make_name(self, testname):
> # Jenkins will display special pages when the test has certain names,
> # so add '_' so the tests don't match those names
> # https://jenkins-ci.org/issue/18062
> @@ -148,17 +176,68 @@ class JUnitBackend(FileBackend):
> if full_test_name in _JUNIT_SPECIAL_NAMES:
> testname += '_'
> full_test_name = testname + self._test_suffix
> + return full_test_name
> +
> + def __make_subcase(self, testname, result, err):
> + """Create a <testcase> element for subtests.
> +
> + This method is used to create a <testcase> element to nest inside of a
> + <testsuite> element when that element represents a test with subtests.
> + This differs from __make_case in that it doesn't add as much metadata
> + to the <testcase>, since that was attached to the <testsuite> by
> + _write, and that it doesn't handle incomplete cases, since subtests
> + cannot have incomplete as a status (though that could change).
> +
> + """
> + full_test_name = self.__make_name(testname)
> + element = etree.Element('testcase',
> + name=full_test_name,
> + status=str(result))
> +
> + # replace special characters and make case insensitive
> + lname = self.__normalize_name(testname)
> +
> + expected_result = "pass"
> +
> + if lname in self._expected_failures:
> + expected_result = "failure"
> + # a test can either fail or crash, but not both
> + assert lname not in self._expected_crashes
> +
> + if lname in self._expected_crashes:
> + expected_result = "error"
> +
> + self.__add_result(element, result, err, expected_result)
> +
> + return element
> +
> + def __make_case(self, testname, classname, data):
> + """Create a <testcase> element and return it.
> +
> + Specifically, this is used to create "normal" test case, one that
> + doesn't contain any subtests. __make_subcase is used to create a
> + <testcase> which belongs inside a nested <testsuite> node.
> +
> + Arguments:
> + testname -- the name of the test
> + classname -- the name of the group (to use piglit terminology)
> + data -- A TestResult instance
> +
> + """
> + full_test_name = self.__make_name(testname)
>
> # Create the root element
> - element = etree.Element('testcase', name=full_test_name,
> + element = etree.Element('testcase',
> + name=full_test_name,
> classname=classname,
> - # Incomplete will not have a time.
> time=str(data.time.total),
> status=str(data.result))
>
> # If this is an incomplete status then none of these values will be
> # available, nor
> if data.result != 'incomplete':
> + expected_result = "pass"
> +
> # Add stdout
> out = etree.SubElement(element, 'system-out')
> out.text = data.out
> @@ -171,7 +250,8 @@ class JUnitBackend(FileBackend):
> err.text = data.err
> err.text += '\n\nstart time: {}\nend time: {}\n'.format(
> data.time.start, data.time.end)
> - expected_result = "pass"
> +
> + element.extend([err, out])
>
> # replace special characters and make case insensitive
> lname = self.__normalize_name(classname, testname)
> @@ -184,29 +264,34 @@ class JUnitBackend(FileBackend):
> if lname in self._expected_crashes:
> expected_result = "error"
>
> - self.__add_result(element, data, err, expected_result)
> + self.__add_result(element, data.result, err, expected_result)
> else:
> etree.SubElement(element, 'failure', message='Incomplete run.')
>
> return element
>
> @staticmethod
> - def __normalize_name(classname, testname):
> - name = (classname + "." + testname).lower()
> + def __normalize_name(testname, classname=None):
> + """Nomralize the test name to what is stored in the expected statuses.
> + """
> + if classname is not None:
> + name = (classname + "." + testname).lower()
> + else:
> + name = testname.lower()
> name = name.replace("=", ".")
> name = name.replace(":", ".")
> return name
>
> @staticmethod
> - def __add_result(element, data, err, expected_result):
> - """Add a <skipped>, <failure>, or <error> if necissary."""
> + def __add_result(element, result, err, expected_result):
> + """Add a <skipped>, <failure>, or <error> if necessary."""
> res = None
> # Add relevant result value, if the result is pass then it doesn't
> # need one of these statuses
> - if data.result == 'skip':
> + if result == 'skip':
> res = etree.SubElement(element, 'skipped')
>
> - elif data.result in ['fail', 'dmesg-warn', 'dmesg-fail']:
> + elif result in ['fail', 'dmesg-warn', 'dmesg-fail']:
> if expected_result == "failure":
> err.text += "\n\nWARN: passing test as an expected failure"
> res = etree.SubElement(element, 'skipped',
> @@ -214,7 +299,7 @@ class JUnitBackend(FileBackend):
> else:
> res = etree.SubElement(element, 'failure')
>
> - elif data.result == 'crash':
> + elif result == 'crash':
> if expected_result == "error":
> err.text += "\n\nWARN: passing test as an expected crash"
> res = etree.SubElement(element, 'skipped',
> @@ -229,7 +314,7 @@ class JUnitBackend(FileBackend):
>
> # Add the piglit type to the failure result
> if res is not None:
> - res.attrib['type'] = str(data.result)
> + res.attrib['type'] = str(result)
>
>
> def _load(results_file):
> @@ -242,6 +327,27 @@ def _load(results_file):
> JUnit document.
>
> """
> + def populate_result(result, test):
> + # 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
> +
> + # 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:])
> +
> + # Try to get the values in stderr for time
> + if 'time start' in result.err:
> + for line in result.err.split('\n'):
> + if line.startswith('time start:'):
> + result.time.start = float(line[len('time start: '):])
> + elif line.startswith('time end:'):
> + result.time.end = float(line[len('time end: '):])
> + break
> +
> run_result = results.TestrunResult()
>
> splitpath = os.path.splitext(results_file)[0].split(os.path.sep)
> @@ -267,25 +373,25 @@ def _load(results_file):
>
> result.result = test.attrib['status']
>
> - # 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
> + populate_result(result, test)
>
> - # 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:])
> + run_result.tests[name] = result
>
> - # Try to get the values in stderr for time
> - if 'time start' in result.err:
> - for line in result.err.split('\n'):
> - if line.startswith('time start:'):
> - result.time.start = float(line[len('time start: '):])
> - elif line.startswith('time end:'):
> - result.time.end = float(line[len('time end: '):])
> - break
> + for test in tree.iterfind('testsuite'):
> + 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['name'].split('.', 1)[1]
> + name = name.replace('.', grouptools.SEPARATOR)
> +
> + # Remove the trailing _ if they were added (such as to api and search)
> + if name.endswith('_'):
> + name = name[:-1]
> +
> + populate_result(result, test)
> +
> + for subtest in test.iterfind('testcase'):
> + result.subtests[subtest.attrib['name']] = subtest.attrib['status']
>
> run_result.tests[name] = result
>
> diff --git a/framework/tests/junit_backends_tests.py b/framework/tests/junit_backends_tests.py
> index 7d5a3fc..ae18e3e 100644
> --- a/framework/tests/junit_backends_tests.py
> +++ b/framework/tests/junit_backends_tests.py
> @@ -29,6 +29,7 @@ try:
> from lxml import etree
> except ImportError:
> import xml.etree.cElementTree as etree
> +import mock
> import nose.tools as nt
> from nose.plugins.skip import SkipTest
>
> @@ -44,7 +45,7 @@ doc_formatter = utils.DocFormatter({'separator': grouptools.SEPARATOR})
> _XML = """\
> <?xml version='1.0' encoding='utf-8'?>
> <testsuites>
> - <testsuite name="piglit" tests="1">
> + <testsuite name="piglit" tests="5">
> <testcase classname="piglit.foo.bar" name="a-test" status="pass" time="1.12345">
> <system-out>this/is/a/command\nThis is stdout</system-out>
> <system-err>this is stderr
> @@ -53,6 +54,24 @@ time start: 1.0
> time end: 4.5
> </system-err>
> </testcase>
> + <testsuite name="piglit.bar" time="1.234" tests="4" failures="1" skipped="1" errors="1">
> + <system-err>this is stderr
> +
> +time start: 1.0
> +time end: 4.5
> +</system-err>
> + <system-out>this/is/a/command\nThis is stdout</system-out>
> + <testcase name="subtest1" status="pass"/>
> + <testcase name="subtest2" status="fail">
> + <failed/>
> + </testcase>
> + <testcase name="subtest3" status="crash">
> + <error/>
> + </testcase>
> + <testcase name="subtest4" status="skip">
> + <skipped/>
> + </testcase>
> + </testsuite>
> </testsuite>
> </testsuites>
> """
> @@ -203,11 +222,12 @@ class TestJUnitLoad(utils.StaticDirectory):
> def setup_class(cls):
> super(TestJUnitLoad, cls).setup_class()
> cls.xml_file = os.path.join(cls.tdir, 'results.xml')
> -
> +
> with open(cls.xml_file, 'w') as f:
> f.write(_XML)
>
> cls.testname = grouptools.join('foo', 'bar', 'a-test')
> + cls.subtestname = 'bar'
>
> @classmethod
> def xml(cls):
> @@ -270,7 +290,6 @@ class TestJUnitLoad(utils.StaticDirectory):
> """backends.junit._load: Totals are calculated."""
> nt.ok_(bool(self.xml()))
>
> -
> @utils.no_error
> def test_load_file(self):
> """backends.junit.load: Loads a file directly"""
> @@ -281,6 +300,48 @@ class TestJUnitLoad(utils.StaticDirectory):
> """backends.junit.load: Loads a directory"""
> backends.junit.REGISTRY.load(self.tdir, 'none')
>
> + def test_subtest_added(self):
> + """backends.junit._load: turns secondlevel <testsuite> into test with stubtests"""
> + xml = self.xml()
> + nt.assert_in(self.subtestname, xml.tests)
> +
> + def test_subtest_time(self):
> + """backends.junit._load: handles time from subtest"""
> + time = self.xml().tests[self.subtestname].time
> + nt.assert_is_instance(time, results.TimeAttribute)
> + nt.eq_(time.start, 1.0)
> + nt.eq_(time.end, 4.5)
> +
> + def test_subtest_out(self):
> + """backends.junit._load: subtest stderr is loaded correctly"""
> + test = self.xml().tests[self.subtestname].out
> + nt.eq_(test, 'This is stdout')
> +
> + def test_subtest_err(self):
> + """backends.junit._load: stderr is loaded correctly."""
> + test = self.xml().tests[self.subtestname].err
> + nt.eq_(test, 'this is stderr\n\ntime start: 1.0\ntime end: 4.5\n')
> +
> + def test_subtest_statuses(self):
> + """backends.juint._load: subtest statuses are restored correctly
> +
> + This is not implemented as separate tests or a generator becuase while
> + it asserts multiple values, it is testing one peice of funcitonality:
> + whether the subtests are restored correctly.
> +
> + """
> + test = self.xml().tests[self.subtestname]
> +
> + subtests = [
> + ('subtest1', 'pass'),
> + ('subtest2', 'fail'),
> + ('subtest3', 'crash'),
> + ('subtest4', 'skip'),
> + ]
> +
> + for name, value in subtests:
> + nt.eq_(test.subtests[name], value)
> +
>
> def test_load_file_name():
> """backends.junit._load: uses the filename for name if filename != 'results'
> @@ -319,3 +380,142 @@ def test_load_default_name():
> test = backends.junit.REGISTRY.load(filename, 'none')
>
> nt.assert_equal(test.name, 'junit result')
> +
> +
> +class TestJunitSubtestWriting(object):
> + """Tests for Junit subtest writing.
> +
> + Junit needs to write out subtests as full tests, so jenkins will consume
> + them correctly.
> +
> + """
> + __patchers = [
> + mock.patch('framework.backends.abstract.shutil.move', mock.Mock()),
> + ]
> +
> + @staticmethod
> + def _make_result():
> + result = results.TestResult()
> + result.time.end = 1.2345
> + result.result = 'pass'
> + result.out = 'this is stdout'
> + result.err = 'this is stderr'
> + result.command = 'foo'
> + result.subtests['foo'] = 'skip'
> + result.subtests['bar'] = 'fail'
> + result.subtests['oink'] = 'crash'
> +
> + test = backends.junit.JUnitBackend('foo')
> + mock_open = mock.mock_open()
> + with mock.patch('framework.backends.abstract.open', mock_open):
> + with test.write_test(grouptools.join('a', 'group', 'test1')) as t:
> + t(result)
> +
> + # Return an xml object
> + # This seems pretty fragile, but I don't see a better way to get waht
> + # we want
> + return etree.fromstring(mock_open.mock_calls[-3][1][0])
> +
> + @classmethod
> + def setup_class(cls):
> + for p in cls.__patchers:
> + p.start()
> +
> + cls.output = cls._make_result()
> +
> + @classmethod
> + def teardown_class(cls):
> + for p in cls.__patchers:
> + p.stop()
> +
> + def test_suite(self):
> + """backends.junit.JUnitBackend.write_test: wraps the cases in a suite"""
> + nt.eq_(self.output.tag, 'testsuite')
> +
> + def test_cases(self):
> + """backends.junit.JUnitBackend.write_test: has one <testcase> per subtest"""
> + nt.eq_(len(self.output.findall('testcase')), 3)
> +
> + @utils.nose_generator
> + def test_metadata(self):
> + """backends.junit.JUnitBackend.write_test: metadata written into the
> + suite
> +
> + """
> + def test(actual, expected):
> + nt.eq_(expected, actual)
> +
> + descrption = ('backends.junit.JUnitBackend.write_test: '
> + '{} is written into the suite')
> +
> + if self.output.tag != 'testsuite':
> + raise Exception('Could not find a testsuite!')
> +
> + tests = [
> + (self.output.find('system-out').text, 'this is stdout',
> + 'system-out'),
> + (self.output.find('system-err').text,
> + 'this is stderr\n\nstart time: 0.0\nend time: 1.2345\n',
> + 'system-err'),
> + (self.output.attrib.get('name'), 'piglit.a.group.test1', 'name'),
> + (self.output.attrib.get('time'), '1.2345', 'timestamp'),
> + (self.output.attrib.get('failures'), '1', 'failures'),
> + (self.output.attrib.get('skipped'), '1', 'skipped'),
> + (self.output.attrib.get('errors'), '1', 'errors'),
> + (self.output.attrib.get('tests'), '3', 'tests'),
> + ]
> +
> + for actual, expected, name in tests:
> + test.description = descrption.format(name)
> + yield test, actual, expected
> +
> + def test_testname(self):
> + """backends.junit.JUnitBackend.write_test: the testname should be the subtest name"""
> + nt.ok_(self.output.find('testcase[@name="foo"]') is not None)
> +
> + def test_fail(self):
> + """Backends.junit.JUnitBackend.write_test: add <failure> if the subtest failed"""
> + nt.eq_(len(self.output.find('testcase[@name="bar"]').findall('failure')), 1)
> +
> + def test_skip(self):
> + """Backends.junit.JUnitBackend.write_test: add <skipped> if the subtest skipped"""
> + nt.eq_(len(self.output.find('testcase[@name="foo"]').findall('skipped')), 1)
> +
> + def test_error(self):
> + """Backends.junit.JUnitBackend.write_test: add <error> if the subtest crashed"""
> + nt.eq_(len(self.output.find('testcase[@name="oink"]').findall('error')), 1)
> +
> +
> +class TestJunitSubtestFinalize(utils.StaticDirectory):
> + @classmethod
> + def setup_class(cls):
> + super(TestJunitSubtestFinalize, cls).setup_class()
> +
> + result = results.TestResult()
> + result.time.end = 1.2345
> + result.result = 'pass'
> + result.out = 'this is stdout'
> + result.err = 'this is stderr'
> + result.command = 'foo'
> + result.subtests['foo'] = 'pass'
> + result.subtests['bar'] = 'fail'
> +
> + test = backends.junit.JUnitBackend(cls.tdir)
> + test.initialize(BACKEND_INITIAL_META)
> + with test.write_test(grouptools.join('a', 'test', 'group', 'test1')) as t:
> + t(result)
> + test.finalize()
> +
> + @utils.not_raises(etree.ParseError)
> + def test_valid_xml(self):
> + """backends.jUnit.JunitBackend.finalize: produces valid xml with subtests"""
> + etree.parse(os.path.join(self.tdir, 'results.xml'))
> +
> + def test_valid_junit(self):
> + """backends.jUnit.JunitBackend.finalize: prodives valid junit with subtests"""
> + if etree.__name__ != 'lxml.etree':
> + raise SkipTest('Test requires lxml features')
> +
> + schema = etree.XMLSchema(file=JUNIT_SCHEMA)
> + xml = etree.parse(os.path.join(self.tdir, 'results.xml'))
> + nt.ok_(schema.validate(xml), msg='xml is not valid')
> --
> 2.6.3
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list