diff --git a/.github/release_commit.py b/.github/release_commit.py new file mode 100644 index 000000000..4b3c61121 --- /dev/null +++ b/.github/release_commit.py @@ -0,0 +1,407 @@ +"""Decide whether a release run may skip the checks its merge run passed. + +After a merge to master, push.yml's merge run runs the checks in +``MERGE_RUN_CHECKS`` (lint, the quality guards, the Alembic v1 and v2 checks +and the cross-database integration tests), and its versioning job then pushes +the release commit, "Update PolicyEngine API", whose own run deploys. That +release run skips those checks only when both of these hold: + +1. The release commit changes nothing but release files: the version in + pyproject.toml's [project] table, the project's own version entry in + uv.lock, CHANGELOG.md, and fragments it removes from the fragment + directory (changelog.d/). Every check then reads the same code as on the + merge commit. +2. The release commit's parent, the merge commit, has a Push run in which + every check in ``MERGE_RUN_CHECKS`` passed. + +Otherwise the release run runs the checks itself, as it did before, and +publishes the tag only when the ones in ``RELEASE_RUN_REQUIRED_CHECKS`` pass. + +.github/scripts/check_release_commit.sh gathers what the decision reads (the +release commit's changes, the parent's copies of pyproject.toml and uv.lock, +and the parent's Push runs and their jobs from the GitHub API) and runs the +``check`` command. + +Commands, run from the repository root of a checkout of the release commit: + + check read the files check_release_commit.sh gathered, print the + reasons the checks cannot be skipped, and write + ``merge_run_checks_passed=true`` or ``false`` and the merge + run's URL to ``--output`` + require-checks for publish-git-tag: read its ``toJSON(needs)`` from the + JOB_RESULTS environment variable, and exit 1 unless either + the release commit check let this run skip the checks and + none of them failed, or every check in + ``RELEASE_RUN_REQUIRED_CHECKS`` passed in this run +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import sys +import tomllib +from dataclasses import dataclass +from pathlib import Path, PurePosixPath + +from changelog_fragments import KEEP_FILE, ConfigurationError, FragmentDefinition + +# The checks a merge run must have passed, by push.yml job id, with the job's +# name. A check that calls a reusable workflow reports each of that +# workflow's jobs as " / ". +MERGE_RUN_CHECKS = { + "lint": "Lint", + "quality-guards": "Quality guards", + "alembic-v1-check": "Alembic v1 qualification", + "alembic-v2-check": "Alembic v2 qualification", + "v2-integration-check": "Cross-database integration tests", +} +# The checks a release run that runs them itself requires before it publishes +# the tag: all of the above but the quality guards, which no deploy job +# required before either. +RELEASE_RUN_REQUIRED_CHECKS = ( + "lint", + "alembic-v1-check", + "alembic-v2-check", + "v2-integration-check", +) +RELEASE_COMMIT_CHECK_JOB = "check-release-commit" +CHECKS_PASSED_OUTPUT = "merge_run_checks_passed" +MERGE_RUN_URL_OUTPUT = "merge_run_url" +PYPROJECT_FILE = "pyproject.toml" +LOCK_FILE = "uv.lock" +CHANGELOG_FILE = "CHANGELOG.md" +REGULAR_FILE_MODE = "100644" +# git diff --raw statuses. +MODIFIED = "M" +DELETED = "D" +# A file's own entry in git diff --raw: ": +# ", then the path. +RAW_DIFF_ENTRY = re.compile( + r":(?P\d{6}) (?P\d{6}) [0-9a-f]+ [0-9a-f]+ " + r"(?P[A-Z])\d*" +) +VERSION_ENTRY = re.compile(r'^version\s*=\s*"[^"\n]*"\s*$') +NAME_ENTRY = re.compile(r'^name\s*=\s*"(?P[^"\n]*)"\s*$') +PACKAGE_HEADER = "[[package]]" +PROJECT_HEADER = "[project]" +SUCCESS = "success" +SKIPPED = "skipped" + + +@dataclass(frozen=True) +class FileChange: + """One path the release commit changes, from git diff --raw.""" + + path: str + status: str + old_mode: str + new_mode: str + + +@dataclass(frozen=True) +class ReleaseFiles: + """The parent's and the release commit's copies of the versioned files.""" + + parent_pyproject: str + release_pyproject: str + parent_lock: str + release_lock: str + + +def parse_raw_diff(raw_diff: str) -> list[FileChange]: + """Return the changes in ``git diff --raw --no-renames -z`` output.""" + fields = raw_diff.split("\0") + if fields and not fields[-1]: + fields.pop() + if len(fields) % 2: + msg = "git diff --raw -z output must alternate entries and paths" + raise ValueError(msg) + changes = [] + for entry, path in zip(fields[0::2], fields[1::2], strict=True): + match = RAW_DIFF_ENTRY.fullmatch(entry) + if match is None: + msg = f"unexpected git diff --raw entry: {entry!r}" + raise ValueError(msg) + changes.append( + FileChange( + path=path, + status=match["status"], + old_mode=match["old_mode"], + new_mode=match["new_mode"], + ) + ) + return changes + + +def normalized_package_name(name: str) -> str: + """Return ``name`` as uv.lock and PEP 503 write it.""" + return re.sub(r"[-_.]+", "-", name).lower() + + +def project_version_index(pyproject_text: str) -> int | None: + """Return the index of the version line in pyproject.toml's [project] table.""" + in_project_table = False + for index, line in enumerate(pyproject_text.splitlines()): + if line.startswith("["): + in_project_table = line.strip() == PROJECT_HEADER + elif in_project_table and VERSION_ENTRY.match(line): + return index + return None + + +def locked_project_version_index(lock_text: str, project_name: str) -> int | None: + """Return the index of the version line in uv.lock's entry for ``project_name``. + + uv.lock writes each package as a ``[[package]]`` table whose own keys, + ``name`` and ``version`` first, come before the next table header. + """ + lines = lock_text.splitlines() + for header_index, header in enumerate(lines): + if header.strip() != PACKAGE_HEADER: + continue + package_name = None + version_index = None + for index in range(header_index + 1, len(lines)): + line = lines[index] + if line.startswith("["): + break + name_match = NAME_ENTRY.match(line) + if name_match is not None: + package_name = normalized_package_name(name_match["name"]) + elif version_index is None and VERSION_ENTRY.match(line): + version_index = index + if package_name == project_name: + return version_index + return None + + +def changes_only_line( + parent_text: str, + release_text: str, + parent_index: int | None, + release_index: int | None, +) -> bool: + """Whether the two texts are the same except for the given line.""" + if parent_index is None or release_index is None or parent_index != release_index: + return False + parent_lines = parent_text.splitlines(keepends=True) + release_lines = release_text.splitlines(keepends=True) + return ( + len(parent_lines) == len(release_lines) + and parent_lines[:parent_index] == release_lines[:release_index] + and parent_lines[parent_index + 1 :] == release_lines[release_index + 1 :] + ) + + +def project_name(pyproject_text: str) -> str: + """Return the normalized [project] name in pyproject.toml.""" + return normalized_package_name(tomllib.loads(pyproject_text)["project"]["name"]) + + +def _is_modified_regular_file(change: FileChange) -> bool: + return ( + change.status == MODIFIED + and change.old_mode == REGULAR_FILE_MODE + and change.new_mode == REGULAR_FILE_MODE + ) + + +def release_change_problems( + changes: list[FileChange], + files: ReleaseFiles, + fragment_directory: str, +) -> list[str]: + """Return why ``changes`` hold more than a release commit may, or nothing.""" + problems = [] + for change in changes: + path = PurePosixPath(change.path) + if change.path == CHANGELOG_FILE: + if not _is_modified_regular_file(change): + problems.append(f"{CHANGELOG_FILE} is not only edited") + elif change.path == PYPROJECT_FILE: + if not _is_modified_regular_file(change) or not changes_only_line( + files.parent_pyproject, + files.release_pyproject, + project_version_index(files.parent_pyproject), + project_version_index(files.release_pyproject), + ): + problems.append( + f"{PYPROJECT_FILE} changes more than the [project] version" + ) + elif change.path == LOCK_FILE: + name = project_name(files.release_pyproject) + if not _is_modified_regular_file(change) or not changes_only_line( + files.parent_lock, + files.release_lock, + locked_project_version_index(files.parent_lock, name), + locked_project_version_index(files.release_lock, name), + ): + problems.append(f"{LOCK_FILE} changes more than {name}'s version") + elif str(path.parent) == fragment_directory and path.name != KEEP_FILE: + if change.status != DELETED: + problems.append( + f"{change.path} is a fragment the release commit does not remove" + ) + else: + problems.append(f"{change.path} is not a release file") + return problems + + +def _check_jobs(jobs: list[dict], check_name: str) -> list[dict]: + """Return the jobs that report the push.yml check named ``check_name``.""" + return [ + job + for job in jobs + if job.get("name") == check_name + or str(job.get("name", "")).startswith(f"{check_name} / ") + ] + + +def run_check_problems(jobs: list[dict]) -> list[str]: + """Return the merge-run checks that did not pass in a run with ``jobs``.""" + problems = [] + for check_name in MERGE_RUN_CHECKS.values(): + check_jobs = _check_jobs(jobs, check_name) + if not check_jobs: + problems.append(f"{check_name} did not run") + continue + problems.extend( + f"{job.get('name')} ended {job.get('status')}, {job.get('conclusion')}" + for job in check_jobs + if job.get("status") != "completed" or job.get("conclusion") != SUCCESS + ) + return problems + + +def passing_merge_run( + runs: list[dict], jobs_by_run: dict[int, list[dict]] +) -> tuple[dict | None, list[str]]: + """Return a run in which every merge-run check passed, or why there is none.""" + if not runs: + return None, ["the parent commit has no Push run"] + problems = [] + for run in runs: + run_problems = run_check_problems(jobs_by_run.get(run["id"], [])) + if not run_problems: + return run, [] + problems.extend( + f"Push run {run.get('html_url', run['id'])}: {run_problem}" + for run_problem in run_problems + ) + return None, problems + + +def _read_text(path: Path) -> str: + return path.read_text(encoding="utf-8") if path.exists() else "" + + +def _write_outputs( + output_path: Path, *, checks_passed: bool, merge_run_url: str +) -> None: + with output_path.open("a", encoding="utf-8") as output: + output.write(f"{CHECKS_PASSED_OUTPUT}={'true' if checks_passed else 'false'}\n") + output.write(f"{MERGE_RUN_URL_OUTPUT}={merge_run_url}\n") + + +def check(arguments: argparse.Namespace) -> int: + """Run the ``check`` command.""" + gathered = Path(arguments.gathered) + problems = [] + try: + fragment_directory = FragmentDefinition.load(Path.cwd()).directory + except ConfigurationError as error: + problems.append(str(error)) + fragment_directory = "" + files = ReleaseFiles( + parent_pyproject=_read_text(gathered / "parent-pyproject.toml"), + release_pyproject=_read_text(Path(PYPROJECT_FILE)), + parent_lock=_read_text(gathered / "parent-uv.lock"), + release_lock=_read_text(Path(LOCK_FILE)), + ) + changes = parse_raw_diff(_read_text(gathered / "changes")) + if fragment_directory: + problems.extend(release_change_problems(changes, files, fragment_directory)) + runs = json.loads(_read_text(gathered / "runs.json") or "{}").get( + "workflow_runs", [] + ) + jobs_by_run = { + run["id"]: json.loads( + _read_text(gathered / f"jobs-{run['id']}.json") or "{}" + ).get("jobs", []) + for run in runs + } + merge_run, run_problems = passing_merge_run(runs, jobs_by_run) + problems.extend(run_problems) + for problem in problems: + print(f"::notice::{problem}") + checks_passed = not problems + merge_run_url = merge_run.get("html_url", "") if merge_run else "" + if checks_passed: + print( + "The release commit changes only release files, and the merge run " + f"{merge_run_url} passed every check on the same code: this run skips " + f"{', '.join(MERGE_RUN_CHECKS.values())}." + ) + else: + print("This run runs the checks itself.") + _write_outputs( + Path(arguments.output), checks_passed=checks_passed, merge_run_url=merge_run_url + ) + return 0 + + +def release_check_problems(job_results: dict) -> list[str]: + """Return why publish-git-tag may not proceed, from its ``toJSON(needs)``.""" + release_commit_check = job_results.get(RELEASE_COMMIT_CHECK_JOB, {}) + skipped_by_design = ( + release_commit_check.get("result") == SUCCESS + and release_commit_check.get("outputs", {}).get(CHECKS_PASSED_OUTPUT) == "true" + ) + allowed_results = {SUCCESS, SKIPPED} if skipped_by_design else {SUCCESS} + problems = [] + for job_id in RELEASE_RUN_REQUIRED_CHECKS: + if job_id not in job_results: + problems.append(f"publish-git-tag does not need {job_id}") + continue + result = job_results[job_id].get("result") + if result not in allowed_results: + problems.append(f"{job_id} ended {result}") + return problems + + +def require_checks(_arguments: argparse.Namespace) -> int: + """Run the ``require-checks`` command.""" + job_results = json.loads(os.environ.get("JOB_RESULTS", "{}")) + problems = release_check_problems(job_results) + for problem in problems: + print(f"::error::{problem}") + if problems: + return 1 + release_commit_check = job_results[RELEASE_COMMIT_CHECK_JOB] + merge_run_url = release_commit_check.get("outputs", {}).get(MERGE_RUN_URL_OUTPUT) + if merge_run_url: + print(f"This run skipped the checks the merge run {merge_run_url} passed.") + else: + print("This run's checks passed.") + return 0 + + +def main(argv: list[str] | None = None) -> int: + """Parse the command line and run one command.""" + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + commands = parser.add_subparsers(dest="command", required=True) + check_parser = commands.add_parser("check") + check_parser.add_argument("--gathered", required=True) + check_parser.add_argument("--output", required=True) + check_parser.set_defaults(handler=check) + require_parser = commands.add_parser("require-checks") + require_parser.set_defaults(handler=require_checks) + arguments = parser.parse_args(argv) + return arguments.handler(arguments) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/scripts/check_release_commit.sh b/.github/scripts/check_release_commit.sh new file mode 100644 index 000000000..ac621df93 --- /dev/null +++ b/.github/scripts/check_release_commit.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash + +# Decide whether this release run may skip the checks its merge run passed, +# for push.yml's check-release-commit job (.github/release_commit.py explains +# the decision). +# +# It gathers what release_commit.py reads: from git, the release commit's +# changes and the parent's copies of pyproject.toml and uv.lock; from the +# GitHub API, the parent's Push runs and the jobs of each. Then +# release_commit.py check writes merge_run_checks_passed (true or false) and +# merge_run_url to GITHUB_OUTPUT. When this script cannot gather something, it +# writes merge_run_checks_passed=false, so the run runs the checks itself, and +# exits 0. +# +# Run from the repository root, in a checkout of the release commit and its +# parent (actions/checkout with fetch-depth: 2). +# +# Environment: +# GH_TOKEN a token that can read this repository's Actions runs +# GITHUB_REPOSITORY the repository, as owner/name +# GITHUB_OUTPUT the step's output file +# PYTHON the Python 3.11+ interpreter (default: python3) + +set -euo pipefail + +: "${GH_TOKEN:?GH_TOKEN is required}" +repository="${GITHUB_REPOSITORY:?GITHUB_REPOSITORY is required}" +output_file="${GITHUB_OUTPUT:?GITHUB_OUTPUT is required}" +python_bin="${PYTHON:-python3}" +release_commit_module="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/release_commit.py" +gathered="$(mktemp -d)" +trap 'rm -rf "${gathered}"' EXIT + +run_the_checks() { + echo "::notice::$1 This run runs the checks itself." + { + echo "merge_run_checks_passed=false" + echo "merge_run_url=" + } >>"${output_file}" + exit 0 +} + +read -r -a commit_and_parents <<<"$(git rev-list --parents -n 1 HEAD)" +if ((${#commit_and_parents[@]} != 2)); then + run_the_checks "The release commit must have exactly one parent." +fi +parent="${commit_and_parents[1]}" + +if ! git diff --raw --no-renames --no-abbrev -z "${parent}" HEAD >"${gathered}/changes"; then + run_the_checks "Could not compare the release commit with its parent ${parent}." +fi +for versioned_file in pyproject.toml uv.lock; do + if ! git show "${parent}:${versioned_file}" >"${gathered}/parent-${versioned_file}"; then + run_the_checks "Could not read ${versioned_file} at the parent ${parent}." + fi +done + +runs_request="repos/${repository}/actions/workflows/push.yml/runs?head_sha=${parent}&event=push&per_page=100" +if ! gh api "${runs_request}" >"${gathered}/runs.json"; then + run_the_checks "Could not list the Push runs of the parent ${parent}." +fi +if ! run_ids="$(jq -r '.workflow_runs[].id' "${gathered}/runs.json")"; then + run_the_checks "Could not read the Push runs of the parent ${parent}." +fi +for run_id in ${run_ids}; do + if ! gh api "repos/${repository}/actions/runs/${run_id}/jobs?per_page=100" >"${gathered}/jobs-${run_id}.json"; then + run_the_checks "Could not list the jobs of Push run ${run_id}." + fi +done + +"${python_bin}" "${release_commit_module}" check --gathered "${gathered}" --output "${output_file}" diff --git a/.github/scripts/run_release_tests.sh b/.github/scripts/run_release_tests.sh index f6fc2f9de..6de2574aa 100644 --- a/.github/scripts/run_release_tests.sh +++ b/.github/scripts/run_release_tests.sh @@ -1,16 +1,30 @@ #!/usr/bin/env bash -# Run the release test suite (make test) in push.yml's release-tests job, -# which the staging deploy needs. +# Run one shard of the release test suite (make test) in push.yml's +# release-tests job, which the staging deploy needs. +# +# make test collects the whole suite, and the pytest plugin +# tests/release_test_shard_plugin.py keeps only this shard's tests and writes +# what it collected and kept to RELEASE_TEST_SHARD_REPORT_DIRECTORY +# (scripts/release_test_shards.py explains the split). # # Deployment settings (route implementations, database policies, Cloud Run and # v2 database targets, and signed-in user authentication) must not leak into # the tests. The release-tests job sets none of them, and this script removes # them anyway, as it did when the tests ran inside the staging deploy job. The # local Redis runtime-cache settings come from the step's env. +# +# Environment: +# RELEASE_TEST_SHARD_INDEX this shard, from 1 +# RELEASE_TEST_SHARD_COUNT how many shards the suite runs in +# RELEASE_TEST_SHARD_REPORT_DIRECTORY where the plugin writes its report set -euo pipefail +: "${RELEASE_TEST_SHARD_INDEX:?RELEASE_TEST_SHARD_INDEX is required}" +: "${RELEASE_TEST_SHARD_COUNT:?RELEASE_TEST_SHARD_COUNT is required}" +: "${RELEASE_TEST_SHARD_REPORT_DIRECTORY:?RELEASE_TEST_SHARD_REPORT_DIRECTORY is required}" + exec env \ -u DEPLOYMENT_ENVIRONMENT \ -u ROUTE_IMPL_HEALTH \ @@ -39,4 +53,5 @@ exec env \ -u PUBLIC_API_AUTH_REQUIRED \ -u PUBLIC_API_AUTH_ISSUER \ -u PUBLIC_API_AUTH_AUDIENCE \ + PYTEST_ADDOPTS="-p tests.release_test_shard_plugin" \ make test diff --git a/.github/workflows/migrate-v1-cloud-sql.yml b/.github/workflows/migrate-v1-cloud-sql.yml index 16dc5198e..69ca0de3d 100644 --- a/.github/workflows/migrate-v1-cloud-sql.yml +++ b/.github/workflows/migrate-v1-cloud-sql.yml @@ -28,6 +28,10 @@ on: jobs: migrate: name: Upgrade and verify v1 Cloud SQL + # push.yml calls this from a release job that may follow skipped + # checks. GitHub would skip this job too unless its if: calls a status + # function; the calling job has already checked what it needs. + if: ${{ !cancelled() }} runs-on: ubuntu-latest timeout-minutes: 30 environment: ${{ inputs.github_environment }} diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 7cecdb294..38be9209c 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -14,10 +14,49 @@ concurrency: cancel-in-progress: false jobs: + check-release-commit: + # Whether this release run may skip the checks its merge run passed: the + # release commit changes only release files, and the merge commit's Push + # run passed every check (.github/release_commit.py). It runs on every + # push, so the checks that need it never need a skipped job; on a merge + # commit its steps do nothing, and the checks run. + name: Check the release commit + runs-on: ubuntu-latest + if: github.repository == 'PolicyEngine/policyengine-api' + permissions: + contents: read + actions: read + outputs: + merge_run_checks_passed: ${{ steps.release_commit.outputs.merge_run_checks_passed }} + merge_run_url: ${{ steps.release_commit.outputs.merge_run_url }} + steps: + - name: Checkout the release commit and its parent + if: github.event.head_commit.message == 'Update PolicyEngine API' + uses: actions/checkout@v4 + with: + fetch-depth: 2 + - name: Setup Python + if: github.event.head_commit.message == 'Update PolicyEngine API' + uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Decide whether this run may skip the merge run's checks + id: release_commit + if: github.event.head_commit.message == 'Update PolicyEngine API' + run: bash .github/scripts/check_release_commit.sh + env: + GH_TOKEN: ${{ github.token }} + lint: name: Lint runs-on: ubuntu-latest - if: github.repository == 'PolicyEngine/policyengine-api' + needs: check-release-commit + # Skipped on a release run whose merge run passed it on the same code. + # !cancelled() runs it when the release commit check fails. + if: | + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') + && (needs.check-release-commit.outputs.merge_run_checks_passed != 'true') steps: - name: Checkout repo uses: actions/checkout@v4 @@ -35,7 +74,13 @@ jobs: quality-guards: name: Quality guards runs-on: ubuntu-latest - if: github.repository == 'PolicyEngine/policyengine-api' + needs: check-release-commit + # Skipped on a release run whose merge run passed it on the same code. + # !cancelled() runs it when the release commit check fails. + if: | + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') + && (needs.check-release-commit.outputs.merge_run_checks_passed != 'true') steps: - name: Checkout repo uses: actions/checkout@v4 @@ -50,17 +95,35 @@ jobs: alembic-v1-check: name: Alembic v1 qualification - if: github.repository == 'PolicyEngine/policyengine-api' + needs: check-release-commit + # Skipped on a release run whose merge run passed it on the same code. + # !cancelled() runs it when the release commit check fails. + if: | + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') + && (needs.check-release-commit.outputs.merge_run_checks_passed != 'true') uses: ./.github/workflows/alembic-v1-check.yml alembic-v2-check: name: Alembic v2 qualification - if: github.repository == 'PolicyEngine/policyengine-api' + needs: check-release-commit + # Skipped on a release run whose merge run passed it on the same code. + # !cancelled() runs it when the release commit check fails. + if: | + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') + && (needs.check-release-commit.outputs.merge_run_checks_passed != 'true') uses: ./.github/workflows/alembic-v2-check.yml v2-integration-check: name: Cross-database integration tests - if: github.repository == 'PolicyEngine/policyengine-api' + needs: check-release-commit + # Skipped on a release run whose merge run passed it on the same code. + # !cancelled() runs it when the release commit check fails. + if: | + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') + && (needs.check-release-commit.outputs.merge_run_checks_passed != 'true') uses: ./.github/workflows/v2-integration-check.yml secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} @@ -140,13 +203,21 @@ jobs: runs-on: ubuntu-latest needs: - ensure-staging-model-version-aligns-with-sim-api + - check-release-commit - lint - alembic-v1-check - alembic-v2-check - v2-integration-check + # The checks may be skipped, so this job runs whatever they ended, and its + # first step after checkout requires that the merge run passed them on the + # same code or that this run did. Every release job after it states which + # jobs it needs to have passed: GitHub would otherwise skip it, because a + # job it needs needed a skipped one. if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.ensure-staging-model-version-aligns-with-sim-api.result == 'success' steps: - name: Checkout repo uses: actions/checkout@v4 @@ -154,6 +225,10 @@ jobs: uses: actions/setup-python@v5 with: python-version: "3.12" + - name: Require the merge run's checks or this run's + run: python .github/release_commit.py require-checks + env: + JOB_RESULTS: ${{ toJSON(needs) }} - name: Publish Git Tag run: ".github/publish-git-tag.sh" @@ -163,8 +238,11 @@ jobs: - ensure-staging-model-version-aligns-with-sim-api - publish-git-tag if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.ensure-staging-model-version-aligns-with-sim-api.result == 'success' + && needs.publish-git-tag.result == 'success' uses: ./.github/workflows/migrate-v1-cloud-sql.yml with: deployment_environment: staging @@ -179,8 +257,10 @@ jobs: needs: - publish-git-tag if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.publish-git-tag.result == 'success' uses: ./.github/workflows/seed-v2-database.yml with: deployment_environment: staging @@ -193,8 +273,10 @@ jobs: # staging database jobs, and the staging deploy needs it, so the tests pass # before GCP authentication, image publication or candidate deployment. # It keeps the staging environment and the variables the tests read when - # they ran inside the staging deploy job. - name: Run release tests + # they ran inside the staging deploy job. The suite runs in six shards, one + # runner each, split by file (scripts/release_test_shards.py), and + # check-release-test-shards checks that each test ran in exactly one. + name: Run release tests (shard ${{ matrix.shard_index }} of 6) runs-on: ubuntu-latest if: | (github.repository == 'PolicyEngine/policyengine-api') @@ -210,6 +292,11 @@ jobs: UV_PYTHON: "3.12" permissions: contents: read + strategy: + # Every shard runs to the end, so one release run shows every failure. + fail-fast: false + matrix: + shard_index: [1, 2, 3, 4, 5, 6] steps: - name: Checkout repo uses: actions/checkout@v4 @@ -230,6 +317,9 @@ jobs: - name: Run release tests run: bash .github/scripts/run_release_tests.sh env: + RELEASE_TEST_SHARD_INDEX: ${{ matrix.shard_index }} + RELEASE_TEST_SHARD_COUNT: "6" + RELEASE_TEST_SHARD_REPORT_DIRECTORY: ${{ runner.temp }}/release-test-shard RUNTIME_CACHE_MODE: local RUNTIME_CACHE_URL: redis://127.0.0.1:6379/0 RUNTIME_CACHE_ENVIRONMENT: test @@ -243,6 +333,41 @@ jobs: GATEWAY_AUTH_AUDIENCE: ${{ secrets.GATEWAY_AUTH_AUDIENCE }} GATEWAY_AUTH_CLIENT_ID: ${{ secrets.GATEWAY_AUTH_CLIENT_ID }} GATEWAY_AUTH_CLIENT_SECRET_RESOURCE: ${{ secrets.GATEWAY_AUTH_CLIENT_SECRET_RESOURCE }} + - name: Upload the tests this shard collected and ran + uses: actions/upload-artifact@v4 + with: + name: release-test-shard-${{ matrix.shard_index }} + path: ${{ runner.temp }}/release-test-shard + if-no-files-found: error + retention-days: 7 + + check-release-test-shards: + # Fails unless every shard collected the same tests and each test ran in + # exactly one shard (scripts/release_test_shards.py check-coverage). + name: Check that every release test ran in exactly one shard + runs-on: ubuntu-latest + needs: release-tests + if: | + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') + && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.release-tests.result == 'success' + permissions: + contents: read + steps: + - name: Checkout repo + uses: actions/checkout@v4 + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Download every shard's report + uses: actions/download-artifact@v4 + with: + pattern: release-test-shard-* + path: ${{ runner.temp }}/release-test-shards + - name: Check that every release test ran in exactly one shard + run: python -m scripts.release_test_shards check-coverage --shard-count 6 "${{ runner.temp }}/release-test-shards" deploy-cloud-run-staging: name: Deploy and test staging Cloud Run candidate @@ -253,9 +378,17 @@ jobs: - migrate-v1-staging-cloud-sql - seed-v2-staging-database - release-tests + - check-release-test-shards if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.ensure-staging-model-version-aligns-with-sim-api.result == 'success' + && needs.publish-git-tag.result == 'success' + && needs.migrate-v1-staging-cloud-sql.result == 'success' + && needs.seed-v2-staging-database.result == 'success' + && needs.release-tests.result == 'success' + && needs.check-release-test-shards.result == 'success' environment: staging env: DEPLOYMENT_ENVIRONMENT: staging @@ -390,8 +523,10 @@ jobs: timeout-minutes: 20 needs: deploy-cloud-run-staging if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.deploy-cloud-run-staging.result == 'success' env: # uv installs uv.lock on the Cloud Run image's Python, not the 3.14 # that .python-version selects. @@ -422,8 +557,11 @@ jobs: - deploy-cloud-run-staging - parallel-live-simulation-tests-staging-cloud-run if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.deploy-cloud-run-staging.result == 'success' + && needs.parallel-live-simulation-tests-staging-cloud-run.result == 'success' environment: staging env: CLOUD_RUN_SERVICE: policyengine-api-staging @@ -488,8 +626,10 @@ jobs: runs-on: ubuntu-latest needs: promote-cloud-run-staging if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.promote-cloud-run-staging.result == 'success' environment: production env: SIM_ENTRYPOINT: ${{ vars.SIM_ENTRYPOINT }} @@ -508,8 +648,10 @@ jobs: name: Upgrade production v1 Cloud SQL schema needs: ensure-production-model-version-aligns-with-sim-api if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.ensure-production-model-version-aligns-with-sim-api.result == 'success' uses: ./.github/workflows/migrate-v1-cloud-sql.yml with: deployment_environment: production @@ -523,8 +665,10 @@ jobs: needs: - ensure-production-model-version-aligns-with-sim-api if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.ensure-production-model-version-aligns-with-sim-api.result == 'success' uses: ./.github/workflows/seed-v2-database.yml with: deployment_environment: production @@ -537,8 +681,11 @@ jobs: - migrate-v1-production-cloud-sql - seed-v2-production-database if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.migrate-v1-production-cloud-sql.result == 'success' + && needs.seed-v2-production-database.result == 'success' environment: production env: DEPLOYMENT_ENVIRONMENT: production @@ -682,8 +829,10 @@ jobs: runs-on: ubuntu-latest needs: deploy-cloud-run-candidate if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.deploy-cloud-run-candidate.result == 'success' environment: production permissions: contents: read @@ -709,8 +858,10 @@ jobs: runs-on: ubuntu-latest needs: deploy-cloud-run-candidate if: | - (github.repository == 'PolicyEngine/policyengine-api') + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.deploy-cloud-run-candidate.result == 'success' permissions: contents: read packages: write diff --git a/.github/workflows/seed-v2-database.yml b/.github/workflows/seed-v2-database.yml index 91bb67777..a55c023ee 100644 --- a/.github/workflows/seed-v2-database.yml +++ b/.github/workflows/seed-v2-database.yml @@ -17,6 +17,10 @@ on: jobs: seed: name: Upgrade schema and seed v2 database + # push.yml calls this from a release job that may follow skipped + # checks. GitHub would skip this job too unless its if: calls a status + # function; the calling job has already checked what it needs. + if: ${{ !cancelled() }} runs-on: ubuntu-latest timeout-minutes: 30 environment: ${{ inputs.deployment_environment }} diff --git a/changelog.d/split-release-tests.changed.md b/changelog.d/split-release-tests.changed.md new file mode 100644 index 000000000..2e8b431e1 --- /dev/null +++ b/changelog.d/split-release-tests.changed.md @@ -0,0 +1 @@ +The release run splits its test suite across six runners and checks that each test ran exactly once, and skips lint, the quality guards, both Alembic checks and the cross-database integration tests when the release commit changes only release files and its merge run passed them; the staging deploy starts about 10 minutes sooner. diff --git a/docs/engineering/skills/alembic-migrations.md b/docs/engineering/skills/alembic-migrations.md index e8cda9a72..a5953e2a2 100644 --- a/docs/engineering/skills/alembic-migrations.md +++ b/docs/engineering/skills/alembic-migrations.md @@ -338,8 +338,11 @@ shared or deployed database. - Pull requests first detect whether the v1 Alembic surface changed. Relevant PRs invoke `.github/workflows/alembic-v1-check.yml`; unrelated PRs skip it. -- Every push to `master` runs the reusable disposable-MySQL qualification next - to lint, regardless of which paths changed. +- Every merge to `master` runs the reusable disposable-MySQL qualification next + to lint, regardless of which paths changed. The release commit's run skips it + only when `check-release-commit` confirms that the release commit changes + nothing but release files and that the merge commit's run passed it + (`.github/release_commit.py`); otherwise the release run runs it again. - The release migration job runs before either staging deployment. It refuses unversioned or invalid history, takes a backup only when revisions are pending, upgrades under the advisory lock, and requires zero post-migration diff --git a/docs/engineering/skills/ci-workflows.md b/docs/engineering/skills/ci-workflows.md index 94955090b..d8f32a4ed 100644 --- a/docs/engineering/skills/ci-workflows.md +++ b/docs/engineering/skills/ci-workflows.md @@ -40,6 +40,34 @@ at least one failure path. Examples: `tests/unit/test_install_locked_ruff_script.py` and `tests/unit/test_cloud_run_deploy_scripts.py`. +## Release jobs in push.yml + +A release run (the run for the "Update PolicyEngine API" commit) may skip the +checks its merge run passed (`check-release-commit`, `.github/release_commit.py`). +GitHub skips a job whose needs include a skipped job, directly or through the +jobs between them, unless its `if:` calls a status function. So every release +job with `needs` starts its `if:` with `!cancelled()` and checks the result of +each job it needs, and each job of a reusable workflow a release job calls has +`if: ${{ !cancelled() }}`: + +```yaml + needs: + - migrate-v1-production-cloud-sql + - seed-v2-production-database + if: | + !cancelled() + && (github.repository == 'PolicyEngine/policyengine-api') + && (github.event.head_commit.message == 'Update PolicyEngine API') + && needs.migrate-v1-production-cloud-sql.result == 'success' + && needs.seed-v2-production-database.result == 'success' +``` + +Only `publish-git-tag` needs the checks that may be skipped; its +`release_commit.py require-checks` step decides whether their results let the +release continue. `tests/unit/test_release_run_checks.py` checks all of this. +A release job that left the status check to GitHub would be skipped whenever +the checks were, and the run would still finish green. + ## Enforcement and baselines The `ci-scripts` quality guard (`python scripts/run_quality_guards.py`, or diff --git a/docs/engineering/skills/testing.md b/docs/engineering/skills/testing.md index ae7b22354..4cc69fc89 100644 --- a/docs/engineering/skills/testing.md +++ b/docs/engineering/skills/testing.md @@ -176,6 +176,29 @@ needs it, so none of its steps start until the tests pass. The job keeps the `staging` environment and the variables the tests read when they ran inside the deployment job, and has no GCP credentials. +The release test suite runs in six shards, one runner each, so that it +finishes before the staging database jobs do. `run_release_tests.sh` loads the +pytest plugin `tests/release_test_shard_plugin.py`: every runner collects the +whole suite and keeps the tests in the files `scripts/release_test_shards.py` +assigns to its shard, heaviest file first, by the measured seconds in +`MEASURED_FILE_SECONDS` or else by test count. A file's tests stay in one +shard, so its fixtures are set up once. The `check-release-test-shards` job +then fails, and the staging deployment with it, unless every shard collected +the same tests and each test ran in exactly one shard. When a test file +becomes much slower or faster, measure it again with `--durations=0` and +update `MEASURED_FILE_SECONDS`; an out-of-date value only makes the shards +finish less evenly. + +The release run skips lint, the quality guards, both Alembic checks and the +cross-database integration tests when its merge run passed them on the same +code: `check-release-commit` (`.github/release_commit.py`) requires that the +release commit changes only the `[project]` version in `pyproject.toml`, the +project's own version entry in `uv.lock`, `CHANGELOG.md` and removed changelog +fragments, and that the merge commit's Push run passed all five checks. +Otherwise the release run runs them, and `publish-git-tag` requires lint, both +Alembic checks and the integration tests to pass, as before. The release test +suite always runs. + Staging deployment checks run the live integration suite against the tagged Cloud Run staging URL before promoting the exact tested revision to the service URL. No production deployment may begin until the diff --git a/scripts/release_test_shards.py b/scripts/release_test_shards.py new file mode 100644 index 000000000..d38fadf27 --- /dev/null +++ b/scripts/release_test_shards.py @@ -0,0 +1,300 @@ +"""Split the release test suite into shards, and check that each test ran once. + +push.yml's release-tests job runs ``make test`` on ``RELEASE_TEST_SHARD_COUNT`` +runners at once, one shard each. Every runner collects the whole suite, and the +pytest plugin in tests/release_test_shard_plugin.py keeps only the tests in the +files ``assign_files_to_shards`` gives its shard, ``RELEASE_TEST_SHARD_INDEX`` +(1 to the count). A file's tests stay together, so its module- and class-scoped +fixtures are set up once. + +Files are assigned heaviest first, each to the shard with the least weight so +far, ties going to the lower shard number; equal weights are taken in path +order. A file's weight is its measured seconds in ``MEASURED_FILE_SECONDS``, or +else its number of tests times ``SECONDS_PER_UNMEASURED_TEST``. The +assignment depends only on the collected files and their test counts, so every +runner that collects the same tests computes the same one, and each test runs +in exactly one shard. A weight that is out of date only makes the shards finish +less evenly. + +Each runner writes what it collected and what it kept to +``RELEASE_TEST_SHARD_REPORT_DIRECTORY``. push.yml's check-release-test-shards +job downloads every shard's report and runs this module's ``check-coverage`` +command, which fails unless every shard reported, all of them collected the same +tests, and each collected test was kept by exactly one shard. + +Usage: python -m scripts.release_test_shards check-coverage --shard-count N DIR +where DIR holds one ``release-test-shard-`` directory per shard. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from collections import Counter +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterable, Mapping, Sequence + +SHARD_INDEX_VARIABLE = "RELEASE_TEST_SHARD_INDEX" +SHARD_COUNT_VARIABLE = "RELEASE_TEST_SHARD_COUNT" +REPORT_DIRECTORY_VARIABLE = "RELEASE_TEST_SHARD_REPORT_DIRECTORY" +REPORT_DIRECTORY_PREFIX = "release-test-shard-" +SHARD_FILE = "shard.json" +COLLECTED_FILE = "collected.txt" +SELECTED_FILE = "selected.txt" +# How many problems of one kind check-coverage prints before it summarizes. +LISTED_PROBLEM_LIMIT = 20 +# Seconds each file's tests took, setup and teardown included, in a local run of +# make test's paths with --durations=0 on 2026-10-29 (only files over 10 +# seconds). Only the ratios matter. +MEASURED_FILE_SECONDS = { + "tests/unit/test_country_spm.py": 166, + "tests/unit/v2/test_import_side_effects.py": 152, + "tests/unit/v2/test_user_policy_routes.py": 32, + "tests/unit/test_asgi_factory.py": 29, + "tests/unit/v2/test_user_route_auth.py": 27, + "tests/unit/test_changelog_fragment_check.py": 24, + "tests/unit/v2/test_household_routes.py": 19, + "tests/unit/v2/test_user_household_routes.py": 17, + "tests/unit/routes/test_canonical_spm.py": 15, + "tests/unit/test_bundle_update_pins.py": 15, + "tests/unit/data/test_v1_database_migration.py": 15, + "tests/unit/routes/test_economy_spm_query.py": 15, + "tests/to_refactor/api/test_api.py": 15, + "tests/unit/routes/test_user_policy_route_outcomes.py": 15, + "tests/unit/routes/test_policy_dual_write_routes.py": 14, + "tests/unit/libs/test_simulation_entrypoint.py": 14, + "tests/unit/test_relock_pull_request_script.py": 13, + "tests/unit/services/test_budget_window_cache.py": 13, + "tests/contract/test_v1_route_contracts.py": 12, + "tests/unit/test_stage6_native_metadata.py": 11, +} +# The mean time of a test in the other files, in the same run. +SECONDS_PER_UNMEASURED_TEST = 0.04 + + +@dataclass(frozen=True) +class ShardSettings: + """Which shard this runner runs, of how many, and where it reports.""" + + index: int + count: int + report_directory: Path + + +@dataclass(frozen=True) +class ShardReport: + """What one shard collected and kept, as its runner wrote them.""" + + index: int + count: int + collected: tuple[str, ...] + selected: tuple[str, ...] + + +def shard_settings(environ: Mapping[str, str]) -> ShardSettings: + """Read the shard settings, raising ValueError when one is missing or wrong.""" + missing = [ + name + for name in ( + SHARD_INDEX_VARIABLE, + SHARD_COUNT_VARIABLE, + REPORT_DIRECTORY_VARIABLE, + ) + if not environ.get(name) + ] + if missing: + msg = f"release test sharding needs {', '.join(missing)}" + raise ValueError(msg) + index_text = environ[SHARD_INDEX_VARIABLE] + count_text = environ[SHARD_COUNT_VARIABLE] + if not (index_text.isdigit() and count_text.isdigit()): + msg = ( + f"{SHARD_INDEX_VARIABLE} and {SHARD_COUNT_VARIABLE} must be whole " + f"numbers, not {index_text!r} and {count_text!r}" + ) + raise ValueError(msg) + index = int(index_text) + count = int(count_text) + if not 1 <= index <= count: + msg = ( + f"{SHARD_INDEX_VARIABLE}={index} must be from 1 to " + f"{SHARD_COUNT_VARIABLE}={count}" + ) + raise ValueError(msg) + return ShardSettings( + index=index, + count=count, + report_directory=Path(environ[REPORT_DIRECTORY_VARIABLE]), + ) + + +def node_file(node_id: str) -> str: + """Return the file a pytest node ID belongs to.""" + return node_id.split("::", 1)[0] + + +def file_weights(node_ids: Iterable[str]) -> dict[str, float]: + """Return each collected file's weight, from MEASURED_FILE_SECONDS or its tests.""" + tests_per_file = Counter(node_file(node_id) for node_id in node_ids) + return { + path: MEASURED_FILE_SECONDS.get(path, test_count * SECONDS_PER_UNMEASURED_TEST) + for path, test_count in tests_per_file.items() + } + + +def assign_files_to_shards( + weights: Mapping[str, float], shard_count: int +) -> dict[str, int]: + """Return the shard (1 to ``shard_count``) each file of ``weights`` runs in.""" + if shard_count < 1: + msg = f"there must be at least one shard, not {shard_count}" + raise ValueError(msg) + shard_totals = dict.fromkeys(range(1, shard_count + 1), 0.0) + assignment = {} + for path in sorted(weights, key=lambda path: (-weights[path], path)): + lightest_shard = min( + shard_totals, key=lambda shard: (shard_totals[shard], shard) + ) + assignment[path] = lightest_shard + shard_totals[lightest_shard] += weights[path] + return assignment + + +def shard_node_ids( + node_ids: Sequence[str], shard_index: int, shard_count: int +) -> list[str]: + """Return the node IDs, in collection order, that shard ``shard_index`` runs.""" + assignment = assign_files_to_shards(file_weights(node_ids), shard_count) + return [ + node_id for node_id in node_ids if assignment[node_file(node_id)] == shard_index + ] + + +def write_report( + settings: ShardSettings, collected: Sequence[str], selected: Sequence[str] +) -> None: + """Write what this shard collected and kept to its report directory.""" + settings.report_directory.mkdir(parents=True, exist_ok=True) + (settings.report_directory / SHARD_FILE).write_text( + json.dumps({"index": settings.index, "count": settings.count}) + "\n", + encoding="utf-8", + ) + for file_name, node_ids in ( + (COLLECTED_FILE, collected), + (SELECTED_FILE, selected), + ): + (settings.report_directory / file_name).write_text( + "".join(f"{node_id}\n" for node_id in node_ids), encoding="utf-8" + ) + + +def read_report(report_directory: Path) -> ShardReport: + """Read one shard's report, as write_report wrote it.""" + shard = json.loads((report_directory / SHARD_FILE).read_text(encoding="utf-8")) + return ShardReport( + index=shard["index"], + count=shard["count"], + collected=tuple( + (report_directory / COLLECTED_FILE).read_text(encoding="utf-8").splitlines() + ), + selected=tuple( + (report_directory / SELECTED_FILE).read_text(encoding="utf-8").splitlines() + ), + ) + + +def _listed(kind: str, node_ids: Sequence[str]) -> list[str]: + problems = [f"{kind}: {node_id}" for node_id in node_ids[:LISTED_PROBLEM_LIMIT]] + if len(node_ids) > LISTED_PROBLEM_LIMIT: + problems.append(f"{kind}: {len(node_ids) - LISTED_PROBLEM_LIMIT} more") + return problems + + +def coverage_problems( + reports: Mapping[int, ShardReport], shard_count: int +) -> list[str]: + """Return why the shards did not run each collected test exactly once.""" + problems = [ + f"shard {index} of {shard_count} did not report" + for index in range(1, shard_count + 1) + if index not in reports + ] + for index, report in sorted(reports.items()): + if (report.index, report.count) != (index, shard_count): + problems.append( + f"the report for shard {index} of {shard_count} says shard " + f"{report.index} of {report.count}" + ) + if problems: + return problems + collected = reports[1].collected + for index, report in sorted(reports.items()): + if set(report.collected) != set(collected): + problems.append( + f"shard {index} collected different tests from shard 1: " + f"{len(set(report.collected) - set(collected))} more, " + f"{len(set(collected) - set(report.collected))} fewer" + ) + runs_per_test = Counter( + node_id for report in reports.values() for node_id in report.selected + ) + problems += _listed( + "collected but run by no shard", + [node_id for node_id in collected if node_id not in runs_per_test], + ) + problems += _listed( + "run by more than one shard", + sorted(node_id for node_id, runs in runs_per_test.items() if runs > 1), + ) + problems += _listed( + "run but not collected", + sorted(set(runs_per_test) - set(collected)), + ) + return problems + + +def check_coverage(report_root: Path, shard_count: int) -> list[str]: + """Read every shard's report under ``report_root`` and return the problems.""" + reports = {} + for index in range(1, shard_count + 1): + report_directory = report_root / f"{REPORT_DIRECTORY_PREFIX}{index}" + if (report_directory / SHARD_FILE).exists(): + reports[index] = read_report(report_directory) + return coverage_problems(reports, shard_count) + + +def main(argv: Sequence[str] | None = None) -> int: + """Run ``check-coverage`` and return the shell status.""" + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + commands = parser.add_subparsers(dest="command", required=True) + coverage_parser = commands.add_parser("check-coverage") + coverage_parser.add_argument("--shard-count", type=int, required=True) + coverage_parser.add_argument("report_root", type=Path) + arguments = parser.parse_args(argv) + problems = check_coverage(arguments.report_root, arguments.shard_count) + for problem in problems: + print(f"::error::{problem}") + if problems: + return 1 + reports = [ + read_report(arguments.report_root / f"{REPORT_DIRECTORY_PREFIX}{index}") + for index in range(1, arguments.shard_count + 1) + ] + print( + f"Each of the {len(reports[0].collected)} collected release tests ran in " + f"exactly one of {arguments.shard_count} shards: " + + ", ".join( + f"shard {report.index} ran {len(report.selected)}" for report in reports + ) + + "." + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/release_test_shard_plugin.py b/tests/release_test_shard_plugin.py new file mode 100644 index 000000000..5d362a229 --- /dev/null +++ b/tests/release_test_shard_plugin.py @@ -0,0 +1,37 @@ +"""Pytest plugin that runs one shard of the release test suite. + +.github/scripts/run_release_tests.sh loads it with ``-p +tests.release_test_shard_plugin`` in push.yml's release-tests job. After +collection it keeps only the tests in the files scripts/release_test_shards.py +assigns to this runner's shard (``RELEASE_TEST_SHARD_INDEX`` of +``RELEASE_TEST_SHARD_COUNT``), reports the rest as deselected, and writes what +it collected and kept to ``RELEASE_TEST_SHARD_REPORT_DIRECTORY`` for the +check-release-test-shards job. +""" + +from __future__ import annotations + +import os + +import pytest + +from scripts.release_test_shards import shard_node_ids, shard_settings, write_report + + +@pytest.hookimpl(trylast=True) +def pytest_collection_modifyitems( + config: pytest.Config, items: list[pytest.Item] +) -> None: + """Keep only this shard's tests, after every other plugin has filtered them.""" + try: + settings = shard_settings(os.environ) + except ValueError as error: + raise pytest.UsageError(str(error)) from error + collected = [item.nodeid for item in items] + selected_node_ids = set(shard_node_ids(collected, settings.index, settings.count)) + selected = [item for item in items if item.nodeid in selected_node_ids] + deselected = [item for item in items if item.nodeid not in selected_node_ids] + if deselected: + config.hook.pytest_deselected(items=deselected) + items[:] = selected + write_report(settings, collected, [item.nodeid for item in selected]) diff --git a/tests/unit/changelog_fragment_support.py b/tests/unit/changelog_fragment_support.py index 4db012db0..1bfbacbeb 100644 --- a/tests/unit/changelog_fragment_support.py +++ b/tests/unit/changelog_fragment_support.py @@ -1,10 +1,10 @@ -"""Load .github/changelog_fragments.py and .github/bump_version.py for tests. +"""Load .github/changelog_fragments.py and the modules that import it, for tests. -``python .github/bump_version.py`` puts .github/ first on ``sys.path``, so the -bump script imports ``changelog_fragments`` from the file beside it. These -helpers load both files by path and register each under its module name, so -the bump script's import finds the fragment module without a change to -``sys.path``. +``python .github/bump_version.py`` and ``python .github/release_commit.py`` +put .github/ first on ``sys.path``, so each imports ``changelog_fragments`` +from the file beside it. These helpers load the files by path and register +each under its module name, so those imports find the fragment module without +a change to ``sys.path``. """ from __future__ import annotations @@ -50,3 +50,9 @@ def bump_version() -> ModuleType: """Return .github/bump_version.py, after the fragment module it imports.""" changelog_fragments() return load_github_module("bump_version") + + +def release_commit() -> ModuleType: + """Return .github/release_commit.py, after the fragment module it imports.""" + changelog_fragments() + return load_github_module("release_commit") diff --git a/tests/unit/test_cloud_run_push_workflow.py b/tests/unit/test_cloud_run_push_workflow.py index c4ed071b1..60483f256 100644 --- a/tests/unit/test_cloud_run_push_workflow.py +++ b/tests/unit/test_cloud_run_push_workflow.py @@ -31,6 +31,10 @@ LIVE_TESTS_STEP = "Run live staging tests, one pytest worker per test" LIVE_TESTS_SCRIPT = REPO / ".github/scripts/run_live_staging_tests.sh" RELEASE_TESTS_JOB = "release-tests" +RELEASE_COMMIT_CONDITION = ( + "(github.repository == 'PolicyEngine/policyengine-api')\n" + "&& (github.event.head_commit.message == 'Update PolicyEngine API')\n" +) RELEASE_TESTS_STEP = "Run release tests" RELEASE_TESTS_COMMAND = "bash .github/scripts/run_release_tests.sh" # The variables the release tests read when they ran inside the staging deploy @@ -334,7 +338,8 @@ def test_release_tests_run_in_their_own_job_before_the_staging_deploy(): # They start with the run, beside the checks and the staging database jobs, # on the same release commits as the deploy. assert job_needs(release_tests) == [] - assert release_tests["if"] == staging_deploy["if"] + assert release_tests["if"] == RELEASE_COMMIT_CONDITION + assert RELEASE_COMMIT_CONDITION.splitlines()[1] in staging_deploy["if"] # No GCP credentials: the tests ran before GCP authentication in the deploy # job too. assert release_tests["permissions"] == {"contents": "read"} @@ -349,7 +354,13 @@ def test_release_tests_job_keeps_the_inputs_the_tests_read_in_the_deploy_job(): assert release_tests["environment"] == "staging" assert release_tests["env"] == RELEASE_TESTS_JOB_ENV assert release_tests_step["run"] == RELEASE_TESTS_COMMAND - assert release_tests_step["env"] == RELEASE_TESTS_STEP_ENV + # Each shard also gets its index, the shard count and where to report + # (tests/unit/test_release_test_shards.py). + assert { + name: value + for name, value in release_tests_step["env"].items() + if not name.startswith("RELEASE_TEST_SHARD_") + } == RELEASE_TESTS_STEP_ENV def test_push_workflow_uses_local_redis_for_predeployment_test_suite(): diff --git a/tests/unit/test_deploy_pipeline_scripts.py b/tests/unit/test_deploy_pipeline_scripts.py index 956b09f9c..c2c0db5cb 100644 --- a/tests/unit/test_deploy_pipeline_scripts.py +++ b/tests/unit/test_deploy_pipeline_scripts.py @@ -105,12 +105,21 @@ def _fake_make(tmp_path: Path, exit_code: int = 0) -> tuple[Path, Path]: return bin_dir, env_dump +def _shard_env(tmp_path: Path) -> dict[str, str]: + return { + "RELEASE_TEST_SHARD_INDEX": "2", + "RELEASE_TEST_SHARD_COUNT": "6", + "RELEASE_TEST_SHARD_REPORT_DIRECTORY": str(tmp_path / "shard-report"), + } + + def test_release_tests_drop_deployment_settings(tmp_path: Path) -> None: bin_dir, env_dump = _fake_make(tmp_path) result = _run( "run_release_tests.sh", env={ + **_shard_env(tmp_path), "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "ROUTE_IMPL_POLICY": "fastapi_native", "DB_WRITE_HOUSEHOLD": "dual", @@ -137,6 +146,10 @@ def test_release_tests_drop_deployment_settings(tmp_path: Path) -> None: assert "PUBLIC_API_AUTH_AUDIENCE" not in names assert "RUNTIME_CACHE_MODE=local" in dumped assert "RUNTIME_CACHE_URL=redis://127.0.0.1:6379/0" in dumped + # make test runs this runner's shard of the suite. + assert "PYTEST_ADDOPTS=-p tests.release_test_shard_plugin" in dumped + assert "RELEASE_TEST_SHARD_INDEX=2" in dumped + assert "RELEASE_TEST_SHARD_COUNT=6" in dumped def test_release_tests_fail_when_make_test_fails(tmp_path: Path) -> None: @@ -144,12 +157,28 @@ def test_release_tests_fail_when_make_test_fails(tmp_path: Path) -> None: result = _run( "run_release_tests.sh", - env={"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}"}, + env={ + **_shard_env(tmp_path), + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + }, ) assert result.returncode == 3 +def test_release_tests_refuse_to_run_without_a_shard(tmp_path: Path) -> None: + bin_dir, env_dump = _fake_make(tmp_path) + + result = _run( + "run_release_tests.sh", + env={"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}"}, + ) + + assert result.returncode != 0 + assert "RELEASE_TEST_SHARD_INDEX is required" in result.stderr + assert not env_dump.exists() + + # --- compute_cloud_run_metadata.sh ---------------------------------------- SHA = "0123456789abcdef0123456789abcdef01234567" diff --git a/tests/unit/test_release_commit.py b/tests/unit/test_release_commit.py new file mode 100644 index 000000000..b9e02a00f --- /dev/null +++ b/tests/unit/test_release_commit.py @@ -0,0 +1,601 @@ +"""Tests for the release commit check: .github/release_commit.py and its script. + +A release run (the run for the "Update PolicyEngine API" commit) may skip the +checks its merge run passed only when the release commit changes nothing but +release files, and the merge commit's Push run passed every one of those +checks. ``check_release_commit.sh`` gathers the release commit's changes, the +parent's copies of pyproject.toml and uv.lock, and the parent's Push runs, and +``release_commit.py check`` decides. ``release_commit.py require-checks`` stops +publish-git-tag unless the run skipped the checks for that reason or passed +them itself. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from typing import TYPE_CHECKING + +import pytest +import yaml + +from tests.unit.changelog_fragment_support import REPO, release_commit + +if TYPE_CHECKING: + from pathlib import Path + +release_commit_module = release_commit() + +CHECK_SCRIPT = REPO / ".github/scripts/check_release_commit.sh" +REPOSITORY = "PolicyEngine/policyengine-api" +MERGE_RUN_URL = "https://github.com/PolicyEngine/policyengine-api/actions/runs/101" +PYPROJECT = """[build-system] +requires = ["hatchling"] + +[project] +name = "policyengine-api" +version = "5.14.2" +dependencies = [ + "flask", +] + +[tool.example] +version = "1.0.0" + +[tool.towncrier] +directory = "changelog.d" + +[[tool.towncrier.type]] +directory = "fixed" +name = "Fixed" +showcontent = true +""" +LOCK = """version = 1 +revision = 3 +requires-python = ">=3.11" + +[[package]] +name = "flask" +version = "3.1.1" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "policyengine-api" +version = "5.14.2" +source = { editable = "." } +dependencies = [ + { name = "flask" }, +] + +[package.metadata] +requires-dist = [{ name = "flask" }] +""" +CHANGELOG = "## [5.14.2] - 2026-10-29\n\n### Fixed\n\n- An earlier fix.\n" +FRAGMENT = "changelog.d/economy-missing-policy.fixed.md" + + +def _bumped(text: str) -> str: + return text.replace('version = "5.14.2"', 'version = "5.14.3"') + + +# --- What the release commit changes ---------------------------------------- + + +def _change(path: str, status: str = "M", mode: str = "100644"): + old_mode = "000000" if status == "A" else mode + new_mode = "000000" if status == "D" else mode + return release_commit_module.FileChange( + path=path, status=status, old_mode=old_mode, new_mode=new_mode + ) + + +def _release_files( + *, + release_pyproject: str | None = None, + release_lock: str | None = None, +): + return release_commit_module.ReleaseFiles( + parent_pyproject=PYPROJECT, + release_pyproject=( + _bumped(PYPROJECT) if release_pyproject is None else release_pyproject + ), + parent_lock=LOCK, + release_lock=_bumped(LOCK) if release_lock is None else release_lock, + ) + + +RELEASE_CHANGES = [ + _change("CHANGELOG.md"), + _change(FRAGMENT, status="D"), + _change("pyproject.toml"), + _change("uv.lock"), +] + + +def _problems(changes, files=None) -> list[str]: + return release_commit_module.release_change_problems( + changes, files or _release_files(), "changelog.d" + ) + + +def test_a_release_commit_changes_only_release_files(): + assert _problems(RELEASE_CHANGES) == [] + + +def test_the_project_version_lines_are_found_in_their_own_tables(): + pyproject_lines = PYPROJECT.splitlines() + lock_lines = LOCK.splitlines() + + pyproject_index = release_commit_module.project_version_index(PYPROJECT) + lock_index = release_commit_module.locked_project_version_index( + LOCK, "policyengine-api" + ) + + assert pyproject_lines[pyproject_index] == 'version = "5.14.2"' + assert pyproject_lines[pyproject_index - 1] == 'name = "policyengine-api"' + assert lock_lines[lock_index] == 'version = "5.14.2"' + assert lock_lines[lock_index - 1] == 'name = "policyengine-api"' + + +def test_any_other_file_is_not_a_release_file(): + changes = [*RELEASE_CHANGES, _change("policyengine_api/api.py")] + + assert _problems(changes) == ["policyengine_api/api.py is not a release file"] + + +def test_pyproject_may_change_only_the_project_version(): + with_a_dependency = _bumped(PYPROJECT).replace('"flask",', '"flask",\n "rq",') + other_version = PYPROJECT.replace('version = "1.0.0"', 'version = "1.0.1"') + + for release_pyproject in (with_a_dependency, other_version): + files = _release_files(release_pyproject=release_pyproject) + assert _problems(RELEASE_CHANGES, files) == [ + "pyproject.toml changes more than the [project] version" + ] + + +def test_uv_lock_may_change_only_the_projects_own_version(): + other_package = _bumped(LOCK).replace('version = "3.1.1"', 'version = "3.1.2"') + reresolved = LOCK.replace('version = "3.1.1"', 'version = "3.1.2"') + added_dependency = _bumped(LOCK).replace( + ' { name = "flask" },', ' { name = "flask" },\n { name = "rq" },' + ) + + for release_lock in (other_package, reresolved, added_dependency): + files = _release_files(release_lock=release_lock) + assert _problems(RELEASE_CHANGES, files) == [ + "uv.lock changes more than policyengine-api's version" + ] + + +def test_release_files_must_stay_edited_regular_files(): + changes = [ + _change("CHANGELOG.md", status="D"), + _change("pyproject.toml", mode="100755"), + _change("uv.lock", status="T"), + ] + + assert _problems(changes) == [ + "CHANGELOG.md is not only edited", + "pyproject.toml changes more than the [project] version", + "uv.lock changes more than policyengine-api's version", + ] + + +def test_the_release_commit_may_only_remove_fragments(): + changes = [ + _change("changelog.d/4200.fixed.md", status="A"), + _change("changelog.d/4201.fixed.md"), + _change("changelog.d/.gitkeep", status="D"), + _change("changelog.d/fixed/4202.md", status="D"), + ] + + assert _problems(changes) == [ + "changelog.d/4200.fixed.md is a fragment the release commit does not remove", + "changelog.d/4201.fixed.md is a fragment the release commit does not remove", + "changelog.d/.gitkeep is not a release file", + "changelog.d/fixed/4202.md is not a release file", + ] + + +def test_git_diff_raw_output_is_parsed_per_path(): + raw_diff = ( + ":100644 100644 aaaa bbbb M\0CHANGELOG.md\0" + ":100644 000000 cccc 0000 D\0changelog.d/4200.fixed.md\0" + ) + + assert release_commit_module.parse_raw_diff(raw_diff) == [ + _change("CHANGELOG.md"), + _change("changelog.d/4200.fixed.md", status="D"), + ] + + +# --- Whether the merge run passed the checks ---------------------------------- + + +def _passing_jobs() -> list[dict]: + return [ + {"name": "Lint", "status": "completed", "conclusion": "success"}, + {"name": "Quality guards", "status": "completed", "conclusion": "success"}, + { + "name": "Alembic v1 qualification / Alembic MySQL lifecycle", + "status": "completed", + "conclusion": "success", + }, + { + "name": "Alembic v2 qualification / V2 Postgres migration lifecycle", + "status": "completed", + "conclusion": "success", + }, + { + "name": ( + "Cross-database integration tests / MySQL, PostgreSQL, and Redis " + "integration" + ), + "status": "completed", + "conclusion": "success", + }, + {"name": "Update versioning", "status": "completed", "conclusion": "success"}, + ] + + +def test_a_merge_run_that_passed_every_check_lets_the_release_run_skip_them(): + run = {"id": 101, "html_url": MERGE_RUN_URL} + + merge_run, problems = release_commit_module.passing_merge_run( + [run], {101: _passing_jobs()} + ) + + assert merge_run == run + assert problems == [] + + +@pytest.mark.parametrize( + ("job_name", "status", "conclusion", "problem"), + [ + ( + "Quality guards", + "completed", + "failure", + "Quality guards ended completed, failure", + ), + ("Lint", "in_progress", None, "Lint ended in_progress, None"), + ("Lint", "completed", "skipped", "Lint ended completed, skipped"), + ], +) +def test_a_check_that_did_not_pass_in_the_merge_run_runs_again( + job_name, status, conclusion, problem +): + jobs = _passing_jobs() + for job in jobs: + if job["name"] == job_name: + job.update(status=status, conclusion=conclusion) + + merge_run, problems = release_commit_module.passing_merge_run( + [{"id": 101, "html_url": MERGE_RUN_URL}], {101: jobs} + ) + + assert merge_run is None + assert problems == [f"Push run {MERGE_RUN_URL}: {problem}"] + + +def test_a_check_missing_from_the_merge_run_runs_again(): + jobs = [job for job in _passing_jobs() if not job["name"].startswith("Alembic v2")] + + merge_run, problems = release_commit_module.passing_merge_run( + [{"id": 101, "html_url": MERGE_RUN_URL}], {101: jobs} + ) + + assert merge_run is None + assert problems == [ + f"Push run {MERGE_RUN_URL}: Alembic v2 qualification did not run" + ] + + +def test_a_parent_without_a_push_run_runs_the_checks_again(): + assert release_commit_module.passing_merge_run([], {}) == ( + None, + ["the parent commit has no Push run"], + ) + + +def test_the_merge_run_checks_are_push_yml_jobs_with_those_names(): + jobs = yaml.safe_load((REPO / ".github/workflows/push.yml").read_text())["jobs"] + + for job_id, job_name in release_commit_module.MERGE_RUN_CHECKS.items(): + assert jobs[job_id]["name"] == job_name + # tests/unit/test_release_run_checks.py checks push.yml with copies of these. + assert tuple(release_commit_module.MERGE_RUN_CHECKS) == ( + "lint", + "quality-guards", + "alembic-v1-check", + "alembic-v2-check", + "v2-integration-check", + ) + assert set(release_commit_module.RELEASE_RUN_REQUIRED_CHECKS) == set( + release_commit_module.MERGE_RUN_CHECKS + ) - {"quality-guards"} + + +# --- publish-git-tag's requirement -------------------------------------------- + + +def _job_results(*, checks_passed: str, release_check: str, check_result: str): + results = { + "ensure-staging-model-version-aligns-with-sim-api": { + "result": "success", + "outputs": {}, + }, + "check-release-commit": { + "result": release_check, + "outputs": { + "merge_run_checks_passed": checks_passed, + "merge_run_url": MERGE_RUN_URL if checks_passed == "true" else "", + }, + }, + } + for job_id in release_commit_module.RELEASE_RUN_REQUIRED_CHECKS: + results[job_id] = {"result": check_result, "outputs": {}} + return results + + +@pytest.mark.parametrize( + ("checks_passed", "release_check", "check_result", "problems"), + [ + # The merge run passed the checks on the same code: they were skipped. + ("true", "success", "skipped", []), + # The release commit check found a reason to run them, and they passed. + ("false", "success", "success", []), + # The release commit check itself failed, and the checks passed. + ("", "failure", "success", []), + # Checks that ran must pass, whatever the release commit check said. + ("true", "success", "failure", ["lint ended failure"]), + ("false", "success", "failure", ["lint ended failure"]), + # Checks skipped without the release commit check's say-so stop the tag. + ("false", "success", "skipped", ["lint ended skipped"]), + ("true", "failure", "skipped", ["lint ended skipped"]), + ], +) +def test_publish_git_tag_requires_the_merge_run_or_this_run_to_pass_the_checks( + checks_passed, release_check, check_result, problems +): + job_results = _job_results( + checks_passed=checks_passed, + release_check=release_check, + check_result=check_result, + ) + for job_id in ("alembic-v1-check", "alembic-v2-check", "v2-integration-check"): + if problems: + job_results[job_id]["result"] = "success" + + assert release_commit_module.release_check_problems(job_results) == problems + + +def test_publish_git_tag_must_need_every_required_check(): + job_results = _job_results( + checks_passed="true", release_check="success", check_result="skipped" + ) + del job_results["alembic-v1-check"] + + assert release_commit_module.release_check_problems(job_results) == [ + "publish-git-tag does not need alembic-v1-check" + ] + + +def _require_checks(job_results: dict) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(REPO / ".github/release_commit.py"), "require-checks"], + cwd=REPO, + env={**os.environ, "JOB_RESULTS": json.dumps(job_results)}, + capture_output=True, + text=True, + check=False, + ) + + +def test_require_checks_exits_1_and_names_the_check_that_did_not_pass(): + result = _require_checks( + _job_results( + checks_passed="false", release_check="success", check_result="failure" + ) + ) + + assert result.returncode == 1 + assert "::error::lint ended failure" in result.stdout + + +def test_require_checks_names_the_merge_run_whose_checks_this_run_skipped(): + result = _require_checks( + _job_results( + checks_passed="true", release_check="success", check_result="skipped" + ) + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert MERGE_RUN_URL in result.stdout + + +# --- check_release_commit.sh, end to end ---------------------------------------- + + +def _git_environment() -> dict[str, str]: + """Return an environment that ignores the user's and system git settings.""" + return { + **os.environ, + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@example.com", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@example.com", + } + + +def _git(repository: Path, *arguments: str) -> str: + return subprocess.run( + ["git", *arguments], + cwd=repository, + env=_git_environment(), + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def _write(repository: Path, relative_path: str, text: str) -> None: + path = repository / relative_path + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +def _merge_commit_repository(tmp_path: Path) -> tuple[Path, str]: + """Return a repository whose HEAD is a merge commit with one pending fragment.""" + repository = tmp_path / "repository" + repository.mkdir() + _git(repository, "init", "--quiet", "--initial-branch", "master") + _write(repository, "pyproject.toml", PYPROJECT) + _write(repository, "uv.lock", LOCK) + _write(repository, "CHANGELOG.md", CHANGELOG) + _write(repository, "changelog.d/.gitkeep", "") + _write(repository, FRAGMENT, "Return 404 for a missing policy.\n") + _write(repository, "policyengine_api/api.py", "app = None\n") + _git(repository, "add", "--all") + _git(repository, "commit", "--quiet", "--message", "Merge pull request #4116") + return repository, _git(repository, "rev-parse", "HEAD") + + +def _commit_release(repository: Path, *, extra_change: bool = False) -> None: + """Commit what the versioning job commits, and optionally one change more.""" + _write(repository, "pyproject.toml", _bumped(PYPROJECT)) + _write(repository, "uv.lock", _bumped(LOCK)) + _write( + repository, + "CHANGELOG.md", + "## [5.14.3] - 2026-10-29\n\n### Fixed\n\n- Return 404.\n\n" + CHANGELOG, + ) + (repository / FRAGMENT).unlink() + if extra_change: + _write(repository, "policyengine_api/api.py", "app = object()\n") + _git(repository, "add", "--all") + _git(repository, "commit", "--quiet", "--message", "Update PolicyEngine API") + + +def _fake_gh(tmp_path: Path, merge_commit: str, jobs: list[dict]) -> Path: + """Put a gh first on PATH that answers the two Actions API requests.""" + responses = tmp_path / "responses" + responses.mkdir() + runs_path = ( + f"repos/{REPOSITORY}/actions/workflows/push.yml/runs" + f"?head_sha={merge_commit}&event=push&per_page=100" + ) + (responses / "runs.json").write_text( + json.dumps({"workflow_runs": [{"id": 101, "html_url": MERGE_RUN_URL}]}) + ) + (responses / "jobs.json").write_text(json.dumps({"jobs": jobs})) + jobs_path = f"repos/{REPOSITORY}/actions/runs/101/jobs?per_page=100" + bin_directory = tmp_path / "bin" + bin_directory.mkdir() + gh = bin_directory / "gh" + gh.write_text( + "#!/usr/bin/env bash\n" + f'if [[ "$*" == "api {runs_path}" ]]; then cat "{responses}/runs.json"\n' + f'elif [[ "$*" == "api {jobs_path}" ]]; then cat "{responses}/jobs.json"\n' + 'else echo "unexpected gh call: $*" >&2; exit 1; fi\n' + ) + gh.chmod(0o755) + return bin_directory + + +def _run_check(repository: Path, bin_directory: Path, tmp_path: Path): + output_path = tmp_path / "github_output" + output_path.touch() + result = subprocess.run( + ["bash", str(CHECK_SCRIPT)], + cwd=repository, + env={ + **_git_environment(), + "PATH": f"{bin_directory}{os.pathsep}{os.environ['PATH']}", + "GH_TOKEN": "fake-token", + "GITHUB_REPOSITORY": REPOSITORY, + "GITHUB_OUTPUT": str(output_path), + "PYTHON": sys.executable, + }, + capture_output=True, + text=True, + check=False, + ) + outputs = dict( + line.split("=", 1) for line in output_path.read_text().splitlines() if line + ) + return result, outputs + + +def test_a_release_commit_whose_merge_run_passed_skips_the_checks(tmp_path: Path): + repository, merge_commit = _merge_commit_repository(tmp_path) + _commit_release(repository) + bin_directory = _fake_gh(tmp_path, merge_commit, _passing_jobs()) + + result, outputs = _run_check(repository, bin_directory, tmp_path) + + assert result.returncode == 0, result.stdout + result.stderr + assert outputs == { + "merge_run_checks_passed": "true", + "merge_run_url": MERGE_RUN_URL, + } + + +def test_a_release_commit_with_another_change_runs_the_checks(tmp_path: Path): + repository, merge_commit = _merge_commit_repository(tmp_path) + _commit_release(repository, extra_change=True) + bin_directory = _fake_gh(tmp_path, merge_commit, _passing_jobs()) + + result, outputs = _run_check(repository, bin_directory, tmp_path) + + assert result.returncode == 0, result.stdout + result.stderr + assert outputs["merge_run_checks_passed"] == "false" + assert "::notice::policyengine_api/api.py is not a release file" in result.stdout + + +def test_a_release_commit_whose_merge_run_failed_a_check_runs_them(tmp_path: Path): + repository, merge_commit = _merge_commit_repository(tmp_path) + _commit_release(repository) + jobs = _passing_jobs() + jobs[0]["conclusion"] = "failure" + bin_directory = _fake_gh(tmp_path, merge_commit, jobs) + + result, outputs = _run_check(repository, bin_directory, tmp_path) + + assert result.returncode == 0, result.stdout + result.stderr + assert outputs["merge_run_checks_passed"] == "false" + assert "Lint ended completed, failure" in result.stdout + + +def test_the_checks_run_when_the_actions_api_cannot_be_read(tmp_path: Path): + repository, _merge_commit = _merge_commit_repository(tmp_path) + _commit_release(repository) + bin_directory = _fake_gh(tmp_path, "another-commit", _passing_jobs()) + + result, outputs = _run_check(repository, bin_directory, tmp_path) + + assert result.returncode == 0, result.stdout + result.stderr + assert outputs == {"merge_run_checks_passed": "false", "merge_run_url": ""} + assert "Could not list the Push runs of the parent" in result.stdout + + +def test_the_checks_run_when_the_release_commit_is_a_merge(tmp_path: Path): + repository, merge_commit = _merge_commit_repository(tmp_path) + _git(repository, "checkout", "--quiet", "-b", "other") + _write(repository, "README.md", "Other.\n") + _git(repository, "add", "--all") + _git(repository, "commit", "--quiet", "--message", "Other") + _git(repository, "checkout", "--quiet", "master") + _git(repository, "merge", "--quiet", "--no-ff", "--message", "Merge", "other") + bin_directory = _fake_gh(tmp_path, merge_commit, _passing_jobs()) + + result, outputs = _run_check(repository, bin_directory, tmp_path) + + assert result.returncode == 0, result.stdout + result.stderr + assert outputs["merge_run_checks_passed"] == "false" + assert "exactly one parent" in result.stdout diff --git a/tests/unit/test_release_run_checks.py b/tests/unit/test_release_run_checks.py new file mode 100644 index 000000000..73ceda32d --- /dev/null +++ b/tests/unit/test_release_run_checks.py @@ -0,0 +1,167 @@ +"""push.yml's release runs: the checks they may skip, and that no deploy is skipped. + +A release run skips lint, the quality guards, the Alembic v1 and v2 checks and +the cross-database integration tests when check-release-commit finds that the +release commit changes only release files and its merge run passed them +(.github/release_commit.py; tests/unit/test_release_commit.py). + +GitHub skips a job whose needs include a skipped or failed job, directly or +through the jobs between them, unless the job's ``if:`` calls a status function +such as ``!cancelled()``. Every release job therefore calls ``!cancelled()`` and +checks the result of each job it needs itself, and so does each job of the +reusable workflows they call. Without that, skipping the checks would skip +every deploy job after them, and the run would still finish green. +""" + +from __future__ import annotations + +import pytest +import yaml + +from tests.unit.cloud_run_deploy_support import ( + REPO, + job_needs, + job_step, + push_workflow_jobs, +) + +RELEASE_COMMIT_CHECK_JOB = "check-release-commit" +PUBLISH_TAG_JOB = "publish-git-tag" +REPOSITORY_CONDITION = "github.repository == 'PolicyEngine/policyengine-api'" +RELEASE_COMMIT = "github.event.head_commit.message == 'Update PolicyEngine API'" +RELEASE_CONDITION = f"({REPOSITORY_CONDITION})\n&& ({RELEASE_COMMIT})\n" +SKIP_CONDITION = "needs.check-release-commit.outputs.merge_run_checks_passed != 'true'" +# release_commit.MERGE_RUN_CHECKS and RELEASE_RUN_REQUIRED_CHECKS; that +# module's tests check they match. +CHECK_JOBS = ( + "lint", + "quality-guards", + "alembic-v1-check", + "alembic-v2-check", + "v2-integration-check", +) +REQUIRED_CHECKS = ( + "lint", + "alembic-v1-check", + "alembic-v2-check", + "v2-integration-check", +) + + +def _is_release_job(job: dict) -> bool: + return f"&& ({RELEASE_COMMIT})" in job.get("if", "") + + +def _release_jobs() -> dict[str, dict]: + return { + job_id: job + for job_id, job in push_workflow_jobs().items() + if _is_release_job(job) + } + + +def test_check_release_commit_runs_on_every_push_and_decides_on_release_commits(): + job = push_workflow_jobs()[RELEASE_COMMIT_CHECK_JOB] + decision = job_step(job, "Decide whether this run may skip the merge run's checks") + + # It runs on merge runs too, so the checks that need it never need a + # skipped job there; its steps do nothing on a merge commit. + assert job["if"] == REPOSITORY_CONDITION + assert job_needs(job) == [] + for step in job["steps"]: + assert step["if"] == RELEASE_COMMIT + assert job["permissions"] == {"contents": "read", "actions": "read"} + assert job_step(job, "Checkout the release commit and its parent")["with"] == { + "fetch-depth": 2 + } + assert decision["id"] == "release_commit" + assert decision["run"] == "bash .github/scripts/check_release_commit.sh" + assert decision["env"] == {"GH_TOKEN": "${{ github.token }}"} + assert job["outputs"] == { + "merge_run_checks_passed": ( + "${{ steps.release_commit.outputs.merge_run_checks_passed }}" + ), + "merge_run_url": "${{ steps.release_commit.outputs.merge_run_url }}", + } + + +@pytest.mark.parametrize("check_job", CHECK_JOBS) +def test_each_merge_run_check_runs_unless_the_release_commit_check_skips_it( + check_job, +): + job = push_workflow_jobs()[check_job] + + assert job_needs(job) == [RELEASE_COMMIT_CHECK_JOB] + # !cancelled(): when the release commit check fails, the checks run. + assert job["if"] == ( + f"!cancelled()\n&& ({REPOSITORY_CONDITION})\n&& ({SKIP_CONDITION})\n" + ) + + +def test_publish_git_tag_requires_the_merge_runs_checks_or_its_own(): + job = push_workflow_jobs()[PUBLISH_TAG_JOB] + step_names = [step["name"] for step in job["steps"]] + requirement = job_step(job, "Require the merge run's checks or this run's") + + assert set(job_needs(job)) == { + "ensure-staging-model-version-aligns-with-sim-api", + RELEASE_COMMIT_CHECK_JOB, + *REQUIRED_CHECKS, + } + assert job["if"] == ( + f"!cancelled()\n&& ({REPOSITORY_CONDITION})\n&& ({RELEASE_COMMIT})\n" + "&& needs.ensure-staging-model-version-aligns-with-sim-api.result == " + "'success'\n" + ) + # Before the tag, and so before anything that deploys. + assert step_names.index(requirement["name"]) < step_names.index("Publish Git Tag") + assert requirement["run"] == "python .github/release_commit.py require-checks" + assert requirement["env"] == {"JOB_RESULTS": "${{ toJSON(needs) }}"} + + +def test_every_release_job_checks_each_job_it_needs(): + # publish-git-tag's release_commit.py require-checks step reads the + # results of the checks and of check-release-commit. + read_by_require_checks = {RELEASE_COMMIT_CHECK_JOB, *CHECK_JOBS} + + for job_id, job in _release_jobs().items(): + needs = job_needs(job) + if not needs: + assert job["if"] == RELEASE_CONDITION, job_id + continue + assert job["if"].startswith(f"!cancelled()\n&& ({REPOSITORY_CONDITION})\n"), ( + job_id + ) + for needed_job in needs: + if job_id == PUBLISH_TAG_JOB and needed_job in read_by_require_checks: + continue + assert f"needs.{needed_job}.result == 'success'" in job["if"], ( + f"{job_id} must check that {needed_job} passed" + ) + + +def test_only_publish_git_tag_waits_for_the_checks_a_release_run_may_skip(): + skippable_jobs = {RELEASE_COMMIT_CHECK_JOB, *CHECK_JOBS} + + for job_id, job in _release_jobs().items(): + if job_id != PUBLISH_TAG_JOB: + assert not skippable_jobs & set(job_needs(job)), job_id + + +def test_reusable_workflows_called_by_release_jobs_run_their_jobs_unless_cancelled(): + called_workflows = sorted( + { + job["uses"].removeprefix("./") + for job in _release_jobs().values() + if "uses" in job and job_needs(job) + } + ) + + assert called_workflows == [ + ".github/workflows/migrate-v1-cloud-sql.yml", + ".github/workflows/seed-v2-database.yml", + ] + for called_workflow in called_workflows: + workflow = yaml.safe_load((REPO / called_workflow).read_text(encoding="utf-8")) + for job_id, job in workflow["jobs"].items(): + assert job.get("if") == "${{ !cancelled() }}", (called_workflow, job_id) diff --git a/tests/unit/test_release_test_shards.py b/tests/unit/test_release_test_shards.py new file mode 100644 index 000000000..f411ba844 --- /dev/null +++ b/tests/unit/test_release_test_shards.py @@ -0,0 +1,418 @@ +"""Tests for the release test shards: the split, the plugin and the coverage check. + +push.yml's release-tests job runs make test on six runners, one shard each +(scripts/release_test_shards.py, tests/release_test_shard_plugin.py), and +check-release-test-shards fails unless each collected test ran in exactly one. +""" + +from __future__ import annotations + +import os +import re +import subprocess +import sys +from typing import TYPE_CHECKING + +import pytest + +from scripts import release_test_shards +from tests.unit.cloud_run_deploy_support import ( + REPO, + job_needs, + job_step, + push_workflow_jobs, +) + +if TYPE_CHECKING: + from pathlib import Path + +SHARD_COUNT = 6 +NODE_IDS = [ + "tests/unit/test_alpha.py::test_one", + "tests/unit/test_alpha.py::test_two", + "tests/unit/test_beta.py::test_one", + "tests/unit/test_gamma.py::TestGroup::test_one[case-a]", + "tests/unit/test_gamma.py::TestGroup::test_one[case-b]", + "tests/unit/test_delta.py::test_one", + "tests/unit/test_country_spm.py::test_heavy", +] + + +def _make_test_paths() -> list[str]: + """Return the paths make test passes to pytest.""" + makefile = (REPO / "Makefile").read_text(encoding="utf-8") + recipe = makefile[makefile.index("\ntest:\n") :] + pytest_line = next(line for line in recipe.splitlines() if " -m pytest " in line) + arguments = pytest_line.split(" -m pytest ", 1)[1].split() + return [argument for argument in arguments if argument.startswith("tests/")] + + +def _make_test_files() -> list[str]: + """Return the test files under make test's paths, without importing them.""" + test_files = [] + for test_path in _make_test_paths(): + path = REPO / test_path + if path.is_file(): + test_files.append(test_path) + continue + test_files += [ + str(found.relative_to(REPO)) + for found in sorted(path.rglob("test_*.py")) + if "__pycache__" not in found.parts + ] + return test_files + + +# --- The split ------------------------------------------------------------------ + + +def test_every_node_id_runs_in_exactly_one_shard(): + shards = [ + release_test_shards.shard_node_ids(NODE_IDS, shard_index, SHARD_COUNT) + for shard_index in range(1, SHARD_COUNT + 1) + ] + assigned = [node_id for shard in shards for node_id in shard] + + assert sorted(assigned) == sorted(NODE_IDS) + assert len(assigned) == len(set(assigned)) + + +def test_a_files_tests_stay_in_one_shard_in_collection_order(): + for shard_index in range(1, SHARD_COUNT + 1): + shard = release_test_shards.shard_node_ids(NODE_IDS, shard_index, SHARD_COUNT) + assert shard == [node_id for node_id in NODE_IDS if node_id in shard] + gamma_tests = [node_id for node_id in shard if "test_gamma.py" in node_id] + assert gamma_tests in ([], NODE_IDS[3:5]) + + +def test_the_split_depends_only_on_the_collected_tests(): + reordered = list(reversed(NODE_IDS)) + + for shard_index in range(1, SHARD_COUNT + 1): + assert set( + release_test_shards.shard_node_ids(NODE_IDS, shard_index, SHARD_COUNT) + ) == set( + release_test_shards.shard_node_ids(reordered, shard_index, SHARD_COUNT) + ) + + +def test_files_go_heaviest_first_to_the_lightest_shard(): + weights = {"a.py": 10.0, "b.py": 7.0, "c.py": 4.0, "d.py": 4.0, "e.py": 2.0} + + assert release_test_shards.assign_files_to_shards(weights, 2) == { + "a.py": 1, + "b.py": 2, + "c.py": 2, + "d.py": 1, + "e.py": 2, + } + + +def test_a_measured_file_weighs_its_seconds_and_another_its_tests(): + weights = release_test_shards.file_weights(NODE_IDS) + + assert ( + weights["tests/unit/test_country_spm.py"] + == (release_test_shards.MEASURED_FILE_SECONDS["tests/unit/test_country_spm.py"]) + ) + assert weights["tests/unit/test_gamma.py"] == pytest.approx( + 2 * release_test_shards.SECONDS_PER_UNMEASURED_TEST + ) + + +def test_every_make_test_file_is_assigned_to_exactly_one_of_six_shards(): + test_files = _make_test_files() + assignment = release_test_shards.assign_files_to_shards( + release_test_shards.file_weights(test_files), SHARD_COUNT + ) + + assert sorted(assignment) == sorted(test_files) + assert set(assignment.values()) == set(range(1, SHARD_COUNT + 1)) + + +def test_every_measured_file_is_a_make_test_file(): + # A renamed or removed file would keep a weight nothing uses; measure again. + assert set(release_test_shards.MEASURED_FILE_SECONDS) <= set(_make_test_files()) + + +@pytest.mark.parametrize( + ("environ", "message"), + [ + ({}, "needs RELEASE_TEST_SHARD_INDEX, RELEASE_TEST_SHARD_COUNT"), + ( + { + "RELEASE_TEST_SHARD_INDEX": "7", + "RELEASE_TEST_SHARD_COUNT": "6", + "RELEASE_TEST_SHARD_REPORT_DIRECTORY": "reports", + }, + "RELEASE_TEST_SHARD_INDEX=7 must be from 1 to RELEASE_TEST_SHARD_COUNT=6", + ), + ( + { + "RELEASE_TEST_SHARD_INDEX": "one", + "RELEASE_TEST_SHARD_COUNT": "6", + "RELEASE_TEST_SHARD_REPORT_DIRECTORY": "reports", + }, + "must be whole numbers", + ), + ], +) +def test_shard_settings_refuse_a_missing_or_impossible_shard(environ, message): + with pytest.raises(ValueError, match=re.escape(message)): + release_test_shards.shard_settings(environ) + + +# --- The coverage check ----------------------------------------------------------- + + +def _report(index: int, selected: list[str], collected: list[str] | None = None): + return release_test_shards.ShardReport( + index=index, + count=2, + collected=tuple(NODE_IDS if collected is None else collected), + selected=tuple(selected), + ) + + +def test_shards_that_run_each_collected_test_once_pass(): + reports = {1: _report(1, NODE_IDS[:3]), 2: _report(2, NODE_IDS[3:])} + + assert release_test_shards.coverage_problems(reports, 2) == [] + + +def test_a_test_no_shard_ran_fails_the_check(): + reports = {1: _report(1, NODE_IDS[:3]), 2: _report(2, NODE_IDS[4:])} + + assert release_test_shards.coverage_problems(reports, 2) == [ + f"collected but run by no shard: {NODE_IDS[3]}" + ] + + +def test_a_test_two_shards_ran_fails_the_check(): + reports = {1: _report(1, NODE_IDS[:4]), 2: _report(2, NODE_IDS[3:])} + + assert release_test_shards.coverage_problems(reports, 2) == [ + f"run by more than one shard: {NODE_IDS[3]}" + ] + + +def test_shards_that_collected_different_tests_fail_the_check(): + reports = { + 1: _report(1, NODE_IDS[:3]), + 2: _report(2, NODE_IDS[3:-1], collected=NODE_IDS[:-1]), + } + + assert release_test_shards.coverage_problems(reports, 2) == [ + "shard 2 collected different tests from shard 1: 0 more, 1 fewer", + f"collected but run by no shard: {NODE_IDS[-1]}", + ] + + +def test_a_shard_that_did_not_report_fails_the_check(): + assert release_test_shards.coverage_problems({1: _report(1, NODE_IDS)}, 2) == [ + "shard 2 of 2 did not report" + ] + + +# --- The plugin, end to end ------------------------------------------------------- + + +def _write_test_suite(root: Path) -> None: + """Write 5 test files of 3 tests each under ``root``/sample_tests.""" + (root / "sample_tests").mkdir() + for file_index in range(5): + test_functions = "".join( + f"def test_{test_index}():\n pass\n\n\n" for test_index in range(3) + ) + (root / "sample_tests" / f"test_file_{file_index}.py").write_text( + test_functions + ) + + +def _run_shard(root: Path, report_root: Path, shard_index: int, shard_count: int): + return subprocess.run( + [ + sys.executable, + "-m", + "pytest", + "-p", + "tests.release_test_shard_plugin", + "-p", + "no:cacheprovider", + "--rootdir", + str(root), + "-q", + str(root / "sample_tests"), + ], + cwd=root, + env={ + **os.environ, + "PYTHONPATH": str(REPO), + "RELEASE_TEST_SHARD_INDEX": str(shard_index), + "RELEASE_TEST_SHARD_COUNT": str(shard_count), + "RELEASE_TEST_SHARD_REPORT_DIRECTORY": str( + report_root / f"release-test-shard-{shard_index}" + ), + }, + capture_output=True, + text=True, + check=False, + ) + + +def test_the_plugin_runs_each_test_in_exactly_one_shard(tmp_path: Path): + suite_root = tmp_path / "suite" + suite_root.mkdir() + _write_test_suite(suite_root) + report_root = tmp_path / "reports" + shard_count = 3 + + results = [ + _run_shard(suite_root, report_root, shard_index, shard_count) + for shard_index in range(1, shard_count + 1) + ] + coverage = subprocess.run( + [ + sys.executable, + "-m", + "scripts.release_test_shards", + "check-coverage", + "--shard-count", + str(shard_count), + str(report_root), + ], + cwd=REPO, + capture_output=True, + text=True, + check=False, + ) + + for result in results: + assert result.returncode == 0, result.stdout + result.stderr + assert "deselected" in result.stdout + assert coverage.returncode == 0, coverage.stdout + coverage.stderr + assert "Each of the 15 collected release tests ran in exactly one of 3 shards" in ( + coverage.stdout + ) + + +def test_the_coverage_check_fails_when_a_shard_did_not_report(tmp_path: Path): + suite_root = tmp_path / "suite" + suite_root.mkdir() + _write_test_suite(suite_root) + report_root = tmp_path / "reports" + _run_shard(suite_root, report_root, 1, 2) + + coverage = subprocess.run( + [ + sys.executable, + "-m", + "scripts.release_test_shards", + "check-coverage", + "--shard-count", + "2", + str(report_root), + ], + cwd=REPO, + capture_output=True, + text=True, + check=False, + ) + + assert coverage.returncode == 1 + assert "::error::shard 2 of 2 did not report" in coverage.stdout + + +def test_the_plugin_refuses_to_run_without_shard_settings(tmp_path: Path): + suite_root = tmp_path / "suite" + suite_root.mkdir() + _write_test_suite(suite_root) + + result = subprocess.run( + [ + sys.executable, + "-m", + "pytest", + "-p", + "tests.release_test_shard_plugin", + "-p", + "no:cacheprovider", + "--rootdir", + str(suite_root), + str(suite_root / "sample_tests"), + ], + cwd=suite_root, + env={ + name: value + for name, value in {**os.environ, "PYTHONPATH": str(REPO)}.items() + if not name.startswith("RELEASE_TEST_SHARD_") + }, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode != 0 + assert "release test sharding needs RELEASE_TEST_SHARD_INDEX" in ( + result.stdout + result.stderr + ) + + +# --- push.yml ------------------------------------------------------------------- + + +def test_the_release_tests_run_in_six_shards_that_report_what_they_ran(): + release_tests = push_workflow_jobs()["release-tests"] + run_step = job_step(release_tests, "Run release tests") + upload_step = job_step( + release_tests, "Upload the tests this shard collected and ran" + ) + + assert release_tests["strategy"]["matrix"]["shard_index"] == list( + range(1, SHARD_COUNT + 1) + ) + assert release_tests["strategy"]["fail-fast"] is False + assert run_step["env"]["RELEASE_TEST_SHARD_INDEX"] == "${{ matrix.shard_index }}" + assert run_step["env"]["RELEASE_TEST_SHARD_COUNT"] == str(SHARD_COUNT) + report_directory = run_step["env"]["RELEASE_TEST_SHARD_REPORT_DIRECTORY"] + assert upload_step["uses"] == "actions/upload-artifact@v4" + assert upload_step["with"] == { + "name": "release-test-shard-${{ matrix.shard_index }}", + "path": report_directory, + "if-no-files-found": "error", + "retention-days": 7, + } + + +def test_the_staging_deploy_needs_every_release_test_to_have_run_once(): + jobs = push_workflow_jobs() + shard_check = jobs["check-release-test-shards"] + download_step = job_step(shard_check, "Download every shard's report") + coverage_step = job_step( + shard_check, "Check that every release test ran in exactly one shard" + ) + + assert job_needs(shard_check) == ["release-tests"] + assert "needs.release-tests.result == 'success'" in shard_check["if"] + assert download_step["uses"] == "actions/download-artifact@v4" + assert download_step["with"]["pattern"] == "release-test-shard-*" + assert coverage_step["run"] == ( + "python -m scripts.release_test_shards check-coverage " + f"--shard-count {SHARD_COUNT} " + f'"{download_step["with"]["path"]}"' + ) + assert {"release-tests", "check-release-test-shards"} <= set( + job_needs(jobs["deploy-cloud-run-staging"]) + ) + + +def test_run_release_tests_loads_the_shard_plugin(): + script = (REPO / ".github/scripts/run_release_tests.sh").read_text(encoding="utf-8") + + assert "-p tests.release_test_shard_plugin" in script + for variable in ( + "RELEASE_TEST_SHARD_INDEX", + "RELEASE_TEST_SHARD_COUNT", + "RELEASE_TEST_SHARD_REPORT_DIRECTORY", + ): + assert f"${{{variable}:?" in script