have the xml parser get test suite info

Summary: This modifies the XML parser to group test results by test suites, and collect metadata about the test suites.

Test Plan: unit tests

Reviewers: anupc

Reviewed By: anupc

Subscribers: changesbot, kylec

Differential Revision: https://tails.corp.dropbox.com/D223822

changes/artifacts/xunit.py

@@ -10,7 +10,7 @@
 from changes.artifacts.xml import DelegateParser
 from changes.config import statsreporter
 from changes.constants import Result
-from changes.models.testresult import TestResult, TestResultManager
+from changes.models.testresult import TestResult, TestResultManager, TestSuite
 from changes.utils.agg import aggregate_result
 from changes.utils.http import build_web_uri
 
@@ -29,8 +29,8 @@ def process(self, fp, artifact):
 
         return test_list
 
-    @statsreporter.timer('xunithandler_get_tests')
-    def get_tests(self, fp):
+    @statsreporter.timer('xunithandler_get_test_suites')
+    def get_test_suites(self, fp):
         try:
             start = fp.tell()
             try:
@@ -49,6 +49,23 @@ def get_tests(self, fp):
             self.report_malformed()
             return []
 
+    @statsreporter.timer('xunithandler_aggregate_tests_from_suites')
+    def aggregate_tests_from_suites(self, test_suites):
+        tests = []
+        for suite in test_suites:
+            tests += suite.test_results
+
+        # avoid deduplicating twice
+        if len(test_suites) > 1:
+            return _deduplicate_testresults(tests)
+        else:
+            return tests
+
+    @statsreporter.timer('xunithandler_get_tests')
+    def get_tests(self, fp):
+        suites = self.get_test_suites(fp)
+        return self.aggregate_tests_from_suites(suites)
+
 
 class XunitDelegate(DelegateParser):
     """
@@ -77,8 +94,7 @@ def parse(self, fp):
         self._parser.Parse(contents, True)
         if not isinstance(self._subparser, XunitBaseParser):
             raise ArtifactParseError('Empty file found')
-        results = _deduplicate_testresults(self._subparser.results)
-        return results
+        return self._subparser.test_suites
 
     def xml_decl(self, version, encoding, standalone):
         if self._encoding:
@@ -100,12 +116,13 @@ class XunitBaseParser(object):
     """
     Base class for Xunit parsers
     """
+    logger = logging.getLogger('xunit')
 
     def __init__(self, step, parser):
         self.step = step
         self._parser = parser
 
-        self.results = []
+        self.test_suites = []
         self._current_result = None
 
         self._is_message = False
@@ -146,7 +163,12 @@ def start(self, tag, attrs):
         if tag == 'testsuites':
             pass
         elif tag == 'testsuite':
-            pass
+            if attrs.get('time'):
+                duration_ms = float(attrs['time']) * 1000
+            else:
+                duration_ms = None
+            suite = TestSuite(step=self.step, name=attrs.get('name', None), duration=duration_ms)
+            self.test_suites.append(suite)
         elif tag == 'testcase':
             # If there's a previous failure in addition to stdout or stderr,
             # prioritize showing the previous failure because that's what's
@@ -215,8 +237,18 @@ def data(self, data):
     def end(self, tag):
         if self._is_message:
             self.close_message()
-        if tag == 'testsuites' or tag == 'testsuite':
+        if tag == 'testsuites':
             pass
+        elif tag == 'testsuite':
+            self.test_suites[-1].test_results = _deduplicate_testresults(self.test_suites[-1].test_results)
+            if self.test_suites[-1].duration is None:
+                # NOTE: it is inaccurate to just sum up the duration of individual
+                # tests, because tests may be run in parallel
+                self.logger.warning('Test suite does not have timing information; (step=%s, build=%s)',
+                                    self.step.id.hex, self.step.job.build_id.hex)
+            self.test_suites[-1].result = aggregate_result([t.result for t in self.test_suites[-1].test_results])
+            if self.test_suites[-1].date_created is None:
+                self.test_suites[-1].date_created = min([t.date_created for t in self.test_suites[-1].test_results])
         elif tag == 'testcase':
             if self._current_result.result == Result.unknown:
                 # Default result is passing
@@ -229,7 +261,7 @@ def end(self, tag):
                 elif self._current_result.result == Result.skipped:
                     self._current_result.result = Result.quarantined_skipped
             self._test_is_quarantined = None
-            self.results.append(self._current_result)
+            self.test_suites[-1].test_results.append(self._current_result)
             self._current_result = None
         elif tag == 'test-artifacts':
             pass

changes/models/testresult.py

@@ -71,6 +71,18 @@ def name(self):
     id = name
 
 
+class TestSuite(object):
+    """A test suite is a collection of test results.
+    """
+    def __init__(self, step, name=None, result=None, duration=None, date_created=None):
+        self.step = step
+        self.name = name
+        self.duration = duration
+        self.date_created = date_created
+        self.result = result or Result.unknown
+        self.test_results = []
+
+
 class TestResultManager(object):
     def __init__(self, step, artifact):
         self.step = step

changes/testutils/fixtures.py

@@ -36,7 +36,7 @@
 from changes.utils.slugs import slugify
 
 __all__ = ('Fixtures', 'SAMPLE_COVERAGE', 'SAMPLE_DIFF_BYTES', 'SAMPLE_DIFF', 'SAMPLE_XUNIT',
-           'SAMPLE_XUNIT_DOUBLE_CASES',
+           'SAMPLE_XUNIT_DOUBLE_CASES', 'SAMPLE_XUNIT_MULTIPLE_SUITES',
            'SAMPLE_XUNIT_TESTARTIFACTS')
 
 
@@ -94,6 +94,62 @@
     </testcase>
 </testsuite>"""
 
+# this has multiple test suites, and one of the test cases exists in all suites
+# the third suite also has a duplicate test case.
+SAMPLE_XUNIT_MULTIPLE_SUITES = """<?xml version="1.0" encoding="utf-8"?>
+<testsuites>
+    <testsuite errors="1" failures="0" skips="0" tests="0">
+        <testcase classname="" name="tests.test_report" time="0" owner="foo">
+            <failure message="collection failure">tests/test_report.py:1: in &lt;module&gt;
+    &gt;   import mock
+    E   ImportError: No module named mock</failure>
+        </testcase>
+        <testcase classname="tests.test_report.ParseTestResultsTest" name="test_simple" time="0.00165796279907" rerun="1"/>
+        <testcase classname="test_simple.SampleTest" name="test_falsehood" time="0.50" rerun="3">
+            <system-out>Running SampleTest</system-out>
+            <error message="test setup failure">test_simple.py:4: in tearDown
+        1/0
+    E   ZeroDivisionError: integer division or modulo by zero</error>
+            <system-out>Running SampleTest</system-out>
+        </testcase>
+    </testsuite>
+    <testsuite errors="1" failures="0" name="suite2" skips="0" tests="0" time="0.077">
+        <testcase classname="" name="tests2.test_report" time="0" owner="foo">
+            <failure message="collection failure">tests/test_report.py:1: in &lt;module&gt;
+    &gt;   import mock
+    E   ImportError: No module named mock</failure>
+        </testcase>
+        <testcase classname="tests2.test_report.ParseTestResultsTest" name="test_simple" time="0.00165796279907" rerun="1"/>
+        <testcase classname="test_simple.SampleTest" name="test_falsehood" time="0.50" rerun="3">
+            <system-out>Running SampleTest</system-out>
+            <error message="test setup failure">test_simple.py:4: in tearDown
+        1/0
+    E   ZeroDivisionError: integer division or modulo by zero</error>
+            <system-out>Running SampleTest</system-out>
+        </testcase>
+    </testsuite>
+    <testsuite errors="1" failures="0" name="" skips="0" tests="0">
+        <testcase classname="" name="tests3.test_report" owner="foo">
+            <failure message="collection failure">tests/test_report.py:1: in &lt;module&gt;
+    &gt;   import mock
+    E   ImportError: No module named mock</failure>
+        </testcase>
+        <testcase classname="" name="tests3.test_report" owner="foo">
+            <failure message="collection failure">tests/test_report.py:1: in &lt;module&gt;
+    &gt;   import mock
+    E   ImportError: No module named mock</failure>
+        </testcase>
+        <testcase classname="tests3.test_report.ParseTestResultsTest" name="test_simple" time="0.00165796279907" rerun="1"/>
+        <testcase classname="test_simple.SampleTest" name="test_falsehood" time="0.50" rerun="3">
+            <system-out>Running SampleTest</system-out>
+            <error message="test setup failure">test_simple.py:4: in tearDown
+        1/0
+    E   ZeroDivisionError: integer division or modulo by zero</error>
+            <system-out>Running SampleTest</system-out>
+        </testcase>
+    </testsuite>
+</testsuites>"""
+
 SAMPLE_XUNIT_DOUBLE_CASES = """<?xml version="1.0" encoding="utf-8"?>
 <testsuite errors="1" failures="2" name="pytest" skips="0" tests="2" time="0.019">
   <testcase classname="test_simple.SampleTest" name="test_falsehood" time="0.25" rerun="3">

tests/changes/artifacts/test_xunit.py

@@ -1,7 +1,9 @@
-import uuid
+import logging
+import mock
 import os
+import pytest
 import time
-import logging
+import uuid
 
 from cStringIO import StringIO
 
@@ -10,10 +12,87 @@
 from changes.models.failurereason import FailureReason
 from changes.models.jobstep import JobStep
 from changes.models.testresult import TestResult
-from changes.testutils import SAMPLE_XUNIT, SAMPLE_XUNIT_DOUBLE_CASES
+from changes.testutils import (
+    SAMPLE_XUNIT, SAMPLE_XUNIT_DOUBLE_CASES, SAMPLE_XUNIT_MULTIPLE_SUITES
+)
 from changes.testutils.cases import TestCase
 
 
+@pytest.mark.parametrize('xml, suite_name, duration', [
+    (SAMPLE_XUNIT, "", 77),
+    (SAMPLE_XUNIT_DOUBLE_CASES, "pytest", 19),
+])
+def test_get_test_single_suite(xml, suite_name, duration):
+    jobstep = JobStep(
+        id=uuid.uuid4(),
+        project_id=uuid.uuid4(),
+        job_id=uuid.uuid4(),
+    )
+
+    fp = StringIO(xml)
+
+    handler = XunitHandler(jobstep)
+    suites = handler.get_test_suites(fp)
+    assert len(suites) == 1
+    assert suites[0].name == suite_name
+    assert suites[0].duration == duration
+    assert suites[0].result == Result.failed
+
+    # test the equivalence of get_tests and get_test_suites in the case where
+    # there is only one test suite, so that we can call get_tests directly
+    # in the rest of this file.
+    fp.seek(0)
+    other_results = handler.get_tests(fp)
+
+    results = suites[0].test_results
+    assert len(results) == len(other_results)
+    for i in range(len(results)):
+        assert other_results[i].step == results[i].step
+        assert other_results[i].step == results[i].step
+        assert other_results[i]._name == results[i]._name
+        assert other_results[i]._package == results[i]._package
+        assert other_results[i].message == results[i].message
+        assert other_results[i].result is results[i].result
+        assert other_results[i].duration == results[i].duration
+        assert other_results[i].reruns == results[i].reruns
+        assert other_results[i].artifacts == results[i].artifacts
+        assert other_results[i].owner == results[i].owner
+        assert other_results[i].message_offsets == results[i].message_offsets
+
+
+def test_get_test_suite_multiple():
+    jobstep = JobStep(
+        id=uuid.uuid4(),
+        project_id=uuid.uuid4(),
+        job_id=uuid.uuid4(),
+    )
+    # needed for logging when a test suite has no duration
+    jobstep.job = mock.MagicMock()
+
+    fp = StringIO(SAMPLE_XUNIT_MULTIPLE_SUITES)
+
+    handler = XunitHandler(jobstep)
+    suites = handler.get_test_suites(fp)
+    assert len(suites) == 3
+    assert suites[0].name is None
+    assert suites[0].duration is None
+    assert suites[0].result == Result.failed
+    assert len(suites[0].test_results) == 3
+
+    assert suites[1].name == 'suite2'
+    assert suites[1].duration == 77
+    assert suites[1].result == Result.failed
+    assert len(suites[1].test_results) == 3
+
+    assert suites[2].name == ''
+    assert suites[2].duration is None
+    assert suites[2].result == Result.failed
+    assert len(suites[2].test_results) == 3
+
+    tests = handler.aggregate_tests_from_suites(suites)
+    assert len(tests) == 7  # 10 test cases, 3 of which are duplicates
+
+
 def test_result_generation():
     jobstep = JobStep(
         id=uuid.uuid4(),