On Fri, Dec 04, 2015 at 04:21:00PM -0800, [email protected] wrote: > From: Dylan Baker <[email protected]> > > 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: [email protected] > cc: [email protected] > Signed-off-by: Dylan Baker <[email protected]> > --- > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
