fix(nb-tools,#16164): check_exec_ratchet — exit 2 « instrument indisponible » à l'épuisement EAGAIN, fin du faux vert

scripts/notebook_tools/check_exec_ratchet.py

@@ -21,54 +21,57 @@
 
 Head verdicts read the working tree (in CI the checkout IS the head), base
 verdicts read the blob via `git show`. Exit code 1 iff at least one
-CLEAN -> non-CLEAN regression exists.
+CLEAN -> non-CLEAN regression exists. Exit code 2 iff git could not be
+spawned at all (instrument unavailable, #16164) -- never confuse "could not
+measure" with "measured 0".
 """
 
 import argparse
-import errno
 import json
 import subprocess
 import sys
-import time
 from pathlib import Path
 
 sys.path.insert(0, str(Path(__file__).resolve().parent))
 
 from check_exec_sequence import code_exec_counts, sequence_verdict
+from fork_retry import run_with_fork_retry
 
 # Same exclusions as notebook-execution-required.yml's detect step plus the
 # checkpoints rule of the tier-1 scanner: archived, papermill-output and
 # research copies are not deliverable notebooks.
 EXCLUDE_MARKERS = ("/.ipynb_checkpoints/", "/archive/", "/_output/",
                    "/research/")
 
-# EAGAIN au spawn (contention de processus, p.ex. pytest-xdist -n 4 sur le
-# runner) est transitoire : le `except OSError: return None` historique
-# transformait ce pic de charge en "changed notebooks : 0" -> faux vert CI
-# (flake diagnostique sur #16125, tentative 3). On retente borne avant de
-# rendre l'echec.
-_EAGAIN_ERRNOS = (errno.EAGAIN, getattr(errno, "EWOULDBLOCK", errno.EAGAIN))
-_EAGAIN_ATTEMPTS = 3
-_EAGAIN_BACKOFF = (0.05, 0.15)  # avant les 2e et 3e tentatives
 
+class InstrumentUnavailable(RuntimeError):
+    """git n'a pas pu etre lance : le ratchet n'a rien mesure (#16164).
 
-def _est_eagain(exc):
-    return isinstance(exc, OSError) and exc.errno in _EAGAIN_ERRNOS
+    Epuisement des retries EAGAIN, ou OSError non transitoire au spawn
+    (ENOENT, EACCES, ...). Distinct de ``returncode != 0`` (git A repondu :
+    l'instrument a tourne, sa reponse vaut None). Decision #16164 :
+    fail-closed sur instrument indisponible, en convergence avec le canon
+    (check_kernel_suffix_canon.py, dont l'OSError propage) -- un garde ne
+    rend jamais un verdict sur un arbre qu'il n'a pas lu.
+    """
+
+    def __init__(self, exc):
+        super().__init__(
+            f"git spawn failed ({exc.__class__.__name__}: "
+            f"errno={getattr(exc, 'errno', None)} {exc})")
 
 
 def git(*args, cwd=None):
-    """Run a git command, returning stdout (utf-8) or None on failure."""
-    for tentative in range(_EAGAIN_ATTEMPTS):
-        try:
-            out = subprocess.run(["git", *args], cwd=cwd, capture_output=True,
-                                 encoding="utf-8", errors="replace",
-                                 check=False)
-            return out.stdout if out.returncode == 0 else None
-        except OSError as exc:
-            if not _est_eagain(exc) or tentative == _EAGAIN_ATTEMPTS - 1:
-                return None
-            time.sleep(_EAGAIN_BACKOFF[tentative])
-    return None
+    """Run a git command, returning stdout (utf-8), None on git-level
+    failure (returncode != 0), or raising InstrumentUnavailable when the
+    spawn itself failed after bounded fork retries (#16217 primitive)."""
+    try:
+        out = run_with_fork_retry(
+            ["git", *args], cwd=cwd, capture_output=True,
+            encoding="utf-8", errors="replace", check=False)
+    except OSError as exc:
+        raise InstrumentUnavailable(exc) from exc
+    return out.stdout if out.returncode == 0 else None
 
 
 def resolve_base(base, cwd=None):
@@ -157,8 +160,20 @@ def main():
                     help="Machine-readable output")
     args = ap.parse_args()
 
-    resolved = resolve_base(args.base)
-    records = ratchet(args.base)
+    try:
+        resolved = resolve_base(args.base)
+        records = ratchet(args.base)
+    except InstrumentUnavailable as exc:
+        # #16164 : "n'a pas pu mesurer" n'est pas "a mesure 0". Le faux vert
+        # historique (changed notebooks : 0 + exit 0) confondait les deux ;
+        # exit 2 est distinct de 1 (regression) pour que CI comme humains
+        # distinguent l'instrument en panne de l'arbre propre.
+        print(f"instrument indisponible : {exc}", file=sys.stderr)
+        print("le ratchet n'a rien mesure -- contention de processus "
+              "(EAGAIN, p.ex. pytest-xdist -n 4) ou git absent du PATH ; "
+              "relancer le job, ne pas lire ceci comme « 0 changements ».",
+              file=sys.stderr)
+        sys.exit(2)
     regressions = [r for r in records if r["regression"]]
 
     if args.as_json:

scripts/notebook_tools/tests/test_check_exec_ratchet.py

@@ -5,6 +5,7 @@
 never required to improve; added notebooks are reported, not failed. No
 network, no kernel.
 """
+import errno
 import json
 import subprocess
 import sys
@@ -248,50 +249,79 @@ class TestTransientSpawnRetry:
     """EAGAIN au spawn = contention de processus transitoire (pytest-xdist
     -n 4 sur le runner). Le `except OSError: return None` historique
     transformait ce pic en "changed notebooks : 0" -> faux vert CI (flake
-    #16125) : le wrapper doit retenter borne, pas rendre None au premier
-    echec de spawn."""
-
-    def _patch_run(self, monkeypatch, etat, resultat_ok):
-        import errno as _errno
-
-        def faux_run(*args, **kwargs):
-            etat["appels"] += 1
-            if etat["appels"] <= etat["echecs"]:
-                raise BlockingIOError(_errno.EAGAIN,
-                                      "Resource temporarily unavailable")
-            return resultat_ok
-
-        monkeypatch.setattr(ratchet.time, "sleep",
-                            lambda s: etat["dors"].append(s))
-        monkeypatch.setattr(ratchet.subprocess, "run", faux_run)
-
-    def test_eagain_retente_puis_passe(self, monkeypatch):
-        etat = {"appels": 0, "echecs": 2, "dors": []}
-        ok = subprocess.CompletedProcess(args=(), returncode=0, stdout="ok\n")
-        self._patch_run(monkeypatch, etat, ok)
-        assert ratchet.git("status") == "ok\n"
-        assert etat["appels"] == 3
-        assert etat["dors"] == list(ratchet._EAGAIN_BACKOFF)
-
-    def test_autre_oserror_rend_none_immediatement(self, monkeypatch):
-        etat = {"appels": 0, "echecs": 1, "dors": []}
-        import errno as _errno
-
-        def faux_run(*args, **kwargs):
-            etat["appels"] += 1
-            raise OSError(_errno.ENOENT, "git introuvable")
-
-        monkeypatch.setattr(ratchet.time, "sleep",
-                            lambda s: etat["dors"].append(s))
-        monkeypatch.setattr(ratchet.subprocess, "run", faux_run)
-        assert ratchet.git("status") is None
-        assert etat["appels"] == 1
-        assert etat["dors"] == []
-
-    def test_eagain_epuise_rend_none_avec_backoff_complet(self, monkeypatch):
-        etat = {"appels": 0, "echecs": 99, "dors": []}
-        ok = subprocess.CompletedProcess(args=(), returncode=0, stdout="ok\n")
-        self._patch_run(monkeypatch, etat, ok)
-        assert ratchet.git("status") is None
-        assert etat["appels"] == ratchet._EAGAIN_ATTEMPTS
-        assert etat["dors"] == list(ratchet._EAGAIN_BACKOFF)
+    #16125).
+
+    Depuis #16164 + #16217 la reprise bornee est mutualisee dans
+    fork_retry.run_with_fork_retry (couverture propre dans
+    test_fork_retry.py). Ce qui reste specifique au ratchet : la traduction
+    de l'OSError remontee en InstrumentUnavailable, et l'absence de retry
+    sur une OSError non transitoire."""
+
+    def _fork_vers_erreur(self, monkeypatch, exc):
+        import fork_retry
+        monkeypatch.setattr(fork_retry.subprocess, "run",
+                            lambda *a, **kw: (_ for _ in ()).throw(exc))
+
+    def test_eagain_epuise_leve_instrument_indisponible(self, monkeypatch):
+        # A l'epuisement des retries, l'echec de spawn monte au CLI (exit 2)
+        # au lieu du faux vert « changed notebooks : 0 ».
+        import fork_retry
+        monkeypatch.setattr(fork_retry.time, "sleep", lambda s: None)
+        self._fork_vers_erreur(
+            monkeypatch,
+            BlockingIOError(errno.EAGAIN, "Resource temporarily unavailable"))
+        with pytest.raises(ratchet.InstrumentUnavailable):
+            ratchet.git("status")
+
+    def test_autre_oserror_leve_instrument_indisponible_sans_retry(
+            self, monkeypatch):
+        # git absent / EACCES n'est pas une reponse : un seul appel, pas de
+        # retry (le filtre etroit est couvert par test_fork_retry.py ; ici on
+        # epingle que le ratchet le respecte).
+        self._fork_vers_erreur(
+            monkeypatch, OSError(errno.ENOENT, "git introuvable"))
+        with pytest.raises(ratchet.InstrumentUnavailable):
+            ratchet.git("status")
+
+
+class TestInstrumentIndisponible:
+    """#16164 : « n'a pas pu mesurer » n'est pas « a mesure 0 ».
+
+    Le contrat lenient historique (git() -> None -> « changed notebooks :
+    0 » -> exit 0) confondait l'echec de spawn avec une mesure nulle. La
+    decision arbitree ici : fail-closed sur instrument indisponible, en
+    convergence avec le canon (check_kernel_suffix_canon.py laisse
+    l'OSError propager) ; exit 2 reste distinct de 1 (regression) et de
+    0 (mesure faite, rien a signaler).
+    """
+
+    def test_verdict_at_base_ne_dit_pas_absent_si_le_blob_est_illisible(
+            self, monkeypatch):
+        # git show en echec de SPAWN n'est pas un blob absent : avant #16164
+        # l'OSError avaldee faisait lire ABSENT (donc « ajoute par la PR »)
+        # a un notebook existant a la base.
+        def faux_git(*args, **kwargs):
+            raise ratchet.InstrumentUnavailable(
+                OSError(errno.EAGAIN, "Resource temporarily unavailable"))
+
+        monkeypatch.setattr(ratchet, "git", faux_git)
+        with pytest.raises(ratchet.InstrumentUnavailable):
+            ratchet.verdict_at_base("origin/main", "a.ipynb")
+
+    def test_cli_exit_2_instrument_indisponible(self, monkeypatch, capsys):
+        # distinct de 0 (mesure propre) et de 1 (regression) ; le message
+        # nomme l'echec de spawn et ne imprime PAS « changed notebooks ».
+        def faux_git(*args, **kwargs):
+            raise ratchet.InstrumentUnavailable(
+                OSError(errno.EAGAIN, "Resource temporarily unavailable"))
+
+        monkeypatch.setattr(ratchet, "git", faux_git)
+        monkeypatch.setattr(sys, "argv",
+                            ["check_exec_ratchet.py", "origin/main"])
+        with pytest.raises(SystemExit) as sortie:
+            ratchet.main()
+        assert sortie.value.code == 2
+        capture = capsys.readouterr()
+        assert "instrument indisponible" in capture.err
+        assert "changed notebooks" not in capture.out
+        assert "changed notebooks" not in capture.err