GRM-51: fix: release pipeline determinism with lock, rebase, and consistent classification
This commit is contained in:
+10
-6
@@ -68,24 +68,28 @@ jobs:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 0
|
||||
- name: Set up environment
|
||||
run: make setup
|
||||
- name: Detect changed paths
|
||||
id: detect
|
||||
env:
|
||||
PYTHONPATH: src
|
||||
run: |
|
||||
set -euo pipefail
|
||||
. .venv/bin/activate
|
||||
BASE="origin/master"
|
||||
HEAD="${{ github.event.pull_request.head.sha || github.sha }}"
|
||||
# Check if any Ansible-related files changed
|
||||
ANSIBLE_CHANGED=$(git diff --name-only "$BASE" "$HEAD" -- ansible/ .ansible-lint 2>/dev/null | head -1)
|
||||
if [ -n "$ANSIBLE_CHANGED" ]; then
|
||||
# Use classify_changes.py for consistent file classification
|
||||
ANSIBLE_RESULT=$(python3 scripts/ci/classify_changes.py --base "$BASE" --head "$HEAD" --check ansible --quiet)
|
||||
if [ "$ANSIBLE_RESULT" = "true" ]; then
|
||||
echo "ansible-changed=true" >> "$GITHUB_OUTPUT"
|
||||
echo "Ansible files changed — molecule tests will run."
|
||||
else
|
||||
echo "ansible-changed=false" >> "$GITHUB_OUTPUT"
|
||||
echo "No Ansible files changed — skipping molecule tests."
|
||||
fi
|
||||
# Check if any user-facing files changed (src/, ansible/, pyproject.toml)
|
||||
USER_FACING=$(git diff --name-only "$BASE" "$HEAD" -- src/gitea_runner_manager/ ansible/ pyproject.toml 2>/dev/null | head -1)
|
||||
if [ -n "$USER_FACING" ]; then
|
||||
USER_FACING_RESULT=$(python3 scripts/ci/classify_changes.py --base "$BASE" --head "$HEAD" --check user-facing --quiet)
|
||||
if [ "$USER_FACING_RESULT" = "true" ]; then
|
||||
echo "user-facing-changed=true" >> "$GITHUB_OUTPUT"
|
||||
echo "User-facing files changed — release dry-run will run."
|
||||
else
|
||||
|
||||
@@ -176,7 +176,13 @@ def get_latest_tag() -> str:
|
||||
@click.option("--base", default=None, help="Base ref (default: latest tag).")
|
||||
@click.option("--head", default="HEAD", help="Head ref (default: HEAD).")
|
||||
@click.option("--quiet", is_flag=True, default=False, help="Only output true/false.")
|
||||
def main(base: str | None, head: str, quiet: bool) -> None:
|
||||
@click.option(
|
||||
"--check",
|
||||
type=click.Choice(["all", "ansible", "user-facing"]),
|
||||
default="all",
|
||||
help="Check specific category: all (default), ansible, or user-facing.",
|
||||
)
|
||||
def main(base: str | None, head: str, quiet: bool, check: str) -> None:
|
||||
if base is None:
|
||||
base = get_latest_tag()
|
||||
if not base:
|
||||
@@ -194,6 +200,34 @@ def main(base: str | None, head: str, quiet: bool) -> None:
|
||||
click.echo(_("No changes between {base} and {head}.", base=base, head=head))
|
||||
return
|
||||
|
||||
if check == "ansible":
|
||||
# Check only for Ansible-related file changes
|
||||
ansible_files = [f for f in files if f.startswith("ansible/") or f == ".ansible-lint"]
|
||||
has_ansible = bool(ansible_files)
|
||||
if quiet:
|
||||
click.echo("true" if has_ansible else "false")
|
||||
return
|
||||
click.echo(_("\nAnsible files changed ({count}):", count=len(ansible_files)))
|
||||
for f in ansible_files:
|
||||
click.echo(f" {f}")
|
||||
click.echo(_("\nResult: {status}", status="Ansible changes detected" if has_ansible else "No Ansible changes"))
|
||||
return
|
||||
|
||||
if check == "user-facing":
|
||||
# Check only for user-facing file changes (inverse of workflow-only)
|
||||
user_files = [f for f in files if is_user_facing(f)]
|
||||
has_user = bool(user_files)
|
||||
if quiet:
|
||||
click.echo("true" if has_user else "false")
|
||||
return
|
||||
click.echo(_("\nUser-facing files changed ({count}):", count=len(user_files)))
|
||||
for f in user_files:
|
||||
click.echo(f" {f}")
|
||||
click.echo(
|
||||
_("\nResult: {status}", status="User-facing changes detected" if has_user else "No user-facing changes")
|
||||
)
|
||||
return
|
||||
|
||||
result = classify_changes(files)
|
||||
has_user = bool(result["user_facing"])
|
||||
|
||||
|
||||
+29
-13
@@ -105,24 +105,25 @@ def get_changelog(new_version: str) -> str:
|
||||
|
||||
|
||||
def has_unreleased_changes(bumped_version: str | None = None) -> bool:
|
||||
"""Check if there are conventional commits since the last tag.
|
||||
"""Check if there are unreleased conventional commits since the last tag.
|
||||
|
||||
If ``bumped_version`` is provided (from a prior git-cliff call), reuses it
|
||||
to avoid a duplicate subprocess invocation.
|
||||
Uses ``git log`` to check for commits between the last tag and HEAD.
|
||||
This is more reliable than comparing version strings — if git-cliff
|
||||
bumps to the same version (e.g., two fix commits between tags), the
|
||||
version comparison would incorrectly report "no unreleased changes"
|
||||
even though there are commits that haven't been released yet.
|
||||
"""
|
||||
if bumped_version is None:
|
||||
result = run_cmd(
|
||||
["git-cliff", "--bumped-version", "--config", CLIFF_CONFIG],
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return False
|
||||
bumped_version = result.stdout.strip().lstrip("v")
|
||||
latest = get_latest_tag()
|
||||
if not latest:
|
||||
return True
|
||||
current = latest.lstrip("v")
|
||||
return bumped_version != current
|
||||
# Check for any commits since the last tag
|
||||
result = run_cmd(
|
||||
["git", "log", f"{latest}..HEAD", "--oneline"],
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return False
|
||||
return bool(result.stdout.strip())
|
||||
|
||||
|
||||
def update_init_version(new_version: str) -> None:
|
||||
@@ -264,6 +265,18 @@ def main(dry_run: bool, skip_tests: bool) -> None:
|
||||
)
|
||||
)
|
||||
|
||||
# Release lock: if HEAD is already a release commit, another release
|
||||
# run is in progress (or already completed). Skip to prevent duplicate tags.
|
||||
head_msg = run_cmd(["git", "log", "-1", "--pretty=%s"]).stdout.strip()
|
||||
if re.match(r"^release: v\d+\.\d+\.\d+", head_msg):
|
||||
click.echo(
|
||||
_(
|
||||
"HEAD is already a release commit ('{msg}'). Another release may have just completed. Skipping.",
|
||||
msg=head_msg,
|
||||
)
|
||||
)
|
||||
return
|
||||
|
||||
# Check if any user-facing files changed since the last tag.
|
||||
# If only workflow/infra files changed, skip the release entirely.
|
||||
latest_tag = get_latest_tag()
|
||||
@@ -326,6 +339,9 @@ def main(dry_run: bool, skip_tests: bool) -> None:
|
||||
committed = commit_release_changes(new_version)
|
||||
if committed:
|
||||
click.echo(_("Created release commit."))
|
||||
# Pull --rebase before push to handle the case where master
|
||||
# advanced between checkout and commit (e.g., another merge).
|
||||
run_cmd(["git", "pull", "--rebase", "origin", "master"], check=False)
|
||||
run_cmd(["git", "push", "origin", "master"])
|
||||
click.echo(_("Pushed release commit to master."))
|
||||
else:
|
||||
|
||||
@@ -252,3 +252,63 @@ class TestMain:
|
||||
result = runner.invoke(main, ["--base", "v0.2.0", "--head", "HEAD"])
|
||||
assert result.exit_code == 0
|
||||
assert "release needed" in result.output
|
||||
|
||||
@patch("scripts.ci.classify_changes.get_changed_files")
|
||||
@patch("scripts.ci.classify_changes.get_latest_tag", return_value="v0.3.0")
|
||||
def test_check_ansible_true(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None:
|
||||
"""--check ansible with Ansible changes outputs true."""
|
||||
mock_changes.return_value = ["ansible/tasks/main.yml", ".gitea/workflows/ci.yml"]
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, ["--check", "ansible", "--quiet"])
|
||||
assert result.exit_code == 0
|
||||
assert "true" in result.output
|
||||
|
||||
@patch("scripts.ci.classify_changes.get_changed_files")
|
||||
@patch("scripts.ci.classify_changes.get_latest_tag", return_value="v0.3.0")
|
||||
def test_check_ansible_false(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None:
|
||||
"""--check ansible with no Ansible changes outputs false."""
|
||||
mock_changes.return_value = ["src/gitea_runner_manager/cli.py", ".gitea/workflows/ci.yml"]
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, ["--check", "ansible", "--quiet"])
|
||||
assert result.exit_code == 0
|
||||
assert "false" in result.output
|
||||
|
||||
@patch("scripts.ci.classify_changes.get_changed_files")
|
||||
@patch("scripts.ci.classify_changes.get_latest_tag", return_value="v0.3.0")
|
||||
def test_check_user_facing_true(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None:
|
||||
"""--check user-facing with user-facing changes outputs true."""
|
||||
mock_changes.return_value = ["src/gitea_runner_manager/cli.py", ".gitea/workflows/ci.yml"]
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, ["--check", "user-facing", "--quiet"])
|
||||
assert result.exit_code == 0
|
||||
assert "true" in result.output
|
||||
|
||||
@patch("scripts.ci.classify_changes.get_changed_files")
|
||||
@patch("scripts.ci.classify_changes.get_latest_tag", return_value="v0.3.0")
|
||||
def test_check_user_facing_false(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None:
|
||||
"""--check user-facing with only workflow changes outputs false."""
|
||||
mock_changes.return_value = [".gitea/workflows/ci.yml", "tests/test_foo.py"]
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, ["--check", "user-facing", "--quiet"])
|
||||
assert result.exit_code == 0
|
||||
assert "false" in result.output
|
||||
|
||||
@patch("scripts.ci.classify_changes.get_changed_files")
|
||||
@patch("scripts.ci.classify_changes.get_latest_tag", return_value="v0.3.0")
|
||||
def test_check_ansible_non_quiet(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None:
|
||||
"""--check ansible in non-quiet mode prints file list."""
|
||||
mock_changes.return_value = ["ansible/tasks/main.yml"]
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, ["--check", "ansible"])
|
||||
assert result.exit_code == 0
|
||||
assert "Ansible changes detected" in result.output
|
||||
|
||||
@patch("scripts.ci.classify_changes.get_changed_files")
|
||||
@patch("scripts.ci.classify_changes.get_latest_tag", return_value="v0.3.0")
|
||||
def test_check_user_facing_non_quiet(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None:
|
||||
"""--check user-facing in non-quiet mode prints file list."""
|
||||
mock_changes.return_value = ["src/gitea_runner_manager/cli.py"]
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, ["--check", "user-facing"])
|
||||
assert result.exit_code == 0
|
||||
assert "User-facing changes detected" in result.output
|
||||
|
||||
+31
-19
@@ -99,37 +99,29 @@ class TestGetChangelog:
|
||||
|
||||
class TestHasUnreleasedChanges:
|
||||
@patch("scripts.ci.release.get_latest_tag")
|
||||
def test_with_bumped_version_no_changes(self, mock_latest: MagicMock) -> None:
|
||||
mock_latest.return_value = "v0.2.0"
|
||||
assert has_unreleased_changes(bumped_version="0.2.0") is False
|
||||
|
||||
@patch("scripts.ci.release.get_latest_tag")
|
||||
def test_with_bumped_version_has_changes(self, mock_latest: MagicMock) -> None:
|
||||
mock_latest.return_value = "v0.2.0"
|
||||
assert has_unreleased_changes(bumped_version="0.3.0") is True
|
||||
|
||||
@patch("scripts.ci.release.get_latest_tag")
|
||||
def test_with_bumped_version_no_tags(self, mock_latest: MagicMock) -> None:
|
||||
def test_no_tags_has_changes(self, mock_latest: MagicMock) -> None:
|
||||
mock_latest.return_value = ""
|
||||
assert has_unreleased_changes(bumped_version="0.1.0") is True
|
||||
assert has_unreleased_changes() is True
|
||||
|
||||
@patch("scripts.ci.release.get_latest_tag")
|
||||
@patch("scripts.ci.release.run_cmd")
|
||||
def test_without_bumped_version_no_changes(self, mock_run_cmd: MagicMock, mock_latest: MagicMock) -> None:
|
||||
mock_run_cmd.return_value = MagicMock(returncode=0, stdout="v0.2.0\n")
|
||||
def test_no_commits_since_tag(self, mock_run_cmd: MagicMock, mock_latest: MagicMock) -> None:
|
||||
mock_run_cmd.return_value = MagicMock(returncode=0, stdout="")
|
||||
mock_latest.return_value = "v0.2.0"
|
||||
assert has_unreleased_changes() is False
|
||||
|
||||
@patch("scripts.ci.release.get_latest_tag")
|
||||
@patch("scripts.ci.release.run_cmd")
|
||||
def test_without_bumped_version_has_changes(self, mock_run_cmd: MagicMock, mock_latest: MagicMock) -> None:
|
||||
mock_run_cmd.return_value = MagicMock(returncode=0, stdout="v0.3.0\n")
|
||||
def test_commits_since_tag(self, mock_run_cmd: MagicMock, mock_latest: MagicMock) -> None:
|
||||
mock_run_cmd.return_value = MagicMock(returncode=0, stdout="abc123 fix: bug\n")
|
||||
mock_latest.return_value = "v0.2.0"
|
||||
assert has_unreleased_changes() is True
|
||||
|
||||
@patch("scripts.ci.release.get_latest_tag")
|
||||
@patch("scripts.ci.release.run_cmd")
|
||||
def test_cliff_fails_returns_false(self, mock_run_cmd: MagicMock) -> None:
|
||||
def test_git_log_fails_returns_false(self, mock_run_cmd: MagicMock, mock_latest: MagicMock) -> None:
|
||||
mock_run_cmd.return_value = MagicMock(returncode=1, stdout="")
|
||||
mock_latest.return_value = "v0.2.0"
|
||||
assert has_unreleased_changes() is False
|
||||
|
||||
|
||||
@@ -313,6 +305,22 @@ class TestMain:
|
||||
assert result.exit_code == 0
|
||||
assert "Dry-run mode" in result.output
|
||||
|
||||
@patch.dict("os.environ", {})
|
||||
@patch("scripts.ci.release.has_user_facing_changes", return_value=True)
|
||||
@patch("scripts.ci.release.run_cmd")
|
||||
def test_release_lock_skips_when_head_is_release_commit(self, mock_run_cmd: MagicMock, mock_uf: MagicMock) -> None:
|
||||
"""If HEAD is already a release commit, should skip to prevent duplicate releases."""
|
||||
# First call: git rev-parse (master), second: git log -1 (release commit)
|
||||
mock_run_cmd.side_effect = [
|
||||
MagicMock(returncode=0, stdout="master\n", stderr=""),
|
||||
MagicMock(returncode=0, stdout="release: v0.5.0\n", stderr=""),
|
||||
]
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, [])
|
||||
assert result.exit_code == 0
|
||||
assert "already a release commit" in result.output
|
||||
assert "Skipping" in result.output
|
||||
|
||||
@patch.dict("os.environ", {})
|
||||
@patch("scripts.ci.release.has_user_facing_changes", return_value=True)
|
||||
@patch("scripts.ci.release.has_unreleased_changes", return_value=False)
|
||||
@@ -543,10 +551,11 @@ class TestMain:
|
||||
mock_user: MagicMock,
|
||||
) -> None:
|
||||
"""If tests fail, release aborts — no commit, no tag."""
|
||||
# First call: git rev-parse (master), then make lint-ruff (success),
|
||||
# then make pytest-cov (failure)
|
||||
# Calls: git rev-parse (master), git log -1 (release lock check),
|
||||
# make lint-ruff (success), make pytest-cov (failure)
|
||||
mock_run_cmd.side_effect = [
|
||||
MagicMock(returncode=0, stdout="master\n", stderr=""),
|
||||
MagicMock(returncode=0, stdout="GRM-50 fix: something\n", stderr=""),
|
||||
MagicMock(returncode=0, stdout="", stderr=""),
|
||||
MagicMock(returncode=1, stdout="", stderr="test failure"),
|
||||
]
|
||||
@@ -582,8 +591,11 @@ class TestMain:
|
||||
mock_user: MagicMock,
|
||||
) -> None:
|
||||
"""If lint fails, release aborts — no commit, no tag."""
|
||||
# Calls: git rev-parse (master), git log -1 (release lock check),
|
||||
# make lint-ruff (failure)
|
||||
mock_run_cmd.side_effect = [
|
||||
MagicMock(returncode=0, stdout="master\n", stderr=""),
|
||||
MagicMock(returncode=0, stdout="GRM-50 fix: something\n", stderr=""),
|
||||
MagicMock(returncode=1, stdout="", stderr="lint error"),
|
||||
]
|
||||
runner = CliRunner()
|
||||
|
||||
Reference in New Issue
Block a user