Skip to content

Commit ee7fc1f

Browse files
authored
MAINT: Run CI on release branches (#2510)
1 parent 2369bc9 commit ee7fc1f

7 files changed

Lines changed: 227 additions & 12 deletions

File tree

.github/workflows/build_and_test.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,18 @@ on:
99
push:
1010
branches:
1111
- "main"
12+
- "releases/v*"
1213
pull_request:
1314
branches:
1415
- "main"
15-
- "release/**"
16+
- "releases/**"
1617
merge_group:
1718
workflow_dispatch:
1819

1920
concurrency:
2021
# This ensures after each commit the old jobs are cancelled and the new ones
2122
# run instead.
22-
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
23+
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
2324
cancel-in-progress: true
2425

2526
jobs:
@@ -75,8 +76,12 @@ jobs:
7576
# latent violations in untouched files, including the tests/integration,
7677
# tests/partner_integration, and tests/end_to_end tiers, are caught before
7778
# merge instead of only on the post-merge run against main.
79+
shell: bash
7880
run: |
7981
git fetch origin main
82+
if [ -n "${GITHUB_BASE_REF:-}" ]; then
83+
git fetch origin "$GITHUB_BASE_REF"
84+
fi
8085
uv run pre-commit run --all-files
8186
8287
# Main job runs only if pre-commit succeeded

.github/workflows/diff_cover.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,25 @@
11
# Single place for all coverage checks: overall threshold + diff coverage on PRs.
22
# Runs once on Ubuntu/Python 3.12 instead of across the full OS x Python matrix.
3+
#
4+
# Release branches run on push only, for the overall coverage threshold. The diff-coverage
5+
# step is deliberately limited to pull requests targeting main, because it compares against
6+
# origin/main and a release branch differs from main by the entire release delta.
37

48
name: coverage
59

610
on:
711
push:
812
branches:
913
- "main"
14+
- "releases/v*"
1015
pull_request:
1116
branches:
1217
- "main"
13-
- "release/**"
1418
merge_group:
19+
workflow_dispatch:
1520

1621
concurrency:
17-
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
22+
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
1823
cancel-in-progress: true
1924

2025
jobs:

.github/workflows/docker_build.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,19 @@ on:
66
push:
77
branches:
88
- "main"
9+
- "releases/v*"
910
pull_request:
1011
branches:
1112
- "main"
12-
- "release/**"
13+
- "releases/**"
1314
merge_group:
1415
workflow_dispatch:
1516

1617
permissions:
1718
contents: read
1819

1920
concurrency:
20-
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
21+
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
2122
cancel-in-progress: true
2223

2324
jobs:

.github/workflows/frontend_tests.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,16 @@ on:
77
push:
88
branches:
99
- "main"
10+
- "releases/v*"
1011
pull_request:
1112
branches:
1213
- "main"
13-
- "release/**"
14+
- "releases/**"
1415
merge_group:
1516
workflow_dispatch:
1617

1718
concurrency:
18-
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
19+
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
1920
cancel-in-progress: true
2021

2122
env:

build_scripts/enforce_alembic_revision_immutability.py

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66
77
Checks staged changes (local pre-commit), the full branch diff against origin/main (CI PRs),
88
and the previous commit (CI merge-queue / push-to-main).
9+
10+
The two history checks are skipped on release branches, which legitimately diverge from main.
911
"""
1012

1113
import os
1214
import subprocess
1315
import sys
1416

1517
_VERSIONS_PATH = "pyrit/memory/alembic/versions/"
18+
_MERGE_QUEUE_REF_PREFIX = "refs/heads/gh-readonly-queue/"
1619

1720

1821
def _git(*args: str) -> subprocess.CompletedProcess[str]:
@@ -42,24 +45,59 @@ def _fail_ci(reason: str) -> bool:
4245
return False
4346

4447

48+
def _on_release_branch() -> bool:
49+
"""
50+
Report whether the checks below are running against a release branch.
51+
52+
A release branch is cut from an earlier tag and carries cherry-picked commits, so it
53+
legitimately differs from ``main`` in ways the history checks below would read as
54+
edits to already-released revisions.
55+
"""
56+
# push events; pull_request events set GITHUB_REF to refs/pull/<n>/merge instead,
57+
# which carries no branch name, so the target branch has to come from GITHUB_BASE_REF.
58+
github_ref = os.environ.get("GITHUB_REF", "")
59+
base_ref = os.environ.get("GITHUB_BASE_REF", "")
60+
if github_ref or base_ref:
61+
# merge_group events run on a temporary queue branch named
62+
# refs/heads/gh-readonly-queue/<target branch>/pr-<n>-<sha> and leave GITHUB_BASE_REF
63+
# unset, so the target branch has to be read back out of the ref itself.
64+
if github_ref.startswith(_MERGE_QUEUE_REF_PREFIX):
65+
github_ref = f"refs/heads/{github_ref[len(_MERGE_QUEUE_REF_PREFIX) :]}"
66+
return github_ref.startswith("refs/heads/releases/") or base_ref.startswith("releases/")
67+
# Neither variable is set outside CI, so fall back to the checked-out branch.
68+
return _git_stdout("rev-parse", "--abbrev-ref", "HEAD").startswith("releases/")
69+
70+
4571
def has_revision_violations() -> bool:
4672
# Local pre-commit: check staged changes
4773
violations = _get_violations(["--cached"])
4874
if violations:
4975
_report(violations)
5076
return True
5177

52-
# CI (PR): diff branch against its merge-base with origin/main.
78+
# A release branch carries cherry-picked fixes that amend already-released revisions on
79+
# purpose, so comparing it against `main` reports intentional work as violations. A pull
80+
# request is still comparable against its own base, so only the push and merge queue paths
81+
# are skipped. `git cherry-pick` does not run pre-commit, so the staged check above rarely
82+
# fires on those paths either: review is the remaining control there.
83+
base_ref = os.environ.get("GITHUB_BASE_REF", "")
84+
if _on_release_branch() and not base_ref:
85+
return False
86+
87+
# CI (PR): diff branch against its merge-base with the branch it targets. A pull request
88+
# into a release branch has to compare against that branch, because everything the release
89+
# branch already carries is not part of the change under review.
5390
# The three-dot syntax (A...B) resolves to ``git diff $(merge-base A B) B``
5491
# automatically, so we don't need a separate merge-base call. When
55-
# origin/main is missing (shallow clone) git exits non-zero.
56-
pr_diff = _git("diff", "--name-status", "origin/main...HEAD", "--", _VERSIONS_PATH)
92+
# the base is missing (shallow clone) git exits non-zero.
93+
base = f"origin/{base_ref}" if base_ref else "origin/main"
94+
pr_diff = _git("diff", "--name-status", f"{base}...HEAD", "--", _VERSIONS_PATH)
5795
if pr_diff.returncode == 0:
5896
violations = [line for line in pr_diff.stdout.strip().splitlines() if line and not line.startswith("A")]
5997
if violations:
6098
_report(violations)
6199
return True
62-
elif _fail_ci("origin/main is not available (shallow clone?)"):
100+
elif _fail_ci(f"{base} is not available (shallow clone?)"):
63101
return True
64102

65103
# CI (merge-queue / push-to-main): on main the branch *is* origin/main, so

doc/contributing/11_memory_models.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ await initialize_pyrit_async("SQLite", skip_schema_migration=True)
9393

9494
Once a migration revision is committed, it **must not be modified or deleted**. This is enforced by a pre-commit hook (`enforce_alembic_revision_immutability`). If you need to fix a migration, create a new revision instead.
9595

96+
A release branch is the one exception. A patch release cherry-picks a fix onto a branch cut from an earlier tag, and that fix may legitimately amend a revision that has already shipped, so the hook does not compare history on a release branch. Review is the control there.
97+
9698
### Pre-commit hooks
9799

98100
Two hooks run automatically when you touch memory-related files:
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT license.
3+
4+
import os
5+
import subprocess
6+
from types import SimpleNamespace
7+
from unittest.mock import patch
8+
9+
import pytest
10+
11+
from build_scripts.enforce_alembic_revision_immutability import (
12+
_on_release_branch,
13+
has_revision_violations,
14+
)
15+
16+
MODIFIED_REVISION = "M\tpyrit/memory/alembic/versions/b2f4c6a8d1e3_add_conversations_table.py"
17+
18+
19+
def _completed(stdout: str = "", returncode: int = 0) -> subprocess.CompletedProcess:
20+
return subprocess.CompletedProcess(args=["git"], returncode=returncode, stdout=stdout, stderr="")
21+
22+
23+
@pytest.mark.parametrize(
24+
"environment, expected",
25+
[
26+
({"GITHUB_REF": "refs/heads/releases/v1.1.0"}, True),
27+
({"GITHUB_REF": "refs/heads/releases/v1.0.1"}, True),
28+
({"GITHUB_REF": "refs/heads/main"}, False),
29+
({"GITHUB_REF": "refs/heads/releases-notes"}, False),
30+
({"GITHUB_REF": "refs/tags/v1.1.0"}, False),
31+
({"GITHUB_REF": "refs/pull/42/merge", "GITHUB_BASE_REF": "releases/v1.1.0"}, True),
32+
({"GITHUB_REF": "refs/pull/42/merge", "GITHUB_BASE_REF": "main"}, False),
33+
({"GITHUB_REF": "refs/heads/gh-readonly-queue/releases/v1.1.0/pr-42-abc123"}, True),
34+
({"GITHUB_REF": "refs/heads/gh-readonly-queue/main/pr-42-abc123"}, False),
35+
({}, False),
36+
],
37+
)
38+
def test_on_release_branch_recognizes_release_refs(environment: dict[str, str], expected: bool) -> None:
39+
"""pull_request events carry the target branch in GITHUB_BASE_REF; merge_group embeds it in GITHUB_REF."""
40+
with patch.dict("os.environ", environment, clear=True):
41+
with patch("build_scripts.enforce_alembic_revision_immutability._git_stdout", return_value="main"):
42+
assert _on_release_branch() is expected
43+
44+
45+
@pytest.mark.parametrize(
46+
"checked_out_branch, expected",
47+
[("releases/v1.1.0", True), ("main", False), ("HEAD", False)],
48+
)
49+
def test_on_release_branch_falls_back_to_checked_out_branch(checked_out_branch: str, expected: bool) -> None:
50+
"""Runs outside GitHub Actions have no ref variables, leaving the branch name as the only signal."""
51+
with patch.dict("os.environ", {}, clear=True):
52+
with patch(
53+
"build_scripts.enforce_alembic_revision_immutability._git_stdout",
54+
return_value=checked_out_branch,
55+
) as mock_git_stdout:
56+
assert _on_release_branch() is expected
57+
58+
assert mock_git_stdout.call_args.args == ("rev-parse", "--abbrev-ref", "HEAD")
59+
60+
61+
def test_on_release_branch_ignores_branch_name_when_ci_refs_are_present() -> None:
62+
"""A PR from a release-named source branch into main must still be enforced."""
63+
environment = {"GITHUB_REF": "refs/pull/42/merge", "GITHUB_BASE_REF": "main"}
64+
with patch.dict("os.environ", environment, clear=True):
65+
with patch(
66+
"build_scripts.enforce_alembic_revision_immutability._git_stdout",
67+
return_value="releases/v1.1.0",
68+
) as mock_git_stdout:
69+
assert _on_release_branch() is False
70+
71+
mock_git_stdout.assert_not_called()
72+
73+
74+
def test_release_branch_push_skips_history_checks() -> None:
75+
"""A release branch push shares neither origin/main nor a comparable previous commit."""
76+
77+
def _fail_if_called(*args, **kwargs):
78+
raise AssertionError(f"history check ran on a release branch push: {args}")
79+
80+
with patch.dict(os.environ, {"GITHUB_BASE_REF": ""}, clear=False):
81+
with patch("build_scripts.enforce_alembic_revision_immutability._on_release_branch", return_value=True):
82+
with patch("build_scripts.enforce_alembic_revision_immutability._get_violations", return_value=[]):
83+
with patch("build_scripts.enforce_alembic_revision_immutability._git", side_effect=_fail_if_called):
84+
assert has_revision_violations() is False
85+
86+
87+
def test_release_pull_request_compares_against_its_base() -> None:
88+
"""A pull request into a release branch is comparable against that branch."""
89+
calls: list[tuple] = []
90+
91+
def _record(*args, **kwargs):
92+
calls.append(args)
93+
return SimpleNamespace(returncode=0, stdout="", stderr="")
94+
95+
with patch.dict(os.environ, {"GITHUB_BASE_REF": "releases/v1.1.0"}, clear=False):
96+
with patch("build_scripts.enforce_alembic_revision_immutability._on_release_branch", return_value=True):
97+
with patch("build_scripts.enforce_alembic_revision_immutability._get_violations", return_value=[]):
98+
with patch("build_scripts.enforce_alembic_revision_immutability._git", side_effect=_record):
99+
assert has_revision_violations() is False
100+
101+
assert any("origin/releases/v1.1.0...HEAD" in call for call in calls)
102+
assert not any("origin/main...HEAD" in call for call in calls)
103+
104+
105+
def test_release_pull_request_reports_modified_revision() -> None:
106+
"""The base comparison still catches a revision the pull request itself edits."""
107+
108+
def _modified(*args, **kwargs):
109+
if "diff" in args and any(arg == "origin/releases/v1.1.0...HEAD" for arg in args):
110+
return SimpleNamespace(returncode=0, stdout=f"M\t{MODIFIED_REVISION}\n", stderr="")
111+
return SimpleNamespace(returncode=0, stdout="", stderr="")
112+
113+
with patch.dict(os.environ, {"GITHUB_BASE_REF": "releases/v1.1.0"}, clear=False):
114+
with patch("build_scripts.enforce_alembic_revision_immutability._on_release_branch", return_value=True):
115+
with patch("build_scripts.enforce_alembic_revision_immutability._get_violations", return_value=[]):
116+
with patch("build_scripts.enforce_alembic_revision_immutability._git", side_effect=_modified):
117+
assert has_revision_violations() is True
118+
119+
120+
def test_release_branch_still_reports_staged_violations() -> None:
121+
"""Skipping the history checks must not stop the staged-change check."""
122+
with patch("build_scripts.enforce_alembic_revision_immutability._on_release_branch", return_value=True):
123+
with patch(
124+
"build_scripts.enforce_alembic_revision_immutability._get_violations",
125+
return_value=[MODIFIED_REVISION],
126+
):
127+
assert has_revision_violations() is True
128+
129+
130+
def test_branch_comparison_still_runs_off_release_branches() -> None:
131+
"""Positive control: the origin/main comparison must keep catching violations everywhere else."""
132+
with patch.dict(os.environ, {"GITHUB_BASE_REF": ""}, clear=False):
133+
with patch("build_scripts.enforce_alembic_revision_immutability._on_release_branch", return_value=False):
134+
with patch("build_scripts.enforce_alembic_revision_immutability._get_violations", return_value=[]):
135+
with patch(
136+
"build_scripts.enforce_alembic_revision_immutability._git",
137+
return_value=_completed(stdout=MODIFIED_REVISION),
138+
) as mock_git:
139+
assert has_revision_violations() is True
140+
141+
assert mock_git.call_args.args[:3] == ("diff", "--name-status", "origin/main...HEAD")
142+
143+
144+
def test_previous_commit_check_still_runs_off_release_branches() -> None:
145+
"""Positive control: the HEAD~1..HEAD check must keep catching violations everywhere else."""
146+
147+
def _violations_for(diff_spec: list[str]) -> list[str]:
148+
return [MODIFIED_REVISION] if diff_spec == ["HEAD~1..HEAD"] else []
149+
150+
with patch("build_scripts.enforce_alembic_revision_immutability._on_release_branch", return_value=False):
151+
with patch(
152+
"build_scripts.enforce_alembic_revision_immutability._get_violations",
153+
side_effect=_violations_for,
154+
):
155+
with patch("build_scripts.enforce_alembic_revision_immutability._git", return_value=_completed()):
156+
assert has_revision_violations() is True
157+
158+
159+
def test_clean_history_off_release_branches_passes() -> None:
160+
with patch("build_scripts.enforce_alembic_revision_immutability._on_release_branch", return_value=False):
161+
with patch("build_scripts.enforce_alembic_revision_immutability._get_violations", return_value=[]):
162+
with patch("build_scripts.enforce_alembic_revision_immutability._git", return_value=_completed()):
163+
assert has_revision_violations() is False

0 commit comments

Comments
 (0)