Repository: kiwitcms/junit.xml-plugin (language: python)

Issue:
Uploading testcase leads to TypeError: unsupported operand type(s) for +: 'NoneType' and 'datetime.timedelta'
Was just testing out the 12.2 version of the junit.xml-plugin and got this:

https://github.com/michaelkaye/throwaway-test-repo/actions/runs/4609146419/jobs/8145874766

#88 may have introduced this.

```
Traceback (most recent call last):
  File "/home/runner/.local/bin/tcms-junit.xml-plugin", line 9, in <module>
    main(sys.argv)
  File "/home/runner/.local/lib/python3.10/site-packages/tcms_junit_plugin/__init__.py", line 179, in main
    plugin.parse(getattr(args, "filename.xml"))
  File "/home/runner/.local/lib/python3.10/site-packages/tcms_junit_plugin/__init__.py", line 140, in parse
    start_date, stop_date = self.testexecution_timestamps(xml_case)
  File "/home/runner/.local/lib/python3.10/site-packages/tcms_junit_plugin/__init__.py", line 75, in testexecution_timestamps
    stop_date = start_date + timedelta(seconds=xml_case.time)
TypeError: unsupported operand type(s) for +: 'NoneType' and 'datetime.timedelta'
```

I'm guessing this is because the data is as so - https://github.com/michaelkaye/throwaway-test-repo/blob/main/junit-files/results-3d58a84d7a978de29ba10f2a6c03d7bf.xml

The timestamp is on the testSuite; the testCases have time (durations) but no individual timestamp. Who knows what the 'correct' format should be, but maybe we should be a bit defensive against bad requests.
Uploading testcase leads to TypeError: unsupported operand type(s) for +: 'NoneType' and 'datetime.timedelta'
Was just testing out the 12.2 version of the junit.xml-plugin and got this:

https://github.com/michaelkaye/throwaway-test-repo/actions/runs/4609146419/jobs/8145874766

#88 may have introduced this.

```
Traceback (most recent call last):
  File "/home/runner/.local/bin/tcms-junit.xml-plugin", line 9, in <module>
    main(sys.argv)
  File "/home/runner/.local/lib/python3.10/site-packages/tcms_junit_plugin/__init__.py", line 179, in main
    plugin.parse(getattr(args, "filename.xml"))
  File "/home/runner/.local/lib/python3.10/site-packages/tcms_junit_plugin/__init__.py", line 140, in parse
    start_date, stop_date = self.testexecution_timestamps(xml_case)
  File "/home/runner/.local/lib/python3.10/site-packages/tcms_junit_plugin/__init__.py", line 75, in testexecution_timestamps
    stop_date = start_date + timedelta(seconds=xml_case.time)
TypeError: unsupported operand type(s) for +: 'NoneType' and 'datetime.timedelta'
```

I'm guessing this is because the data is as so - https://github.com/michaelkaye/throwaway-test-repo/blob/main/junit-files/results-3d58a84d7a978de29ba10f2a6c03d7bf.xml

The timestamp is on the testSuite; the testCases have time (durations) but no individual timestamp. Who knows what the 'correct' format should be, but maybe we should be a bit defensive against bad requests.


Discussion / hints:
I could have a go at merging times from the TestSuite into the TestCases, if that's what is wanted. I do want the actual times ending up in kiwi; it's better than having the timings (which are normally over ~15min for a test run) batched up into a few milliseconds.
> The timestamp is on the testSuite; the testCases have time (durations) but no individual timestamp. Who knows what the 'correct' format should be, but maybe we should be a bit defensive against bad requests.

Sigh. The joy of having a "standard format" where nobody knows that the standard is. 

Being defensive here is an easy fix which effectively negates the entire new functionality b/c Kiwi TCMS understands `start_date` & `stop_date` and the current information within the `<testsuite>` tag is currently lost during parsing. If we don't have the start time we can't set the stop time either. 

I will add this issue on the backlog + a reference to the current XML file for further investigation. 

```xml
<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="Mocha Tests" time="73.132" tests="4" failures="0">
  <testsuite name="" timestamp="2023-03-06T13:18:40" tests="0" file="cypress/e2e/integration-manager/send_event.spec.ts" time="0.000" failures="0">
  </testsuite>
  <testsuite name="Root Suite Integration Manager: Send Event" timestamp="2023-03-06T13:18:40" tests="4" time="73.081" failures="0">
    <testcase name="Integration Manager: Send Event should send a state event" time="21.077" classname="should send a state event">
    </testcase>
    <testcase name="Integration Manager: Send Event should send a state event with empty content" time="16.205" classname="should send a state event with empty content">
    </testcase>
    <testcase name="Integration Manager: Send Event should send a state event with empty state key" time="16.372" classname="should send a state event with empty state key">
    </testcase>
    <testcase name="Integration Manager: Send Event should fail to send an event type which is not allowed" time="16.032" classname="should fail to send an event type which is not allowed">
    </testcase>
  </testsuite>
</testsuites>
```

> I could have a go at merging times from the TestSuite into the TestCases, if that's what is wanted. I do want the actual times ending up in kiwi; it's better than having the timings (which are normally over ~15min for a test run) batched up into a few milliseconds.

Feel free to give it a try. 

What's needed here IMO is properly looping over testsuite's instead of jumping over them and then after that's in place we can make a go at supporting the timestamps feature by passing the starting value from the testsuite tag. 
I could have a go at merging times from the TestSuite into the TestCases, if that's what is wanted. I do want the actual times ending up in kiwi; it's better than having the timings (which are normally over ~15min for a test run) batched up into a few milliseconds.
> The timestamp is on the testSuite; the testCases have time (durations) but no individual timestamp. Who knows what the 'correct' format should be, but maybe we should be a bit defensive against bad requests.

Sigh. The joy of having a "standard format" where nobody knows that the standard is. 

Being defensive here is an easy fix which effectively negates the entire new functionality b/c Kiwi TCMS understands `start_date` & `stop_date` and the current information within the `<testsuite>` tag is currently lost during parsing. If we don't have the start time we can't set the stop time either. 

I will add this issue on the backlog + a reference to the current XML file for further investigation. 

```xml
<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="Mocha Tests" time="73.132" tests="4" failures="0">
  <testsuite name="" timestamp="2023-03-06T13:18:40" tests="0" file="cypress/e2e/integration-manager/send_event.spec.ts" time="0.000" failures="0">
  </testsuite>
  <testsuite name="Root Suite Integration Manager: Send Event" timestamp="2023-03-06T13:18:40" tests="4" time="73.081" failures="0">
    <testcase name="Integration Manager: Send Event should send a state event" time="21.077" classname="should send a state event">
    </testcase>
    <testcase name="Integration Manager: Send Event should send a state event with empty content" time="16.205" classname="should send a state event with empty content">
    </testcase>
    <testcase name="Integration Manager: Send Event should send a state event with empty state key" time="16.372" classname="should send a state event with empty state key">
    </testcase>
    <testcase name="Integration Manager: Send Event should fail to send an event type which is not allowed" time="16.032" classname="should fail to send an event type which is not allowed">
    </testcase>
  </testsuite>
</testsuites>
```

> I could have a go at merging times from the TestSuite into the TestCases, if that's what is wanted. I do want the actual times ending up in kiwi; it's better than having the timings (which are normally over ~15min for a test run) batched up into a few milliseconds.

Feel free to give it a try. 

What's needed here IMO is properly looping over testsuite's instead of jumping over them and then after that's in place we can make a go at supporting the timestamps feature by passing the starting value from the testsuite tag. 

Fix patch:
diff --git a/tcms_junit_plugin/__init__.py b/tcms_junit_plugin/__init__.py
index a2d8969..bbd7e96 100644
--- a/tcms_junit_plugin/__init__.py
+++ b/tcms_junit_plugin/__init__.py
@@ -46,7 +46,7 @@ def extract_logs(xml_case):
 ```
 """
 
-    def testcase_summary(self, xml_case):
+    def testcase_summary(self, xml_suite, xml_case):
         """
         This method will generate the TestCase summary which is sent to
         Kiwi TCMS. It may be overriden for more flexibility!
@@ -54,12 +54,12 @@ def testcase_summary(self, xml_case):
         values = {
             "classname": xml_case.classname,
             "name": xml_case.name,
-            "suitename": getattr(xml_case, "suitename", None),
+            "suitename": getattr(xml_suite, "name", None),
         }
         return Template(self.summary_template).substitute(values)
 
     # pylint: disable=protected-access
-    def testexecution_timestamps(self, xml_case):
+    def testexecution_timestamps(self, xml_suite, xml_case):
         """
         This method will return (start_date, stop_date) if information
         is present!
@@ -67,15 +67,55 @@ def testexecution_timestamps(self, xml_case):
         start_date = None
         stop_date = None
 
+        # some runners, e.g. Mocha.js reports a single starting timestamp
+        # as a <testsuite> attribute. Take that if available!
+        if "timestamp" in xml_suite._elem.attrib:
+            start_date = self.parse_timestamp(xml_suite._elem.attrib["timestamp"])
+
+        # update start_date if individual case contains this information
         if "timestamp" in xml_case._elem.attrib:
-            start_date = datetime.strptime(
-                xml_case._elem.attrib["timestamp"], "%Y-%m-%dT%H:%M:%S.%f"
-            )
+            start_date = self.parse_timestamp(xml_case._elem.attrib["timestamp"])
         if "time" in xml_case._elem.attrib:
             stop_date = start_date + timedelta(seconds=xml_case.time)
 
         return (start_date, stop_date)
 
+    def parse_timestamp(self, value):
+        """
+        Try different timestamp formats until one of them finally works!
+        """
+        # IMPORTANT: keep the format strings ordered such that the parser extracts
+        # as much information as possible
+        for format_string in [
+            "%Y-%m-%dT%H:%M:%S.%f",  # Nose2
+            "%Y-%m-%dT%H:%M:%S",  # Mocha.js
+            "%Y-%m-%d %H:%M:%S",  # Katalon Studio
+        ]:
+            try:
+                return datetime.strptime(value, format_string)
+            except ValueError:
+                pass
+
+        raise ValueError(f"Unknown timestamp format {value}")
+
+    def parse_as_testsuites(self, xml_path):
+        xml = JUnitXml.fromfile(xml_path)
+        # apparently junit.xml may contain either a <testsuites> tag,
+        # e.g. Katalon Studio, Mocha.js
+        if xml._tag == "testsuites":  # pylint: disable=protected-access
+            return xml
+
+        # or a single <testsuite> tag,
+        # e.g. Nose2, py.test
+        if xml._tag == "testsuite":  # pylint: disable=protected-access
+            # transforms the input data to a format which contains the <testsuites> tag
+            new_xml = JUnitXml()
+            # see JUnitXml.__iadd__() method
+            new_xml += xml
+            return new_xml
+
+        raise RuntimeError(f"Unknown XML root tag {xml._tag}")
+
     def parse(
         self, junit_filenames, progress_cb=None
     ):  # pylint: disable=too-many-branches, too-many-locals
@@ -85,69 +125,56 @@ def parse(
             if self.verbose:
                 print(f"Parsing {junit_xml} ...")
 
-            xml = JUnitXml.fromfile(junit_xml)
-            # apparently junit.xml may contain either a <testsuites> tag,
-            # e.g. Katalon Studio
-            if xml._tag == "testsuites":  # pylint: disable=protected-access
-                cases = []
-                for suite in xml:
-                    for case in suite:
-                        # Retain the suite name (if present) with each
-                        # testcase.
-                        if suite.name:
-                            case.suitename = suite.name
-                        cases.append(case)
-            # or directly <testsuite> (only 1) tag - nose & py.test
-            else:
-                cases = list(xml)
-
-            for xml_case in cases:
-                summary = self.testcase_summary(xml_case)[:255]
-
-                test_case, _ = self.backend.test_case_get_or_create(summary)
-                self.backend.add_test_case_to_plan(test_case["id"], self.backend.plan_id)
-
-                comment = self.backend.created_by_text
-                if not xml_case.result:
-                    status_id = self.backend.get_status_id("PASSED")
-
-                # note: since junitpartser v2.0 the result attribute holds
-                # a list of values b/c pytest can produce files which contain
-                # multiple results for the same test case. We take the first!
-                for result in xml_case.result:
-                    comment = result.tostring().decode().strip()
-                    comment = f"```{comment}```"
-
-                    if isinstance(result, Failure):
-                        status_id = self.backend.get_status_id("FAILED")
-                        comment += self.extract_logs(xml_case)
-                        break
-
-                    if isinstance(result, Error):
-                        status_id = self.backend.get_status_id("ERROR")
-                        comment += self.extract_logs(xml_case)
-                        break
-
-                    if isinstance(result, Skipped):
-                        status_id = self.backend.get_status_id("WAIVED")
-                        comment = result.message
-                        break
-
-                for execution in self.backend.add_test_case_to_run(
-                    test_case["id"],
-                    self.backend.run_id,
-                ):
-                    start_date, stop_date = self.testexecution_timestamps(xml_case)
-                    self.backend.update_test_execution(
-                        execution["id"],
-                        status_id,
-                        comment=comment,
-                        start_date=start_date,
-                        stop_date=stop_date,
-                    )
-
-                if progress_cb:
-                    progress_cb()
+            xml = self.parse_as_testsuites(junit_xml)
+
+            for xml_suite in xml:
+                for xml_case in xml_suite:
+                    summary = self.testcase_summary(xml_suite, xml_case)[:255]
+
+                    test_case, _ = self.backend.test_case_get_or_create(summary)
+                    self.backend.add_test_case_to_plan(test_case["id"], self.backend.plan_id)
+
+                    comment = self.backend.created_by_text
+                    if not xml_case.result:
+                        status_id = self.backend.get_status_id("PASSED")
+
+                    # note: since junitpartser v2.0 the result attribute holds
+                    # a list of values b/c pytest can produce files which contain
+                    # multiple results for the same test case. We take the first!
+                    for result in xml_case.result:
+                        comment = result.tostring().decode().strip()
+                        comment = f"```{comment}```"
+
+                        if isinstance(result, Failure):
+                            status_id = self.backend.get_status_id("FAILED")
+                            comment += self.extract_logs(xml_case)
+                            break
+
+                        if isinstance(result, Error):
+                            status_id = self.backend.get_status_id("ERROR")
+                            comment += self.extract_logs(xml_case)
+                            break
+
+                        if isinstance(result, Skipped):
+                            status_id = self.backend.get_status_id("WAIVED")
+                            comment = result.message
+                            break
+
+                    for execution in self.backend.add_test_case_to_run(
+                        test_case["id"],
+                        self.backend.run_id,
+                    ):
+                        start_date, stop_date = self.testexecution_timestamps(xml_suite, xml_case)
+                        self.backend.update_test_execution(
+                            execution["id"],
+                            status_id,
+                            comment=comment,
+                            start_date=start_date,
+                            stop_date=stop_date,
+                        )
+
+                    if progress_cb:
+                        progress_cb()
 
         self.backend.finish_test_run()
 


Test patch:
diff --git a/tests/bin/make-junit.xml b/tests/bin/make-junit.xml
index b2471ee..0a032c4 100755
--- a/tests/bin/make-junit.xml
+++ b/tests/bin/make-junit.xml
@@ -4,4 +4,7 @@ set -e
 
 # Execute this from the project root directory
 nose2 --plugin nose2.plugins.junitxml --junit-xml
-./tcms-junit.xml-plugin --verbose nose2-junit.xml tests/data/9.xml
+./tcms-junit.xml-plugin --verbose \
+    nose2-junit.xml \
+    tests/data/9.xml \
+    tests/data/mocha.js.xml
diff --git a/tests/data/mocha.js.xml b/tests/data/mocha.js.xml
new file mode 100644
index 0000000..63852a9
--- /dev/null
+++ b/tests/data/mocha.js.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<testsuites name="Mocha Tests" time="73.132" tests="4" failures="0">
+  <testsuite name="" timestamp="2023-03-06T13:18:40" tests="0" file="cypress/e2e/integration-manager/send_event.spec.ts" time="0.000" failures="0">
+  </testsuite>
+  <testsuite name="Root Suite Integration Manager: Send Event" timestamp="2023-03-06T13:18:40" tests="4" time="73.081" failures="0">
+    <testcase name="Integration Manager: Send Event should send a state event" time="21.077" classname="should send a state event">
+    </testcase>
+    <testcase name="Integration Manager: Send Event should send a state event with empty content" time="16.205" classname="should send a state event with empty content">
+    </testcase>
+    <testcase name="Integration Manager: Send Event should send a state event with empty state key" time="16.372" classname="should send a state event with empty state key">
+    </testcase>
+    <testcase name="Integration Manager: Send Event should fail to send an event type which is not allowed" time="16.032" classname="should fail to send an event type which is not allowed">
+    </testcase>
+  </testsuite>
+</testsuites>