fix(ci-insights): Always fill test metrics from reports (#347)

The goal of this change is to ensure we are always filling metrics from
a test report. Before this, an early-return skipped the recording for the
setup and teardown phase leading to incomplete metrics (total_duration,
etc.)

Author: remyduthu

pytest_mergify/__init__.py

@@ -352,6 +352,9 @@ def pytest_exception_interact(
             )
 
     def pytest_runtest_logreport(self, report: _pytest.reports.TestReport) -> None:
+        if self.mergify_ci.flaky_detector:
+            self.mergify_ci.flaky_detector.try_fill_metrics_from_report(report)
+
         if self.tracer is None:
             return
 
@@ -377,15 +380,13 @@ def pytest_runtest_logreport(self, report: _pytest.reports.TestReport) -> None:
             }
         )
 
-        if not self.mergify_ci.flaky_detector:
-            return
-
-        if not self.mergify_ci.flaky_detector.try_fill_metrics_from_report(report):
-            return
-
-        test_span.set_attributes({"cicd.test.flaky_detection": True})
-        if self.mergify_ci.flaky_detector.mode == "new":
-            test_span.set_attributes({"cicd.test.new": True})
+        if (
+            self.mergify_ci.flaky_detector
+            and self.mergify_ci.flaky_detector.is_rerunning_test(report.nodeid)
+        ):
+            test_span.set_attributes({"cicd.test.flaky_detection": True})
+            if self.mergify_ci.flaky_detector.mode == "new":
+                test_span.set_attributes({"cicd.test.new": True})
 
 
 def pytest_addoption(parser: _pytest.config.argparsing.Parser) -> None:

pytest_mergify/flaky_detection.py

@@ -179,23 +179,31 @@ def _fetch_context(self) -> _FlakyDetectionContext:
 
         return result
 
-    def try_fill_metrics_from_report(self, report: _pytest.reports.TestReport) -> bool:
-        if report.outcome not in ["failed", "passed", "rerun"]:
-            return False
-
+    def try_fill_metrics_from_report(self, report: _pytest.reports.TestReport) -> None:
         test = report.nodeid
 
+        if report.outcome == "skipped":
+            # Remove metrics for skipped tests. Setup phase may have passed and
+            # initialized metrics before call phase was skipped.
+            self._test_metrics.pop(test, None)
+            return
+
         if test not in self._tests_to_process:
-            return False
+            return
 
         if len(test) > self._context.max_test_name_length:
             self._over_length_tests.add(test)
-            return False
+            return
+
+        if test not in self._test_metrics:
+            if report.when != "setup":
+                # Metrics have been removed (e.g. for a skipped test), do nothing.
+                return
 
-        metrics = self._test_metrics.setdefault(test, _TestMetrics())
-        metrics.fill_from_report(report)
+            # Initialize metrics after setup phase.
+            self._test_metrics[test] = _TestMetrics()
 
-        return True
+        self._test_metrics[test].fill_from_report(report)
 
     def prepare_for_session(self, session: _pytest.main.Session) -> None:
         tests_in_session = {item.nodeid for item in session.items}
@@ -242,7 +250,9 @@ def is_test_too_slow(self, test: str) -> bool:
         )
 
     def is_rerunning_test(self, test: str) -> bool:
-        return test in self._test_metrics
+        return (
+            metrics := self._test_metrics.get(test)
+        ) is not None and metrics.rerun_count >= 1
 
     def is_last_rerun_for_test(self, test: str) -> bool:
         metrics = self._test_metrics[test]

tests/test_flaky_detection.py

@@ -6,6 +6,7 @@
 import freezegun
 import pytest
 
+import pytest_mergify
 from pytest_mergify import flaky_detection
 
 _NOW = datetime.datetime(
@@ -73,23 +74,19 @@ def make_report(
     detector._context = _make_flaky_detection_context(max_test_name_length=100)
     detector._tests_to_process = ["foo"]
 
-    detector.try_fill_metrics_from_report(
-        make_report(nodeid="foo", when="setup", duration=1)
-    )
-    detector.try_fill_metrics_from_report(
-        make_report(nodeid="foo", when="call", duration=2)
-    )
-    detector.try_fill_metrics_from_report(
+    plugin = pytest_mergify.PytestMergify()
+    plugin.mergify_ci = pytest_mergify.ci_insights.MergifyCIInsights()
+    plugin.mergify_ci.flaky_detector = detector
+
+    plugin.pytest_runtest_logreport(make_report(nodeid="foo", when="setup", duration=1))
+    plugin.pytest_runtest_logreport(make_report(nodeid="foo", when="call", duration=2))
+    plugin.pytest_runtest_logreport(
         make_report(nodeid="foo", when="teardown", duration=3)
     )
 
-    detector.try_fill_metrics_from_report(
-        make_report(nodeid="foo", when="setup", duration=4)
-    )
-    detector.try_fill_metrics_from_report(
-        make_report(nodeid="foo", when="call", duration=5)
-    )
-    detector.try_fill_metrics_from_report(
+    plugin.pytest_runtest_logreport(make_report(nodeid="foo", when="setup", duration=4))
+    plugin.pytest_runtest_logreport(make_report(nodeid="foo", when="call", duration=5))
+    plugin.pytest_runtest_logreport(
         make_report(nodeid="foo", when="teardown", duration=6)
     )