From 0c074f84585f35c79e001d49915539cab6e46d06 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 17:58:44 +0100 Subject: [PATCH] fix: run mypy and shellcheck in pr_checks validate.yml runs mypy against its baseline and checks that shellcheck is installed, and pr_checks ran neither. The guard test only knew the python, vermin and npm commands, so it missed both. A step can now pipe its output into a second command, as mypy into mypy-baseline filter. --- CONTRIBUTING.md | 2 ++ scripts/generate_readme.py | 4 ++-- scripts/pr_checks.py | 33 ++++++++++++++++++++++++++------- tests/test_contributing_text.py | 4 ++-- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff2a2baec..e85902a47 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -82,8 +82,10 @@ does, and names at the end each check that failed or could not run: ```bash python scripts/validate_pr.py --changed # the BIOS files the branch adds python scripts/validate_schemas.py --source-only # profiles and platform files +mypy | mypy-baseline filter --no-colors # no type error beyond mypy-baseline.txt python scripts/profile_sync.py --added --strict # each profile the branch adds vermin --no-tips -t=3.8- --violations install.py # install.py stays on Python 3.8 +shellcheck --version # the launcher test runs shellcheck python scripts/export_native.py --all --fetch --output-dir tmp/native # files the exporter tests compare with python scripts/refresh_data_dirs.py # data directories the pack-building tests read python scripts/generate_site.py # the pages the site tests read diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index 572f3d1eb..4489173df 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -940,8 +940,8 @@ def contributing_text( .md for the site); *site_words* names the site from where the text is read. """ pip_line, floor_line = ci_install_lines() - width = max(len(step.command) for step in pr_checks.STEPS) - steps = "\n".join(f"{step.command:<{width}} # {step.why}" for step in pr_checks.STEPS) + width = max(len(pr_checks.shown(step)) for step in pr_checks.STEPS) + steps = "\n".join(f"{pr_checks.shown(step):<{width}} # {step.why}" for step in pr_checks.STEPS) return f"""{title} ## Add a BIOS file diff --git a/scripts/pr_checks.py b/scripts/pr_checks.py index 0acd4082c..a5eba4f9e 100644 --- a/scripts/pr_checks.py +++ b/scripts/pr_checks.py @@ -30,6 +30,9 @@ class Step: needs: str = "" provider: str = "" env: dict[str, str] = field(default_factory=dict) + # A second command that reads the first one's output, as a shell pipe + # would: mypy's report goes through mypy-baseline filter. + then: str = "" # The order of validate.yml: data fetched, site generated and built, then the @@ -37,9 +40,13 @@ class Step: STEPS: tuple[Step, ...] = ( Step("python scripts/validate_pr.py --changed", "the BIOS files the branch adds"), Step("python scripts/validate_schemas.py --source-only", "profiles and platform files"), + Step("mypy", "no type error beyond mypy-baseline.txt", needs="mypy", + provider="the pip line of CONTRIBUTING.md", then="mypy-baseline filter --no-colors"), Step("python scripts/profile_sync.py --added --strict", "each profile the branch adds"), Step("vermin --no-tips -t=3.8- --violations install.py", "install.py stays on Python 3.8", needs="vermin", provider="the pip line of CONTRIBUTING.md"), + Step("shellcheck --version", "the launcher test runs shellcheck", needs="shellcheck", + provider="the shellcheck package"), Step("python scripts/export_native.py --all --fetch --output-dir tmp/native", "files the exporter tests compare with"), Step("python scripts/refresh_data_dirs.py", "data directories the pack-building tests read"), @@ -57,24 +64,36 @@ STEPS: tuple[Step, ...] = ( ) -def argv(step: Step) -> list[str]: +def argv(command: str) -> list[str]: """The command, with `python` run by the interpreter running this script.""" - words = step.command.split() + words = command.split() return [sys.executable, *words[1:]] if words[0] == "python" else words +def shown(step: Step) -> str: + return f"{step.command} | {step.then}" if step.then else step.command + + +def execute(step: Step) -> int: + env = {**os.environ, **step.env} + if not step.then: + return subprocess.run(argv(step.command), cwd=ROOT, env=env).returncode + first = subprocess.run(argv(step.command), cwd=ROOT, env=env, stdout=subprocess.PIPE) + return subprocess.run(argv(step.then), cwd=ROOT, env=env, input=first.stdout).returncode + + def run(steps: tuple[Step, ...] = STEPS) -> int: failed: list[str] = [] not_run: list[str] = [] for step in steps: - print(f"\n== {step.command}", flush=True) + print(f"\n== {shown(step)}", flush=True) if step.needs and shutil.which(step.needs) is None: print(f"not run: {step.needs} is not installed ({step.provider})") - not_run.append(f"{step.command} ({step.needs}: {step.provider})") + not_run.append(f"{shown(step)} ({step.needs}: {step.provider})") continue - code = subprocess.run(argv(step), cwd=ROOT, env={**os.environ, **step.env}).returncode + code = execute(step) if code: - failed.append(f"{step.command} (exit {code})") + failed.append(f"{shown(step)} (exit {code})") print() for line in failed: print(f"FAILED {line}") @@ -90,7 +109,7 @@ def main() -> int: args = parser.parse_args() if args.list: for step in STEPS: - print(step.command) + print(shown(step)) return 0 return run() diff --git a/tests/test_contributing_text.py b/tests/test_contributing_text.py index c79ea1413..6c08e2f60 100644 --- a/tests/test_contributing_text.py +++ b/tests/test_contributing_text.py @@ -221,7 +221,7 @@ class OneCommandRunsTheChecks(unittest.TestCase): skipping one (refresh_data_dirs) failed the pack tests with no hint.""" CHECK = re.compile(r"(?:xargs -0 |run: |^\s*)((?:python3?\.?8? -m unittest|python scripts/\w+\.py" - r"|vermin|npm (?:ci|run build)))") + r"|vermin|npm (?:ci|run build)|mypy-baseline filter|mypy|shellcheck))") def test_every_check_ci_runs_is_a_step(self): import pr_checks @@ -230,7 +230,7 @@ class OneCommandRunsTheChecks(unittest.TestCase): ran = {m.group(1) for line in workflow.splitlines() if not line.strip().startswith("#") for m in [self.CHECK.search(line)] if m} self.assertGreaterEqual(len(ran), 10) - steps = [step.command for step in pr_checks.STEPS] + steps = [c for step in pr_checks.STEPS for c in (step.command, step.then) if c] for check in sorted(ran): with self.subTest(check=check): self.assertTrue(any(command.startswith(check) for command in steps))