Rework inner code for JUnit reports

Now we have unified code for both task exporter and verification
reporter. Also, difference in attributes order between python versions
are fixed.

Covers https://bugs.python.org/issue34160

Change-Id: I9a86e995d2ecb78a3ec9a69eaa5815296fcb4a6e

rally/common/io/junit.py

@@ -1,4 +1,3 @@
-# Copyright 2015: eNovance
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -13,56 +12,150 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import collections
+import datetime as dt
 import xml.etree.ElementTree as ET
 
+from rally.common import version
 
-class JUnit(object):
-    SUCCESS = "success"
-    FAILURE = "failure"
-    ERROR = "error"
 
-    def __init__(self, test_suite_name):
+def _prettify_xml(elem, level=0):
-        self.test_suite_name = test_suite_name
+    """Adds indents.
-        self.test_cases = []
-        self.n_tests = 0
-        self.n_failures = 0
-        self.n_errors = 0
-        self.total_time = 0.0
 
-    def add_test(self, test_name, time, outcome=SUCCESS, message=""):
+    Code of this method was copied from
-        class_name, name = test_name.split(".", 1)
+        http://effbot.org/zone/element-lib.htm#prettyprint
-        self.test_cases.append({
-            "classname": class_name,
-            "name": name,
-            "time": str("%.2f" % time),
-            "outcome": outcome,
-            "message": message
-        })
 
-        if outcome == JUnit.FAILURE:
+    """
-            self.n_failures += 1
+    i = "\n" + level * "  "
-        elif outcome == JUnit.ERROR:
+    if len(elem):
-            self.n_errors += 1
+        if not elem.text or not elem.text.strip():
-        elif outcome != JUnit.SUCCESS:
+            elem.text = i + "  "
-            raise ValueError("Unexpected outcome %s" % outcome)
+        if not elem.tail or not elem.tail.strip():
-
+            elem.tail = i
-        self.n_tests += 1
+        for elem in elem:
-        self.total_time += time
+            _prettify_xml(elem, level + 1)
-
+        if not elem.tail or not elem.tail.strip():
-    def to_xml(self):
+            elem.tail = i
-        xml = ET.Element("testsuite", {
-            "name": self.test_suite_name,
-            "tests": str(self.n_tests),
-            "time": str("%.2f" % self.total_time),
-            "failures": str(self.n_failures),
-            "errors": str(self.n_errors),
-        })
-        for test_case in self.test_cases:
-            outcome = test_case.pop("outcome")
-            message = test_case.pop("message")
-            if outcome in [JUnit.FAILURE, JUnit.ERROR]:
-                sub = ET.SubElement(xml, "testcase", test_case)
-                sub.append(ET.Element(outcome, {"message": message}))
     else:
-                xml.append(ET.Element("testcase", test_case))
+        if level and (not elem.tail or not elem.tail.strip()):
-        return ET.tostring(xml, encoding="utf-8").decode("utf-8")
+            elem.tail = i
+
+
+def _filter_attrs(**attrs):
+    return collections.OrderedDict(
+        (k, v) for k, v in sorted(attrs.items()) if v is not None)
+
+
+class _TestCase(object):
+    def __init__(self, parent, classname, name, id=None, time=None,
+                 timestamp=None):
+        self._parent = parent
+        attrs = _filter_attrs(id=id, time=time, classname=classname,
+                              name=name, timestamp=timestamp)
+        self._elem = ET.SubElement(self._parent._elem, "testcase", **attrs)
+
+    def _add_details(self, tag=None, text=None, *comments):
+        if tag:
+            elem = ET.SubElement(self._elem, tag)
+            if text:
+                elem.text = text
+        for comment in comments:
+            if comment:
+                self._elem.append(ET.Comment(comment))
+
+    def mark_as_failed(self, details):
+        self._add_details("failure", details)
+        self._parent._increment("failures")
+
+    def mark_as_uxsuccess(self, reason=None):
+        # NOTE(andreykurilin): junit doesn't support uxsuccess
+        #   status, so let's display it like "fail" with proper comment.
+        self.mark_as_failed(
+            f"It is an unexpected success. The test "
+            f"should fail due to: {reason or 'Unknown reason'}"
+        )
+
+    def mark_as_xfail(self, reason=None, details=None):
+        reason = (f"It is an expected failure due to: "
+                  f"{reason or 'Unknown reason'}")
+        self._add_details(None, None, reason, details)
+
+    def mark_as_skipped(self, reason):
+        self._add_details("skipped", reason or "Unknown reason")
+        self._parent._increment("skipped")
+
+
+class _TestSuite(object):
+    def __init__(self, parent, id, time, timestamp):
+        self._parent = parent
+        attrs = _filter_attrs(id=id, time=time, tests="0",
+                              errors="0", skipped="0",
+                              failures="0", timestamp=timestamp)
+        self._elem = ET.SubElement(self._parent, "testsuite", **attrs)
+
+        self._finalized = False
+        self._calculate = True
+        self._total = 0
+        self._skipped = 0
+        self._failures = 0
+
+    def _finalize(self):
+        if not self._finalized and self._calculate:
+            self._setup_final_stats(tests=str(self._total),
+                                    skipped=str(self._skipped),
+                                    failures=str(self._failures))
+        self._finalized = True
+
+    def _setup_final_stats(self, tests, skipped, failures):
+        self._elem.set("tests", tests)
+        self._elem.set("skipped", skipped)
+        self._elem.set("failures", failures)
+
+    def setup_final_stats(self, tests, skipped, failures):
+        """Turn off calculation of final stats."""
+        self._calculate = False
+        self._setup_final_stats(tests, skipped, failures)
+
+    def _increment(self, status):
+        if self._calculate:
+            key = f"_{status}"
+            value = getattr(self, key) + 1
+            setattr(self, key, value)
+            self._finalized = False
+
+    def add_test_case(self, classname, name, id=None, time=None,
+                      timestamp=None):
+        self._increment("total")
+        return _TestCase(self, id=id, classname=classname, name=name,
+                         time=time, timestamp=timestamp)
+
+
+class JUnitXML(object):
+    """A helper class to build JUnit-XML report without knowing XML."""
+
+    def __init__(self):
+        self._root = ET.Element("testsuites")
+        self._test_suites = []
+
+        self._root.append(
+            ET.Comment("Report is generated by Rally %s at %s" % (
+                version.version_string(),
+                dt.datetime.utcnow().isoformat()))
+        )
+
+    def __str__(self):
+        return self.to_string()
+
+    def to_string(self):
+        for test_suite in self._test_suites:
+            test_suite._finalize()
+
+        _prettify_xml(self._root)
+
+        return ET.tostring(self._root, encoding="utf-8").decode("utf-8")
+
+    def add_test_suite(self, id, time, timestamp):
+        test_suite = _TestSuite(
+            self._root, id=id, time=time, timestamp=timestamp)
+        self._test_suites.append(test_suite)
+        return test_suite

rally/common/utils.py

@@ -32,6 +32,7 @@ import uuid
 
 from six import moves
 
+from rally.common.io import junit
 from rally.common import logging
 from rally import exceptions
 
@@ -786,23 +787,6 @@ class BackupHelper(object):
                 shutil.rmtree(path)
 
 
+@logging.log_deprecated("it was an inner helper.", rally_version="3.0.0")
 def prettify_xml(elem, level=0):
-    """Adds indents.
+    return junit._prettify_xml(elem, level=level)
-
-    Code of this method was copied from
-        http://effbot.org/zone/element-lib.htm#prettyprint
-
-    """
-    i = "\n" + level * "  "
-    if len(elem):
-        if not elem.text or not elem.text.strip():
-            elem.text = i + "  "
-        if not elem.tail or not elem.tail.strip():
-            elem.tail = i
-        for elem in elem:
-            prettify_xml(elem, level + 1)
-        if not elem.tail or not elem.tail.strip():
-            elem.tail = i
-    else:
-        if level and (not elem.tail or not elem.tail.strip()):
-            elem.tail = i

rally/plugins/common/exporters/junit.py

@@ -15,11 +15,8 @@
 import datetime as dt
 import itertools
 import os
-import xml.etree.ElementTree as ET
 
-from rally.common import utils
+from rally.common.io import junit
-from rally.common import version
-from rally import consts
 from rally.task import exporter
 
 
@@ -59,57 +56,37 @@ class JUnitXMLExporter(exporter.TaskExporter):
     """
 
     def generate(self):
-        root = ET.Element("testsuites")
+        root = junit.JUnitXML()
-        root.append(ET.Comment("Report is generated by Rally %s at %s" % (
-            version.version_string(),
-            dt.datetime.utcnow().strftime(consts.TimeFormat.ISO8601))))
 
         for t in self.tasks_results:
             created_at = dt.datetime.strptime(t["created_at"],
                                               "%Y-%m-%dT%H:%M:%S")
             updated_at = dt.datetime.strptime(t["updated_at"],
                                               "%Y-%m-%dT%H:%M:%S")
-            task = {
+            test_suite = root.add_test_suite(
-                "id": t["uuid"],
+                id=t["uuid"],
-                "tests": 0,
+                time="%.2f" % (updated_at - created_at).total_seconds(),
-                "errors": "0",
+                timestamp=t["created_at"]
-                "skipped": "0",
+            )
-                "failures": 0,
-                "time": "%.2f" % (updated_at - created_at).total_seconds(),
-                "timestamp": t["created_at"],
-            }
-            test_cases = []
             for workload in itertools.chain(
                     *[s["workloads"] for s in t["subtasks"]]):
                 class_name, name = workload["name"].split(".", 1)
-                test_case = {
+                test_case = test_suite.add_test_case(
-                    "id": workload["uuid"],
+                    id=workload["uuid"],
-                    "time": "%.2f" % workload["full_duration"],
+                    time="%.2f" % workload["full_duration"],
-                    "name": name,
+                    classname=class_name,
-                    "classname": class_name,
+                    name=name,
-                    "timestamp": workload["created_at"]
+                    timestamp=workload["created_at"]
-                }
+                )
                 if not workload["pass_sla"]:
-                    task["failures"] += 1
+                    details = "\n".join(
-                    test_case["failure"] = "\n".join(
                         [s["detail"]
                          for s in workload["sla_results"]["sla"]
-                         if not s["success"]])
+                         if not s["success"]]
-                test_cases.append(test_case)
+                    )
+                    test_case.mark_as_failed(details)
 
-            task["tests"] = str(len(test_cases))
+        raw_report = root.to_string()
-            task["failures"] = str(task["failures"])
-
-            testsuite = ET.SubElement(root, "testsuite", task)
-            for test_case in test_cases:
-                failure = test_case.pop("failure", None)
-                test_case = ET.SubElement(testsuite, "testcase", test_case)
-                if failure:
-                    ET.SubElement(test_case, "failure").text = failure
-
-        utils.prettify_xml(root)
-
-        raw_report = ET.tostring(root, encoding="utf-8").decode("utf-8")
 
         if self.output_destination:
             return {"files": {self.output_destination: raw_report},

rally/plugins/common/verification/reporters.py

@@ -13,13 +13,10 @@
 #    under the License.
 
 import collections
-import datetime as dt
 import json
 import re
-import xml.etree.ElementTree as ET
 
-from rally.common import utils
+from rally.common.io import junit
-from rally.common import version
 from rally import consts
 from rally.ui import utils as ui_utils
 from rally.verification import reporter
@@ -410,74 +407,55 @@ class JUnitXMLReporter(reporter.VerificationReporter):
         pass
 
     def generate(self):
-        root = ET.Element("testsuites")
+        report = junit.JUnitXML()
-
-        root.append(ET.Comment("Report is generated by Rally %s at %s" % (
-            version.version_string(),
-            dt.datetime.utcnow().strftime(TIME_FORMAT))))
 
         for v in self.verifications:
-            verification = ET.SubElement(root, "testsuite", {
+            test_suite = report.add_test_suite(
-                "id": v.uuid,
+                id=v.uuid,
-                "time": str(v.tests_duration),
+                time=str(v.tests_duration),
-                "tests": str(v.tests_count),
+                timestamp=v.created_at.strftime(TIME_FORMAT)
-                "errors": "0",
+            )
-                "skipped": str(v.skipped),
+            test_suite.setup_final_stats(
-                "failures": str(v.failures + v.unexpected_success),
+                tests=str(v.tests_count),
-                "timestamp": v.created_at.strftime(TIME_FORMAT)
+                skipped=str(v.skipped),
-            })
+                failures=str(v.failures + v.unexpected_success)
+            )
+
             tests = sorted(v.tests.values(),
                            key=lambda t: (t.get("timestamp", ""), t["name"]))
             for result in tests:
                 class_name, name = result["name"].rsplit(".", 1)
-                test_case = {
-                    "time": result["duration"],
-                    "name": name, "classname": class_name
-                }
 
                 test_id = [tag[3:] for tag in result.get("tags", [])
                            if tag.startswith("id-")]
-                if test_id:
-                    test_case["id"] = test_id[0]
-                if "timestamp" in result:
-                    test_case["timestamp"] = result["timestamp"]
 
-                test_case_element = ET.SubElement(verification, "testcase",
+                test_case = test_suite.add_test_case(
-                                                  test_case)
+                    id=(test_id[0] if test_id else None),
+                    time=result["duration"], name=name, classname=class_name,
+                    timestamp=result.get("timestamp"))
+
                 if result["status"] == "success":
                     # nothing to add
                     pass
                 elif result["status"] == "uxsuccess":
-                    # NOTE(andreykurilin): junit doesn't support uxsuccess
+                    test_case.mark_as_uxsuccess(
-                    #   status, so let's display it like "fail" with proper
+                        result.get("reason"))
-                    # comment.
-                    failure = ET.SubElement(test_case_element, "failure")
-                    failure.text = ("It is an unexpected success. The test "
-                                    "should fail due to: %s" %
-                                    result.get("reason", "Unknown reason"))
                 elif result["status"] == "fail":
-                    failure = ET.SubElement(test_case_element, "failure")
+                    test_case.mark_as_failed(
-                    failure.text = result.get("traceback", None)
+                        result.get("traceback", None))
                 elif result["status"] == "xfail":
-                    # NOTE(andreykurilin): junit doesn't support xfail status,
-                    # so let's display it like "success" with proper comment
-                    test_case_element.append(ET.Comment(
-                        "It is an expected failure due to: %s" %
-                        result.get("reason", "Unknown reason")))
                     trace = result.get("traceback", None)
-                    if trace:
+                    test_case.mark_as_xfail(
-                        test_case_element.append(ET.Comment(
+                        result.get("reason", None),
-                            "Traceback:\n%s" % trace))
+                        f"Traceback:\n{trace}" if trace else None)
                 elif result["status"] == "skip":
-                    skipped = ET.SubElement(test_case_element, "skipped")
+                    test_case.mark_as_skipped(
-                    skipped.text = result.get("reason", "Unknown reason")
+                        result.get("reason", None))
                 else:
                     # wtf is it?! we should add validation of results...
                     pass
 
-            utils.prettify_xml(root)
+        raw_report = report.to_string()
-
-        raw_report = ET.tostring(root, encoding="utf-8").decode("utf-8")
         if self.output_destination:
             return {"files": {self.output_destination: raw_report},
                     "open": self.output_destination}

tests/ci/playbooks/rally-install/pre.yaml

@@ -46,6 +46,7 @@
           # NOTE(pabelanger): We run apt-get update to ensure we dont have a stale
           # package cache in the gate.
           sudo apt-get update
+          sudo apt-get install python3.6-dev
 
     - name: Install bindep
       shell:

tests/unit/common/io/test_junit.py

@@ -13,7 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import sys
+import mock
 
 from rally.common.io import junit
 from tests.unit import test
@@ -22,33 +22,40 @@ from tests.unit import test
 class JUnitTestCase(test.TestCase):
     def setUp(self):
         super(JUnitTestCase, self).setUp()
-        if sys.version_info >= (3, 8):
+        p_mock_datetime = mock.patch("rally.common.io.junit.dt.datetime")
-            self.skipTest("This test case is failing due to changed order of "
+        self.mock_datetime = p_mock_datetime.start()
-                          "xml tag parameters.")
+        isoformat = self.mock_datetime.utcnow.return_value.isoformat
+        isoformat.return_value = "TIME"
+        self.addCleanup(p_mock_datetime.stop)
+
+        p_mock_version = mock.patch("rally.common.version.version_string",
+                                    return_value="VERSION")
+        self.mock_version = p_mock_version.start()
+        self.addCleanup(p_mock_version.stop)
 
     def test_basic_testsuite(self):
-        j = junit.JUnit("test")
+        j = junit.JUnitXML()
-        j.add_test("Foo.Bar", 3.14, outcome=junit.JUnit.SUCCESS)
+        test_suite = j.add_test_suite("uuid1", time="58.51", timestamp="3")
-        j.add_test("Foo.Baz", 13.37, outcome=junit.JUnit.FAILURE,
+        test_suite.add_test_case(classname="Foo", name="Bar", time="3.14")
-                   message="fail_message")
+        t = test_suite.add_test_case(classname="Foo", name="Baz", time="13.37")
-        j.add_test("Eggs.Spam", 42.00, outcome=junit.JUnit.ERROR)
+        t.mark_as_failed("fail_message")
 
-        expected = """
+        expected = """<testsuites>
-<testsuite errors="1" failures="1" name="test" tests="3" time="58.51">
+  <!--Report is generated by Rally VERSION at TIME-->
+  <testsuite errors="0" failures="1" id="uuid1" skipped="0" tests="2" time="58.51" timestamp="3">
     <testcase classname="Foo" name="Bar" time="3.14" />
     <testcase classname="Foo" name="Baz" time="13.37">
-<failure message="fail_message" /></testcase>
+      <failure>fail_message</failure>
-<testcase classname="Eggs" name="Spam" time="42.00">
+    </testcase>
-<error message="" /></testcase></testsuite>"""
+  </testsuite>
-        self.assertEqual(expected.replace("\n", ""), j.to_xml())
+</testsuites>
+"""  # noqa: E501
+        self.assertEqual(expected, j.to_string())
 
     def test_empty_testsuite(self):
-        j = junit.JUnit("test")
+        j = junit.JUnitXML()
-        expected = """
+        expected = """<testsuites>
-<testsuite errors="0" failures="0" name="test" tests="0" time="0.00" />"""
+  <!--Report is generated by Rally VERSION at TIME-->
-        self.assertEqual(expected.replace("\n", ""), j.to_xml())
+</testsuites>
-
+"""
-    def test_invalid_outcome(self):
+        self.assertEqual(expected, str(j))
-        j = junit.JUnit("test")
-        self.assertRaises(ValueError, j.add_test, "Foo.Bar", 1.23,
-                          outcome=1024)

tests/unit/plugins/common/exporters/test_junit.py

@@ -14,7 +14,6 @@
 
 import datetime as dt
 import os
-import sys
 
 import mock
 
@@ -66,20 +65,15 @@ def get_tasks_results():
 class JUnitXMLExporterTestCase(test.TestCase):
     def setUp(self):
         super(JUnitXMLExporterTestCase, self).setUp()
-        if sys.version_info >= (3, 8):
-            self.skipTest("This test case is failing due to changed order of "
-                          "xml tag parameters.")
         self.datetime = dt.datetime
 
-        patcher = mock.patch("rally.plugins.common.exporters.junit.dt")
+        patcher = mock.patch("rally.common.io.junit.dt")
         self.dt = patcher.start()
-        self.dt.datetime.strptime.side_effect = self.datetime.strptime
+        self.dt.datetime.utcnow.return_value.isoformat.return_value = "$TIME"
         self.addCleanup(patcher.stop)
 
-    @mock.patch("rally.plugins.common.exporters.junit.version.version_string")
+    @mock.patch("rally.common.version.version_string")
     def test_generate(self, mock_version_string):
-        now = self.dt.datetime.utcnow.return_value
-        now.strftime.return_value = "$TIME"
         mock_version_string.return_value = "$VERSION"
 
         with open(os.path.join(os.path.dirname(__file__),

tests/unit/plugins/common/verification/test_reporters.py

@@ -15,7 +15,6 @@
 import collections
 import datetime as dt
 import os
-import sys
 
 import ddt
 import mock
@@ -388,16 +387,11 @@ class HTMLReporterTestCase(test.TestCase):
 
 
 class JUnitXMLReporterTestCase(test.TestCase):
-    def setUp(self):
-        super(JUnitXMLReporterTestCase, self).setUp()
-        if sys.version_info >= (3, 8):
-            self.skipTest("This test case is failing due to changed order of "
-                          "xml tag parameters.")
 
-    @mock.patch("%s.dt" % PATH)
+    @mock.patch("rally.common.io.junit.dt")
-    @mock.patch("%s.version.version_string" % PATH)
+    @mock.patch("rally.common.version.version_string")
     def test_generate(self, mock_version_string, mock_dt):
-        mock_dt.datetime.utcnow.return_value.strftime.return_value = "TIME"
+        mock_dt.datetime.utcnow.return_value.isoformat.return_value = "TIME"
         # release when junit reporter was introduced
         mock_version_string.return_value = "0.8.0"
         with open(os.path.join(os.path.dirname(__file__),