Enhancement/lchib 614 utilize surefire report extended by rerun feature v2

smart_tests/commands/record/case_event.py

@@ -1,6 +1,6 @@
 import datetime
 import sys
-from typing import Any, Callable, Dict, Generator
+from typing import Any, Callable, Dict, Generator, List, Optional, Tuple
 
 import dateutil.parser
 from dateutil.tz import tzlocal
@@ -12,7 +12,13 @@
 
 POSSIBLE_RESULTS = (Failure, Error, Skipped)
 
-CaseEventType = Dict[str, str]
+# Surefire's rerun/flaky extension tags. junitparser's TestCase.ITER_TYPES (and thus
+# case.result/case.system_out/case.system_err) doesn't know about these, so they're
+# read directly off the raw element.
+RERUN_TAGS = ("rerunFailure", "rerunError")
+FLAKY_TAGS = ("flakyFailure", "flakyError")
+
+CaseEventType = Dict[str, Any]
 CaseEventGenerator = Generator[CaseEventType, None, None]
 
 # function that computes TestPath from a test case
@@ -83,17 +89,15 @@ def from_case_and_suite(
         suite: TestSuite,
         report_file: str,
         data_builder: DataBuilder
-    ) -> Dict:
-        "Builds a JSON representation of CaseEvent from JUnitPaser objects"
-
-        # TODO: reconsider the initial value of the status.
-        status = CaseEvent.TEST_PASSED
-        for r in case.result:
-            if any(isinstance(r, s) for s in (Failure, Error)):
-                status = CaseEvent.TEST_FAILED
-                break
-            elif isinstance(r, Skipped):
-                status = CaseEvent.TEST_SKIPPED
+    ) -> List[Dict]:
+        """
+        Builds JSON representations of CaseEvent(s) from JUnitParser objects.
+
+        Normally a <testcase> maps to a single event. But Surefire's rerun/flaky
+        reports pack multiple attempts into one <testcase> (see RERUN_TAGS/FLAKY_TAGS);
+        each attempt is expanded into its own FAILED event here, in addition to the
+        testcase's own final PASSED/FAILED/SKIPPED result.
+        """
 
         def path_canonicalizer(test_path: TestPath) -> TestPath:
             if sys.platform == "win32":
@@ -114,6 +118,12 @@ def stdout(case: TestCase) -> str:
 
             return ""
 
+        # TODO(Improvement): when <system-err> is present, it currently replaces the failure's own
+        # text/message below instead of being appended to it. A test can both fail AND
+        # print to stderr (e.g. a logged warning before the assertion), so this can hide
+        # the actual failure detail behind unrelated captured output. Consider
+        # concatenating (failure text/message first, then system-err) instead of
+        # returning early here.
         def stderr(case: TestCase) -> str:
             """
             case for:
@@ -142,15 +152,64 @@ def stderr(case: TestCase) -> str:
 
             return stderr
 
-        return CaseEvent.create(
-            test_path=path_canonicalizer(path_builder(case, suite, report_file)),
-            duration_secs=case.time,
-            status=status,
-            stdout=stdout(case),
-            stderr=stderr(case),
-            timestamp=suite.timestamp,
-            data=data_builder(case),
-        )
+        def final_result() -> Tuple[int, str, str, float]:
+            """
+            The testcase's own final result: unchanged from before this method supported reruns.
+            <testcase time> is defined by Surefire as the duration of THIS run (the first failing
+            run, or the last successful run), so this is the one attempt that keeps it.
+            """
+            # TODO: reconsider the initial value of the status.
+            status = CaseEvent.TEST_PASSED
+            for r in case.result:
+                if any(isinstance(r, s) for s in (Failure, Error)):
+                    status = CaseEvent.TEST_FAILED
+                    break
+                elif isinstance(r, Skipped):
+                    status = CaseEvent.TEST_SKIPPED
+            return status, stdout(case), stderr(case), case.time
+
+        def attempt_result(elem) -> Tuple[int, str, str, float]:
+            """
+            One <flakyFailure>/<flakyError>/<rerunFailure>/<rerunError> attempt: always FAILED.
+            Surefire doesn't record a separate duration for these, so duration is 0 (avoids
+            double-counting the testcase's total time against final_result()'s duration).
+            """
+            def child_text(tag: str) -> Optional[str]:
+                child = elem.find(tag)
+                return child.text if child is not None and child.text else None
+
+            attempt_stdout = child_text("system-out") or ""
+            attempt_stderr = child_text("system-err") or child_text("stackTrace") or elem.attrib.get("message") or ""
+            return CaseEvent.TEST_FAILED, attempt_stdout, attempt_stderr, 0.0
+
+        rerun_elems = [c for c in case._elem if c.tag in RERUN_TAGS]
+        flaky_elems = [c for c in case._elem if c.tag in FLAKY_TAGS]
+
+        if rerun_elems:
+            # failed on the first attempt and on every rerun
+            attempts = [final_result()] + [attempt_result(e) for e in rerun_elems]
+        elif flaky_elems:
+            # failed on earlier attempts, passed on the last one
+            attempts = [attempt_result(e) for e in flaky_elems] + [final_result()]
+        else:
+            # no reruns: unchanged, single-event behavior
+            attempts = [final_result()]
+
+        test_path = path_canonicalizer(path_builder(case, suite, report_file))
+        data = data_builder(case)
+
+        return [
+            CaseEvent.create(
+                test_path=test_path,
+                duration_secs=attempt_duration,
+                status=attempt_status,
+                stdout=attempt_stdout,
+                stderr=attempt_stderr,
+                timestamp=suite.timestamp,
+                data=data,
+            )
+            for attempt_status, attempt_stdout, attempt_stderr, attempt_duration in attempts
+        ]
 
     @classmethod
     def create(cls, test_path: TestPath, duration_secs: float, status,

smart_tests/commands/record/tests.py

@@ -6,7 +6,7 @@
 from concurrent.futures import ThreadPoolExecutor, as_completed
 from pathlib import Path
 from time import time_ns
-from typing import Annotated, Callable, Dict, Generator, List, Tuple, Union
+from typing import Annotated, Callable, Dict, Generator, Iterable, List, Tuple, Union
 
 import click
 from dateutil.parser import ParserError, parse
@@ -118,7 +118,8 @@ def parse(report: str) -> Generator[CaseEventType, None, None]:
             try:
                 for suite in testsuites:
                     for case in suite:
-                        yield CaseEvent.from_case_and_suite(self.path_builder, case, suite, report, self.metadata_builder)
+                        yield from CaseEvent.from_case_and_suite(
+                            self.path_builder, case, suite, report, self.metadata_builder)
             except Exception as e:
                 warn_and_exit_if_fail_fast_mode(
                     "Warning: error parsing JUnitXml file {filename}: {error}".format(
@@ -388,23 +389,6 @@ def send(payload: Dict[str, Union[str, List]]) -> None:
             nonlocal is_observation
             is_observation = res.json().get("testSession", {}).get("isObservation", False)
 
-        def recorded_result() -> Tuple[int, int, int, float]:
-            test_count = 0
-            success_count = 0
-            fail_count = 0
-            duration = float(0)
-
-            for tc in testcases(self.reports):
-                test_count += 1
-                status = tc.get("status")
-                if status == 0:
-                    fail_count += 1
-                elif status == 1:
-                    success_count += 1
-                duration += float(tc.get("duration") or 0)  # sec
-
-            return test_count, success_count, fail_count, duration / 60   # sec to min
-
         try:
             start = time_ns()
             tc = testcases(self.reports)
@@ -478,7 +462,7 @@ def recorded_result() -> Tuple[int, int, int, float]:
                 return
 
         file_count = len(self.reports)
-        test_count, success_count, fail_count, duration = recorded_result()
+        test_count, success_count, fail_count, duration = summarize_case_events(testcases(self.reports))
 
         click.echo(
             f"Smart Tests recorded tests for build "
@@ -516,6 +500,36 @@ def _get_test_session_url(self) -> str:
 tests = Group(name="tests", callback=RecordTests, help="Record test results")
 
 
+def summarize_case_events(events: Iterable[CaseEventType]) -> Tuple[int, int, int, float]:
+    """
+    Summarizes a stream of case events into (test_count, success_count, fail_count, duration_minutes),
+    for the "Tests found / Tests passed / Tests failed" table printed after `record tests` runs.
+
+    Grouped by testPath rather than counted per event: a flaky/rerun testcase produces multiple
+    events (see CaseEvent.from_case_and_suite), but it's still a single test. A test that ever
+    passed counts as passed (even if it also failed on earlier attempts); a test that only ever
+    failed counts as failed.
+    """
+    tests: Dict[Tuple, Dict[str, bool]] = {}
+    duration = float(0)
+
+    for tc in events:
+        key = tuple((p["type"], p["name"]) for p in tc.get("testPath", []))
+        entry = tests.setdefault(key, {"passed": False, "failed": False})
+        status = tc.get("status")
+        if status == 1:
+            entry["passed"] = True
+        elif status == 0:
+            entry["failed"] = True
+        duration += float(tc.get("duration") or 0)  # sec
+
+    test_count = len(tests)
+    success_count = sum(1 for e in tests.values() if e["passed"])
+    fail_count = sum(1 for e in tests.values() if e["failed"] and not e["passed"])
+
+    return test_count, success_count, fail_count, duration / 60   # sec to min
+
+
 # if we fail to determine the timestamp of the build, we err on the side of collecting more test reports
 # than no test reports, so we use the 'epoch' timestamp
 INVALID_TIMESTAMP = datetime.datetime.fromtimestamp(0)

tests/commands/record/test_tests.py

@@ -8,7 +8,7 @@
 
 import responses  # type: ignore
 
-from smart_tests.commands.record.tests import INVALID_TIMESTAMP, parse_launchable_timeformat
+from smart_tests.commands.record.tests import INVALID_TIMESTAMP, parse_launchable_timeformat, summarize_case_events
 from smart_tests.utils.http_client import get_base_url
 from tests.cli_test_case import CliTestCase
 
@@ -107,6 +107,50 @@ def test_parse_launchable_timeformat(self):
 
         self.assertEqual(INVALID_TIMESTAMP, parse_launchable_timeformat(t3))
 
+    def _case_event(self, test_path, status, duration=0.0):
+        return {"type": "case", "testPath": test_path, "status": status, "duration": duration,
+                "stdout": "", "stderr": "", "data": None}
+
+    def test_summarize_case_events_single_event_per_test(self):
+        """One event per test (the common case, unaffected by flaky/rerun expansion)."""
+        passing_path = [{"type": "class", "name": "AppTest"}, {"type": "testcase", "name": "shouldWork"}]
+        failing_path = [{"type": "class", "name": "BrokenTest"}, {"type": "testcase", "name": "neverWorks"}]
+        events = [
+            self._case_event(passing_path, status=1, duration=0.01),
+            self._case_event(failing_path, status=0, duration=0.02),
+        ]
+
+        test_count, success_count, fail_count, duration = summarize_case_events(events)
+
+        self.assertEqual((test_count, success_count, fail_count), (2, 1, 1))
+        self.assertAlmostEqual(duration, (0.01 + 0.02) / 60)
+
+    def test_summarize_case_events_rerun_counts_as_one_test(self):
+        """A flaky test (4 FAILED attempts + 1 final PASSED, same testPath) must count as ONE
+        test, not five -- this is exactly the bug the raw per-event counting had."""
+        flaky_path = [{"type": "class", "name": "FlakyTest"}, {"type": "testcase", "name": "passesEventually"}]
+        events = [self._case_event(flaky_path, status=0, duration=0.0) for _ in range(4)]
+        events.append(self._case_event(flaky_path, status=1, duration=0.045))
+
+        test_count, success_count, fail_count, duration = summarize_case_events(events)
+
+        self.assertEqual((test_count, success_count, fail_count), (1, 1, 0))
+        self.assertAlmostEqual(duration, 0.045 / 60)
+
+    def test_summarize_case_events_normal_and_rerun_mixed(self):
+        """A normal test (1 event, PASSED) plus a rerun-fail test (5 events, all FAILED, same
+        testPath) should report 2 tests found / 1 passed / 1 failed -- not 6 / 1 / 5."""
+        normal_path = [{"type": "class", "name": "AppTest"}, {"type": "testcase", "name": "shouldWork"}]
+        rerun_fail_path = [{"type": "class", "name": "AlwaysFailingTest"}, {"type": "testcase", "name": "neverPasses"}]
+        events = [self._case_event(normal_path, status=1, duration=0.002)]
+        events.append(self._case_event(rerun_fail_path, status=0, duration=0.001))
+        events.extend(self._case_event(rerun_fail_path, status=0, duration=0.0) for _ in range(4))
+
+        test_count, success_count, fail_count, duration = summarize_case_events(events)
+
+        self.assertEqual((test_count, success_count, fail_count), (2, 1, 1))
+        self.assertAlmostEqual(duration, (0.002 + 0.001) / 60)
+
     @responses.activate
     @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token})
     def test_when_total_test_duration_zero(self):

tests/data/maven/record_test_flaky_error_result.json

@@ -0,0 +1,45 @@
+{
+    "events": [
+        {
+            "type": "case",
+            "testPath": [
+                {
+                    "type": "class",
+                    "name": "com.cloudbees.app.FlakyErrorDemoTest"
+                },
+                {
+                    "type": "testcase",
+                    "name": "throwsUnexpectedExceptionThenSucceeds"
+                }
+            ],
+            "duration": 0.0,
+            "status": 0,
+            "stdout": "",
+            "stderr": "java.lang.NullPointerException: Cannot invoke \"String.length()\" because \"value\" is null\n    at com.cloudbees.app.FlakyErrorDemoTest.throwsUnexpectedExceptionThenSucceeds(FlakyErrorDemoTest.java:16)\n    at java.base/java.lang.reflect.Method.invoke(Method.java:583)\n    at java.base/java.util.ArrayList.forEach(ArrayList.java:1612)\n    at java.base/java.util.ArrayList.forEach(ArrayList.java:1612)\n",
+            "data": null
+        },
+        {
+            "type": "case",
+            "testPath": [
+                {
+                    "type": "class",
+                    "name": "com.cloudbees.app.FlakyErrorDemoTest"
+                },
+                {
+                    "type": "testcase",
+                    "name": "throwsUnexpectedExceptionThenSucceeds"
+                }
+            ],
+            "duration": 0.001,
+            "status": 1,
+            "stdout": "",
+            "stderr": "",
+            "data": null
+        }
+    ],
+    "testRunner": "maven",
+    "group": "",
+    "noBuild": false,
+    "flavors": [],
+    "testSuite": ""
+}