diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index b1a6a1d..50ba456 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -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 diff --git a/scripts/ci/classify_changes.py b/scripts/ci/classify_changes.py index d8a878c..6440b57 100644 --- a/scripts/ci/classify_changes.py +++ b/scripts/ci/classify_changes.py @@ -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"]) diff --git a/scripts/ci/release.py b/scripts/ci/release.py index 0e0ccdb..1158395 100644 --- a/scripts/ci/release.py +++ b/scripts/ci/release.py @@ -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: diff --git a/tests/unit/test_classify_changes.py b/tests/unit/test_classify_changes.py index 62aac7a..5dbc5ac 100644 --- a/tests/unit/test_classify_changes.py +++ b/tests/unit/test_classify_changes.py @@ -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 diff --git a/tests/unit/test_release.py b/tests/unit/test_release.py index f29bf67..fa02a17 100644 --- a/tests/unit/test_release.py +++ b/tests/unit/test_release.py @@ -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()