diff --git a/README.md b/README.md index b54da0e9..89e8a444 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,10 @@ Only a few inputs are required to get started: For the full input and output reference, see [Configuration reference](docs/configuration_reference.md). For how label → chapter mapping and aggregation works, see [Custom Chapters Behavior](docs/configuration_reference.md#custom-chapters-behavior). -> **Important**: tag defined by `tag-name` must exist in the repository; otherwise, the action fails. +> **Important**: In compare mode (`from-tag-name` provided), **both** `tag-name` and +> `from-tag-name` must exist as git tags in the repository. The action checks each tag +> via the GitHub API before calling compare and exits with a tag-specific error if either +> is absent. ## Example Workflow @@ -136,14 +139,26 @@ on: workflow_dispatch: inputs: tag-name: - description: 'Existing git tag to use for this draft release. Syntax: "v[0-9]+.[0-9]+.[0-9]+". Ensure the tag is created and pushed before running.' + description: 'Tag name to create and use for this draft release. Syntax: "v[0-9]+.[0-9]+.[0-9]+".' required: true + from-tag-name: + description: 'Existing tag to use as the start of the release range (compare mode). Syntax: "v[0-9]+.[0-9]+.[0-9]+". Must already exist in the repository.' + required: false jobs: release: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Create and push tag + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + git tag ${{ github.event.inputs.tag-name }} + git push origin ${{ github.event.inputs.tag-name }} - name: Generate Release Notes id: notes @@ -152,6 +167,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: tag-name: ${{ github.event.inputs.tag-name }} + from-tag-name: ${{ github.event.inputs.from-tag-name }} chapters: | - {"title": "New Features 🎉", "labels": "enhancement, feature", "order": 10} - {"title": "Bugfixes 🛠", "labels": "error, bug", "order": 20} diff --git a/action.yml b/action.yml index 0b1e5fee..50b32f47 100644 --- a/action.yml +++ b/action.yml @@ -42,7 +42,9 @@ inputs: required: false default: 'both' from-tag-name: - description: 'The tag name of the previous release to use as a start reference point for the current release notes.' + description: | + The tag name of the previous release to use as a start reference point for the current release notes. + When provided, activates compare mode. Both this tag and 'tag-name' must exist as git tags. required: false default: '' hierarchy: diff --git a/docs/features/compare_mode.md b/docs/features/compare_mode.md index bdf9efc4..2c5fc10d 100644 --- a/docs/features/compare_mode.md +++ b/docs/features/compare_mode.md @@ -43,6 +43,10 @@ Generating release notes for **`v2.6.5`** (previous: `v2.6.4`): Compare mode is active **when `from-tag-name` is explicitly provided**. When it is absent the existing timestamp path runs unchanged. +> **Prerequisite — both tags must exist:** Before the compare API is called, the action +> looks up each tag via `get_git_ref("tags/")`. If either tag is absent the action +> exits immediately with a tag-specific error message naming the missing tag. + ### Step 1 — Graph-based commit selection Instead of asking "what happened after time T?", the action asks GitHub: *"what commits @@ -106,11 +110,14 @@ from-tag-name provided? ┌──┴──────────────────────┐ YES (compare mode) NO (timestamp mode) │ │ - GitHub Compare API: get_commits(since=data.since) - commits unique to to-tag get_pulls(state=closed) + Validate both tags exist get_commits(since=data.since) + (exit with error if missing) get_pulls(state=closed) │ │ - extract PR numbers FilterByRelease drops - from commit messages PRs/commits before since + GitHub Compare API: FilterByRelease drops + commits unique to to-tag PRs/commits before since + │ + extract PR numbers + from commit messages │ fetch each PR by number │ diff --git a/docs/motivation.md b/docs/motivation.md index f3e05b62..dcffd78e 100644 --- a/docs/motivation.md +++ b/docs/motivation.md @@ -14,7 +14,7 @@ Release documentation often drifts: missing PR summaries, unlabeled issues, or m |-----------|-------------| | Determinism | Same inputs produce the same release notes; no hidden heuristics. | | Explicit Configuration | Chapters defined in YAML; no magic label groupings. | -| Fail Safe | If explicit notes missing, optionally fall back to CodeRabbit; never silently fabricate content. | +| Fail Safe | If explicit notes missing, optionally fall back to CodeRabbit; never silently fabricate content. In compare mode, both tags must exist before the compare API is called — the action exits immediately with a clear error rather than confusing 404 responses. | | Transparency | Service Chapters surface hygiene issues instead of hiding them. | | Extensibility | Row formats, hierarchy, and duplicity policy are pluggable via inputs. | | Minimal Boilerplate | Only a tag and basic chapters required for first adoption. | diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 23a59a01..96464dae 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -168,10 +168,9 @@ def get_from_tag_name() -> str: @staticmethod def is_from_tag_name_defined() -> bool: """ - Check if the from-tag name is defined in the action inputs. + Check if the from-tag name is defined (normalizes to a non-empty tag). """ - value = ActionInputs.get_from_tag_name() - return value.strip() != "" + return ActionInputs.get_from_tag_name() != "" @staticmethod def get_chapters() -> list[dict[str, str]]: @@ -602,8 +601,14 @@ def validate_inputs() -> None: errors.append("Tag name must be a non-empty string.") from_tag_name = ActionInputs.get_from_tag_name() + raw_from_tag_name = get_action_input(FROM_TAG_NAME, default="") if not isinstance(from_tag_name, str): errors.append("From tag name must be a string.") + elif raw_from_tag_name != "" and from_tag_name == "": + errors.append( + "'from-tag-name' must not be empty when running in compare mode. " + "Both 'tag-name' and 'from-tag-name' must refer to existing tags in the repository." + ) chapters = ActionInputs.get_chapters() if len(chapters) == 0: diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index dc32e271..f3f236b1 100644 --- a/release_notes_generator/data/miner.py +++ b/release_notes_generator/data/miner.py @@ -26,7 +26,7 @@ from typing import Optional, Callable import semver -from github import Github +from github import Github, GithubException from github.GitRelease import GitRelease from github.Issue import Issue from github.PullRequest import PullRequest @@ -102,17 +102,20 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: - Extract PR numbers from commit messages and fetch those PRs. - Filter out commits that already have a PR reference to avoid duplication. """ - logger.info( - "Compare mode: using repo.compare('%s', '%s').", - ActionInputs.get_from_tag_name(), - ActionInputs.get_tag_name(), - ) - comparison = self._safe_call(repo.compare)(ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name()) + from_tag = ActionInputs.get_from_tag_name() + to_tag = ActionInputs.get_tag_name() + + logger.info("Compare mode: using repo.compare('%s', '%s').", from_tag, to_tag) + + self._validate_tag_exists(repo, from_tag) + self._validate_tag_exists(repo, to_tag) + + comparison = self._safe_call(repo.compare)(from_tag, to_tag) if comparison is None: logger.error( "Compare API returned no result for '%s'...'%s'. Ending!", - ActionInputs.get_from_tag_name(), - ActionInputs.get_tag_name(), + from_tag, + to_tag, ) sys.exit(1) compare_commits: list[GithubCommit] = list(comparison.commits) @@ -157,6 +160,35 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: len(data.pull_requests), ) + def _validate_tag_exists(self, repo: Repository, tag: str) -> None: + try: + repo.get_git_ref(f"tags/{tag}") + except GithubException as e: + if e.status == 404: + logger.error( + "Tag '%s' does not exist in repository '%s'. " + "Both 'tag-name' and 'from-tag-name' must exist as git tags before compare mode is used. Ending!", + tag, + repo.full_name, + ) + else: + logger.error( + "GitHub API error validating tag '%s' in repository '%s' (HTTP %s): %s. Ending!", + tag, + repo.full_name, + e.status, + e.data, + ) + sys.exit(1) + except Exception as exc: + logger.error( + "Unexpected error validating tag '%s' in repository '%s': %s. Ending!", + tag, + repo.full_name, + exc, + ) + sys.exit(1) + def _handle_since_time_mode(self, repo: Repository, data: MinedData) -> None: """ Handle since-time mode: mine issues, PRs, and commits based on release timestamp. diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index 2c6a3554..a7007689 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -20,7 +20,7 @@ from datetime import datetime from typing import Optional -from github import Github +from github import Github, GithubException from github.Commit import Commit from github.GitRelease import GitRelease from github.Issue import Issue @@ -587,6 +587,7 @@ def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4" release_mock.published_at = published_at release_mock.tag_name = from_tag mock_repo.get_release.return_value = release_mock + mock_repo.get_git_ref.return_value = mocker.Mock() mock_repo.get_issues.return_value = [] comparison_mock = mocker.Mock() @@ -786,3 +787,87 @@ def test_mine_data_timestamp_mode_compare_shas_empty(mocker, mock_repo): data = miner.mine_data() assert data.compare_commit_shas == set() + + +# --- compare mode tag-ref existence validation --- + + +def test_mine_data_compare_mode_from_tag_not_found_exits(mocker, mock_repo): + """from-tag-name absent as git ref → specific error logged and sys.exit(1).""" + not_found = GithubException(404, {"message": "Not Found"}, {}) + mock_repo.get_git_ref.side_effect = lambda ref: (_ for _ in ()).throw(not_found) if ref == "tags/v2.6.3" else None + error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") + + miner = _make_compare_miner(mocker, mock_repo) + + with pytest.raises(SystemExit) as exc_info: + miner.mine_data() + + assert exc_info.value.code == 1 + logged_messages = " ".join(str(call) for call in error_mock.call_args_list) + assert "v2.6.3" in logged_messages + mock_repo.compare.assert_not_called() + + +def test_mine_data_compare_mode_to_tag_not_found_exits(mocker, mock_repo): + """tag-name absent as git ref → specific error logged and sys.exit(1).""" + not_found = GithubException(404, {"message": "Not Found"}, {}) + mock_repo.get_git_ref.side_effect = lambda ref: (_ for _ in ()).throw(not_found) if ref == "tags/v2.6.4" else None + error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") + + miner = _make_compare_miner(mocker, mock_repo) + + with pytest.raises(SystemExit) as exc_info: + miner.mine_data() + + assert exc_info.value.code == 1 + logged_messages = " ".join(str(call) for call in error_mock.call_args_list) + assert "v2.6.4" in logged_messages + mock_repo.compare.assert_not_called() + + +def test_mine_data_compare_mode_from_tag_api_error_exits(mocker, mock_repo): + """Non-404 GitHub error on from-tag validation → API failure logged and sys.exit(1).""" + api_error = GithubException(503, {"message": "Service Unavailable"}, {}) + mock_repo.get_git_ref.side_effect = api_error + error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") + + miner = _make_compare_miner(mocker, mock_repo) + + with pytest.raises(SystemExit) as exc_info: + miner.mine_data() + + assert exc_info.value.code == 1 + logged_messages = " ".join(str(call) for call in error_mock.call_args_list) + assert "503" in logged_messages + mock_repo.compare.assert_not_called() + + +def test_mine_data_compare_mode_both_tags_exist_calls_compare(mocker, mock_repo): + """Both tags exist as git refs → repo.compare() is called normally.""" + mock_repo.get_git_ref.return_value = None # no exception = tag exists + + commit_mock = mocker.Mock() + commit_mock.sha = "abc123" + commit_mock.commit.message = "Fix something (#1)" + + miner = _make_compare_miner(mocker, mock_repo, compare_commits=[commit_mock]) + miner.mine_data() + + mock_repo.compare.assert_called_once_with("v2.6.3", "v2.6.4") + + +def test_mine_data_compare_mode_network_error_on_tag_validation_exits(mocker, mock_repo): + """Network-layer exception (non-GithubException) on tag ref lookup → error logged and sys.exit(1).""" + mock_repo.get_git_ref.side_effect = OSError("Connection timed out") + error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") + + miner = _make_compare_miner(mocker, mock_repo) + + with pytest.raises(SystemExit) as exc_info: + miner.mine_data() + + assert exc_info.value.code == 1 + logged_messages = " ".join(str(call) for call in error_mock.call_args_list) + assert "Connection timed out" in logged_messages + mock_repo.compare.assert_not_called() diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 4e1d4d14..da6220b6 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -176,6 +176,33 @@ def test_get_from_tag_name_empty(mocker): assert ActionInputs.get_from_tag_name() == "" +# --- is_from_tag_name_defined --- + + +def test_is_from_tag_name_defined_raw_when_env_var_set(monkeypatch): + """Env var present with a valid tag → True.""" + monkeypatch.setenv("INPUT_FROM_TAG_NAME", "v1.0.0") + assert ActionInputs.is_from_tag_name_defined() is True + + +def test_is_from_tag_name_defined_raw_when_env_var_whitespace_only(monkeypatch): + """Env var present but whitespace-only → False (normalizes to empty string).""" + monkeypatch.setenv("INPUT_FROM_TAG_NAME", " ") + assert ActionInputs.is_from_tag_name_defined() is False + + +def test_is_from_tag_name_defined_raw_when_env_var_empty(monkeypatch): + """Env var empty string (action.yml default) → False (compare mode not requested).""" + monkeypatch.setenv("INPUT_FROM_TAG_NAME", "") + assert ActionInputs.is_from_tag_name_defined() is False + + +def test_is_from_tag_name_defined_raw_when_env_var_absent(monkeypatch): + """Env var absent → False.""" + monkeypatch.delenv("INPUT_FROM_TAG_NAME", raising=False) + assert ActionInputs.is_from_tag_name_defined() is False + + def test_get_from_tag_name_invalid_format(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="v1.2.beta") with pytest.raises(ValueError) as excinfo: @@ -185,6 +212,22 @@ def test_get_from_tag_name_invalid_format(mocker): ) +def test_validate_inputs_compare_mode_whitespace_from_tag_name_fails(mocker): + """from-tag-name provided as whitespace-only in compare mode → error in buffer and sys.exit(1).""" + case = success_case.copy() + case["get_from_tag_name"] = "" + patchers = apply_mocks(case, mocker) + try: + mocker.patch.dict("os.environ", {"INPUT_FROM_TAG_NAME": " "}) + mock_error = mocker.patch("release_notes_generator.action_inputs.logger.error") + mock_exit = mocker.patch("sys.exit") + ActionInputs.validate_inputs() + assert any("from-tag-name" in str(c) for c in mock_error.call_args_list) + mock_exit.assert_called_once_with(1) + finally: + stop_mocks(patchers) + + def test_get_chapters_success(mocker): mocker.patch( "release_notes_generator.action_inputs.get_action_input", return_value='[{"title": "Title", "label": "Label"}]'