<div dir="ltr">According to the schema it's allowed. The schema is in the piglit repo at "framework/tests/schema/junit-7.xsd"</div><div class="gmail_extra"> <div class="gmail_quote">On Sat, Dec 5, 2015 at 4:36 AM, Jose Fonseca <<a href="mailto:jfonseca@vmware.com" target="_blank">jfonseca@vmware.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 05/12/15 00:21, <a href="mailto:baker.dylan.c@gmail.com" target="_blank">baker.dylan.c@gmail.com</a> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> From: Dylan Baker <<a href="mailto:baker.dylan.c@gmail.com" target="_blank">baker.dylan.c@gmail.com</a>> 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 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. </blockquote> </div></div> Having the sub-tests being a testsuite is OK, but I'm not sure nesting a testsuite inside a testsuite is reliable. I don't know if JUnit generated by other frameworks ever have that. And what exactly is the semantics for names. I wonder if it wouldn't be better to ensure that the `testsuite` tag is always a child from the root `testsuites` tag. Jose<div class="HOEnZb"><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Also adds test for the new feature. cc: <a href="mailto:mark.a.janes@intel.com" target="_blank">mark.a.janes@intel.com</a> cc: <a href="mailto:jfonseca@vmware.com" target="_blank">jfonseca@vmware.com</a> Signed-off-by: Dylan Baker <<a href="mailto:dylanx.c.baker@intel.com" target="_blank">dylanx.c.baker@intel.com</a>> --- 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 # <a href="https://jenkins-ci.org/issue/18062" rel="noreferrer" target="_blank">https://jenkins-ci.org/issue/18062</a> @@ -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(<a href="http://test.name" rel="noreferrer" target="_blank">test.name</a>, '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') </blockquote> </div></div></blockquote></div> </div>