Skip to content

ci: Scripts Tests (CPU) -- 3e mode de rouge : un E2E parse stdout avant sa precondition et jette le diagnostic de l'outil (prune_merged_worktrees) #17292

Description

@jsboige

Le défaut

Scripts Tests (CPU) — check REQUIS — porte un troisième mode de rouge, distinct des deux décrits dans #17253 : un test qui parse stdout avant d'établir sa précondition et qui jette le stderr de l'outil. Le rouge se lit alors JSONDecodeError, ce qui n'accuse rien d'utile.

FAILED scripts/tests/test_prune_merged_worktrees.py::TestEndToEnd::test_dry_run_exits_1_when_refusals
scripts/tests/test_prune_merged_worktrees.py:1369: in test_dry_run_exits_1_when_refusals
    out = json.loads(proc.stdout)
E   json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
= 1 failed, 14468 passed, 97 skipped, 8 xfailed in 325.54s

Job 106479334905, run 35643830660, PR #17289, 2026-09-21T19:16Z.

Ce que le code établit (lu, pas supposé)

Le test (test_prune_merged_worktrees.py, l.1358-1375) fait, dans cet ordre :

proc = subprocess.run([..., "--json"], capture_output=True, text=True, ...)
assert proc.returncode in (0, 1), f"unexpected exit: {proc.returncode}"
out = json.loads(proc.stdout)          # <-- l.1369, AVANT toute précondition
if out["scanned"] == 0:
    pytest.skip("no worktree present (CI checkout shallow)")

Sa docstring annonce pourtant l'intention inverse : « CI : skip si scanned=0 OU si aucun worktree main n'est présent ». La précondition est donc écrite, mais évaluée après l'endroit où elle serait nécessaire.

Le script (scripts/ci/prune_merged_worktrees.py) n'explique pas le couple observé :

Chemin Sortie Code
--json nominal (l.1352) JSON toujours imprimé 0 ou 1
ERROR: l.1280-1281 (list_worktrees échoue) stdout vide 2
ERROR diagnosing … l.1298-1299 stdout vide 2

Les deux chemins d'erreur rendent 2, or le test a vu rc ∈ {0,1} avec stdout vide. Ce couple est la signature d'une exception non rattrapée au milieu de main() : Python rend alors 1, n'écrit rien sur stdout, et dépose le traceback sur stderr. Le script ne rattrape que RuntimeError (l.1277-1299) — tout autre type (CalledProcessError, OSError, KeyError, URLError…) s'échappe. capture_output=True avale alors le traceback, et le test le jette : le seul artefact qui nommait la cause est perdu.

Deux défauts, pas un :

  1. Contrat --json troué : une exception non rattrapée sort en rc=1, indistinguable du rc=1 documenté = « des refus ont été observés ». Un appelant ne peut pas séparer « l'outil a tourné et refusé » de « l'outil n'a pas pu tourner ».
  2. Le test convertit une panne d'outil en erreur de parsing, et jette le diagnostic.

La mesure : flaky, cross-lane, sans changement de code

Observé le 2026-09-21 Verdict de Scripts Tests (CPU)
PR #17289 (fix/16878-…, 19:16Z) failure — ce test
PR #17286 (fix/16962-…, 18:56Z) failure — ce test
PR #17285 (18:50Z) failure — ce test
branche feat/14831-… (19:02Z) failure
branche feat/nav-chain-reachability (18:50Z) failure
main a8aafc3f (19:08Z) failure
main b5c64988 (19:11Z) success

Aucun des deux commits main ne touche scripts/ci/prune_merged_worktrees.py ni son test (git log --oneline origin/main -- sur les deux fichiers : dernier changement = #15373). Le verdict change donc selon l'état du runner, pas selon le contenu : c'est un flaky, et il rougit des PRs de plusieurs lanes à la fois.

Deux fausses pistes écartées par la mesure (à ne pas refaire)

  • gh injoignable n'est pas ce mode. Reproduit ici avec GH_HOST=127.0.0.1 : rc=2 + ERROR diagnosing …: gh pr list failed: …, stdout vide. Or le test aurait alors échoué sur assert proc.returncode in (0, 1) avec « unexpected exit: 2 » — pas sur json.loads. Ce chemin est donc un autre mode, et il illustre exactement pourquoi le contrat --json doit nommer ses échecs.
  • Un traceback vu sur stderr en rc=0 n'est pas un défaut : c'était un BrokenPipeError provoqué par mon propre | head sur la mesure. (Le rc d'une mesure n'est pas une mesure.)

Ce que je ne prétends pas : je n'ai pas reproduit l'exception exacte du runner. J'établis le trou de contrat (couple rc ∈ {0,1} + stdout vide impossible par les chemins d'erreur nommés) et l'aveuglement du test. Le type d'exception reste à capturer sur la prochaine occurrence — ce que le critère 1 rend possible.

Ce que ça coûte

Ce rouge gèle la merge de la flotte (check requis), il est imputé à la PR alors qu'il vient du runner, et la prochaine occurrence sera tout aussi opaque : le test jettera de nouveau le traceback.

Critère d'acceptation

  1. Le diagnostic remonte : sur stdout vide (ou non-JSON), le test rapporte le stderr du sous-processus verbatim dans son message — plus jamais un JSONDecodeError nu.
  2. rc=1 redevient non ambigu : une exception inattendue dans le script sort par son code d'erreur documenté (2) avec le traceback sur stderr ; rc=1 ne signifie plus que « des refus ont été observés ».
  3. La précondition annoncée est évaluée avant d'être nécessaire : « pas de worktree exploitable dans ce checkout CI » reste un skip nommé (rc=2 ou scanned == 0), jamais un silence ; toute autre absence de sortie reste un échec dur.
  4. Fail-closed préservé : un vrai désaccord (worktree main absent alors qu'il est présent, verdict inattendu, refus disparu) échoue toujours durement — le correctif ne doit pas transformer le test en skip permanent. Un test qui ne peut plus échouer n'est pas un correctif.

Hors périmètre

Les deux variantes de #17253 (fetch promisor #17231 ; job tué en SIGKILL) et leur cause racine (quota d'installation 403, pression mémoire du runner). Cette issue traite le test de prune_merged_worktrees et le contrat d'erreur du script qu'il pilote.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions