internal(pytest-mergify): honour an empty and a refused test selection

clients/pytest-mergify/README.md

@@ -48,12 +48,21 @@ bisection — only the tests that failed on the previous attempt are
 informative. The plugin asks Mergify whether the current run is such a rerun
 and, if so, runs only those tests; the rest are reported as deselected.
 
-Nothing to configure: the plugin uses the token and job identity it already
-has, and Mergify decides. Any other situation — a normal run, a rerun Mergify
-has no previous results for, an unreachable API — runs the full suite, so the
-feature can only remove work, never coverage. The feature is also enabled per
-organization on Mergify's side, so it stays inactive until your organization
-is opted in.
+Two other answers exist. Mergify may say the previous attempt of this job
+already ran every one of these tests and they all passed: the run then executes
+no test and exits green, and still reports itself so the attempt is visible.
+Or Mergify may stop the run outright. That happens when several runs of this
+job report under the same name and run the same tests: Mergify cannot tell
+which one the current run repeats, and it will not guess which tests to skip.
+The run then **fails**, showing Mergify's explanation of what it saw — usually
+asking you to give each of those runs its own `MERGIFY_TEST_JOB_NAME`.
+
+Otherwise there is nothing to configure: the plugin uses the token and job
+identity it already has, and Mergify decides. Every remaining situation — a
+normal run, a rerun Mergify has no previous results for, an unreachable API, an
+answer from a newer Mergify this plugin does not understand — runs the full
+suite, so the feature never costs coverage. It is also enabled per organization
+on Mergify's side, so it stays inactive until your organization is opted in.
 
 Set `MERGIFY_TEST_SELECTION_DISABLE=true` in your CI to opt out: the plugin
 then always runs the full suite and never queries the endpoint. It is scoped

clients/pytest-mergify/python/pytest_mergify/__init__.py

@@ -294,7 +294,16 @@ def pytest_collection_modifyitems(
         self.mergify_ci.on_tests_collected([item.nodeid for item in items])
 
         if self.mergify_ci.test_selection:
-            self.mergify_ci.test_selection.filter_items(config, items)
+            try:
+                self.mergify_ci.test_selection.filter_items(config, items)
+            except pytest.UsageError:
+                # A refusal ends the run here, before any test span exists. The
+                # session is still uploaded (from `pytest_sessionfinish`), and
+                # without this it would reach Mergify as a finished, error-free
+                # session holding no test — the exact shape of a run that was
+                # legitimately told to execute nothing.
+                self.has_error = True
+                raise
 
     def pytest_collection_finish(self, session: _pytest.main.Session) -> None:
         detector = self.mergify_ci.flaky_detector
@@ -321,6 +330,7 @@ def pytest_collection_finish(self, session: _pytest.main.Session) -> None:
     def pytest_sessionfinish(
         self,
         session: _pytest.main.Session,
+        exitstatus: int,
     ) -> typing.Generator[None, None, None]:
         # xdist worker: export metrics via workeroutput (independent of tracer).
         if _is_xdist_worker(session.config):
@@ -335,17 +345,45 @@ def pytest_sessionfinish(
                         self.mergify_ci.test_retrier.to_serializable_metrics()
                     )
 
-        if not self._traces_enabled or self._session_span is None:
-            yield
-            return
-
         yield
 
         # Export here rather than in the terminal summary: the summary's token
         # checks return early on a run with nothing to upload, but the capture
         # path (tests) and the debug path have spans to hand off regardless.
+        # Called unconditionally -- it holds the "traces off / already exported"
+        # guard itself, and stating it twice is how the two drift apart.
         self._finalize_and_export()
 
+        self._green_a_run_told_to_execute_nothing(session, exitstatus)
+
+    def _green_a_run_told_to_execute_nothing(
+        self,
+        session: _pytest.main.Session,
+        exitstatus: int,
+    ) -> None:
+        """Turn "no tests ran" into a pass when Mergify asked for exactly that.
+
+        An `empty` selection deselects the whole collection, which pytest
+        reports as exit code 5 — the code that means "you thought you were
+        running tests and you were not". Here the run did precisely what it was
+        told, so it exits 0.
+
+        Outside the export above rather than inside it, because the verdict a
+        job reports does not depend on whether this run had traces to upload:
+        `PYTEST_MERGIFY_DEBUG`, or a token the plugin never got, must not turn a
+        deliberately empty run red.
+        """
+        selection = self.mergify_ci.test_selection
+        if (
+            exitstatus == pytest.ExitCode.NO_TESTS_COLLECTED
+            and selection is not None
+            and selection.selection == "empty"
+            # A run that collected nothing to begin with is not a run Mergify
+            # emptied, and its exit code 5 is the real answer.
+            and selection.deselected_count > 0
+        ):
+            session.exitstatus = pytest.ExitCode.OK
+
     def _finalize_and_export(self) -> None:
         """Close the session span and export every collected span, exactly once.
 

clients/pytest-mergify/python/pytest_mergify/ci_insights.py

@@ -333,6 +333,12 @@ def _load_test_selection(self, collection_fingerprint: str) -> None:
                 selection=fetched["selection"],
                 reason=fetched["reason"],
                 tests=fetched["tests"],
+                # `.get`, unlike the keys above, even though the binding sets
+                # this key on every answer: the plugin already holds a wording
+                # for a refusal that arrives without one, so a binding that
+                # stopped setting it should reach the user as that fallback
+                # rather than as a `KeyError` crashing their pytest session.
+                message=fetched.get("message"),
             )
 
     def _load_run_context(self) -> None:

clients/pytest-mergify/python/pytest_mergify/test_selection.py

@@ -3,6 +3,43 @@
 
 import _pytest.config
 import _pytest.nodes
+import pytest
+
+
+# What a refusal says when the server sent no wording of its own. The copy
+# belongs to the server -- it can be corrected there without publishing a
+# client, and it alone knows which job it is talking about -- so this is a
+# fallback, not the message.
+#
+# Written for someone who has jobs and runs, not for someone who knows how
+# Mergify stores them: "test session", "previous attempt" and "the run this one
+# continues" are our vocabulary, and a reader meeting them in a red build
+# learns nothing. So it opens with what happened to THEM, then why, then the
+# fix with its documentation, then the way out when the fix does not apply.
+#
+# It still asserts no cause. A build matrix is the likely producer of several
+# runs under one name, not the only one -- a job rerun on the same revision
+# leaves the same signature, and renaming per matrix leg would not fix it.
+# Hence the condition on the remedy, and the last line.
+#
+# It names no job, unlike the server's message, which formats one in: the
+# plugin would have to carry the job name on every answer to enrich a string
+# that renders only if the marshalling loses `message` -- a field on every
+# answer to improve one that is not supposed to appear.
+FALLBACK_REFUSAL_MESSAGE = (
+    "Mergify Test Selection stopped this run.\n"
+    "\n"
+    "Several runs of this job report to Mergify under the same name, and they"
+    " run the same tests — so Mergify cannot tell which one this run repeats,"
+    " and it will not guess which tests to skip.\n"
+    "\n"
+    "If this job runs more than once (a build matrix, for example), give each"
+    " run its own name with MERGIFY_TEST_JOB_NAME:\n"
+    "https://docs.mergify.com/ci-insights/test-frameworks/pytest/\n"
+    "\n"
+    "If this job only runs once, this is unexpected — please contact Mergify"
+    " support."
+)
 
 
 @dataclasses.dataclass
@@ -17,21 +54,45 @@ class TestSelection:
     tests the previous attempt did, so the request cannot be made before the
     collection is known. The bundled binding
     (`CiApiClient.fetch_test_selection`) fetches the answer and it is injected
-    here. Every error, timeout, or unknown situation degrades to running the
-    full suite — this feature can only remove work, never correctness.
+    here.
+
+    Four answers are understood:
+
+    * `full` -- run everything.
+    * `subset` -- run only `tests`.
+    * `empty` -- run nothing: the predecessor's attempt of this job already ran
+      these tests and they passed. The run exits green having executed none of
+      them, and still uploads its session.
+    * `refused` -- Mergify holds several candidate sessions for this job and
+      will not guess between them. The run FAILS, showing the server's own
+      explanation (`message`), or `FALLBACK_REFUSAL_MESSAGE` if it sent none.
+
+    Every error, timeout, and every answer outside that list degrades to
+    running the full suite — the feature can remove work, never correctness,
+    and a client is routinely older than the server it talks to.
     """
 
-    selection: typing.Literal["full", "subset"] = "full"
+    selection: typing.Literal["full", "subset", "empty", "refused"] = "full"
     reason: str = "not_requested"
     tests: typing.List[str] = dataclasses.field(default_factory=list)
+    # What the server wants shown to the CI user about this answer, when it has
+    # something to say -- today only a refusal does. Shown verbatim: the wording
+    # is the server's so it can be improved without publishing a client.
+    message: typing.Optional[str] = None
     init_error_msg: typing.Optional[str] = None
     kept_count: typing.Optional[int] = dataclasses.field(init=False, default=None)
     deselected_count: int = dataclasses.field(init=False, default=0)
 
     def __post_init__(self) -> None:
-        # A subset is only honoured with a non-empty list; anything else (a
-        # `full` answer, or a `subset` the server sent empty) runs everything.
-        if not (self.selection == "subset" and self.tests):
+        # `empty` and `refused` are answers in themselves and carry no tests.
+        # A subset is only honoured with a non-empty list; anything else -- a
+        # `full` answer, a `subset` the server sent empty, or a variant this
+        # client predates -- runs everything. Acting on a value we cannot
+        # reason about is the one outcome that loses coverage silently, on a
+        # run that reports green.
+        if self.selection in ("empty", "refused"):
+            self.tests = []
+        elif not (self.selection == "subset" and self.tests):
             self.selection = "full"
             self.tests = []
 
@@ -40,14 +101,31 @@ def filter_items(
         config: _pytest.config.Config,
         items: typing.List[_pytest.nodes.Item],
     ) -> None:
-        """Reduce the collected items to the served subset, in place.
+        """Apply the served answer to the collected items, in place.
 
         Matching is by exact nodeid — the identifiers Mergify serves are the
         ones this plugin previously uploaded. Served names absent from the
         collection are ignored; if NOTHING matches (e.g. the tests were
         renamed since the previous attempt), the full suite runs — an empty
         reduced run would turn green without testing anything.
+
+        Raises `pytest.UsageError` on a refusal, which is what fails the run,
+        carrying the server's explanation of it.
         """
+        if self.selection == "refused":
+            # Deliberately not the degradation path. Everywhere else, a shape
+            # Mergify cannot resolve costs time and nothing else; here it is
+            # Mergify saying it holds several candidate predecessors for this
+            # job, which means one job name is standing for several runs. That
+            # keeps the reporting wrong for every future attempt, so it has to
+            # be seen and fixed rather than absorbed into a full run nobody
+            # notices.
+            raise pytest.UsageError(self.message or FALLBACK_REFUSAL_MESSAGE)
+
+        if self.selection == "empty":
+            self._deselect_everything(config, items)
+            return
+
         if self.selection != "subset":
             return
 
@@ -66,10 +144,41 @@ def filter_items(
         self.kept_count = len(kept)
         self.deselected_count = len(deselected)
 
+    def _deselect_everything(
+        self,
+        config: _pytest.config.Config,
+        items: typing.List[_pytest.nodes.Item],
+    ) -> None:
+        """Empty the collection through pytest's own deselection path.
+
+        Deselecting rather than stopping the session is what keeps the rest of
+        the run intact: the session still finishes, so it still uploads. A
+        `pytest.exit` here would be shorter and would make the one job that
+        legitimately ran nothing the only one missing from Mergify's reporting.
+
+        A collection that is already empty is left alone, counters included: the
+        run is then red for a reason of its own (a `-k` matching nothing), and
+        recording an application would have this answer both green that exit
+        code and announce a skip over a suite it never emptied.
+        """
+        if not items:
+            return
+
+        self.deselected_count = len(items)
+        deselected = list(items)
+        items[:] = []
+        config.hook.pytest_deselected(items=deselected)
+
     def report(self) -> str:
         report_str = f"""✂️ Test selection
 - Selection: {self.selection} (reason: {self.reason})
 """
         if self.selection == "subset" and self.kept_count is not None:
             report_str += f"- Reduced rerun: executing {self.kept_count} previously-failing test(s), {self.deselected_count} deselected\n"
+        elif self.selection == "empty" and self.deselected_count:
+            # "selected", not "collected": this hook runs `trylast`, so the
+            # count is what survived the user's own filters, and pytest's own
+            # header two lines above already spends "collected" on the number
+            # before them.
+            report_str += f"- Skipped rerun: executing no test, the previous attempt of this job ran all {self.deselected_count} selected test(s) and they passed\n"
         return report_str

clients/pytest-mergify/src/lib.rs

@@ -114,8 +114,8 @@ impl CiApiClient {
         }
     }
 
-    /// The test selection as a dict (`selection`, `reason`, `tests`), or `None`
-    /// when test selection is not enabled for the repository.
+    /// The test selection as a dict (`selection`, `reason`, `tests`, `message`),
+    /// or `None` when test selection is not enabled for the repository.
     ///
     /// `collection_fingerprint` is what [`compute_test_collection_fingerprint`]
     /// returned for the tests this run collected — which is why the plugin asks
@@ -201,13 +201,19 @@ fn test_selection_dict(py: Python<'_>, selection: &TestSelection) -> PyResult<Py
     let dict = PyDict::new(py);
     dict.set_item("selection", &selection.selection)?;
     dict.set_item("reason", &selection.reason)?;
-    // `tests` is `None` for any answer that carries no subset -- `full`, and
-    // any variant this client predates -- since a `subset` without it is
-    // rejected upstream. An empty list is the right value for the plugin: it
-    // keeps the key present, so `TestSelection(**dict)` never raises, and the
-    // Python-side normalisation reads it as "nothing to select" and runs
-    // everything.
+    // `tests` is `None` for every answer that carries no subset -- `full`,
+    // `empty`, `refused`, and any variant this client predates -- since a
+    // `subset` without it is rejected upstream. An empty list is the right
+    // value for the plugin: it keeps the key present, so `TestSelection(**dict)`
+    // never raises, and what the answer then means is decided on the Python
+    // side from `selection` alone, never from the emptiness of this list.
     dict.set_item("tests", selection.tests.clone().unwrap_or_default())?;
+    // Unlike `tests`, this one is handed over as-is rather than defaulted: the
+    // dict then mirrors the wire, where the key is simply absent from every
+    // answer carrying no copy. Nothing downstream distinguishes `None` from an
+    // empty string -- both fall to the plugin's own wording -- so this is about
+    // the dict describing the answer honestly, not about a signal being read.
+    dict.set_item("message", selection.message.clone())?;
     Ok(dict.into())
 }