diff --git a/scripts/ci/guard_test_root.py b/scripts/ci/guard_test_root.py index 88b8ec81bb..17a7fe2775 100644 --- a/scripts/ci/guard_test_root.py +++ b/scripts/ci/guard_test_root.py @@ -35,41 +35,373 @@ REPO_ROOT = Path(__file__).resolve().parents[2] DEFAULT_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "scripts-tests.yml" -# On matche le bloc pytest multi-lignes : pytest \ puis les chemins -# (termines par \) jusqu'a `-n ` qui signale la fin des args pytest. -PYTEST_BLOCK_RE = re.compile( - r"pytest\s*\\\s*\n" - r"((?:[^\n]*\\\s*\n)+?)" - r"\s*-n\s", + +class PytestBlockNotFound(Exception): + """Le workflow contient `pytest` mais le bloc multi-lignes ne matche + pas. C'est un defaut de lecture -- le garde ne peut pas verifier + quoi que ce soit et DOIT le signaler plutot que rendre `ok=True` a + tort. Cf. CONCERNS coordinateur (c.86) sur #18896 + CONCERNS + adjoint po-2025 (c.87) sur #18951 (inline pytest + derniere ligne + sans continuation).""" + + +class UnknownPytestOption(Exception): + """Le bloc pytest utilise une option longue hors des deux sets + documentes (booleen ou a-valeur). Le garde REFUSE plutot que de + supposer que l'option prend une valeur au token suivant -- cette + supposition est un faux vert mesure par l'adjoint po-2025 c.93 sur + #18951 (comment 5967232538) : `pytest --trace-config X Y` rendait + `paths=[Y]` parce que `--trace-config` etait suppose prendre une + valeur, X etait avale, et le test racine etait invisible. + + Cf. CONCERNS adjoint po-2025 c.93 sur #18951 : la politique est + "toute option longue doit etre dans un set explicite (booleen ou + a-valeur) ou on refuse" -- extension de la politique c.86 "soit on + mesure, soit on refuse" a la classe d'arite. + """ + +# On matche un APPEL pytest complet : `pytest` (ou `python -m pytest`) +# suivi d'une liste d'arguments qui peut tenir sur 1 ligne (inline) ou +# sur N lignes (multi-lignes YAML block scalar, chaque ligne +# indentee). La fin de l'appel est signalee par une ligne vide, un +# retour en col 0 (debut d'une autre cle YAML), ou la fin de fichier. +# Cf. #18896 (CONCERNS c.86) : la version precedente exigeait `-n` en +# fin de bloc, ce qui rendait l extraction VIDE pour les workflows +# pytest valides sans xdist. Cf. #18951 (CONCERNS c.87) : la version +# c.86 ne capturait pas la derniere ligne d'arguments sans +# continuation, et ignorait completement les appels inline -- le +# contrat "soit on mesure, soit on refuse" n'etait pas tenu sur ces +# deux cas. +# +# Strategie : on trouve la ligne qui contient `pytest` (eventuellement +# apres `python -m`). Le bloc d'arguments est forme de toutes les +# lignes consecutives qui sont (a) NON VIDES, (b) indentées au moins +# autant que la ligne `pytest` (donc on reste dans le meme bloc YAML), +# et (c) qui ne sont pas une autre cle YAML (commencent par un +# alphabetique ou un caractere special YAML). La ligne peut finir par +# `\` (multi-lignes) ou par autre chose (fin de l'appel). +PYTEST_INVOCATION_RE = re.compile( + r"(?:(?<=\s)|(?<=\A)|(?<=\=))(?:python[ \t]+(?:-m[ \t]+)?)?pytest(?=[ \t]+\S)", re.MULTILINE, ) +# Options pytest qui sont BOOLEENNES (sans valeur). Une option +# booleenne ne doit pas declencher `skip_next` dans le parser -- sinon +# le token suivant (un chemin) est avale comme valeur de l'option. +# Cf. CONCERNS adjoint po-2025 c.9 sur #18951 : `pytest --verbose +# scripts/notebook_tools/tests/` rendait `paths=[]` car `--verbose` +# etait traite comme `--xxx` (valeur) et le chemin etait avale. +# Reference : https://docs.pytest.org/en/stable/reference/reference.html#command-line-flags +PYTEST_BOOLEAN_OPTIONS: frozenset[str] = frozenset({ + # Short boolean + "-v", "-q", "-s", "-x", "-l", "-h", + # Long boolean + "--verbose", "--quiet", "--exitfirst", "--capture=no", "-s", + "--strict", "--strict-markers", "--strict-config", + "--doctest-modules", "--doctest-ellipsis", "--doctest-glob", + "--continue-on-collection-errors", + "--co", "--collect-only", + "--showlocals", + "--lf", "--last-failed", + "--ff", "--failed-first", + "--sw", "--stepwise", + "--sw-skip", "--stepwise-skip", + "--nf", "--new-first", + "--cache-show", "--cache-clear", + "--ci", + "--runxfail", + "--no-header", "--no-summary", "--no-cov", "--no-cov-on-fail", + "--benchmark-disable", "--benchmark-only", "--benchmark-skip", + # c.92 adjoint review (comment 5966800233) : --disable-warnings + # and its alias --disable-pytest-warnings are recognized by + # `python -m pytest --help` and were missing from this list. + # Without them, `pytest --disable-warnings X/` ate X as a value. + "--disable-warnings", "--disable-pytest-warnings", + # Documentation flags (c.92 -- no value, exit immediately). + "--help", "--version", + "-h", +}) + +# Options pytest qui prennent une VALEUR au token suivant (ou via `=`). +# Une option a valeur DOIT declencher `skip_next` pour avaler le token +# suivant comme valeur. Si une option longue n'est ni dans +# PYTEST_BOOLEAN_OPTIONS ni dans PYTEST_VALUE_OPTIONS, le garde REFUSE +# plutot que de supposer son arite -- politique "toute option longue doit +# etre dans un set explicite ou on refuse" (extension c.94 de la politique +# c.86 "soit on mesure, soit on refuse", CONCERNS adjoint po-2025 c.93 +# sur #18951). +# +# Source : grep sur `.github/workflows/*.yml` pour `--xxx=VAL` et +# `--xxx VAL`. 3 options mesurees au c.94 : +# --durations : affiche les N tests les plus lents +# --durations-min : filtre par seuil (en secondes) +# --tb : format de traceback (short/long/line/no/auto/native) +# Cf. https://docs.pytest.org/en/stable/reference/reference.html#command-line-flags +PYTEST_VALUE_OPTIONS: frozenset[str] = frozenset({ + "--durations", + "--durations-min", + "--tb", + # c.94 : --dist est utilise dans .github/workflows/scripts-tests.yml + # sous la forme `-n 4 --dist loadscope --tb=short -q`. Sans + # whitelist, la nouvelle politique "refuser les options longues + # hors set" (c.94) leve `UnknownPytestOption` sur le workflow + # reel du depot. Cf. test_parse_with_xdist_returns_all_paths + + # test_parse_filters_long_option_values. + "--dist", +}) + def parse_collected_paths(workflow: Path) -> list[str]: """Lit la liste des chemins collectes par pytest depuis le YAML. - Les arguments de pytest sont multi-lignes, termines par \\. On extrait le - bloc, on nettoie les continuations de ligne, on splitte sur les espaces, - on filtre tout ce qui ressemble a un flag (-x, --dist, ...) ou un - nombre. + Strategie : on cherche le premier appel `pytest` (avec ou sans + `python -m`). Une fois trouve, on collecte tous les tokens qui + suivent sur la meme ligne (cas inline) et sur les lignes suivantes + qui sont dans le meme bloc YAML (meme niveau d'indentation ou plus, + ou continuation par backslash). On s'arrete a la premiere ligne qui : + - est vide, ou + - commence en col 0 (sortie du bloc YAML), ou + - commence par un caractere YAML (`-`, `#`, etc., debut d'une + autre entree de liste). + + Si le workflow ne contient pas du tout `pytest`, on rend une liste + vide (le workflow n'est pas dans le scope de cette garde). Si + `pytest` est present mais qu'on ne peut pas extraire d'arguments + (par exemple "pytest \" seul sans chemins), on leve + ``PytestBlockNotFound`` : le garde DOIT refuser plutot que rendre + ``ok=True`` sans rien verifier (#18896 c.86, #18951 c.87). """ text = workflow.read_text(encoding="utf-8") - match = PYTEST_BLOCK_RE.search(text) - if not match: + m = _find_pytest_invocation(text) + if m is None: return [] - block = match.group(1) - joined = re.sub(r"\s*\\\s*\n\s*", " ", block) + # Si le match provient d'un scalaire YAML decode, on travaille sur + # la version decodee (le match pointe sur du texte decode, pas sur + # le texte brut). Cf. CONCERNS adjoint po-2025 c.9 sur #18951. + if _is_decoded_match(m): + work_text = m.decoded_text + pytest_end = m.end() + else: + work_text = text + pytest_end = m.end() + # Capture du bloc d'arguments. On collecte toutes les lignes qui + # font partie du meme appel pytest (meme bloc YAML, ou inline). + rest = work_text[pytest_end:] + lines = rest.split("\n") + args_lines: list[str] = [] + if lines: + # La ligne qui contient pytest (apres le mot pytest) -- cas + # inline : on prend tout jusqu'au \n. + first_line = lines[0] + # Si la ligne se termine par `\\`, c'est une continuation. + first_has_cont = first_line.rstrip().endswith("\\") + args_lines.append(first_line) + # Pour les lignes suivantes (cas multi-lignes), on continue tant + # qu'on est dans le meme bloc YAML. Une ligne vide, une ligne en + # col 0 (sans indentation), ou une ligne qui commence par `-`/`#` + # met fin au bloc. + base_indent = _line_indent(work_text, m.start()) + for line in lines[1:]: + if not line.strip(): + break # ligne vide + line_indent = _line_indent_of(line) + # Si on est dans une continuation `\\`, on accepte les lignes + # indentees jusqu'a la fin du bloc. + if line_indent == 0: + break # retour en col 0 : sortie du bloc YAML + if line.lstrip().startswith(("-", "#")) and line_indent <= base_indent: + break # nouvelle entree de liste ou commentaire en col 0 + # On accepte la ligne (avec ou sans \). + args_lines.append(line) + if not line.rstrip().endswith("\\"): + # Derniere ligne du bloc d'arguments. + break + # Joindre les lignes en nettoyant les continuations. + joined = " ".join(args_lines) + joined = re.sub(r"[ \t]*\\[ \t]*\n[ \t]*", " ", joined) + # Filtrer les `\` orphelins (sans continuation) et les separateurs + # YAML residuels. + joined = re.sub(r"(? int: + """Indentation de la ligne qui contient la position pos dans text.""" + start = text.rfind("\n", 0, pos) + 1 + line = text[start:pos] + return len(line) - len(line.lstrip()) + + +def _line_indent_of(line: str) -> int: + """Indentation d'une ligne donnee.""" + return len(line) - len(line.lstrip()) + + +def _find_pytest_invocation(text: str) -> re.Match | None: + """Trouve le premier appel pytest qui n'est pas dans un commentaire + YAML ni dans une chaine quotée non-scalaire. Heuristique : on + considere pytest comme un appel si (a) la ligne ne commence pas + par `#`, et (b) pytest est precede d'un whitespace ou debut de + ligne, et (c) pytest est suivi d'un separateur d'argument (`\\s` ou + fin de ligne) -- un `pytest-x` dans une dependances pip n'est pas + un appel. + + Cas special CONCERNS adjoint po-2025 c.9 sur #18951 : un + `run: "python -m pytest scripts/notebook_tools/tests/ -q"` est un + **scalaire YAML valide** (la ligne entiere est entre quotes, c'est + legal en GitHub Actions). L'ancienne implementation rejetait + pytest dans cette chaîne (quote_count impair) et rendait + `paths=[]`. Le fix : si la ligne est un scalaire YAML entre + quotes et contient pytest, on decode le contenu (retire les + quotes) et on re-matche la regex sur la version decodee. + + Renvoie soit un match direct sur `text`, soit un objet + ``_DecodedMatch`` (namedtuple-like) qui wrap un match interne + + le texte decode. Voir ``_is_decoded_match``. + """ + return _find_pytest_in_decoded_or_raw(text) + + +def _is_decoded_match(m) -> bool: + """Vrai si le match provient d'un scalaire YAML decode.""" + return isinstance(m, _DecodedMatch) + + +class _DecodedMatch: + """Wrapper d'un re.Match + le texte decode sur lequel il a ete + trouve. Necessaire parce que ``re.Match`` n'accepte pas + l'attribution d'attributs (objet immutable).""" + __slots__ = ("match", "decoded_text") + + def __init__(self, match: re.Match, decoded_text: str): + self.match = match + self.decoded_text = decoded_text + + def start(self) -> int: + return self.match.start() + + def end(self) -> int: + return self.match.end() + + +def _find_pytest_in_decoded_or_raw(text: str): + """Implementation complete : essaie d'abord sur le texte brut, puis + sur les scalaires YAML decodes (run: "..." ou run: '...').""" + m = _find_pytest_raw(text) + if m is not None: + return m + for _line_start, _line_end, decoded in _iter_yaml_scalar_lines(text): + m = _find_pytest_raw(decoded) + if m is not None: + return _DecodedMatch(m, decoded) + return None + + +def _iter_yaml_scalar_lines(text: str): + """Iterateur sur les lignes de la forme `cle: "..."` ou `cle: '...'` + ou `- cle: "..."` (sequence item). Renvoie (line_start, line_end, decoded).""" + for line in text.splitlines(keepends=True): + stripped = line.lstrip() + # Forme : `key: "value"`, `key: 'value'`, ou `- key: "value"` + # (sequence item contenant un mapping). + if ":" not in stripped: + continue + # Retirer le prefix `- ` (sequence item) si present. + if stripped.startswith("-"): + stripped = stripped[1:].lstrip() + if ":" not in stripped: + continue + key, _, rest = stripped.partition(":") + rest = rest.strip() + if not key.replace("-", "").replace("_", "").isalnum(): + continue + if len(rest) >= 2 and rest[0] == rest[-1] and rest[0] in ('"', "'"): + decoded = rest[1:-1] + line_start = text.find(line) + line_end = line_start + len(line) + yield (line_start, line_end, decoded) + + +def _find_pytest_raw(text: str) -> re.Match | None: + """Recherche brute d'un appel pytest dans `text`. Renvoie le match + ou None. Cf. CONCERNS c.86 (sans-xdist) et c.87 (inline).""" + for m in PYTEST_INVOCATION_RE.finditer(text): + line_start = text.rfind("\n", 0, m.start()) + 1 + line_end = text.find("\n", m.end()) + if line_end == -1: + line_end = len(text) + line = text[line_start:line_end] + stripped = line.lstrip() + # Commentaire YAML : skip. + if stripped.startswith("#"): + continue + # Quote check : sur la portion de ligne jusqu'au debut de pytest, + # compter les apostrophes et guillemets. Si le compte est + # impair, pytest est dans une chaîne YAML non-scalaire et + # n'est pas un appel. + prefix = line[: m.start() - line_start] + quote_count = sum(1 for c in prefix if c in ("'", '"')) + if quote_count % 2 == 1: + continue + return m + return None + + def _is_collected(entry: Path, collected_dirs: set[Path], collected_files: set[Path]) -> bool: """Vrai si le fichier entry est dans un chemin collecte. @@ -172,6 +504,21 @@ def main(argv: list[str] | None = None) -> int: try: collected = parse_collected_paths(args.workflow) + except PytestBlockNotFound as e: + # #18896 (c.86) : si le workflow contient `pytest` mais que le + # pattern multi-lignes ne matche pas, c'est un defaut de lecture + # -- le garde rendait `ok=True` sans rien verifier. On remonte + # un statut explicite (rc=2) et un message qui nomme la cause. + print(f"FAIL: {e}", file=sys.stderr) + return 2 + except UnknownPytestOption as e: + # #18951 (c.94) : si le workflow contient une option pytest + # longue hors des sets documentes, c'est un defaut de politique + # -- le garde ne peut pas deviner l'arite, donc refuser plutot + # que rendre `ok=True` en supposant `skip_next` (faux vert + # mesure par l'adjoint po-2025 c.93 sur `--trace-config`). + print(f"FAIL: {e}", file=sys.stderr) + return 2 except Exception as e: print(f"FAIL: impossible de parser {args.workflow}: {e}", file=sys.stderr) return 2 @@ -180,7 +527,7 @@ def main(argv: list[str] | None = None) -> int: if args.json: out = { - "workflow": str(args.workflow.relative_to(REPO_ROOT)), + "workflow": str(args.workflow.resolve().relative_to(REPO_ROOT)), "collected_paths": collected, "violations": [str(v.relative_to(REPO_ROOT)) for v in violations], "ok": not violations, @@ -197,7 +544,11 @@ def main(argv: list[str] | None = None) -> int: print(f"\nDeplacer dans le sous-dossier collecte adapte.") return 1 print(f"OK: tous les test_*.py sont dans les chemins collectes.") - print(f"Chemins collectes (lus depuis {args.workflow.relative_to(REPO_ROOT)}) :") + try: + wf_display = args.workflow.resolve().relative_to(REPO_ROOT) + except ValueError: + wf_display = args.workflow + print(f"Chemins collectes (lus depuis {wf_display}) :") for c in collected: print(f" - {c}") diff --git a/scripts/ci/tests/test_guard_test_root.py b/scripts/ci/tests/test_guard_test_root.py new file mode 100644 index 0000000000..0bbbf6bc62 --- /dev/null +++ b/scripts/ci/tests/test_guard_test_root.py @@ -0,0 +1,634 @@ +"""Tests du parseur `guard_test_root.py` (CONCERNS coordinateur c.86, #18896). + +Trois cas exiges par l'arbitrage du 03/10 : + + 1. bloc pytest valide AVEC `-n` (xdist) : le pattern matche, la liste + des chemins collectes est non-vide. + 2. bloc pytest valide SANS `-n` (juste `-q`) : le pattern matche + (avant le fix c.86, il rendait une liste vide et le garde disait + `ok=True` sans rien verifier -- un faux positif silencieux). + 3. workflow mentionnant `pytest` sans bloc multi-lignes reconnu : + `parse_collected_paths` leve `PytestBlockNotFound` ; le `main` + rend `rc=2` et un message explicite. + +Ces tests sont network-free : le module est importe en local, les +fichiers YAML de test sont crees dans un `tmp_path` pytest. +""" +import importlib.util +import sys +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parents[3] +_GUARD = _REPO_ROOT / "scripts" / "ci" / "guard_test_root.py" +_SPEC = importlib.util.spec_from_file_location("guard_test_root", _GUARD) +assert _SPEC is not None and _SPEC.loader is not None +guard = importlib.util.module_from_spec(_SPEC) +sys.modules["guard_test_root"] = guard +_SPEC.loader.exec_module(guard) + + +# --------------------------------------------------------------------------- +# 1. Bloc pytest valide AVEC `-n` (le cas historique) +# --------------------------------------------------------------------------- + +def test_parse_with_xdist_returns_all_paths(tmp_path): + """Bloc pytest avec `-n 4 --dist loadscope` : 17 chemins collectes + sur le workflow reel. Le test isole un sous-ensemble a 4 chemins + pour eviter une re-execution des autres jobs.""" + wf = tmp_path / "with_xdist.yml" + wf.write_text( + "name: with-xdist\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " runs-on: ubuntu-latest\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" + " scripts/tests \\\n" + " scripts/notebook_tools/tests \\\n" + " scripts/lean/tests \\\n" + " MyIA.AI.Notebooks/GameTheory/tests \\\n" + " -n 4 --dist loadscope --tb=short -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == [ + "scripts/tests", + "scripts/notebook_tools/tests", + "scripts/lean/tests", + "MyIA.AI.Notebooks/GameTheory/tests", + ] + + +# --------------------------------------------------------------------------- +# 2. Bloc pytest valide SANS `-n` (le cas du CONCERNS c.86) +# --------------------------------------------------------------------------- + +def test_parse_without_xdist_returns_paths(tmp_path): + """Bloc pytest avec juste `-q` en queue : avant le fix c.86, le + pattern exigeait `-n\\s` en queue, donc cette fixture rendait une + liste vide et le garde disait `ok=True` sans rien verifier. Le + fix accepte tout argument pytest en queue (le bloc se termine a + la derniere ligne se terminant par `\\`). + """ + wf = tmp_path / "no_xdist.yml" + wf.write_text( + "name: no-xdist\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " runs-on: ubuntu-latest\n" + " steps:\n" + " - run: |\n" + " python -m pytest \\\n" + " scripts/notebook_tools/tests/ \\\n" + " -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/notebook_tools/tests/"] + + +def test_parse_without_any_pytest_options(tmp_path): + """Bloc pytest valide SANS continuation sur la derniere ligne de + chemins. C'est le cas CONCERNS adjoint c.87 : la derniere ligne + d'arguments DOIT etre incluse dans l'extraction, sinon le scope + (ici `scripts/notebook_tools/tests`) disparait du controle. Le + pattern accepte la derniere ligne avec ou sans `\\` final.""" + wf = tmp_path / "bare.yml" + wf.write_text( + "name: bare\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" + " scripts/tests \\\n" + " scripts/notebook_tools/tests\n" + ) + paths = guard.parse_collected_paths(wf) + # La derniere ligne (sans `\\`) doit etre incluse -- sinon le scope + # `scripts/notebook_tools/tests` disparait et un test racine + # `test_uncollected.py` n'est pas detecte. + assert paths == ["scripts/tests", "scripts/notebook_tools/tests"] + + +# --------------------------------------------------------------------------- +# 3. Workflow mentionnant `pytest` sans bloc multi-lignes (defaut) +# --------------------------------------------------------------------------- + +def test_parse_pytest_no_multiline_block_raises(tmp_path): + """Si le workflow contient `pytest \\` (signe d'un appel multi-lignes + intentionnel) mais que PYTEST_BLOCK_RE ne matche pas le bloc + (par exemple parce que la ligne `pytest \\` est seule sans + chemins -- bug de l'auteur), on leve `PytestBlockNotFound`. + Le main rend rc=2 et un message qui nomme la cause -- c'est + l'arbitrage explicite du coordinateur ('s'il ne le prend pas en + charge, elle doit rendre une erreur de lecture, jamais OK').""" + wf = tmp_path / "orphan_block.yml" + wf.write_text( + "name: orphan-block\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" # ligne finale sans chemin -- bug YAML + " -q\n" + ) + import pytest + with pytest.raises(guard.PytestBlockNotFound): + guard.parse_collected_paths(wf) + + +def test_parse_inline_pytest_extracts_paths(tmp_path): + """Un workflow avec pytest INLINE (`pytest scripts/tests/ -q`, + pas de continuation `\\`) DOIT extraire le chemin. C'est le + CONCERNS adjoint c.87 sur #18951 : avant c.87, les appels inline + etaient completement ignores (paths=[]) et un test racine etait + invisible. La garde pretendait `ok:true` sans rien mesurer.""" + wf = tmp_path / "inline.yml" + wf.write_text( + "name: inline\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest scripts/tests/ -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/tests/"] + + +def test_parse_no_pytest_at_all_returns_empty(tmp_path): + """Un workflow sans pytest du tout rend une liste vide -- le + workflow n'est pas dans le scope de cette garde. La distinction + avec le cas 3 (pytest sans bloc) est que pytest est absent.""" + wf = tmp_path / "no_pytest.yml" + wf.write_text( + "name: no-pytest\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: echo 'no tests here'\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == [] + + +# --------------------------------------------------------------------------- +# 4. main() : rc=2 sur pytest sans bloc, rc=0 sur workflow OK +# --------------------------------------------------------------------------- + +def test_main_returns_2_on_pytest_block_not_found(tmp_path, capsys): + """Le main intercepte `PytestBlockNotFound` et rend rc=2 avec un + message explicite sur stderr. Avant le fix c.86, ce cas rendait + rc=0 -- le garde ne verifiait rien et disait 'OK'.""" + wf = tmp_path / "orphan.yml" + wf.write_text( + "name: orphan\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" # bloc YAML bugge : pytest \ sans chemins + " -q\n" + ) + rc = guard.main(["--workflow", str(wf), "--json"]) + captured = capsys.readouterr() + assert rc == 2 + # c.87 : on peut etre leve par PYTEST_BLOCK_RE.search qui ne match + # pas (regex obsolete) ou par paths vide apres extraction (appel + # pytest sans chemins). Les deux cas rendent rc=2 et un message + # FAIL explicite sur stderr. + assert ( + "PYTEST_BLOCK_RE ne matche pas" in captured.err + or "aucun chemin" in captured.err + ), f"Message d'erreur inattendu: {captured.err!r}" + + +def test_main_returns_0_on_clean_workflow(tmp_path, capsys, monkeypatch): + """Workflow propre : main rend rc=0 et liste les chemins collectes. + On monkeypatche REPO_ROOT pour qu'il accepte le chemin tmp_path + dans la sortie JSON (relative_to), ce que la garde reecrite en + c.86 fait deja via `args.workflow.resolve().relative_to(REPO_ROOT)`. + Si le tmp_path est sous REPO_ROOT, pas besoin de patch ; sinon + on elargit REPO_ROOT pour ce test.""" + # Placer le workflow sous REPO_ROOT pour eviter le monkeypatch + rel_wf = _REPO_ROOT / "scripts" / "ci" / "tests" / "_tmp_clean.yml" + rel_wf.write_text( + "name: clean\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" + " scripts/tests \\\n" + " -q\n" + ) + try: + rc = guard.main(["--workflow", str(rel_wf), "--json"]) + captured = capsys.readouterr() + assert rc == 0 + import json + out = json.loads(captured.out) + assert out["ok"] is True + assert "scripts/tests" in out["collected_paths"] + finally: + rel_wf.unlink() + + +# --------------------------------------------------------------------------- +# 5. Controles positifs (temoins du CONCERNS) +# --------------------------------------------------------------------------- + +def test_no_xdist_with_root_test_detects_violation(tmp_path, monkeypatch): + """Temoignage du CONCERNS c.86 : un workflow pytest SANS `-n` qui + pointe vers scripts/notebook_tools/tests/ (un dossier collecte) + doit rendre ok=True si tous les test_*.py de scripts/notebook_tools/ + sont dans scripts/notebook_tools/tests/ (ou dans un sous-dossier + collecte). Si un test_uncollected.py est a la racine de + scripts/notebook_tools/, le garde DOIT le voir -- avant le fix, + il rendait ok=True silencieusement (parse_collected_paths -> [], + find_violations([]) -> []).""" + # Repertoire de test + nb_dir = tmp_path / "scripts" / "notebook_tools" + nb_dir.mkdir(parents=True) + (nb_dir / "tests").mkdir() + (nb_dir / "tests" / "test_alpha.py").write_text("# test\n") + (nb_dir / "test_uncollected.py").write_text("# orphan\n") # racine, hors tests/ + + # Workflow qui pointe vers scripts/notebook_tools/tests/ sans -n + wf = tmp_path / "no_xdist.yml" + wf.write_text( + "name: no-xdist\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " python -m pytest \\\n" + " scripts/notebook_tools/tests/ \\\n" + " -q\n" + ) + + # Repertoire racine du garde : on patche REPO_ROOT pour pointer + # vers notre tmp_path, et on enqueue _in_scope pour accepter + # n'importe quel scripts/notebook_tools/. + monkeypatch.setattr(guard, "REPO_ROOT", tmp_path) + from scripts.ci import guard_test_root # noqa: F401 (already loaded) + monkeypatch.setattr(guard, "_in_scope", lambda p: True) + + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/notebook_tools/tests/"], ( + f"Sans -n, le pattern doit quand meme extraire scripts/notebook_tools/tests/ ; " + f"avant le fix c.86, il rendait [] et find_violations([]) -> [] masquait " + f"test_uncollected.py. paths={paths!r}" + ) + + violations = guard.find_violations(paths) + assert any("test_uncollected.py" in str(v) for v in violations), ( + f"find_violations doit detecter test_uncollected.py a la racine de " + f"scripts/notebook_tools/ ; avant le fix c.86, paths etait [] et " + f"violations etait [] aussi. violations={violations!r}" + ) + + +# --------------------------------------------------------------------------- +# 6. CONCERNS adjoint po-2025 (c.87) sur #18951 +# --------------------------------------------------------------------------- +# +# Deux cas que la version c.86 ne gerait pas : +# - bloc valide SANS continuation sur la derniere ligne (le scope +# `scripts/notebook_tools/tests` disparaissait de l'extraction) +# - appel inline (`pytest scripts/notebook_tools/tests/ -q`, sans +# `\` de continuation) etait completement ignore +# +# Les deux cas rendent `paths: []` et `ok: true` malgre un test racine, +# ce qui viole le contrat "soit on mesure, soit on refuse". +# On les couvre ici avec les memes fixtures que le temoin precedent. + + +def test_inline_pytest_detects_root_test_violation(tmp_path, monkeypatch): + """Temoignage du CONCERNS c.87 #18951 cas 1 : un workflow avec + `python -m pytest scripts/notebook_tools/tests/ -q` (inline, pas de + `\\`) doit extraire le chemin et signaler `test_uncollected.py` a + la racine de scripts/notebook_tools/. Avant c.87, le pattern + matchait que les appels multi-lignes : `PYTEST_INVOCATION_RE` + exigeait `\\` final, donc cet appel etait completement ignore, + `paths` etait `[]`, `ok` etait `true`, et le test racine invisible.""" + nb_dir = tmp_path / "scripts" / "notebook_tools" + nb_dir.mkdir(parents=True) + (nb_dir / "tests").mkdir() + (nb_dir / "tests" / "test_alpha.py").write_text("# test\n") + (nb_dir / "test_uncollected.py").write_text("# orphan\n") + + wf = tmp_path / "inline.yml" + wf.write_text( + "name: inline\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: python -m pytest scripts/notebook_tools/tests/ -q\n" + ) + + monkeypatch.setattr(guard, "REPO_ROOT", tmp_path) + monkeypatch.setattr(guard, "_in_scope", lambda p: True) + + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/notebook_tools/tests/"], ( + f"Appel inline doit etre extrait : paths={paths!r}" + ) + + violations = guard.find_violations(paths) + assert any("test_uncollected.py" in str(v) for v in violations), ( + f"find_violations doit detecter test_uncollected.py ; " + f"violations={violations!r}" + ) + + +def test_block_without_final_continuation_extracts_first(tmp_path, monkeypatch): + """Temoignage du CONCERNS c.87 #18951 cas 2 : un workflow avec + `pytest \\` puis `scripts/tests/ \\` puis `scripts/notebook_tools/tests/` + (sans `\\` sur la derniere ligne) doit extraire les DEUX chemins. + Avant c.87, le pattern s'arretait a la premiere ligne sans `\\`, + donc `scripts/notebook_tools/tests/` etait ignore, et le scope + notebook_tools disparaissait du controle.""" + nb_dir = tmp_path / "scripts" / "notebook_tools" + nb_dir.mkdir(parents=True) + (nb_dir / "tests").mkdir() + (nb_dir / "tests" / "test_alpha.py").write_text("# test\n") + (nb_dir / "test_uncollected.py").write_text("# orphan\n") + + wf = tmp_path / "no_final_cont.yml" + wf.write_text( + "name: no-final-cont\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" + " scripts/tests \\\n" + " scripts/notebook_tools/tests/\n" + ) + + monkeypatch.setattr(guard, "REPO_ROOT", tmp_path) + monkeypatch.setattr(guard, "_in_scope", lambda p: True) + + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/tests", "scripts/notebook_tools/tests/"], ( + f"Les deux chemins doivent etre extraits ; avant c.87, le " + f"second etait ignore. paths={paths!r}" + ) + + violations = guard.find_violations(paths) + assert any("test_uncollected.py" in str(v) for v in violations), ( + f"find_violations doit detecter test_uncollected.py a la racine ; " + f"violations={violations!r}" + ) + + +def test_pytest_with_only_flags_raises(tmp_path): + """Temoignage CONCERNS c.87 : un appel pytest avec uniquement des + flags (pas de chemin) doit lever `PytestBlockNotFound`. C'est le + cas `pytest \\` puis `-q` sans aucun chemin -- le garde ne peut + rien mesurer, donc il DOIT refuser plutot que rendre `ok=True`. + Avant c.87, le test_parse_pytest_no_multiline_block_raises etait + deja couvert pour `PYTEST_BLOCK_RE` qui ne matche pas, mais pas + pour ce cas ou le bloc matche mais ne contient que des flags.""" + wf = tmp_path / "only_flags.yml" + wf.write_text( + "name: only-flags\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" + " -q\n" + ) + import pytest + with pytest.raises(guard.PytestBlockNotFound): + guard.parse_collected_paths(wf) + + +def test_parse_filters_long_option_values(tmp_path): + """Les options `--xxx valeur` (ex. `--dist loadscope`) ne doivent + pas ajouter `valeur` a la liste des chemins collectes. C'est un + filtre de token, pas un changement de pattern -- mais le test + documente la convention.""" + wf = tmp_path / "with_dist.yml" + wf.write_text( + "name: with-dist\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: |\n" + " pytest \\\n" + " scripts/tests \\\n" + " -n 4 --dist loadscope --tb short -q\n" + ) + paths = guard.parse_collected_paths(wf) + # `loadscope` (valeur de `--dist`) et `short` (valeur de `--tb`) + # doivent etre filtres, ainsi que `-n`, `4`, `--dist`, `--tb`, `-q`. + assert paths == ["scripts/tests"], ( + f"Options et valeurs filtrees ; seul le chemin reste. paths={paths!r}" + ) + + +def test_parse_verbose_boolean_does_not_eat_next_path(tmp_path): + """CONCERNS adjoint po-2025 c.9 sur #18951 : un appel + `pytest --verbose scripts/notebook_tools/tests/` rendait + `paths=[]` car `--verbose` (option BOOLEENNE) etait traite comme + une option a valeur (skip_next=True), et le chemin suivant etait + avale. Le fix : distinguer les options booleennes (dans + PYTEST_BOOLEAN_OPTIONS) des options a valeur. `--verbose` ne doit + pas declencher skip_next.""" + wf = tmp_path / "verbose.yml" + wf.write_text( + "name: verbose\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest --verbose scripts/notebook_tools/tests/ -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/notebook_tools/tests/"], ( + f"`--verbose` est booleen, ne doit pas manger le chemin. paths={paths!r}" + ) + + +def test_parse_quoted_yaml_scalar_extracts_paths(tmp_path): + """CONCERNS adjoint po-2025 c.9 sur #18951 : un run YAML sous + forme de scalaire quoté `run: \"python -m pytest scripts/notebook_tools/tests/ -q\"` + etait rejete (pytest dans une chaîne quotée, quote_count impair). + Or, en GitHub Actions, ce scalaire est execute tel quel. Le fix : + decoder le contenu du scalaire et relancer la detection. La garde + doit extraire les chemins corrects et detecter un test racine.""" + wf = tmp_path / "quoted.yml" + wf.write_text( + "name: quoted\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + ' - run: "python -m pytest scripts/notebook_tools/tests/ -q"\n' + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/notebook_tools/tests/"], ( + f"Scalaire YAML quoté doit etre decode. paths={paths!r}" + ) + + +def test_parse_long_option_with_equals_value_does_not_eat_next(tmp_path): + """Les options `--xxx=VAL` (valeur inline) ne doivent pas + declencher skip_next : la valeur est dans le meme token. Cas + typique : `--junitxml=report.xml` ne doit pas manger le chemin + suivant comme valeur.""" + wf = tmp_path / "junitxml.yml" + wf.write_text( + "name: junitxml\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest --junitxml=report.xml scripts/tests/ -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/tests/"], ( + f"`--junitxml=VAL` doit etre absorbé inline. paths={paths!r}" + ) + + +def test_parse_disable_warnings_boolean_does_not_eat_next(tmp_path): + """CONCERNS adjoint po-2025 c.92 sur #18951 (comment 5966800233) : + `pytest --disable-warnings scripts/tests/ -q` rendait + `paths=[scripts/tests/]` comme collectes -- mais + `PYTEST_BOOLEAN_OPTIONS` ne contenait pas `--disable-warnings`, + donc le token etait traite comme une option a valeur (skip_next), + et `scripts/tests/` etait avale. Resultat : paths=[] + ok=True, + faux vert sur un run qui pose un test racine. Le fix : ajouter + `--disable-warnings` et son alias `--disable-pytest-warnings` + a la liste des booleens reconnus.""" + wf = tmp_path / "disable_warnings.yml" + wf.write_text( + "name: dw\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest --disable-warnings scripts/tests/ -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/tests/"], ( + f"`--disable-warnings` est booleen, ne doit pas manger le chemin. " + f"paths={paths!r}" + ) + + +# --------------------------------------------------------------------------- +# 7. CONCERNS adjoint po-2025 c.93 sur #18951 : politique "toute option +# longue doit etre dans un set explicite ou on refuse" (c.94). +# --------------------------------------------------------------------------- +# +# Le contrat : une option pytest longue (--xxx) doit etre soit dans +# PYTEST_BOOLEAN_OPTIONS (booleen), soit dans PYTEST_VALUE_OPTIONS (a valeur), +# soit utilisee sous `--xxx=VAL`. Toute option longue hors set REFUSE +# (UnknownPytestOption) plutot que de supposer `skip_next=True` -- parce +# que l'arite est inconnue et la supposition est un faux vert potentiel +# (cas mesure : `--trace-config`, qui n'est pas booleen et qui a fait +# avaler `scripts/tests/` comme valeur sur #18951). + + +def test_parse_durations_value_option_eats_next(tmp_path): + """Option a valeur whitelisee : `--durations=25` et `--durations 25` + ne doivent pas ajouter `25` aux chemins collectes. La politique + c.94 distingue booleen / a-valeur, donc `--durations 25` declenche + skip_next correctement (la valeur `25` est absorbee).""" + wf = tmp_path / "durations.yml" + wf.write_text( + "name: durations\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest --durations 25 scripts/tests/ -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/tests/"], ( + f"`--durations 25` doit avaler `25` comme valeur. paths={paths!r}" + ) + + +def test_parse_tb_value_option_eats_next(tmp_path): + """Option a valeur whitelisee : `--tb short` ne doit pas ajouter + `short` aux chemins collectes. Pareil que `--durations`.""" + wf = tmp_path / "tb.yml" + wf.write_text( + "name: tb\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest --tb short scripts/tests/ -q\n" + ) + paths = guard.parse_collected_paths(wf) + assert paths == ["scripts/tests/"], ( + f"`--tb short` doit avaler `short` comme valeur. paths={paths!r}" + ) + + +def test_parse_unknown_long_option_raises(tmp_path): + """Politique c.94 : une option longue hors PYTEST_BOOLEAN_OPTIONS et + PYTEST_VALUE_OPTIONS leve `UnknownPytestOption`. Le main rend rc=2. + + Cas fondateur : `--trace-config` (adjoint po-2025 c.93, comment + 5967232538) -- pytest 3.14 l'accepte silencieusement (pas dans + `--help`) et son arite est inconnue. Avant c.94, le repli + `skip_next=True` faisait avaler le chemin suivant comme valeur + (faux vert). Apres c.94, lever une exception explicite.""" + wf = tmp_path / "trace_config.yml" + wf.write_text( + "name: trace-config\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest --trace-config scripts/tests/ -q\n" + ) + import pytest + with pytest.raises(guard.UnknownPytestOption) as exc_info: + guard.parse_collected_paths(wf) + # Le message doit nommer l'option pour faciliter le diagnostic. + assert "--trace-config" in str(exc_info.value), ( + f"Le message doit nommer l'option inconnue. got: {exc_info.value!r}" + ) + + +def test_main_returns_2_on_unknown_pytest_option(tmp_path, capsys): + """Le main intercepte `UnknownPytestOption` et rend rc=2 avec un + message explicite sur stderr (meme contrat que `PytestBlockNotFound`).""" + wf = tmp_path / "unknown_opt.yml" + wf.write_text( + "name: unknown-opt\n" + "on: [push]\n" + "jobs:\n" + " t:\n" + " steps:\n" + " - run: pytest --made-up-flag scripts/tests/ -q\n" + ) + rc = guard.main(["--workflow", str(wf), "--json"]) + captured = capsys.readouterr() + assert rc == 2, ( + f"`UnknownPytestOption` doit donner rc=2 (refus), got {rc}. stderr={captured.err!r}" + ) + assert "--made-up-flag" in captured.err, ( + f"Le message d'erreur doit nommer l'option. stderr={captured.err!r}" + )