GRM-35: fix: enforce tests pass before tagging a release

The release script now runs lint and tests before committing or tagging. If either fails, the release is aborted. Also fixes test_cli_version to use __version__ dynamically. Closes GRM-35
2026-06-21 19:10:30 +00:00
parent 2e5ca5a88f
commit 3dcdde80ad
5 changed files with 191 additions and 5 deletions
@@ -12,6 +12,8 @@ jobs:
        with:
          fetch-depth: 0
          token: ${{ secrets.REPO_TOKEN }}
+      - name: Set up environment
+        run: make setup
      - name: Install git-cliff
        run: |
          GIT_CLIFF_VERSION="2.13.0"
@@ -23,8 +25,6 @@ jobs:
          chmod +x "$HOME/.local/bin/git-cliff"
          echo "$HOME/.local/bin" >> "$GITHUB_PATH"
          "$HOME/.local/bin/git-cliff" --version
-      - name: Install Python dependencies
-        run: python3 -m pip install --break-system-packages requests python-dotenv click
      - name: Configure git
        run: |
          git config user.name "grm-ci-bot"
@@ -33,6 +33,7 @@ jobs:
        env:
          PYTHONPATH: src
        run: |
+          . .venv/bin/activate
          python3 scripts/release.py
      - name: Notify on failure
        if: failure()
@@ -40,6 +41,7 @@ jobs:
          REPO_TOKEN: ${{ secrets.REPO_TOKEN }}
          PYTHONPATH: src
        run: |
+          . .venv/bin/activate
          python3 scripts/notify_failure.py \
            --repo "${{ github.repository }}" \
            --run-id "${{ github.run_id }}" \
@@ -124,13 +124,17 @@ After a PR is merged to master, the release pipeline runs automatically:

 1. **Release workflow** (`.gitea/workflows/release.yml`):
   - Triggers on push to master
+   - Sets up full dev environment (`make setup`) so lint and tests can run
   - Runs `scripts/release.py` which uses **git-cliff** to:
     - Calculate the next semver version from conventional commits since the last tag
     - Update `__version__` in `src/gitea_runner_manager/__init__.py` (single source of truth)
     - Update `CHANGELOG.md` with the new version section
+     - **Run `make lint-ruff` and `make pytest-cov`** to verify the release is healthy
+     - If lint or tests fail, **abort immediately** — no commit, no tag
     - Commit with `release: vX.Y.Z` prefix (cleaner than `chore(release):`)
     - Create an annotated tag `vX.Y.Z` on the release commit
     - Push both the commit and tag to master
+   - `--skip-tests` flag bypasses test verification (emergency use only, not recommended)
   - Loops are prevented by `has_unreleased_changes` — after a release commit is tagged, the next run finds no unreleased changes and exits
   - On failure, creates a Gitea issue via `scripts/notify_failure.py`

@@ -8,6 +8,12 @@ source of truth, read by setuptools via ``dynamic = ["version"]``) and
 with the changelog as the tag message, and pushes both to trigger the publish
 workflow.

+**Test enforcement**: Before committing or tagging, the script runs
+``make lint-ruff`` and ``make pytest-cov`` to verify the release is healthy.
+If either fails, the release is aborted — no commit, no tag.  This ensures
+we never release a version that fails tests.  Use ``--skip-tests`` only for
+emergency releases (not recommended).
+
 The ``release:`` prefix (instead of ``chore(release):``) keeps the history
 clean while still being descriptive.  Loops are prevented by the
 ``has_unreleased_changes`` check — after a release commit is tagged, the next
@@ -18,7 +24,7 @@ last tag, it exits with a message and does nothing.  If the tag already exists
 (e.g., from a partial previous run), it skips tag creation and only pushes.

 Usage:
-  REPO_TOKEN=<token> python3 scripts/release.py [--dry-run]
+  REPO_TOKEN=<token> python3 scripts/release.py [--dry-run] [--skip-tests]
 """

 from __future__ import annotations
@@ -183,6 +189,35 @@ def commit_release_changes(new_version: str) -> bool:
    return True


+def run_tests() -> None:
+    """Run lint and tests to verify the release is healthy.
+
+    This is called *after* version files are updated but *before* the tag is
+    created, ensuring we never tag a release that fails tests.
+    """
+    click.echo(_("Running lint checks..."))
+    lint = run_cmd(["make", "lint-ruff"], check=False)
+    if lint.returncode != 0:
+        raise click.ClickException(
+            _(
+                "Lint failed — refusing to release. Fix lint errors first.\n{stderr}",
+                stderr=lint.stderr.strip() if lint.stderr else lint.stdout.strip(),
+            )
+        )
+    click.echo(_("Lint passed."))
+
+    click.echo(_("Running tests..."))
+    tests = run_cmd(["make", "pytest-cov"], check=False)
+    if tests.returncode != 0:
+        raise click.ClickException(
+            _(
+                "Tests failed — refusing to release. Fix test failures first.\n{stderr}",
+                stderr=tests.stderr.strip() if tests.stderr else tests.stdout.strip(),
+            )
+        )
+    click.echo(_("Tests passed."))
+
+
 def create_and_push_tag(new_version: str, changelog: str, dry_run: bool) -> bool:
    """Create an annotated tag with the changelog as message and push it.

@@ -206,7 +241,13 @@ def create_and_push_tag(new_version: str, changelog: str, dry_run: bool) -> bool

@click.command()
@click.option("--dry-run", is_flag=True, default=False, help="Show what would happen without making changes.")
-def main(dry_run: bool) -> None:
+@click.option(
+    "--skip-tests",
+    is_flag=True,
+    default=False,
+    help="Skip lint and test verification (NOT recommended — only for emergency releases).",
+)
+def main(dry_run: bool, skip_tests: bool) -> None:
    # Ensure we're on master
    branch = run_cmd(["git", "rev-parse", "--abbrev-ref", "HEAD"]).stdout.strip()
    if branch != "master":
@@ -251,6 +292,13 @@ def main(dry_run: bool) -> None:
    update_changelog(changelog)
    click.echo(_("Updated {changelog_file}", changelog_file=CHANGELOG_FILE))

+    # Verify tests pass BEFORE committing or tagging.
+    # This ensures we never release a version that fails tests.
+    if skip_tests:
+        click.echo(_("WARNING: --skip-tests passed — skipping test verification."))
+    else:
+        run_tests()
+
    # Commit version + changelog (Gap 11: use 'release:' prefix, not 'chore(release):')
    committed = commit_release_changes(new_version)
    if committed:
@@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch

 from click.testing import CliRunner

+from gitea_runner_manager import __version__
 from gitea_runner_manager.cli import cli


@@ -12,7 +13,7 @@ class TestCLI:
        runner = CliRunner()
        result = runner.invoke(cli, ["--version"])
        assert result.exit_code == 0
-        assert "0.2.0" in result.output
+        assert __version__ in result.output

    @patch("gitea_runner_manager.cli.RunnerManager")
    def test_install(self, mock_manager_class: MagicMock) -> None:
@@ -15,6 +15,7 @@ from scripts.release import (
    has_unreleased_changes,
    main,
    run_cmd,
+    run_tests,
    tag_exists,
    update_changelog,
    update_init_version,
@@ -269,6 +270,28 @@ class TestCreateAndPushTag:
        mock_run_cmd.assert_not_called()


+class TestRunTests:
+    @patch("scripts.release.run_cmd")
+    def test_lint_and_tests_pass(self, mock_run_cmd: MagicMock) -> None:
+        mock_run_cmd.return_value = MagicMock(returncode=0, stdout="", stderr="")
+        run_tests()  # should not raise
+
+    @patch("scripts.release.run_cmd")
+    def test_lint_fails_raises(self, mock_run_cmd: MagicMock) -> None:
+        mock_run_cmd.return_value = MagicMock(returncode=1, stdout="", stderr="lint error")
+        with pytest.raises(click.ClickException, match="Lint failed"):
+            run_tests()
+
+    @patch("scripts.release.run_cmd")
+    def test_tests_fail_raises(self, mock_run_cmd: MagicMock) -> None:
+        mock_run_cmd.side_effect = [
+            MagicMock(returncode=0, stdout="", stderr=""),  # lint passes
+            MagicMock(returncode=1, stdout="", stderr="test failure"),  # tests fail
+        ]
+        with pytest.raises(click.ClickException, match="Tests failed"):
+            run_tests()
+
+
 class TestMain:
    @patch.dict("os.environ", {})
    @patch("scripts.release.run_cmd")
@@ -351,6 +374,7 @@ class TestMain:
        mock_tag.assert_not_called()

    @patch.dict("os.environ", {})
+    @patch("scripts.release.run_tests")
    @patch("scripts.release.create_and_push_tag", return_value=True)
    @patch("scripts.release.commit_release_changes", return_value=True)
    @patch("scripts.release.update_changelog")
@@ -371,6 +395,7 @@ class TestMain:
        mock_update_changelog: MagicMock,
        mock_commit: MagicMock,
        mock_tag: MagicMock,
+        mock_run_tests: MagicMock,
    ) -> None:
        mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="")
        runner = CliRunner()
@@ -379,10 +404,12 @@ class TestMain:
        assert "Bumping version" in result.output
        mock_update_init.assert_called_once_with("0.2.0")
        mock_update_changelog.assert_called_once_with("changelog")
+        mock_run_tests.assert_called_once()
        mock_commit.assert_called_once_with("0.2.0")
        mock_tag.assert_called_once_with("0.2.0", "changelog", False)

    @patch.dict("os.environ", {})
+    @patch("scripts.release.run_tests")
    @patch("scripts.release.create_and_push_tag", return_value=False)
    @patch("scripts.release.commit_release_changes", return_value=False)
    @patch("scripts.release.update_changelog")
@@ -403,6 +430,7 @@ class TestMain:
        mock_update_changelog: MagicMock,
        mock_commit: MagicMock,
        mock_tag: MagicMock,
+        mock_run_tests: MagicMock,
    ) -> None:
        """When tag already exists, still update files but report existing tag."""
        mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="")
@@ -411,3 +439,106 @@ class TestMain:
        assert result.exit_code == 0
        assert "already existed" in result.output
        mock_tag.assert_called_once_with("0.1.0", "changelog", False)
+
+    @patch.dict("os.environ", {})
+    @patch("scripts.release.create_and_push_tag", return_value=True)
+    @patch("scripts.release.commit_release_changes", return_value=True)
+    @patch("scripts.release.update_changelog")
+    @patch("scripts.release.update_init_version")
+    @patch("scripts.release.get_changelog", return_value="changelog")
+    @patch("scripts.release.get_latest_tag", return_value="v0.1.0")
+    @patch("scripts.release.get_bumped_version", return_value="0.2.0")
+    @patch("scripts.release.has_unreleased_changes", return_value=True)
+    @patch("scripts.release.run_cmd")
+    def test_full_flow_skip_tests(
+        self,
+        mock_run_cmd: MagicMock,
+        mock_has: MagicMock,
+        mock_bumped: MagicMock,
+        mock_latest: MagicMock,
+        mock_changelog: MagicMock,
+        mock_update_init: MagicMock,
+        mock_update_changelog: MagicMock,
+        mock_commit: MagicMock,
+        mock_tag: MagicMock,
+    ) -> None:
+        """--skip-tests bypasses test verification."""
+        mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="")
+        runner = CliRunner()
+        result = runner.invoke(main, ["--skip-tests"])
+        assert result.exit_code == 0
+        assert "WARNING: --skip-tests" in result.output
+        # run_tests should NOT be called — verify no "make lint-ruff" or "make pytest-cov" calls
+        make_calls = [c.args[0] for c in mock_run_cmd.call_args_list if c.args[0][:1] == ["make"]]
+        assert make_calls == []
+
+    @patch.dict("os.environ", {})
+    @patch("scripts.release.create_and_push_tag")
+    @patch("scripts.release.commit_release_changes")
+    @patch("scripts.release.update_changelog")
+    @patch("scripts.release.update_init_version")
+    @patch("scripts.release.get_changelog", return_value="changelog")
+    @patch("scripts.release.get_latest_tag", return_value="v0.1.0")
+    @patch("scripts.release.get_bumped_version", return_value="0.2.0")
+    @patch("scripts.release.has_unreleased_changes", return_value=True)
+    @patch("scripts.release.run_cmd")
+    def test_tests_fail_aborts_before_tag(
+        self,
+        mock_run_cmd: MagicMock,
+        mock_has: MagicMock,
+        mock_bumped: MagicMock,
+        mock_latest: MagicMock,
+        mock_changelog: MagicMock,
+        mock_update_init: MagicMock,
+        mock_update_changelog: MagicMock,
+        mock_commit: MagicMock,
+        mock_tag: 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)
+        mock_run_cmd.side_effect = [
+            MagicMock(returncode=0, stdout="master\n", stderr=""),
+            MagicMock(returncode=0, stdout="", stderr=""),
+            MagicMock(returncode=1, stdout="", stderr="test failure"),
+        ]
+        runner = CliRunner()
+        result = runner.invoke(main, [])
+        assert result.exit_code != 0
+        assert "Tests failed" in result.output
+        mock_commit.assert_not_called()
+        mock_tag.assert_not_called()
+
+    @patch.dict("os.environ", {})
+    @patch("scripts.release.create_and_push_tag")
+    @patch("scripts.release.commit_release_changes")
+    @patch("scripts.release.update_changelog")
+    @patch("scripts.release.update_init_version")
+    @patch("scripts.release.get_changelog", return_value="changelog")
+    @patch("scripts.release.get_latest_tag", return_value="v0.1.0")
+    @patch("scripts.release.get_bumped_version", return_value="0.2.0")
+    @patch("scripts.release.has_unreleased_changes", return_value=True)
+    @patch("scripts.release.run_cmd")
+    def test_lint_fail_aborts_before_tag(
+        self,
+        mock_run_cmd: MagicMock,
+        mock_has: MagicMock,
+        mock_bumped: MagicMock,
+        mock_latest: MagicMock,
+        mock_changelog: MagicMock,
+        mock_update_init: MagicMock,
+        mock_update_changelog: MagicMock,
+        mock_commit: MagicMock,
+        mock_tag: MagicMock,
+    ) -> None:
+        """If lint fails, release aborts — no commit, no tag."""
+        mock_run_cmd.side_effect = [
+            MagicMock(returncode=0, stdout="master\n", stderr=""),
+            MagicMock(returncode=1, stdout="", stderr="lint error"),
+        ]
+        runner = CliRunner()
+        result = runner.invoke(main, [])
+        assert result.exit_code != 0
+        assert "Lint failed" in result.output
+        mock_commit.assert_not_called()
+        mock_tag.assert_not_called()