From 7d0845cb28f8eee732da8f23ee1bd42c45eeb0e6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 00:48:21 +0100 Subject: [PATCH 1/4] chore: refresh Cargo.lock for 0.157.1 The release pull request merged before its lockfile refresh landed, leaving workspace crate versions at 0.157.0 in Cargo.lock. This is the same cargo update --workspace result the release workflow produced. --- Cargo.lock | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c795b3af03..8439e44cb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1142,7 +1142,7 @@ checksum = "5b3dc8af474f516a851ff4bd12db780f948b9250ad37211e4eec0bccea54e01b" [[package]] name = "sqlbuild-analysis" -version = "0.157.0" +version = "0.157.1" dependencies = [ "polyglot-sql", "polyglot-sql-function-catalogs", @@ -1164,7 +1164,7 @@ dependencies = [ [[package]] name = "sqlbuild-attachments" -version = "0.157.0" +version = "0.157.1" dependencies = [ "sqlbuild-core", "sqlbuild-sqltext", @@ -1172,7 +1172,7 @@ dependencies = [ [[package]] name = "sqlbuild-cache" -version = "0.157.0" +version = "0.157.1" dependencies = [ "blake3", "rayon", @@ -1185,7 +1185,7 @@ dependencies = [ [[package]] name = "sqlbuild-cli" -version = "0.157.0" +version = "0.157.1" dependencies = [ "blake3", "ctrlc", @@ -1200,7 +1200,7 @@ dependencies = [ [[package]] name = "sqlbuild-config" -version = "0.157.0" +version = "0.157.1" dependencies = [ "saphyr-parser", "sqlbuild-core", @@ -1211,7 +1211,7 @@ dependencies = [ [[package]] name = "sqlbuild-core" -version = "0.157.0" +version = "0.157.1" dependencies = [ "regex", "sha2", @@ -1219,14 +1219,14 @@ dependencies = [ [[package]] name = "sqlbuild-cursor" -version = "0.157.0" +version = "0.157.1" dependencies = [ "sqlbuild-core", ] [[package]] name = "sqlbuild-discovery" -version = "0.157.0" +version = "0.157.1" dependencies = [ "junction", "rayon", @@ -1239,7 +1239,7 @@ dependencies = [ [[package]] name = "sqlbuild-model-config" -version = "0.157.0" +version = "0.157.1" dependencies = [ "regex", "sqlbuild-core", @@ -1247,7 +1247,7 @@ dependencies = [ [[package]] name = "sqlbuild-pysource" -version = "0.157.0" +version = "0.157.1" dependencies = [ "ruff_python_ast", "ruff_python_parser", @@ -1258,7 +1258,7 @@ dependencies = [ [[package]] name = "sqlbuild-python" -version = "0.157.0" +version = "0.157.1" dependencies = [ "fensu-policy", "mimalloc", @@ -1286,7 +1286,7 @@ dependencies = [ [[package]] name = "sqlbuild-refactor" -version = "0.157.0" +version = "0.157.1" dependencies = [ "serde", "serde_json", @@ -1300,7 +1300,7 @@ dependencies = [ [[package]] name = "sqlbuild-render" -version = "0.157.0" +version = "0.157.1" dependencies = [ "serde_json", "sqlbuild-cache", @@ -1310,7 +1310,7 @@ dependencies = [ [[package]] name = "sqlbuild-rules" -version = "0.157.0" +version = "0.157.1" dependencies = [ "blake3", "fensu-policy", @@ -1335,7 +1335,7 @@ dependencies = [ [[package]] name = "sqlbuild-scopes" -version = "0.157.0" +version = "0.157.1" dependencies = [ "serde_json", "sha2", @@ -1348,7 +1348,7 @@ dependencies = [ [[package]] name = "sqlbuild-sqltext" -version = "0.157.0" +version = "0.157.1" dependencies = [ "rayon", "serde", @@ -1358,7 +1358,7 @@ dependencies = [ [[package]] name = "sqlbuild-state" -version = "0.157.0" +version = "0.157.1" dependencies = [ "serde_json", "sha2", From 7698e1395fd5a5cf597e1b706b6fe263be2111a1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 01:12:12 +0100 Subject: [PATCH 2/4] ci: fail the version check when lockfile versions are stale Compare each workspace crate's Cargo.lock entry with its manifest version and the uv.lock project entry with pyproject.toml, offline, so a release whose lockfile refresh did not land fails a check instead of merging. --- crates/README.md | 3 +- scripts/native_layering/_helpers/versions.py | 72 +++++++++++++++++-- scripts/native_layering/constants.py | 4 ++ .../native_layering/_helpers/_test_types.py | 2 + .../native_layering/_helpers/helpers.py | 35 ++++++++- .../native_layering/_helpers/test_versions.py | 37 ++++++++++ 6 files changed, 142 insertions(+), 11 deletions(-) diff --git a/crates/README.md b/crates/README.md index d2172a022a..ec54e19504 100644 --- a/crates/README.md +++ b/crates/README.md @@ -52,7 +52,8 @@ in `[workspace.metadata.native-layers]` in the workspace `Cargo.toml`, and - a crate below `sqlbuild-analysis` reaches polyglot. The same check fails when the workspace version, the `pyproject.toml` version and -`.release-please-manifest.json` disagree, because the version is part of the build identity. +`.release-please-manifest.json` disagree, or when a workspace crate's `Cargo.lock` entry carries a +version other than its manifest's, because the version is part of the build identity. `fensu check` applies the Rust structure rules inside each crate: other crates use items through `main/` entry points, `models.rs`, `types.rs` and `constants.rs`, not through `_helpers/`. diff --git a/scripts/native_layering/_helpers/versions.py b/scripts/native_layering/_helpers/versions.py index 1b3f6940d4..8cbcbd57a8 100644 --- a/scripts/native_layering/_helpers/versions.py +++ b/scripts/native_layering/_helpers/versions.py @@ -8,8 +8,12 @@ from typing import Any from scripts.native_layering.constants import ( + CARGO_LOCKFILE, CARGO_MANIFEST, CLI_DISTRIBUTION_PIN_PREFIX, + LOCK_SOURCE_KEY, + PYPROJECT_LOCK_SOURCE, + PYPROJECT_LOCKFILE, PYPROJECT_MANIFEST, RELEASE_MANIFEST, RELEASE_MANIFEST_PACKAGE, @@ -17,16 +21,15 @@ def get_native_version_errors(root: Path) -> tuple[str, ...]: - """Return an error when the Cargo workspace, Python package and release versions differ.""" + """Return errors when the Cargo, Python, lockfile and release versions disagree.""" with (root / CARGO_MANIFEST).open("rb") as handle: cargo: dict[str, Any] = tomllib.load(handle) with (root / PYPROJECT_MANIFEST).open("rb") as handle: pyproject: dict[str, Any] = tomllib.load(handle) release: dict[str, Any] = json.loads((root / RELEASE_MANIFEST).read_text(encoding="utf-8")) + workspace_version: Any = cargo.get("workspace", {}).get("package", {}).get("version") versions: dict[str, Any] = { - f"{CARGO_MANIFEST} [workspace.package] version": cargo.get("workspace", {}) - .get("package", {}) - .get("version"), + f"{CARGO_MANIFEST} [workspace.package] version": workspace_version, f"{PYPROJECT_MANIFEST} [project] version": pyproject.get("project", {}).get("version"), f"{RELEASE_MANIFEST} version": release.get(RELEASE_MANIFEST_PACKAGE), f"{PYPROJECT_MANIFEST} sqlbuild-cli dependency": next( @@ -38,7 +41,62 @@ def get_native_version_errors(root: Path) -> tuple[str, ...]: None, ), } - if len(set(versions.values())) == 1: + errors: list[str] = [] + if len(set(versions.values())) != 1: + listed: str = ", ".join(f"{source} is {version}" for source, version in versions.items()) + errors.append(f"Release versions differ: {listed}.") + errors.extend( + _cargo_lockfile_errors(root=root, cargo=cargo, workspace_version=workspace_version) + ) + errors.extend(_python_lockfile_errors(root=root, pyproject=pyproject)) + return tuple(errors) + + +def _python_lockfile_errors(*, root: Path, pyproject: dict[str, Any]) -> tuple[str, ...]: + """Report a uv.lock entry for the project that carries a version other than pyproject's.""" + project: dict[str, Any] = pyproject.get("project", {}) + with (root / PYPROJECT_LOCKFILE).open("rb") as handle: + lock: dict[str, Any] = tomllib.load(handle) + locked: list[Any] = [ + entry.get("version") + for entry in lock.get("package", ()) + if entry.get("name") == project.get("name") + and entry.get(LOCK_SOURCE_KEY) == PYPROJECT_LOCK_SOURCE + ] + if locked == [project.get("version")]: + return () + listed: str = ", ".join(map(str, locked)) or "no version" + return ( + f"{PYPROJECT_LOCKFILE} locks {project.get('name')} at {listed}, " + f"not {PYPROJECT_MANIFEST} {project.get('version')}. Run `uv lock`.", + ) + + +def _cargo_lockfile_errors( + *, root: Path, cargo: dict[str, Any], workspace_version: Any +) -> tuple[str, ...]: + """Report workspace crates whose Cargo.lock entry differs from their manifest version.""" + expected: dict[str, Any] = {} + for pattern in cargo.get("workspace", {}).get("members", ()): + for directory in sorted(root.glob(pattern)): + with (directory / CARGO_MANIFEST).open("rb") as handle: + package: dict[str, Any] = tomllib.load(handle).get("package", {}) + version: Any = package.get("version") + if isinstance(version, dict) and version.get("workspace") is True: + version = workspace_version + expected[package.get("name")] = version + with (root / CARGO_LOCKFILE).open("rb") as handle: + lock: dict[str, Any] = tomllib.load(handle) + stale: list[str] = sorted( + f"{entry['name']} is {entry.get('version')}, not {expected[entry['name']]}" + for entry in lock.get("package", ()) + if LOCK_SOURCE_KEY not in entry + and entry.get("name") in expected + and entry.get("version") != expected[entry["name"]] + ) + if not stale: return () - listed: str = ", ".join(f"{source} is {version}" for source, version in versions.items()) - return (f"Release versions differ: {listed}.",) + return ( + f"{CARGO_LOCKFILE} workspace crate versions are stale: {'; '.join(stale)}. " + "Run `cargo update --workspace`.", + ) diff --git a/scripts/native_layering/constants.py b/scripts/native_layering/constants.py index 34b20f736e..cf19fd3f89 100644 --- a/scripts/native_layering/constants.py +++ b/scripts/native_layering/constants.py @@ -5,7 +5,11 @@ POLYGLOT_PACKAGE: str = "polyglot-sql" DEPENDENCY_TABLES: tuple[str, ...] = ("dependencies", "build-dependencies", "dev-dependencies") CARGO_MANIFEST: str = "Cargo.toml" +CARGO_LOCKFILE: str = "Cargo.lock" PYPROJECT_MANIFEST: str = "pyproject.toml" +PYPROJECT_LOCKFILE: str = "uv.lock" +LOCK_SOURCE_KEY: str = "source" +PYPROJECT_LOCK_SOURCE: dict[str, str] = {"editable": "."} RELEASE_MANIFEST: str = ".release-please-manifest.json" RELEASE_MANIFEST_PACKAGE: str = "." CLI_DISTRIBUTION_PIN_PREFIX: str = "sqlbuild-cli==" diff --git a/tests/unit/scripts/native_layering/_helpers/_test_types.py b/tests/unit/scripts/native_layering/_helpers/_test_types.py index 468e6a1fbc..1737ba390f 100644 --- a/tests/unit/scripts/native_layering/_helpers/_test_types.py +++ b/tests/unit/scripts/native_layering/_helpers/_test_types.py @@ -25,3 +25,5 @@ class NativeVersionTestCase: release_version: str cli_version: str expected_errors: tuple[str, ...] + lock_versions: dict[str, str] = field(default_factory=dict) + python_lock_version: str | None = None diff --git a/tests/unit/scripts/native_layering/_helpers/helpers.py b/tests/unit/scripts/native_layering/_helpers/helpers.py index 1508abe90b..db682bd797 100644 --- a/tests/unit/scripts/native_layering/_helpers/helpers.py +++ b/tests/unit/scripts/native_layering/_helpers/helpers.py @@ -43,12 +43,41 @@ def _lock_package(name: str, dependencies: tuple[str, ...]) -> str: def write_versions(root: Path, test_case: NativeVersionTestCase) -> None: - """Write the four release version sources the version check compares.""" + """Write the release version sources and the lockfile entries the version check compares. + + Every crate in `lock_versions` is a workspace member that inherits the workspace version, and + its Cargo.lock entry carries the mapped version; a registry crate rides along unchanged. The + uv.lock project entry carries `python_lock_version`, defaulting to the pyproject version. + """ + members: str = ", ".join(f'"crates/{name}"' for name in test_case.lock_versions) (root / "Cargo.toml").write_text( - f'[workspace.package]\nversion = "{test_case.workspace_version}"\n', encoding="utf-8" + f"[workspace]\nmembers = [{members}]\n\n" + f'[workspace.package]\nversion = "{test_case.workspace_version}"\n', + encoding="utf-8", + ) + lock: list[str] = [ + "version = 4\n", + '[[package]]\nname = "serde"\nversion = "1.0.0"\n' + 'source = "registry+https://github.com/rust-lang/crates.io-index"\n', + ] + for name, version in test_case.lock_versions.items(): + crate: Path = root / "crates" / name + crate.mkdir(parents=True) + (crate / "Cargo.toml").write_text( + f'[package]\nname = "{name}"\nversion.workspace = true\n', encoding="utf-8" + ) + lock.append(f'[[package]]\nname = "{name}"\nversion = "{version}"\n') + (root / "Cargo.lock").write_text("\n".join(lock), encoding="utf-8") + python_lock_version: str = test_case.python_lock_version or test_case.pyproject_version + (root / "uv.lock").write_text( + "version = 1\n\n" + '[[package]]\nname = "example"\n' + f'version = "{python_lock_version}"\nsource = {{ editable = "." }}\n\n' + '[[package]]\nname = "example-cli"\nsource = { directory = "crates/example-cli" }\n', + encoding="utf-8", ) (root / "pyproject.toml").write_text( - f'[project]\nversion = "{test_case.pyproject_version}"\n' + f'[project]\nname = "example"\nversion = "{test_case.pyproject_version}"\n' f'dependencies = ["sqlbuild-cli=={test_case.cli_version}"]\n', encoding="utf-8", ) diff --git a/tests/unit/scripts/native_layering/_helpers/test_versions.py b/tests/unit/scripts/native_layering/_helpers/test_versions.py index 563a8b3ad7..8706b2d762 100644 --- a/tests/unit/scripts/native_layering/_helpers/test_versions.py +++ b/tests/unit/scripts/native_layering/_helpers/test_versions.py @@ -20,6 +20,7 @@ release_version="1.2.0", cli_version="1.2.0", expected_errors=(), + lock_versions={"example-core": "1.2.0", "example-cli": "1.2.0"}, ), ], ids=lambda case: case.description, @@ -61,6 +62,42 @@ def test_given_matching_versions_when_checking_then_returns_no_errors( "pyproject.toml sqlbuild-cli dependency is 1.1.9.", ), ), + NativeVersionTestCase( + description="lockfile kept the previous workspace version after a release bump", + workspace_version="1.2.0", + pyproject_version="1.2.0", + release_version="1.2.0", + cli_version="1.2.0", + lock_versions={"example-core": "1.1.9", "example-cli": "1.1.9"}, + expected_errors=( + "Cargo.lock workspace crate versions are stale: example-cli is 1.1.9, not 1.2.0; " + "example-core is 1.1.9, not 1.2.0. Run `cargo update --workspace`.", + ), + ), + NativeVersionTestCase( + description="uv lockfile kept the previous project version after a release bump", + workspace_version="1.2.0", + pyproject_version="1.2.0", + release_version="1.2.0", + cli_version="1.2.0", + lock_versions={"example-cli": "1.2.0"}, + python_lock_version="1.1.9", + expected_errors=( + "uv.lock locks example at 1.1.9, not pyproject.toml 1.2.0. Run `uv lock`.", + ), + ), + NativeVersionTestCase( + description="one workspace crate lags in the lockfile", + workspace_version="1.2.0", + pyproject_version="1.2.0", + release_version="1.2.0", + cli_version="1.2.0", + lock_versions={"example-core": "1.2.0", "example-cli": "1.1.9"}, + expected_errors=( + "Cargo.lock workspace crate versions are stale: example-cli is 1.1.9, not 1.2.0. " + "Run `cargo update --workspace`.", + ), + ), ], ids=lambda case: case.description, ) From 0eeb1effa2909f2e083e3021de4c807a2f8fdfe9 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 01:26:50 +0100 Subject: [PATCH 3/4] ci: let only the trusted release checks pass a release pull request's Verify Release Please pull requests are version-only, so the Verify workflow skipped their suites and its Verify check run succeeded within seconds. That check run satisfied the required Verify before the release job pushed the lockfile refresh and before its trusted checks posted Verify on the refreshed head. On same-repository release-please-- branches the Verify workflow's aggregate job now reports as "Release pull request suites" (it still always runs; a skipped job would count as passing). The only Verify on such a head is the commit status from the release checks, so the pull request waits until they pass on its current head, and every later push waits again. The release checks also run the offline version check before dispatching anything. A check-release-gate static check, part of check-ci, models how the ruleset resolves Verify for release, feature, fork and merge group pull requests against the real workflow files. --- .github/workflows/release-please.yml | 17 +- .github/workflows/verify.yml | 17 +- Makefile | 8 +- scripts/check_release_gate.py | 14 ++ scripts/release_gate/__init__.py | 1 + scripts/release_gate/_helpers/__init__.py | 1 + scripts/release_gate/_helpers/expressions.py | 152 ++++++++++++++++ scripts/release_gate/_helpers/gate.py | 112 ++++++++++++ scripts/release_gate/_helpers/resolution.py | 26 +++ scripts/release_gate/constants.py | 134 ++++++++++++++ scripts/release_gate/exceptions.py | 5 + scripts/release_gate/main/__init__.py | 1 + scripts/release_gate/main/check.py | 28 +++ scripts/release_gate/models.py | 47 +++++ scripts/release_gate/types.py | 36 ++++ tests/unit/scripts/release_gate/__init__.py | 1 + .../scripts/release_gate/_helpers/__init__.py | 1 + .../release_gate/_helpers/_test_types.py | 69 +++++++ .../scripts/release_gate/_helpers/helpers.py | 45 +++++ .../release_gate/_helpers/test_expressions.py | 115 ++++++++++++ .../release_gate/_helpers/test_gate.py | 168 ++++++++++++++++++ .../release_gate/_helpers/test_resolution.py | 80 +++++++++ 22 files changed, 1067 insertions(+), 11 deletions(-) create mode 100644 scripts/check_release_gate.py create mode 100644 scripts/release_gate/__init__.py create mode 100644 scripts/release_gate/_helpers/__init__.py create mode 100644 scripts/release_gate/_helpers/expressions.py create mode 100644 scripts/release_gate/_helpers/gate.py create mode 100644 scripts/release_gate/_helpers/resolution.py create mode 100644 scripts/release_gate/constants.py create mode 100644 scripts/release_gate/exceptions.py create mode 100644 scripts/release_gate/main/__init__.py create mode 100644 scripts/release_gate/main/check.py create mode 100644 scripts/release_gate/models.py create mode 100644 scripts/release_gate/types.py create mode 100644 tests/unit/scripts/release_gate/__init__.py create mode 100644 tests/unit/scripts/release_gate/_helpers/__init__.py create mode 100644 tests/unit/scripts/release_gate/_helpers/_test_types.py create mode 100644 tests/unit/scripts/release_gate/_helpers/helpers.py create mode 100644 tests/unit/scripts/release_gate/_helpers/test_expressions.py create mode 100644 tests/unit/scripts/release_gate/_helpers/test_gate.py create mode 100644 tests/unit/scripts/release_gate/_helpers/test_resolution.py diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 5c5e60d109..b827895fc0 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -144,12 +144,17 @@ jobs: exit 1 } - # Verify is the required status, so it only succeeds after every trusted check passes, - # including building the distributions that Publish reuses once the release is tagged. - # Verify's test suites skip release pull requests, whose product code already passed - # them on main; release performance runs on main after each merge and does not gate. - # Each dispatched run carries this attempt's request id in its run name and is matched - # on it exactly. + # Verify is the required status. On Release Please branches this commit status is its + # only source, because the Verify workflow reports its job under another name there, so + # the pull request merges only after every trusted check passes on this head, which + # already includes any lockfile refresh. The checks include building the distributions + # that Publish reuses once the release is tagged. Verify's test suites skip release pull + # requests, whose product code already passed them on main; release performance runs on + # main after each merge and does not gate. Each dispatched run carries this attempt's + # request id in its run name and is matched on it exactly. + if ! python3 -m scripts.check_native_layering; then + fail_verify "Release versions or Cargo.lock workspace versions disagree" + fi workflows=(pull-request-metadata.yml version-guard.yml release-distributions.yml) contexts=("Pull Request Metadata" "Block automatic 1.0 releases" "Release distributions") for context in "${contexts[@]}"; do diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 132e236851..cc06a66113 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -104,8 +104,9 @@ jobs: # Release Please pull requests change only release metadata on top of product code that # already passed every suite before it reached main, so the suites are skipped. Their - # required Verify status is posted by the trusted release checks in release-please.yml - # after the metadata, version and distribution checks pass. + # required Verify status is posted only by the trusted release checks in + # release-please.yml after the metadata, version and distribution checks pass; the + # verify job below reports under another name on Release Please branches. if [[ "${EVENT_NAME}" == "pull_request" \ && "${HEAD_REPOSITORY}" == "${GITHUB_REPOSITORY}" \ && "${HEAD_REF}" == release-please--branches--* ]] && release_metadata_only; then @@ -861,7 +862,17 @@ jobs: sdist: false verify: - name: Verify + # The required Verify of a Release Please pull request is the commit status that the trusted + # release checks in release-please.yml post on the branch's current head, after the lockfile + # refresh and every dispatched check. This job reports under another name there, so its quick + # metadata-only pass can never satisfy Verify first; a skipped job would count as passing, so + # the job always runs and only its name changes. Until those checks post Verify on the head, + # the pull request waits for an expected check, and every new push to the branch waits again. + name: >- + ${{ (github.event_name == 'pull_request' + && github.event.pull_request.head.repo.full_name == github.repository + && startsWith(github.event.pull_request.head.ref, 'release-please--')) + && 'Release pull request suites' || 'Verify' }} if: always() needs: [ diff --git a/Makefile b/Makefile index 1882629b73..5ee3df9120 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ SHELL := /bin/bash -.PHONY: verify verify-quick verify-pg coverage cli-preview rust-check check-native-layering check-python-boundary \ +.PHONY: verify verify-quick verify-pg coverage cli-preview rust-check check-native-layering check-python-boundary check-release-gate \ check-e2e-shards test-e2e-duckdb test-e2e-duckdb-build-core \ test-e2e-duckdb-build-incremental \ test-e2e-duckdb-cli-data test-e2e-duckdb-cli test-e2e-duckdb-inspection \ @@ -623,7 +623,7 @@ verify-quick: exit $$status -check-ci: check-e2e-shards check-native-layering check-python-boundary rust-lint +check-ci: check-e2e-shards check-native-layering check-python-boundary check-release-gate rust-lint uv run ruff format --check . uv run ruff check . uv run ty check src tests @@ -639,6 +639,10 @@ check-python-boundary: uv run python -m scripts.check_python_boundary +check-release-gate: + uv run python -m scripts.check_release_gate + + check-pr-metadata: @test -n "$(PR_TITLE)" || { echo "PR_TITLE is required" >&2; exit 2; } @test -n "$(PR_BODY_FILE)" || { echo "PR_BODY_FILE is required" >&2; exit 2; } diff --git a/scripts/check_release_gate.py b/scripts/check_release_gate.py new file mode 100644 index 0000000000..a1f3a39009 --- /dev/null +++ b/scripts/check_release_gate.py @@ -0,0 +1,14 @@ +"""Direct wrapper for the release pull request Verify gate check.""" + +from __future__ import annotations + +from scripts.release_gate.main.check import run_release_gate_check + + +def main(argv: list[str] | None = None) -> int: + """Check the release pull request Verify gate.""" + return run_release_gate_check(argv) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/release_gate/__init__.py b/scripts/release_gate/__init__.py new file mode 100644 index 0000000000..76b2cab051 --- /dev/null +++ b/scripts/release_gate/__init__.py @@ -0,0 +1 @@ +"""Release pull request Verify gate checks.""" diff --git a/scripts/release_gate/_helpers/__init__.py b/scripts/release_gate/_helpers/__init__.py new file mode 100644 index 0000000000..4600e421f0 --- /dev/null +++ b/scripts/release_gate/_helpers/__init__.py @@ -0,0 +1 @@ +"""Release gate helpers.""" diff --git a/scripts/release_gate/_helpers/expressions.py b/scripts/release_gate/_helpers/expressions.py new file mode 100644 index 0000000000..631fdcf786 --- /dev/null +++ b/scripts/release_gate/_helpers/expressions.py @@ -0,0 +1,152 @@ +"""Evaluate the subset of GitHub Actions expressions that workflow job names use.""" + +from __future__ import annotations + +import re +from typing import Any + +from scripts.release_gate.constants import ( + EXPRESSION_LITERALS, + FALSY_VALUES, + STARTS_WITH_ARGUMENTS, + STARTS_WITH_FUNCTION, +) +from scripts.release_gate.exceptions import ReleaseGateError +from scripts.release_gate.types import Operator, TokenKind + +_TEMPLATE: re.Pattern[str] = re.compile(r"\$\{\{(.*?)\}\}", re.DOTALL) +_TOKEN: re.Pattern[str] = re.compile( + r"\s*(?:(?P'(?:[^']|'')*')|(?P&&|\|\||==|!=|!|\(|\)|,)" + r"|(?P[A-Za-z_][A-Za-z0-9_-]*(?:\.[A-Za-z_][A-Za-z0-9_-]*)*))" +) + + +def evaluate_template(*, template: str, contexts: dict[str, Any]) -> str: + """Substitute every `${{ }}` expression in `template` with its value under `contexts`.""" + return _TEMPLATE.sub( + lambda match: _to_string(evaluate(expression=match.group(1), contexts=contexts)), + template, + ) + + +def evaluate(*, expression: str, contexts: dict[str, Any]) -> Any: + """Return the value of one expression under `contexts`.""" + parser: _Parser = _Parser(tokens=_tokenize(expression), contexts=contexts) + value: Any = parser.parse_or() + if parser.peek() is not None: + raise ReleaseGateError(f"Unexpected token {parser.peek()!r} in {expression!r}") + return value + + +def _tokenize(expression: str) -> list[tuple[TokenKind, str]]: + tokens: list[tuple[TokenKind, str]] = [] + position: int = 0 + while expression[position:].strip(): + match: re.Match[str] | None = _TOKEN.match(expression, position) + if match is None or match.lastgroup is None: + raise ReleaseGateError(f"Unsupported expression syntax at {expression[position:]!r}") + tokens.append((TokenKind(match.lastgroup), match.group(match.lastgroup))) + position = match.end() + return tokens + + +class _Parser: + def __init__(self, *, tokens: list[tuple[TokenKind, str]], contexts: dict[str, Any]) -> None: + self._tokens: list[tuple[TokenKind, str]] = tokens + self._contexts: dict[str, Any] = contexts + self._index: int = 0 + + def peek(self) -> str | None: + return self._tokens[self._index][1] if self._index < len(self._tokens) else None + + def _take(self) -> tuple[TokenKind, str]: + if self._index >= len(self._tokens): + raise ReleaseGateError("Unexpected end of expression") + token: tuple[TokenKind, str] = self._tokens[self._index] + self._index += 1 + return token + + def _expect(self, operator: Operator) -> None: + if self._take() != (TokenKind.OPERATOR, operator): + raise ReleaseGateError(f"Expected {operator.value!r}") + + def parse_or(self) -> Any: + value: Any = self._parse_and() + while self.peek() == Operator.OR: + self._take() + right: Any = self._parse_and() + value = value if _truthy(value) else right + return value + + def _parse_and(self) -> Any: + value: Any = self._parse_equality() + while self.peek() == Operator.AND: + self._take() + right: Any = self._parse_equality() + value = right if _truthy(value) else value + return value + + def _parse_equality(self) -> Any: + value: Any = self._parse_unary() + if self.peek() in (Operator.EQUAL, Operator.NOT_EQUAL): + operator: str = self._take()[1] + equal: bool = _equal(left=value, right=self._parse_unary()) + return equal if operator == Operator.EQUAL else not equal + return value + + def _parse_unary(self) -> Any: + if self.peek() == Operator.NOT: + self._take() + return not _truthy(self._parse_unary()) + return self._parse_primary() + + def _parse_primary(self) -> Any: + kind, text = self._take() + if (kind, text) == (TokenKind.OPERATOR, Operator.OPEN): + value: Any = self.parse_or() + self._expect(Operator.CLOSE) + return value + if kind is TokenKind.STRING: + return text[1:-1].replace("''", "'") + if kind is not TokenKind.NAME: + raise ReleaseGateError(f"Unexpected token {text!r}") + if text in EXPRESSION_LITERALS: + return EXPRESSION_LITERALS[text] + if self.peek() == Operator.OPEN: + return self._call(text) + return self._lookup(text) + + def _call(self, function: str) -> Any: + self._expect(Operator.OPEN) + arguments: list[Any] = [self.parse_or()] + while self.peek() == Operator.SEPARATOR: + self._take() + arguments.append(self.parse_or()) + self._expect(Operator.CLOSE) + if function.lower() == STARTS_WITH_FUNCTION and len(arguments) == STARTS_WITH_ARGUMENTS: + return _to_string(arguments[0]).lower().startswith(_to_string(arguments[1]).lower()) + raise ReleaseGateError(f"Unsupported function {function}") + + def _lookup(self, path: str) -> Any: + value: Any = self._contexts + for key in path.split("."): + value = value.get(key) if isinstance(value, dict) else None + return value + + +def _truthy(value: Any) -> bool: + return value not in FALSY_VALUES + + +def _equal(*, left: Any, right: Any) -> bool: + if isinstance(left, str) and isinstance(right, str): + return left.lower() == right.lower() + return left == right + + +def _to_string(value: Any) -> str: + if value is None: + return "" + if isinstance(value, bool): + return str(value).lower() + return str(value) diff --git a/scripts/release_gate/_helpers/gate.py b/scripts/release_gate/_helpers/gate.py new file mode 100644 index 0000000000..e9187dcc56 --- /dev/null +++ b/scripts/release_gate/_helpers/gate.py @@ -0,0 +1,112 @@ +"""Check that only the trusted release checks can satisfy a release pull request's Verify.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import yaml + +from scripts.release_gate._helpers.expressions import evaluate_template +from scripts.release_gate._helpers.resolution import required_check_passes +from scripts.release_gate.constants import ( + ALWAYS_CONDITION, + GATE_SCENARIOS, + PULL_REQUEST_EVENT, + RELEASE_BRANCH_OUTPUT, + RELEASE_CHECKS_JOB, + RELEASE_PLEASE_JOB, + RELEASE_VERIFY_SUCCESS, + RELEASE_WORKFLOW, + REQUIRED_CHECK, + SCENARIO_HEADS, + VERIFY_JOB, + VERIFY_WORKFLOW, +) +from scripts.release_gate.exceptions import ReleaseGateError +from scripts.release_gate.models import CheckRun, GateScenario, WorkflowEvent + + +def get_release_gate_errors(root: Path) -> tuple[str, ...]: + """Return every way the workflows in `root` let something other than release checks pass.""" + verify: dict[str, Any] = _job(root=root, workflow=VERIFY_WORKFLOW, job=VERIFY_JOB) + errors: list[str] = [ + _scenario_error(scenario) + for scenario in GATE_SCENARIOS + if _mergeable(job=verify, scenario=scenario) is not scenario.expected_mergeable + ] + if verify.get("if") != ALWAYS_CONDITION: + errors.append( + f"{VERIFY_WORKFLOW} job {VERIFY_JOB} must run with `if: {ALWAYS_CONDITION}`: " + "a skipped job satisfies a required check." + ) + errors.extend(_release_checks_errors(root)) + return tuple(errors) + + +def verify_check_name(*, job: dict[str, Any], event: WorkflowEvent) -> str: + """Return the check run name the Verify workflow's aggregate job reports for `event`.""" + pull_request: dict[str, Any] = { + "head": {"ref": event.head_ref, "repo": {"full_name": event.head_repository}} + } + contexts: dict[str, Any] = { + "github": { + "event_name": event.event_name, + "repository": event.repository, + "event": {"pull_request": pull_request} + if event.event_name == PULL_REQUEST_EVENT + else {}, + } + } + return evaluate_template(template=str(job.get("name", VERIFY_JOB)), contexts=contexts).strip() + + +def _mergeable(*, job: dict[str, Any], scenario: GateScenario) -> bool: + name: str = verify_check_name(job=job, event=scenario.event) + check_runs: tuple[CheckRun, ...] = tuple( + CheckRun(sha=sha, name=name, conclusion=scenario.verify_conclusion) + for sha in SCENARIO_HEADS + ) + return required_check_passes( + name=REQUIRED_CHECK, + head_sha=scenario.head_sha, + check_runs=check_runs, + statuses=scenario.statuses, + ) + + +def _scenario_error(scenario: GateScenario) -> str: + outcome: str = "stays blocked" if scenario.expected_mergeable else "can merge" + return f"A {scenario.description} {outcome} under {VERIFY_WORKFLOW}." + + +def _release_checks_errors(root: Path) -> list[str]: + job: dict[str, Any] = _job(root=root, workflow=RELEASE_WORKFLOW, job=RELEASE_CHECKS_JOB) + needs: Any = job.get("needs") + errors: list[str] = [] + if RELEASE_PLEASE_JOB not in (needs if isinstance(needs, list) else [needs]): + errors.append( + f"{RELEASE_WORKFLOW} job {RELEASE_CHECKS_JOB} must need {RELEASE_PLEASE_JOB}, " + "so it checks the head after the lockfile refresh." + ) + steps: list[dict[str, Any]] = job.get("steps", []) + if not any(step.get("with", {}).get("ref") == RELEASE_BRANCH_OUTPUT for step in steps): + errors.append( + f"{RELEASE_WORKFLOW} job {RELEASE_CHECKS_JOB} must check out the release branch, " + "so it posts Verify on the branch's current head." + ) + if not any(RELEASE_VERIFY_SUCCESS in str(step.get("run", "")) for step in steps): + errors.append( + f"{RELEASE_WORKFLOW} job {RELEASE_CHECKS_JOB} never posts a successful " + f"{REQUIRED_CHECK} status, so release pull requests could never merge." + ) + return errors + + +def _job(*, root: Path, workflow: str, job: str) -> dict[str, Any]: + document: Any = yaml.safe_load((root / workflow).read_text(encoding="utf-8")) + jobs: Any = document.get("jobs", {}) if isinstance(document, dict) else {} + found: Any = jobs.get(job) if isinstance(jobs, dict) else None + if not isinstance(found, dict): + raise ReleaseGateError(f"{workflow} has no job {job}") + return found diff --git a/scripts/release_gate/_helpers/resolution.py b/scripts/release_gate/_helpers/resolution.py new file mode 100644 index 0000000000..ed042e1c9f --- /dev/null +++ b/scripts/release_gate/_helpers/resolution.py @@ -0,0 +1,26 @@ +"""Model how a ruleset resolves one required check name on a pull request head.""" + +from __future__ import annotations + +from scripts.release_gate.constants import PASSING_STATES +from scripts.release_gate.models import CheckRun, CommitStatus +from scripts.release_gate.types import CheckState + + +def required_check_passes( + *, + name: str, + head_sha: str, + check_runs: tuple[CheckRun, ...], + statuses: tuple[CommitStatus, ...], +) -> bool: + """Return whether the required check `name` lets the pull request at `head_sha` merge.""" + reported: list[CheckState] = [ + run.conclusion for run in check_runs if run.sha == head_sha and run.name == name + ] + reported.extend( + [status.state for status in statuses if status.sha == head_sha and status.context == name][ + -1: + ] + ) + return bool(reported) and all(state in PASSING_STATES for state in reported) diff --git a/scripts/release_gate/constants.py b/scripts/release_gate/constants.py new file mode 100644 index 0000000000..90295d79b2 --- /dev/null +++ b/scripts/release_gate/constants.py @@ -0,0 +1,134 @@ +"""Stable release gate policy constants.""" + +from typing import Any + +from scripts.release_gate.models import CommitStatus, GateScenario, WorkflowEvent +from scripts.release_gate.types import CheckState + +VERIFY_WORKFLOW: str = ".github/workflows/verify.yml" +VERIFY_JOB: str = "verify" +RELEASE_WORKFLOW: str = ".github/workflows/release-please.yml" +RELEASE_PLEASE_JOB: str = "release-please" +RELEASE_CHECKS_JOB: str = "release-checks" +RELEASE_BRANCH_OUTPUT: str = "${{ needs.release-please.outputs.release_branch }}" +REQUIRED_CHECK: str = "Verify" +ALWAYS_CONDITION: str = "always()" +RELEASE_VERIFY_SUCCESS: str = "post_status success Verify" +PASSING_STATES: frozenset[CheckState] = frozenset( + {CheckState.SUCCESS, CheckState.SKIPPED, CheckState.NEUTRAL} +) + +PULL_REQUEST_EVENT: str = "pull_request" +EXPRESSION_LITERALS: dict[str, Any] = {"true": True, "false": False, "null": None} +FALSY_VALUES: tuple[Any, ...] = (None, False, 0, "") +STARTS_WITH_FUNCTION: str = "startswith" +STARTS_WITH_ARGUMENTS: int = 2 + +_REPOSITORY: str = "example-org/example-project" +_RELEASE_BRANCH: str = "release-please--branches--main" +_HEAD: str = "head-1" +_NEXT_HEAD: str = "head-2" +_RELEASE_PULL_REQUEST: WorkflowEvent = WorkflowEvent( + event_name=PULL_REQUEST_EVENT, + repository=_REPOSITORY, + head_ref=_RELEASE_BRANCH, + head_repository=_REPOSITORY, +) +_FEATURE_PULL_REQUEST: WorkflowEvent = WorkflowEvent( + event_name=PULL_REQUEST_EVENT, + repository=_REPOSITORY, + head_ref="feature/inventory-sync", + head_repository=_REPOSITORY, +) +_PENDING: CommitStatus = CommitStatus(sha=_HEAD, context=REQUIRED_CHECK, state=CheckState.PENDING) +_SUCCESS: CommitStatus = CommitStatus(sha=_HEAD, context=REQUIRED_CHECK, state=CheckState.SUCCESS) +_FAILURE: CommitStatus = CommitStatus(sha=_HEAD, context=REQUIRED_CHECK, state=CheckState.FAILURE) + +GATE_SCENARIOS: tuple[GateScenario, ...] = ( + GateScenario( + description="release pull request before the trusted release checks report", + event=_RELEASE_PULL_REQUEST, + verify_conclusion=CheckState.SUCCESS, + statuses=(), + head_sha=_HEAD, + expected_mergeable=False, + ), + GateScenario( + description="release pull request while the trusted release checks run", + event=_RELEASE_PULL_REQUEST, + verify_conclusion=CheckState.SUCCESS, + statuses=(_PENDING,), + head_sha=_HEAD, + expected_mergeable=False, + ), + GateScenario( + description="release pull request after the trusted release checks pass", + event=_RELEASE_PULL_REQUEST, + verify_conclusion=CheckState.SUCCESS, + statuses=(_PENDING, _SUCCESS), + head_sha=_HEAD, + expected_mergeable=True, + ), + GateScenario( + description="release pull request whose trusted release checks failed", + event=_RELEASE_PULL_REQUEST, + verify_conclusion=CheckState.SUCCESS, + statuses=(_PENDING, _FAILURE), + head_sha=_HEAD, + expected_mergeable=False, + ), + GateScenario( + description="release pull request recovered by re-running the trusted release checks", + event=_RELEASE_PULL_REQUEST, + verify_conclusion=CheckState.SUCCESS, + statuses=(_PENDING, _FAILURE, _PENDING, _SUCCESS), + head_sha=_HEAD, + expected_mergeable=True, + ), + GateScenario( + description="release pull request pushed to after its trusted release checks passed", + event=_RELEASE_PULL_REQUEST, + verify_conclusion=CheckState.SUCCESS, + statuses=(_PENDING, _SUCCESS), + head_sha=_NEXT_HEAD, + expected_mergeable=False, + ), + GateScenario( + description="feature pull request whose suites passed", + event=_FEATURE_PULL_REQUEST, + verify_conclusion=CheckState.SUCCESS, + statuses=(), + head_sha=_HEAD, + expected_mergeable=True, + ), + GateScenario( + description="feature pull request whose suites failed", + event=_FEATURE_PULL_REQUEST, + verify_conclusion=CheckState.FAILURE, + statuses=(), + head_sha=_HEAD, + expected_mergeable=False, + ), + GateScenario( + description="fork pull request from a release-like branch whose suites passed", + event=WorkflowEvent( + event_name=PULL_REQUEST_EVENT, + repository=_REPOSITORY, + head_ref=_RELEASE_BRANCH, + head_repository="contributor/example-project", + ), + verify_conclusion=CheckState.SUCCESS, + statuses=(), + head_sha=_HEAD, + expected_mergeable=True, + ), + GateScenario( + description="merge group whose suites passed", + event=WorkflowEvent(event_name="merge_group", repository=_REPOSITORY), + verify_conclusion=CheckState.SUCCESS, + statuses=(), + head_sha=_HEAD, + expected_mergeable=True, + ), +) +SCENARIO_HEADS: tuple[str, ...] = (_HEAD, _NEXT_HEAD) diff --git a/scripts/release_gate/exceptions.py b/scripts/release_gate/exceptions.py new file mode 100644 index 0000000000..d6762a0c10 --- /dev/null +++ b/scripts/release_gate/exceptions.py @@ -0,0 +1,5 @@ +"""Release gate failures.""" + + +class ReleaseGateError(RuntimeError): + """Raised when a workflow or expression cannot be read as the gate check expects.""" diff --git a/scripts/release_gate/main/__init__.py b/scripts/release_gate/main/__init__.py new file mode 100644 index 0000000000..35ae10130c --- /dev/null +++ b/scripts/release_gate/main/__init__.py @@ -0,0 +1 @@ +"""Release gate command entrypoints.""" diff --git a/scripts/release_gate/main/check.py b/scripts/release_gate/main/check.py new file mode 100644 index 0000000000..11194a2fb7 --- /dev/null +++ b/scripts/release_gate/main/check.py @@ -0,0 +1,28 @@ +"""Release gate command entrypoint.""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +from scripts.release_gate._helpers.gate import get_release_gate_errors +from scripts.release_gate.exceptions import ReleaseGateError + + +def run_release_gate_check(argv: list[str] | None = None) -> int: + """Fail when anything but the trusted release checks can pass a release pull request.""" + parser: argparse.ArgumentParser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--root", type=Path, default=Path.cwd()) + arguments: argparse.Namespace = parser.parse_args(argv) + try: + errors: tuple[str, ...] = get_release_gate_errors(arguments.root) + except ReleaseGateError as error: + errors = (str(error),) + if errors: + print("Release pull request Verify gate is invalid:", file=sys.stderr) + for error in errors: + print(f"- {error}", file=sys.stderr) + return 1 + print("Release pull request Verify gate is valid.") + return 0 diff --git a/scripts/release_gate/models.py b/scripts/release_gate/models.py new file mode 100644 index 0000000000..3d66631dbb --- /dev/null +++ b/scripts/release_gate/models.py @@ -0,0 +1,47 @@ +"""Release gate policy models.""" + +from __future__ import annotations + +from dataclasses import dataclass + +from scripts.release_gate.types import CheckState + + +@dataclass(frozen=True) +class WorkflowEvent: + """The parts of a workflow trigger that decide how the Verify job reports.""" + + event_name: str + repository: str + head_ref: str | None = None + head_repository: str | None = None + + +@dataclass(frozen=True) +class CheckRun: + """A check run reported on a commit.""" + + sha: str + name: str + conclusion: CheckState + + +@dataclass(frozen=True) +class CommitStatus: + """A commit status posted on a commit.""" + + sha: str + context: str + state: CheckState + + +@dataclass(frozen=True) +class GateScenario: + """A pull request state and whether its required Verify must let it merge.""" + + description: str + event: WorkflowEvent + verify_conclusion: CheckState + statuses: tuple[CommitStatus, ...] + head_sha: str + expected_mergeable: bool diff --git a/scripts/release_gate/types.py b/scripts/release_gate/types.py new file mode 100644 index 0000000000..e78c5d0988 --- /dev/null +++ b/scripts/release_gate/types.py @@ -0,0 +1,36 @@ +"""Release gate enumerations.""" + +from __future__ import annotations + +from enum import StrEnum + + +class TokenKind(StrEnum): + """Lexical class of a workflow expression token.""" + + STRING = "string" + OPERATOR = "operator" + NAME = "name" + + +class Operator(StrEnum): + """Workflow expression operators and punctuation.""" + + AND = "&&" + OR = "||" + EQUAL = "==" + NOT_EQUAL = "!=" + NOT = "!" + OPEN = "(" + CLOSE = ")" + SEPARATOR = "," + + +class CheckState(StrEnum): + """Check run conclusions and commit status states the gate distinguishes.""" + + SUCCESS = "success" + SKIPPED = "skipped" + NEUTRAL = "neutral" + PENDING = "pending" + FAILURE = "failure" diff --git a/tests/unit/scripts/release_gate/__init__.py b/tests/unit/scripts/release_gate/__init__.py new file mode 100644 index 0000000000..1d9c06d477 --- /dev/null +++ b/tests/unit/scripts/release_gate/__init__.py @@ -0,0 +1 @@ +"""Release gate tooling tests.""" diff --git a/tests/unit/scripts/release_gate/_helpers/__init__.py b/tests/unit/scripts/release_gate/_helpers/__init__.py new file mode 100644 index 0000000000..153ab6c508 --- /dev/null +++ b/tests/unit/scripts/release_gate/_helpers/__init__.py @@ -0,0 +1 @@ +"""Release gate helper tests.""" diff --git a/tests/unit/scripts/release_gate/_helpers/_test_types.py b/tests/unit/scripts/release_gate/_helpers/_test_types.py new file mode 100644 index 0000000000..0792f1eb63 --- /dev/null +++ b/tests/unit/scripts/release_gate/_helpers/_test_types.py @@ -0,0 +1,69 @@ +from dataclasses import dataclass, field +from typing import Any + +from scripts.release_gate.models import CheckRun, CommitStatus + +VALID_RELEASE_CHECKS_JOB: dict[str, Any] = { + "needs": "release-please", + "steps": [ + { + "uses": "actions/checkout@v4", + "with": {"ref": "${{ needs.release-please.outputs.release_branch }}"}, + }, + {"run": 'post_status success Verify "passed"'}, + ], +} + + +@dataclass(frozen=True) +class RepositoryGateTestCase: + description: str + expected_errors: tuple[str, ...] + + +@dataclass(frozen=True) +class GateViolationTestCase: + description: str + verify_job: dict[str, Any] + expected_errors: tuple[str, ...] + release_checks_job: dict[str, Any] = field( + default_factory=lambda: dict(VALID_RELEASE_CHECKS_JOB) + ) + + +@dataclass(frozen=True) +class VerifyCheckNameTestCase: + description: str + event_name: str + head_ref: str | None + head_repository: str | None + expected_name: str + + +@dataclass(frozen=True) +class RequiredCheckTestCase: + description: str + check_runs: tuple[CheckRun, ...] + statuses: tuple[CommitStatus, ...] + expected_passes: bool + + +@dataclass(frozen=True) +class ExpressionTestCase: + description: str + expression: str + expected_value: Any + + +@dataclass(frozen=True) +class TemplateTestCase: + description: str + template: str + expected_value: str + + +@dataclass(frozen=True) +class UnsupportedExpressionTestCase: + description: str + expression: str + expected_message: str diff --git a/tests/unit/scripts/release_gate/_helpers/helpers.py b/tests/unit/scripts/release_gate/_helpers/helpers.py new file mode 100644 index 0000000000..e45160b77f --- /dev/null +++ b/tests/unit/scripts/release_gate/_helpers/helpers.py @@ -0,0 +1,45 @@ +from pathlib import Path +from typing import Any + +import yaml + +REPOSITORY_ROOT: Path = Path(__file__).resolve().parents[5] +REPOSITORY: str = "example-org/example-project" +CONTEXTS: dict[str, Any] = { + "github": { + "event_name": "pull_request", + "repository": REPOSITORY, + "event": {"pull_request": {"head": {"ref": "Release-Please--branches--main"}}}, + } +} + + +def read_verify_job() -> dict[str, Any]: + """Return the repository Verify workflow's aggregate job.""" + workflow: Any = yaml.safe_load( + (REPOSITORY_ROOT / ".github/workflows/verify.yml").read_text(encoding="utf-8") + ) + return workflow["jobs"]["verify"] + + +def write_workflows( + *, root: Path, verify_job: dict[str, Any], release_checks_job: dict[str, Any] +) -> None: + """Write a Verify workflow and a Release Please workflow holding the given jobs.""" + workflows: Path = root / ".github" / "workflows" + workflows.mkdir(parents=True) + (workflows / "verify.yml").write_text( + yaml.safe_dump({"name": "Verify", "jobs": {"verify": verify_job}}), encoding="utf-8" + ) + (workflows / "release-please.yml").write_text( + yaml.safe_dump( + { + "name": "Release Please", + "jobs": { + "release-please": {"runs-on": "ubuntu-24.04"}, + "release-checks": release_checks_job, + }, + } + ), + encoding="utf-8", + ) diff --git a/tests/unit/scripts/release_gate/_helpers/test_expressions.py b/tests/unit/scripts/release_gate/_helpers/test_expressions.py new file mode 100644 index 0000000000..236fef8953 --- /dev/null +++ b/tests/unit/scripts/release_gate/_helpers/test_expressions.py @@ -0,0 +1,115 @@ +import pytest + +from scripts.release_gate._helpers.expressions import evaluate, evaluate_template +from scripts.release_gate.exceptions import ReleaseGateError +from tests.unit.scripts.release_gate._helpers._test_types import ( + ExpressionTestCase, + TemplateTestCase, + UnsupportedExpressionTestCase, +) +from tests.unit.scripts.release_gate._helpers.helpers import CONTEXTS + + +@pytest.mark.parametrize( + "test_case", + [ + ExpressionTestCase( + description="and returns its right operand when the left is truthy", + expression="github.event_name == 'pull_request' && 'Suites'", + expected_value="Suites", + ), + ExpressionTestCase( + description="or falls back when the left is falsy", + expression="github.event_name == 'merge_group' && 'Suites' || 'Verify'", + expected_value="Verify", + ), + ExpressionTestCase( + description="string comparison ignores case", + expression="github.event_name == 'Pull_Request'", + expected_value=True, + ), + ExpressionTestCase( + description="startsWith ignores case", + expression="startsWith(github.event.pull_request.head.ref, 'release-please--')", + expected_value=True, + ), + ExpressionTestCase( + description="a missing property is null", + expression="github.event.merge_group.head_sha", + expected_value=None, + ), + ExpressionTestCase( + description="negation and grouping", + expression="!(github.event_name != 'pull_request') && 'yes' || 'no'", + expected_value="yes", + ), + ExpressionTestCase( + description="quoted apostrophe in a string literal", + expression="'it''s'", + expected_value="it's", + ), + ], + ids=lambda case: case.description, +) +def test_given_expression_when_evaluating_then_returns_github_value( + test_case: ExpressionTestCase, +) -> None: + value: object = evaluate(expression=test_case.expression, contexts=CONTEXTS) + + assert value == test_case.expected_value + + +@pytest.mark.parametrize( + "test_case", + [ + TemplateTestCase( + description="every expression is substituted", + template="${{ github.event_name }} on ${{ github.repository }}", + expected_value="pull_request on example-org/example-project", + ), + TemplateTestCase( + description="null renders empty and booleans render lowercase", + template="[${{ github.missing }}] ${{ github.event_name == 'pull_request' }}", + expected_value="[] true", + ), + ], + ids=lambda case: case.description, +) +def test_given_template_when_evaluating_then_substitutes_expressions( + test_case: TemplateTestCase, +) -> None: + assert evaluate_template(template=test_case.template, contexts=CONTEXTS) == ( + test_case.expected_value + ) + + +@pytest.mark.parametrize( + "test_case", + [ + UnsupportedExpressionTestCase( + description="unsupported function", + expression="contains(github.event_name, 'pull')", + expected_message="Unsupported function contains", + ), + UnsupportedExpressionTestCase( + description="unsupported operator", + expression="github.run_attempt > 1", + expected_message="Unsupported expression syntax", + ), + UnsupportedExpressionTestCase( + description="unbalanced parentheses", + expression="(github.event_name == 'pull_request'", + expected_message="Unexpected end of expression", + ), + ], + ids=lambda case: case.description, +) +def test_given_unsupported_expression_when_evaluating_then_raises_gate_error( + test_case: UnsupportedExpressionTestCase, +) -> None: + with pytest.raises(ReleaseGateError, match=test_case.expected_message): + evaluate(expression=test_case.expression, contexts=CONTEXTS) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/scripts/release_gate/_helpers/test_gate.py b/tests/unit/scripts/release_gate/_helpers/test_gate.py new file mode 100644 index 0000000000..d1c0394dae --- /dev/null +++ b/tests/unit/scripts/release_gate/_helpers/test_gate.py @@ -0,0 +1,168 @@ +from pathlib import Path + +import pytest + +from scripts.release_gate._helpers.gate import get_release_gate_errors, verify_check_name +from scripts.release_gate.models import WorkflowEvent +from tests.unit.scripts.release_gate._helpers._test_types import ( + GateViolationTestCase, + RepositoryGateTestCase, + VerifyCheckNameTestCase, +) +from tests.unit.scripts.release_gate._helpers.helpers import ( + REPOSITORY, + REPOSITORY_ROOT, + read_verify_job, + write_workflows, +) + +RELEASE_AWARE_NAME: str = ( + "${{ (github.event_name == 'pull_request'" + " && github.event.pull_request.head.repo.full_name == github.repository" + " && startsWith(github.event.pull_request.head.ref, 'release-please--'))" + " && 'Release pull request suites' || 'Verify' }}" +) + + +@pytest.mark.parametrize( + "test_case", + [ + RepositoryGateTestCase( + description="repository workflows gate release pull requests on the release checks", + expected_errors=(), + ), + ], + ids=lambda case: case.description, +) +def test_given_repository_workflows_when_checking_release_gate_then_returns_no_errors( + test_case: RepositoryGateTestCase, +) -> None: + assert get_release_gate_errors(REPOSITORY_ROOT) == test_case.expected_errors + + +@pytest.mark.parametrize( + "test_case", + [ + VerifyCheckNameTestCase( + description="Release Please pull request reports under another name", + event_name="pull_request", + head_ref="release-please--branches--main", + head_repository=REPOSITORY, + expected_name="Release pull request suites", + ), + VerifyCheckNameTestCase( + description="feature pull request reports Verify", + event_name="pull_request", + head_ref="feature/inventory-sync", + head_repository=REPOSITORY, + expected_name="Verify", + ), + VerifyCheckNameTestCase( + description="fork pull request from a release-like branch reports Verify", + event_name="pull_request", + head_ref="release-please--branches--main", + head_repository="contributor/example-project", + expected_name="Verify", + ), + VerifyCheckNameTestCase( + description="merge group reports Verify", + event_name="merge_group", + head_ref=None, + head_repository=None, + expected_name="Verify", + ), + VerifyCheckNameTestCase( + description="manual run reports Verify", + event_name="workflow_dispatch", + head_ref=None, + head_repository=None, + expected_name="Verify", + ), + ], + ids=lambda case: case.description, +) +def test_given_event_when_naming_repository_verify_job_then_reports_expected_name( + test_case: VerifyCheckNameTestCase, +) -> None: + event: WorkflowEvent = WorkflowEvent( + event_name=test_case.event_name, + repository=REPOSITORY, + head_ref=test_case.head_ref, + head_repository=test_case.head_repository, + ) + + assert verify_check_name(job=read_verify_job(), event=event) == test_case.expected_name + + +@pytest.mark.parametrize( + "test_case", + [ + GateViolationTestCase( + description="Verify job reports Verify for release pull requests", + verify_job={"name": "Verify", "if": "always()"}, + expected_errors=( + "A release pull request before the trusted release checks report can merge " + "under .github/workflows/verify.yml.", + "A release pull request pushed to after its trusted release checks passed can " + "merge under .github/workflows/verify.yml.", + ), + ), + GateViolationTestCase( + description="Verify job renamed for every pull request", + verify_job={ + "name": "${{ github.event_name == 'pull_request' && 'Suites' || 'Verify' }}", + "if": "always()", + }, + expected_errors=( + "A feature pull request whose suites passed stays blocked under " + ".github/workflows/verify.yml.", + "A fork pull request from a release-like branch whose suites passed stays " + "blocked under .github/workflows/verify.yml.", + ), + ), + GateViolationTestCase( + description="Verify job can be skipped", + verify_job={ + "name": RELEASE_AWARE_NAME, + "if": "needs.changes.outputs.checks == 'true'", + }, + expected_errors=( + ".github/workflows/verify.yml job verify must run with `if: always()`: a skipped " + "job satisfies a required check.", + ), + ), + GateViolationTestCase( + description="release checks never post Verify on the refreshed head", + verify_job={"name": RELEASE_AWARE_NAME, "if": "always()"}, + release_checks_job={"steps": [{"uses": "actions/checkout@v4"}]}, + expected_errors=( + ".github/workflows/release-please.yml job release-checks must need " + "release-please, so it checks the head after the lockfile refresh.", + ".github/workflows/release-please.yml job release-checks must check out the " + "release branch, so it posts Verify on the branch's current head.", + ".github/workflows/release-please.yml job release-checks never posts a " + "successful Verify status, so release pull requests could never merge.", + ), + ), + GateViolationTestCase( + description="release-aware Verify job with valid release checks", + verify_job={"name": RELEASE_AWARE_NAME, "if": "always()"}, + expected_errors=(), + ), + ], + ids=lambda case: case.description, +) +def test_given_workflows_when_checking_release_gate_then_reports_violations( + tmp_path: Path, test_case: GateViolationTestCase +) -> None: + write_workflows( + root=tmp_path, + verify_job=test_case.verify_job, + release_checks_job=test_case.release_checks_job, + ) + + assert get_release_gate_errors(tmp_path) == test_case.expected_errors + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/scripts/release_gate/_helpers/test_resolution.py b/tests/unit/scripts/release_gate/_helpers/test_resolution.py new file mode 100644 index 0000000000..dea8009d7b --- /dev/null +++ b/tests/unit/scripts/release_gate/_helpers/test_resolution.py @@ -0,0 +1,80 @@ +import pytest + +from scripts.release_gate._helpers.resolution import required_check_passes +from scripts.release_gate.models import CheckRun, CommitStatus +from scripts.release_gate.types import CheckState +from tests.unit.scripts.release_gate._helpers._test_types import RequiredCheckTestCase + + +@pytest.mark.parametrize( + "test_case", + [ + RequiredCheckTestCase( + description="nothing reported keeps the check expected", + check_runs=(), + statuses=(), + expected_passes=False, + ), + RequiredCheckTestCase( + description="a differently named check run does not count", + check_runs=(CheckRun(sha="head", name="Suites", conclusion=CheckState.SUCCESS),), + statuses=(), + expected_passes=False, + ), + RequiredCheckTestCase( + description="a passing check run alone passes", + check_runs=(CheckRun(sha="head", name="Verify", conclusion=CheckState.SUCCESS),), + statuses=(), + expected_passes=True, + ), + RequiredCheckTestCase( + description="a skipped check run passes", + check_runs=(CheckRun(sha="head", name="Verify", conclusion=CheckState.SKIPPED),), + statuses=(), + expected_passes=True, + ), + RequiredCheckTestCase( + description="a passing commit status alone passes", + check_runs=(), + statuses=(CommitStatus(sha="head", context="Verify", state=CheckState.SUCCESS),), + expected_passes=True, + ), + RequiredCheckTestCase( + description="a later commit status replaces an earlier failure", + check_runs=(), + statuses=( + CommitStatus(sha="head", context="Verify", state=CheckState.FAILURE), + CommitStatus(sha="head", context="Verify", state=CheckState.SUCCESS), + ), + expected_passes=True, + ), + RequiredCheckTestCase( + description="a check run and a commit status sharing the name must both pass", + check_runs=(CheckRun(sha="head", name="Verify", conclusion=CheckState.SUCCESS),), + statuses=(CommitStatus(sha="head", context="Verify", state=CheckState.PENDING),), + expected_passes=False, + ), + RequiredCheckTestCase( + description="a status on an earlier commit does not count", + check_runs=(), + statuses=(CommitStatus(sha="earlier", context="Verify", state=CheckState.SUCCESS),), + expected_passes=False, + ), + ], + ids=lambda case: case.description, +) +def test_given_reported_checks_when_resolving_required_check_then_returns_expected( + test_case: RequiredCheckTestCase, +) -> None: + passes: bool = required_check_passes( + name="Verify", + head_sha="head", + check_runs=test_case.check_runs, + statuses=test_case.statuses, + ) + + assert passes is test_case.expected_passes + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) From e5cd3bf8d68f9e7be047ea35cd6f84a42e7dde4b Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 01:31:22 +0100 Subject: [PATCH 4/4] ci: match release pull requests by the Release Please branch prefix --- .github/workflows/verify.yml | 2 +- tests/unit/scripts/release_gate/_helpers/test_gate.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index cc06a66113..56abc70ac3 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -871,7 +871,7 @@ jobs: name: >- ${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository - && startsWith(github.event.pull_request.head.ref, 'release-please--')) + && startsWith(github.event.pull_request.head.ref, 'release-please--branches--')) && 'Release pull request suites' || 'Verify' }} if: always() needs: diff --git a/tests/unit/scripts/release_gate/_helpers/test_gate.py b/tests/unit/scripts/release_gate/_helpers/test_gate.py index d1c0394dae..d43aa66213 100644 --- a/tests/unit/scripts/release_gate/_helpers/test_gate.py +++ b/tests/unit/scripts/release_gate/_helpers/test_gate.py @@ -19,7 +19,7 @@ RELEASE_AWARE_NAME: str = ( "${{ (github.event_name == 'pull_request'" " && github.event.pull_request.head.repo.full_name == github.repository" - " && startsWith(github.event.pull_request.head.ref, 'release-please--'))" + " && startsWith(github.event.pull_request.head.ref, 'release-please--branches--'))" " && 'Release pull request suites' || 'Verify' }}" )