From 3f893dfe4a137ebb54648e9ecf55e91c545ccf19 Mon Sep 17 00:00:00 2001 From: Mahdi Shafiei Date: Wed, 30 Sep 2026 13:42:58 -0700 Subject: [PATCH] ci: stand down the test jobs on a merge its pull request already tested A squash or rebase merge onto an unmoved main carries the tree its pull request tested. ci.yml gains an already_tested job that runs substrax's already-tested action, pinned by commit, on a push only. The quality, unit shard, integration and coverage jobs need it and run unless the action answers true, so an empty answer (a manual run, a failed lookup) keeps them running. The coverage contract accepts that one condition on the combined coverage job and still refuses any other: the gate stands the job down only for a tree whose pull request ran it successfully. The CI contract module gains the gate's rules: the compare runs only on a push, it is the shared action pinned to a full SHA, every job consults it, and no job reads an empty answer as a skip. --- .github/workflows/ci.yml | 28 +++++++++++++-- tests/test_ci_concurrency.py | 70 +++++++++++++++++++++++++++++++++--- tests/test_ci_coverage.py | 5 ++- 3 files changed, 95 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 45a5bda..29a8653 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,9 +15,28 @@ env: CI_PYTEST_MARKERS: "not gpu and not gpu_required and not cuda and not slow and not network and not scbench and not spatialbench" jobs: + already_tested: + name: Tree already tested + runs-on: ubuntu-latest + outputs: + skip: ${{ steps.compare.outputs.skip }} + steps: + - uses: actions/checkout@v4 + + # A merge repeats the pull request; a schedule or a manual run re-measures on purpose. When + # this step does not run the output is empty, which every consumer reads as "test it". The + # compare lives in substrax: skip only when this tree is the pull request's and every check + # of it succeeded. + - name: Compare this tree with the pull request that produced it + id: compare + if: github.event_name == 'push' + uses: avitai/substrax/.github/actions/already-tested@ec43d80bbae0040375f746cd1bb07dddb0a0b169 + quality: name: Quality Gate runs-on: ubuntu-latest + needs: [already_tested] + if: needs.already_tested.outputs.skip != 'true' steps: - uses: actions/checkout@v4 @@ -43,7 +62,8 @@ jobs: unit_tests: name: Unit Tests (${{ matrix.shard.id }}) - needs: quality + needs: [quality, already_tested] + if: needs.already_tested.outputs.skip != 'true' runs-on: ubuntu-latest timeout-minutes: 30 strategy: @@ -174,7 +194,8 @@ jobs: integration_tests: name: Integration Tests - needs: unit_tests + needs: [unit_tests, already_tested] + if: needs.already_tested.outputs.skip != 'true' runs-on: ubuntu-latest timeout-minutes: 20 @@ -220,7 +241,8 @@ jobs: coverage: name: Test Coverage - needs: [unit_tests, integration_tests] + needs: [unit_tests, integration_tests, already_tested] + if: needs.already_tested.outputs.skip != 'true' runs-on: ubuntu-latest timeout-minutes: 15 diff --git a/tests/test_ci_concurrency.py b/tests/test_ci_concurrency.py index 1622e49..ccac96c 100644 --- a/tests/test_ci_concurrency.py +++ b/tests/test_ci_concurrency.py @@ -1,19 +1,28 @@ -"""Every workflow a push triggers cancels the run a newer push supersedes. +"""CI spends runner time only on work somebody will read. -Without a concurrency group keyed on the ref, two pushes in a row queue two full runs, and -the older one holds runners (the organisation's few macOS runners above all) for work -nobody will read. +Every workflow a push triggers cancels the run a newer push supersedes: without a concurrency +group keyed on the ref, two pushes in a row queue two full runs, and the older one holds runners +(the organisation's few macOS runners above all) for work nobody will read. + +A merge onto ``main`` does not repeat the CI jobs its pull request already ran over the same tree: +they consult substrax's already-tested action, pinned by commit, which compares only on a push. """ from __future__ import annotations +import re from pathlib import Path from typing import Any +import pytest import yaml WORKFLOWS = Path(__file__).resolve().parents[1] / ".github" / "workflows" +GATE_JOB = "already_tested" +GATE_CONDITION = f"needs.{GATE_JOB}.outputs.skip != 'true'" +GATE_ACTION = re.compile(r"^avitai/substrax/\.github/actions/already-tested@[0-9a-f]{40}$") +GATED_WORKFLOWS = ("ci.yml",) def _documents() -> dict[str, dict[str, Any]]: @@ -65,3 +74,56 @@ def test_every_uv_cache_is_pruned_before_it_is_saved() -> None: assert checked, "no setup-uv step found; the contract is reading the wrong files" assert unpruned == [], f"setup-uv steps saving an unpruned cache: {unpruned}" + + +def _workflow_jobs(name: str) -> dict[str, dict[str, Any]]: + return yaml.safe_load((WORKFLOWS / name).read_text(encoding="utf-8"))["jobs"] + + +@pytest.mark.parametrize("name", GATED_WORKFLOWS) +def test_the_gate_compares_only_on_a_push(name: str) -> None: + """A manual run re-measures on purpose; only a merge repeats a pull request.""" + gate = _workflow_jobs(name)[GATE_JOB] + steps = [step for step in gate["steps"] if "already-tested" in str(step.get("uses", ""))] + + assert [step.get("if") for step in steps] == ["github.event_name == 'push'"] + assert steps[0]["id"] in gate["outputs"]["skip"] + + +@pytest.mark.parametrize("name", GATED_WORKFLOWS) +def test_the_gate_is_the_shared_action_pinned_to_a_commit(name: str) -> None: + """The compare is substrax's already-tested action, pinned by a full commit SHA. + + The action finds the pull request a push merged (squash or rebase) and skips only when that + pull request tested this tree and every one of its checks succeeded; its rules are tested in + substrax. A full commit SHA pins exactly the code that runs. + """ + steps = _workflow_jobs(name)[GATE_JOB]["steps"] + compare = next(step for step in steps if step.get("id") == "compare") + + assert GATE_ACTION.match(compare.get("uses", "")), compare.get("uses") + assert "run" not in compare, "the gate runs the shared action, not an inline script" + + +@pytest.mark.parametrize("name", GATED_WORKFLOWS) +def test_every_job_that_repeats_the_pull_request_consults_the_gate(name: str) -> None: + jobs = _workflow_jobs(name) + ungated = sorted( + job_id + for job_id, job in jobs.items() + if job_id != GATE_JOB + and (job.get("if") != GATE_CONDITION or GATE_JOB not in job.get("needs", [])) + ) + + assert ungated == [], f"{name}: these repeat the pull request without the gate: {ungated}" + + +@pytest.mark.parametrize("name", GATED_WORKFLOWS) +def test_an_unanswered_gate_leaves_the_work_running(name: str) -> None: + """An empty output (no compare, or a lookup that failed) reads as "test it".""" + for job_id, job in _workflow_jobs(name).items(): + if job_id == GATE_JOB: + continue + text = yaml.safe_dump(job) + assert "outputs.skip == " not in text, f"{job_id} tests the gate for equality" + assert "outputs.skip != 'false'" not in text, f"{job_id} runs only on an explicit false" diff --git a/tests/test_ci_coverage.py b/tests/test_ci_coverage.py index d675b5b..8c8f5b7 100644 --- a/tests/test_ci_coverage.py +++ b/tests/test_ci_coverage.py @@ -14,6 +14,9 @@ REPO_ROOT = Path(__file__).resolve().parents[1] WORKFLOWS = REPO_ROOT / ".github" / "workflows" +# The one condition the coverage job may carry: it stands down only on a merge whose pull request +# already ran this job, and every other check, successfully over the same tree. +ALREADY_TESTED = "needs.already_tested.outputs.skip != 'true'" def coverage_cap_violations(workflow: dict, pyproject: dict) -> list[str]: @@ -28,7 +31,7 @@ def coverage_cap_violations(workflow: dict, pyproject: dict) -> list[str]: problems.append(f"[tool.coverage.report] fail_under is {floor}, not at least 80") if not {"push", "pull_request"} <= set(triggers): problems.append(f"CI runs on {sorted(triggers)}, not on both push and pull_request") - if "if" in job: + if job.get("if", ALREADY_TESTED) != ALREADY_TESTED: problems.append(f"the combined coverage job only runs when {job['if']}") if "coverage report" not in commands or "--fail-under=0" in commands: problems.append("the combined coverage job does not run coverage report against fail_under")