handling empty test suites in XML parser

Summary: This fixes errors such as https://forge-sentry.pp.dropbox.com/sentry/changes/issues/503/events/68012/, where the parser would fail when given a test suite with no test cases.

Test Plan: unit tests

Reviewers: anupc, kylec

Reviewed By: kylec

Subscribers: changesbot

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

changes/artifacts/xunit.py

@@ -169,6 +169,17 @@ def start(self, tag, attrs):
                 duration_ms = None
             suite = TestSuite(step=self.step, name=attrs.get('name', None), duration=duration_ms)
             self.test_suites.append(suite)
+
+            # try to assign a result. We will override this if this test suite
+            # has tests
+            if 'failures' in attrs and int(attrs['failures']) > 0:
+                suite.result = Result.failed
+            elif 'errors' in attrs and int(attrs['errors']) > 0:
+                suite.result = Result.failed
+            else:
+                # TODO(naphat) what about 'skipped'? we don't have enough information
+                # to distinguish between skipped and passing
+                suite.result = Result.passed
         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
@@ -246,9 +257,12 @@ def end(self, tag):
                 # 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])
+
+            if len(self.test_suites[-1].test_results) > 0:
+                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

changes/testutils/fixtures.py

@@ -37,7 +37,8 @@
 
 __all__ = ('Fixtures', 'SAMPLE_COVERAGE', 'SAMPLE_DIFF_BYTES', 'SAMPLE_DIFF', 'SAMPLE_XUNIT',
            'SAMPLE_XUNIT_DOUBLE_CASES', 'SAMPLE_XUNIT_MULTIPLE_SUITES',
-           'SAMPLE_XUNIT_MULTIPLE_SUITES_COMPLETE_TIME',
+           'SAMPLE_XUNIT_MULTIPLE_SUITES_COMPLETE_TIME', 'SAMPLE_XUNIT_MULTIPLE_EMPTY_PASSED',
+           'SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_FAILURE', 'SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_ERROR',
            'SAMPLE_XUNIT_TESTARTIFACTS')
 
 
@@ -95,6 +96,30 @@
     </testcase>
 </testsuite>"""
 
+SAMPLE_XUNIT_MULTIPLE_EMPTY_PASSED = """
+<testsuites>
+    <testsuite errors="0" failures="0" name="suite-name" skipped="0" timestamp="2016-08-31"/>
+    <testsuite errors="0" failures="0" name="null" skipped="0" timestamp="2016-08-31">
+        <testcase classname="some.class" name="testName" source_file="/var/output/junit.xml" time="6.11"/>
+    </testsuite>
+</testsuites>"""
+
+SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_FAILURE = """
+<testsuites>
+    <testsuite errors="0" failures="1" name="suite-name" skipped="0" timestamp="2016-08-31"/>
+    <testsuite errors="0" failures="0" name="null" skipped="0" timestamp="2016-08-31">
+        <testcase classname="some.class" name="testName" source_file="/var/output/junit.xml" time="6.11"/>
+    </testsuite>
+</testsuites>"""
+
+SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_ERROR = """
+<testsuites>
+    <testsuite errors="1" failures="0" name="suite-name" skipped="0" timestamp="2016-08-31"/>
+    <testsuite errors="0" failures="0" name="null" skipped="0" timestamp="2016-08-31">
+        <testcase classname="some.class" name="testName" source_file="/var/output/junit.xml" time="6.11"/>
+    </testsuite>
+</testsuites>"""
+
 # 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"?>

tests/changes/artifacts/test_xunit.py

@@ -13,7 +13,9 @@
 from changes.models.jobstep import JobStep
 from changes.models.testresult import TestResult
 from changes.testutils import (
-    SAMPLE_XUNIT, SAMPLE_XUNIT_DOUBLE_CASES, SAMPLE_XUNIT_MULTIPLE_SUITES
+    SAMPLE_XUNIT, SAMPLE_XUNIT_DOUBLE_CASES, SAMPLE_XUNIT_MULTIPLE_SUITES,
+    SAMPLE_XUNIT_MULTIPLE_EMPTY_PASSED, SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_FAILURE,
+    SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_ERROR,
 )
 from changes.testutils.cases import TestCase
 
@@ -93,6 +95,36 @@ def test_get_test_suite_multiple():
     assert len(tests) == 7  # 10 test cases, 3 of which are duplicates
 
 
+@pytest.mark.parametrize('xml,result', [
+    (SAMPLE_XUNIT_MULTIPLE_EMPTY_PASSED, Result.passed),
+    (SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_FAILURE, Result.failed),
+    (SAMPLE_XUNIT_MULTIPLE_EMPTY_FAILED_ERROR, Result.failed),
+])
+def test_get_test_suite_multiple_empty(xml, result):
+    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(xml)
+
+    handler = XunitHandler(jobstep)
+    suites = handler.get_test_suites(fp)
+    assert len(suites) == 2
+    assert suites[0].name == 'suite-name'
+    assert suites[0].duration is None
+    assert suites[0].result is result
+    assert len(suites[0].test_results) == 0
+
+    assert suites[1].name == 'null'
+    assert suites[1].duration is None
+    assert suites[1].result == Result.passed
+    assert len(suites[1].test_results) == 1
+
+
 def test_result_generation():
     jobstep = JobStep(
         id=uuid.uuid4(),