From d5fa3d634ff5d2a153bd20a0b0280e1ee123b9b9 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 26 Jun 2026 15:00:56 +0200 Subject: [PATCH 01/18] Initial solution. --- CONTRIBUTING.md | 7 ++ README.md | 5 +- action.yml | 4 +- docs/features/compare_mode.md | 16 ++- docs/motivation.md | 2 +- release_notes_generator/action_inputs.py | 23 ++++ release_notes_generator/data/miner.py | 31 +++-- release_notes_generator/utils/constants.py | 6 + .../data/test_miner.py | 112 +++++++++++++++++- .../test_action_inputs.py | 40 +++++++ 10 files changed, 231 insertions(+), 15 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ab7c3a5e..5418d96d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,6 +25,13 @@ * Ensure the Pull Request description clearly outlines your solution. * Link your PR to the relevant _Issue_. +## Compare Mode Contract + +When contributing changes that touch compare mode (the `from-tag-name` path): +- Both `tag-name` and `from-tag-name` must exist as git tags before the compare API is called. +- The action validates this via `DataMiner._validate_compare_mode_tags()` and exits with code 1 if either tag is absent. +- Any change to this validation logic must update `docs/features/compare_mode.md` and the tests in `tests/unit/release_notes_generator/data/test_miner.py`. + ## Branch Naming (PID:H-1) Branches MUST start with one of the allowed prefixes: `feature/`, `fix/`, `docs/`, `chore/` Examples: diff --git a/README.md b/README.md index b54da0e9..25fbcf19 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 validates this +> before calling the compare API and exits with a clear error and a list of recent tags +> if either is absent. ## Example Workflow 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..b613a47a 100644 --- a/docs/features/compare_mode.md +++ b/docs/features/compare_mode.md @@ -43,6 +43,11 @@ 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 +> validates that **both `from-tag-name` and `tag-name` exist as git tags** in the repository. +> If either tag is absent the action exits immediately with a clear error message and a +> list of the most-recent tags to help diagnose the problem. + ### Step 1 — Graph-based commit selection Instead of asking "what happened after time T?", the action asks GitHub: *"what commits @@ -106,11 +111,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..2008a029 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 any API call — the action exits immediately with a clear error and a list of recent tags 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..c24dfda2 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -173,6 +173,24 @@ def is_from_tag_name_defined() -> bool: value = ActionInputs.get_from_tag_name() return value.strip() != "" + @staticmethod + def validate_compare_mode_tag_names() -> None: + """ + Validate that both tag-name and from-tag-name are non-empty strings. + + Logs an error and exits if either is empty. Called before GitHub tag-existence + checks so callers receive a clear input-level error before any API call. + Both 'tag-name' and 'from-tag-name' must be non-empty when running in compare mode. + """ + tag = ActionInputs.get_tag_name() + if not tag.strip(): + logger.error("'tag-name' must not be empty when running in compare mode. Ending!") + sys.exit(1) + from_tag = ActionInputs.get_from_tag_name() + if not from_tag.strip(): + logger.error("'from-tag-name' must not be empty when running in compare mode. Ending!") + sys.exit(1) + @staticmethod def get_chapters() -> list[dict[str, str]]: """ @@ -669,6 +687,11 @@ def validate_inputs() -> None: print_empty_chapters = ActionInputs.get_print_empty_chapters() ActionInputs.validate_input(print_empty_chapters, bool, "Print empty chapters must be a boolean.", errors) + # Compare mode: validate both tag names are non-empty strings. + # Only runs when all prior validations passed to avoid cascading errors. + if not errors and isinstance(from_tag_name, str) and from_tag_name.strip(): + ActionInputs.validate_compare_mode_tag_names() + # Log errors if any if errors: for error in errors: diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index dc32e271..6ba8fc46 100644 --- a/release_notes_generator/data/miner.py +++ b/release_notes_generator/data/miner.py @@ -35,6 +35,7 @@ from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.data.utils.bulk_sub_issue_collector import BulkSubIssueCollector +from release_notes_generator.utils.constants import COMPARE_MODE_TAG_MISSING_ERROR from release_notes_generator.model.record.issue_record import IssueRecord from release_notes_generator.model.mined_data import MinedData @@ -93,28 +94,44 @@ def mine_data(self) -> MinedData: return de_duplicated_data + def _validate_tag_exists(self, repo: Repository, tag_name: str) -> bool: + """ + Return True if *tag_name* exists as a git tag in *repo*, False otherwise. + + Uses the safe_call wrapper so any 404 / GithubException is caught and + converted to a None return value, which is treated as "not found". + """ + ref = self._safe_call(repo.get_git_ref)(f"tags/{tag_name}") + return ref is not None + + def _validate_compare_mode_tags(self, repo: Repository) -> None: + """ + Validate that both from-tag-name and tag-name exist as git tags. + + Exits with code 1 on the first missing tag, logging a clear error message. + """ + for tag_name in (ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name()): + if not self._validate_tag_exists(repo, tag_name): + logger.error(COMPARE_MODE_TAG_MISSING_ERROR, tag_name, repo.full_name) + sys.exit(1) + def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: """ Handle comparison mode: mine commits and PRs between two tags. Logic: + - Validate both from_tag and to_tag exist as git tags (fail fast). - Fetch commits between from_tag and to_tag using repo.compare(). - Extract PR numbers from commit messages and fetch those PRs. - Filter out commits that already have a PR reference to avoid duplication. """ + self._validate_compare_mode_tags(repo) 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()) - if comparison is None: - logger.error( - "Compare API returned no result for '%s'...'%s'. Ending!", - ActionInputs.get_from_tag_name(), - ActionInputs.get_tag_name(), - ) - sys.exit(1) compare_commits: list[GithubCommit] = list(comparison.commits) total_commits = getattr(comparison, "total_commits", None) if isinstance(total_commits, int) and total_commits > len(compare_commits): diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index b3efdfdd..2929348d 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -82,6 +82,12 @@ OTHERS_NO_TOPIC, ] +# Compare mode tag validation +COMPARE_MODE_TAG_MISSING_ERROR: str = ( + "Compare mode requires both tags to exist. Tag '%s' not found in repository '%s'." + " Both 'tag-name' and 'from-tag-name' must exist as git tags. Ending!" +) + LINKED_ISSUES_MAX = 10 ISSUES_FOR_PRS: str = """ query {{ diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index 2c6a3554..b7fd388f 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -574,7 +574,8 @@ def test_extract_pr_numbers_multiline_message(mocker): def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4", created_at=datetime(2026, 5, 7), published_at=None, prefer_published=False, - compare_commits=None, get_pull_side_effect=None, total_commits=None): + compare_commits=None, get_pull_side_effect=None, total_commits=None, + from_tag_ref_exists=True, to_tag_ref_exists=True): """Wire a DataMiner for compare-mode mine_data calls.""" mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True) mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value=from_tag) @@ -589,6 +590,13 @@ def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4" mock_repo.get_release.return_value = release_mock mock_repo.get_issues.return_value = [] + # Simulate git-ref existence for each tag in call order (from_tag first, to_tag second) + _ref_results = [ + mocker.Mock() if from_tag_ref_exists else None, + mocker.Mock() if to_tag_ref_exists else None, + ] + mock_repo.get_git_ref.side_effect = _ref_results + comparison_mock = mocker.Mock() comparison_mock.commits = compare_commits if compare_commits is not None else [] if total_commits is not None: @@ -786,3 +794,105 @@ def test_mine_data_timestamp_mode_compare_shas_empty(mocker, mock_repo): data = miner.mine_data() assert data.compare_commit_shas == set() + + +# --- tag existence validation --- + + +def test_validate_tag_exists_returns_true_when_ref_found(mocker, mock_repo): + """get_git_ref returns an object → tag exists.""" + gh = mocker.Mock(spec=Github) + miner = DataMiner(gh, mocker.Mock()) + miner._safe_call = decorator_mock + mock_repo.get_git_ref.return_value = mocker.Mock() + + assert miner._validate_tag_exists(mock_repo, "v1.0.0") is True + mock_repo.get_git_ref.assert_called_once_with("tags/v1.0.0") + + +def test_validate_tag_exists_returns_false_when_ref_not_found(mocker, mock_repo): + """get_git_ref returns None (404 swallowed by safe_call) → tag absent.""" + gh = mocker.Mock(spec=Github) + miner = DataMiner(gh, mocker.Mock()) + miner._safe_call = decorator_mock + mock_repo.get_git_ref.return_value = None + + assert miner._validate_tag_exists(mock_repo, "v9.9.9") is False + + +def test_validate_compare_mode_tags_exits_on_missing_from_tag(mocker, mock_repo): + """from-tag missing → sys.exit(1) with error.""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") + + gh = mocker.Mock(spec=Github) + miner = DataMiner(gh, mocker.Mock()) + miner._safe_call = decorator_mock + # from-tag (first call) → missing; to-tag (second call) → exists + mock_repo.get_git_ref.side_effect = [None, mocker.Mock()] + + mock_exit = mocker.patch("sys.exit") + error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") + + miner._validate_compare_mode_tags(mock_repo) + + mock_exit.assert_called_once_with(1) + assert any("v1.0.0" in str(call) for call in error_mock.call_args_list) + + +def test_validate_compare_mode_tags_exits_on_missing_to_tag(mocker, mock_repo): + """to-tag missing → sys.exit(1) with error.""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") + + gh = mocker.Mock(spec=Github) + miner = DataMiner(gh, mocker.Mock()) + miner._safe_call = decorator_mock + # from-tag → exists; to-tag (second call) → missing + mock_repo.get_git_ref.side_effect = [mocker.Mock(), None] + + mock_exit = mocker.patch("sys.exit") + error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") + + miner._validate_compare_mode_tags(mock_repo) + + mock_exit.assert_called_once_with(1) + assert any("v1.1.0" in str(call) for call in error_mock.call_args_list) + + +def test_validate_compare_mode_tags_passes_when_both_exist(mocker, mock_repo): + """Both tags exist → no exit, no error logged.""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") + + gh = mocker.Mock(spec=Github) + miner = DataMiner(gh, mocker.Mock()) + miner._safe_call = decorator_mock + mock_repo.get_git_ref.return_value = mocker.Mock() + + mock_exit = mocker.patch("sys.exit") + miner._validate_compare_mode_tags(mock_repo) + + mock_exit.assert_not_called() + + +def test_mine_data_compare_mode_exits_when_target_tag_missing(mocker, mock_repo): + """End-to-end: mine_data exits when tag-name does not exist as a git tag.""" + miner = _make_compare_miner(mocker, mock_repo, to_tag_ref_exists=False) + mocker.patch("sys.exit", side_effect=SystemExit(1)) + + with pytest.raises(SystemExit): + miner.mine_data() + + mock_repo.compare.assert_not_called() + + +def test_mine_data_compare_mode_exits_when_from_tag_missing(mocker, mock_repo): + """End-to-end: mine_data exits when from-tag-name does not exist as a git tag.""" + miner = _make_compare_miner(mocker, mock_repo, from_tag_ref_exists=False) + mocker.patch("sys.exit", side_effect=SystemExit(1)) + + with pytest.raises(SystemExit): + miner.mine_data() + + 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..26a716f2 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -185,6 +185,46 @@ def test_get_from_tag_name_invalid_format(mocker): ) +# --- validate_compare_mode_tag_names --- + + +def test_validate_compare_mode_tag_names_both_set(mocker): + """Both tag-name and from-tag-name non-empty → no exit, no error.""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") + mock_exit = mocker.patch("sys.exit") + + ActionInputs.validate_compare_mode_tag_names() + + mock_exit.assert_not_called() + + +def test_validate_compare_mode_tag_names_empty_tag_name_exits(mocker): + """Empty tag-name → sys.exit(1).""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") + mock_exit = mocker.patch("sys.exit") + error_mock = mocker.patch("release_notes_generator.action_inputs.logger.error") + + ActionInputs.validate_compare_mode_tag_names() + + mock_exit.assert_called_once_with(1) + assert any("tag-name" in str(c) for c in error_mock.call_args_list) + + +def test_validate_compare_mode_tag_names_empty_from_tag_name_exits(mocker): + """Empty from-tag-name → sys.exit(1).""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="") + mock_exit = mocker.patch("sys.exit") + error_mock = mocker.patch("release_notes_generator.action_inputs.logger.error") + + ActionInputs.validate_compare_mode_tag_names() + + mock_exit.assert_called_once_with(1) + assert any("from-tag-name" in str(c) for c in error_mock.call_args_list) + + def test_get_chapters_success(mocker): mocker.patch( "release_notes_generator.action_inputs.get_action_input", return_value='[{"title": "Title", "label": "Label"}]' From 5afa957d122c8af009202cb8b15523f29c9e3215 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 26 Jun 2026 15:11:25 +0200 Subject: [PATCH 02/18] fix: improve error handling for missing tags in compare mode --- README.md | 3 +- docs/features/compare_mode.md | 3 +- docs/motivation.md | 2 +- release_notes_generator/data/miner.py | 7 ++ .../data/test_miner.py | 79 +------------------ 5 files changed, 11 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index 25fbcf19..e1c46f49 100644 --- a/README.md +++ b/README.md @@ -124,8 +124,7 @@ For how label → chapter mapping and aggregation works, see [Custom Chapters Be > **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 validates this -> before calling the compare API and exits with a clear error and a list of recent tags -> if either is absent. +> before calling the compare API and exits with a clear error if either is absent. ## Example Workflow diff --git a/docs/features/compare_mode.md b/docs/features/compare_mode.md index b613a47a..7be53178 100644 --- a/docs/features/compare_mode.md +++ b/docs/features/compare_mode.md @@ -45,8 +45,7 @@ the existing timestamp path runs unchanged. > **Prerequisite — both tags must exist:** Before the compare API is called, the action > validates that **both `from-tag-name` and `tag-name` exist as git tags** in the repository. -> If either tag is absent the action exits immediately with a clear error message and a -> list of the most-recent tags to help diagnose the problem. +> If either tag is absent the action exits immediately with a clear error message. ### Step 1 — Graph-based commit selection diff --git a/docs/motivation.md b/docs/motivation.md index 2008a029..9192ea27 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. In compare mode, both tags must exist before any API call — the action exits immediately with a clear error and a list of recent tags rather than confusing 404 responses. | +| Fail Safe | If explicit notes missing, optionally fall back to CodeRabbit; never silently fabricate content. In compare mode, both tags must exist before any API call — 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/data/miner.py b/release_notes_generator/data/miner.py index 6ba8fc46..b1e61538 100644 --- a/release_notes_generator/data/miner.py +++ b/release_notes_generator/data/miner.py @@ -132,6 +132,13 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: ActionInputs.get_tag_name(), ) comparison = self._safe_call(repo.compare)(ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name()) + if comparison is None: + logger.error( + "Compare API returned no result for '%s'...'%s'. Ending!", + ActionInputs.get_from_tag_name(), + ActionInputs.get_tag_name(), + ) + sys.exit(1) compare_commits: list[GithubCommit] = list(comparison.commits) total_commits = getattr(comparison, "total_commits", None) if isinstance(total_commits, int) and total_commits > len(compare_commits): diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index b7fd388f..d636a718 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -796,84 +796,7 @@ def test_mine_data_timestamp_mode_compare_shas_empty(mocker, mock_repo): assert data.compare_commit_shas == set() -# --- tag existence validation --- - - -def test_validate_tag_exists_returns_true_when_ref_found(mocker, mock_repo): - """get_git_ref returns an object → tag exists.""" - gh = mocker.Mock(spec=Github) - miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = decorator_mock - mock_repo.get_git_ref.return_value = mocker.Mock() - - assert miner._validate_tag_exists(mock_repo, "v1.0.0") is True - mock_repo.get_git_ref.assert_called_once_with("tags/v1.0.0") - - -def test_validate_tag_exists_returns_false_when_ref_not_found(mocker, mock_repo): - """get_git_ref returns None (404 swallowed by safe_call) → tag absent.""" - gh = mocker.Mock(spec=Github) - miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = decorator_mock - mock_repo.get_git_ref.return_value = None - - assert miner._validate_tag_exists(mock_repo, "v9.9.9") is False - - -def test_validate_compare_mode_tags_exits_on_missing_from_tag(mocker, mock_repo): - """from-tag missing → sys.exit(1) with error.""" - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") - - gh = mocker.Mock(spec=Github) - miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = decorator_mock - # from-tag (first call) → missing; to-tag (second call) → exists - mock_repo.get_git_ref.side_effect = [None, mocker.Mock()] - - mock_exit = mocker.patch("sys.exit") - error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") - - miner._validate_compare_mode_tags(mock_repo) - - mock_exit.assert_called_once_with(1) - assert any("v1.0.0" in str(call) for call in error_mock.call_args_list) - - -def test_validate_compare_mode_tags_exits_on_missing_to_tag(mocker, mock_repo): - """to-tag missing → sys.exit(1) with error.""" - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") - - gh = mocker.Mock(spec=Github) - miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = decorator_mock - # from-tag → exists; to-tag (second call) → missing - mock_repo.get_git_ref.side_effect = [mocker.Mock(), None] - - mock_exit = mocker.patch("sys.exit") - error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") - - miner._validate_compare_mode_tags(mock_repo) - - mock_exit.assert_called_once_with(1) - assert any("v1.1.0" in str(call) for call in error_mock.call_args_list) - - -def test_validate_compare_mode_tags_passes_when_both_exist(mocker, mock_repo): - """Both tags exist → no exit, no error logged.""" - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") - - gh = mocker.Mock(spec=Github) - miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = decorator_mock - mock_repo.get_git_ref.return_value = mocker.Mock() - - mock_exit = mocker.patch("sys.exit") - miner._validate_compare_mode_tags(mock_repo) - - mock_exit.assert_not_called() +# --- compare mode tag validation (end-to-end via mine_data) --- def test_mine_data_compare_mode_exits_when_target_tag_missing(mocker, mock_repo): From df399f724d911d8d1e500e43a6dab26739146587 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 26 Jun 2026 15:27:01 +0200 Subject: [PATCH 03/18] Fixed review comment. --- release_notes_generator/data/miner.py | 31 +++++++++++++------ release_notes_generator/utils/constants.py | 3 ++ .../data/test_miner.py | 6 ++-- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index b1e61538..58cfcde0 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 @@ -35,7 +35,9 @@ from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.data.utils.bulk_sub_issue_collector import BulkSubIssueCollector -from release_notes_generator.utils.constants import COMPARE_MODE_TAG_MISSING_ERROR +from requests.exceptions import ConnectionError as RequestsConnectionError, RequestException, Timeout + +from release_notes_generator.utils.constants import COMPARE_MODE_TAG_API_ERROR, COMPARE_MODE_TAG_MISSING_ERROR from release_notes_generator.model.record.issue_record import IssueRecord from release_notes_generator.model.mined_data import MinedData @@ -98,21 +100,32 @@ def _validate_tag_exists(self, repo: Repository, tag_name: str) -> bool: """ Return True if *tag_name* exists as a git tag in *repo*, False otherwise. - Uses the safe_call wrapper so any 404 / GithubException is caught and - converted to a None return value, which is treated as "not found". + Calls repo.get_git_ref directly (not via _safe_call) so that a 404 can be + distinguished from other API or network failures. Raises any non-404 + GithubException or network error for the caller to handle. """ - ref = self._safe_call(repo.get_git_ref)(f"tags/{tag_name}") - return ref is not None + try: + repo.get_git_ref(f"tags/{tag_name}") + return True + except GithubException as exc: + if exc.status == 404: + return False + raise def _validate_compare_mode_tags(self, repo: Repository) -> None: """ Validate that both from-tag-name and tag-name exist as git tags. - Exits with code 1 on the first missing tag, logging a clear error message. + Exits with code 1 on the first missing tag (404) or on any API/network + error, logging a distinct message for each failure mode. """ for tag_name in (ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name()): - if not self._validate_tag_exists(repo, tag_name): - logger.error(COMPARE_MODE_TAG_MISSING_ERROR, tag_name, repo.full_name) + try: + if not self._validate_tag_exists(repo, tag_name): + logger.error(COMPARE_MODE_TAG_MISSING_ERROR, tag_name, repo.full_name) + sys.exit(1) + except (GithubException, RequestsConnectionError, Timeout, RequestException) as exc: + logger.error(COMPARE_MODE_TAG_API_ERROR, tag_name, repo.full_name, exc) sys.exit(1) def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index 2929348d..a176c6c8 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -87,6 +87,9 @@ "Compare mode requires both tags to exist. Tag '%s' not found in repository '%s'." " Both 'tag-name' and 'from-tag-name' must exist as git tags. Ending!" ) +COMPARE_MODE_TAG_API_ERROR: str = ( + "GitHub API error while checking tag '%s' in repository '%s': %s. Ending!" +) LINKED_ISSUES_MAX = 10 ISSUES_FOR_PRS: str = """ diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index d636a718..b9ae4567 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 @@ -592,8 +592,8 @@ def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4" # Simulate git-ref existence for each tag in call order (from_tag first, to_tag second) _ref_results = [ - mocker.Mock() if from_tag_ref_exists else None, - mocker.Mock() if to_tag_ref_exists else None, + mocker.Mock() if from_tag_ref_exists else GithubException(404, "Not Found"), + mocker.Mock() if to_tag_ref_exists else GithubException(404, "Not Found"), ] mock_repo.get_git_ref.side_effect = _ref_results From f4e2d167fadff9bbcc871deb1a10c0c537a33047 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 26 Jun 2026 15:37:45 +0200 Subject: [PATCH 04/18] Fixed review comments. --- release_notes_generator/action_inputs.py | 13 ++++++++- .../data/test_miner.py | 1 - .../test_action_inputs.py | 29 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index c24dfda2..c533aa0a 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -173,6 +173,17 @@ def is_from_tag_name_defined() -> bool: value = ActionInputs.get_from_tag_name() return value.strip() != "" + @staticmethod + def is_from_tag_name_provided() -> bool: + """ + Check whether the from-tag-name input was explicitly provided with a non-blank value. + + Reads the raw env var (before normalization) so that whitespace-only values are + still treated as provided, routing them to the fail-fast compare-mode validation + in validate_inputs() rather than silently skipping it. + """ + return os.getenv(f'INPUT_{FROM_TAG_NAME.replace("-", "_").upper()}', "") != "" + @staticmethod def validate_compare_mode_tag_names() -> None: """ @@ -689,7 +700,7 @@ def validate_inputs() -> None: # Compare mode: validate both tag names are non-empty strings. # Only runs when all prior validations passed to avoid cascading errors. - if not errors and isinstance(from_tag_name, str) and from_tag_name.strip(): + if not errors and ActionInputs.is_from_tag_name_provided(): ActionInputs.validate_compare_mode_tag_names() # Log errors if any diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index b9ae4567..11529b5a 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -590,7 +590,6 @@ def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4" mock_repo.get_release.return_value = release_mock mock_repo.get_issues.return_value = [] - # Simulate git-ref existence for each tag in call order (from_tag first, to_tag second) _ref_results = [ mocker.Mock() if from_tag_ref_exists else GithubException(404, "Not Found"), mocker.Mock() if to_tag_ref_exists else GithubException(404, "Not Found"), diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 26a716f2..25292fc3 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_provided --- + + +def test_is_from_tag_name_provided_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_provided() is True + + +def test_is_from_tag_name_provided_when_env_var_whitespace_only(monkeypatch): + """Env var present but whitespace-only → True (user provided a value, even if invalid).""" + monkeypatch.setenv("INPUT_FROM_TAG_NAME", " ") + assert ActionInputs.is_from_tag_name_provided() is True + + +def test_is_from_tag_name_provided_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_provided() is False + + +def test_is_from_tag_name_provided_when_env_var_absent(monkeypatch): + """Env var absent → False.""" + monkeypatch.delenv("INPUT_FROM_TAG_NAME", raising=False) + assert ActionInputs.is_from_tag_name_provided() 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: @@ -193,10 +220,12 @@ def test_validate_compare_mode_tag_names_both_set(mocker): mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") mock_exit = mocker.patch("sys.exit") + mock_error = mocker.patch("release_notes_generator.action_inputs.logger.error") ActionInputs.validate_compare_mode_tag_names() mock_exit.assert_not_called() + mock_error.assert_not_called() def test_validate_compare_mode_tag_names_empty_tag_name_exits(mocker): From 94c0504d16c8065d953e29439c546c79774deb29 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 26 Jun 2026 15:40:02 +0200 Subject: [PATCH 05/18] fix: simplify error message formatting for GitHub API tag checks --- release_notes_generator/utils/constants.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index a176c6c8..af38a17d 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -87,9 +87,7 @@ "Compare mode requires both tags to exist. Tag '%s' not found in repository '%s'." " Both 'tag-name' and 'from-tag-name' must exist as git tags. Ending!" ) -COMPARE_MODE_TAG_API_ERROR: str = ( - "GitHub API error while checking tag '%s' in repository '%s': %s. Ending!" -) +COMPARE_MODE_TAG_API_ERROR: str = "GitHub API error while checking tag '%s' in repository '%s': %s. Ending!" LINKED_ISSUES_MAX = 10 ISSUES_FOR_PRS: str = """ From da97ff2f9e57f20eede14b2906446a6c2679dc2b Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 10:55:51 +0200 Subject: [PATCH 06/18] fix: remove tag validation logic from DataMiner and related tests --- release_notes_generator/data/miner.py | 39 +------------------ .../data/test_miner.py | 33 +--------------- 2 files changed, 3 insertions(+), 69 deletions(-) diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index 58cfcde0..dc32e271 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, GithubException +from github import Github from github.GitRelease import GitRelease from github.Issue import Issue from github.PullRequest import PullRequest @@ -35,9 +35,6 @@ from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.data.utils.bulk_sub_issue_collector import BulkSubIssueCollector -from requests.exceptions import ConnectionError as RequestsConnectionError, RequestException, Timeout - -from release_notes_generator.utils.constants import COMPARE_MODE_TAG_API_ERROR, COMPARE_MODE_TAG_MISSING_ERROR from release_notes_generator.model.record.issue_record import IssueRecord from release_notes_generator.model.mined_data import MinedData @@ -96,49 +93,15 @@ def mine_data(self) -> MinedData: return de_duplicated_data - def _validate_tag_exists(self, repo: Repository, tag_name: str) -> bool: - """ - Return True if *tag_name* exists as a git tag in *repo*, False otherwise. - - Calls repo.get_git_ref directly (not via _safe_call) so that a 404 can be - distinguished from other API or network failures. Raises any non-404 - GithubException or network error for the caller to handle. - """ - try: - repo.get_git_ref(f"tags/{tag_name}") - return True - except GithubException as exc: - if exc.status == 404: - return False - raise - - def _validate_compare_mode_tags(self, repo: Repository) -> None: - """ - Validate that both from-tag-name and tag-name exist as git tags. - - Exits with code 1 on the first missing tag (404) or on any API/network - error, logging a distinct message for each failure mode. - """ - for tag_name in (ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name()): - try: - if not self._validate_tag_exists(repo, tag_name): - logger.error(COMPARE_MODE_TAG_MISSING_ERROR, tag_name, repo.full_name) - sys.exit(1) - except (GithubException, RequestsConnectionError, Timeout, RequestException) as exc: - logger.error(COMPARE_MODE_TAG_API_ERROR, tag_name, repo.full_name, exc) - sys.exit(1) - def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: """ Handle comparison mode: mine commits and PRs between two tags. Logic: - - Validate both from_tag and to_tag exist as git tags (fail fast). - Fetch commits between from_tag and to_tag using repo.compare(). - Extract PR numbers from commit messages and fetch those PRs. - Filter out commits that already have a PR reference to avoid duplication. """ - self._validate_compare_mode_tags(repo) logger.info( "Compare mode: using repo.compare('%s', '%s').", ActionInputs.get_from_tag_name(), diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index 11529b5a..c6831054 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, GithubException +from github import Github from github.Commit import Commit from github.GitRelease import GitRelease from github.Issue import Issue @@ -574,8 +574,7 @@ def test_extract_pr_numbers_multiline_message(mocker): def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4", created_at=datetime(2026, 5, 7), published_at=None, prefer_published=False, - compare_commits=None, get_pull_side_effect=None, total_commits=None, - from_tag_ref_exists=True, to_tag_ref_exists=True): + compare_commits=None, get_pull_side_effect=None, total_commits=None): """Wire a DataMiner for compare-mode mine_data calls.""" mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True) mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value=from_tag) @@ -590,12 +589,6 @@ def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4" mock_repo.get_release.return_value = release_mock mock_repo.get_issues.return_value = [] - _ref_results = [ - mocker.Mock() if from_tag_ref_exists else GithubException(404, "Not Found"), - mocker.Mock() if to_tag_ref_exists else GithubException(404, "Not Found"), - ] - mock_repo.get_git_ref.side_effect = _ref_results - comparison_mock = mocker.Mock() comparison_mock.commits = compare_commits if compare_commits is not None else [] if total_commits is not None: @@ -795,26 +788,4 @@ def test_mine_data_timestamp_mode_compare_shas_empty(mocker, mock_repo): assert data.compare_commit_shas == set() -# --- compare mode tag validation (end-to-end via mine_data) --- - - -def test_mine_data_compare_mode_exits_when_target_tag_missing(mocker, mock_repo): - """End-to-end: mine_data exits when tag-name does not exist as a git tag.""" - miner = _make_compare_miner(mocker, mock_repo, to_tag_ref_exists=False) - mocker.patch("sys.exit", side_effect=SystemExit(1)) - with pytest.raises(SystemExit): - miner.mine_data() - - mock_repo.compare.assert_not_called() - - -def test_mine_data_compare_mode_exits_when_from_tag_missing(mocker, mock_repo): - """End-to-end: mine_data exits when from-tag-name does not exist as a git tag.""" - miner = _make_compare_miner(mocker, mock_repo, from_tag_ref_exists=False) - mocker.patch("sys.exit", side_effect=SystemExit(1)) - - with pytest.raises(SystemExit): - miner.mine_data() - - mock_repo.compare.assert_not_called() From 26b316aface762bc88253513bbab8a2b349beda3 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 11:09:09 +0200 Subject: [PATCH 07/18] fix: update from-tag-name validation to include whitespace-only values --- release_notes_generator/action_inputs.py | 8 ++++---- release_notes_generator/utils/constants.py | 7 ------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index c533aa0a..2f895acf 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -176,11 +176,11 @@ def is_from_tag_name_defined() -> bool: @staticmethod def is_from_tag_name_provided() -> bool: """ - Check whether the from-tag-name input was explicitly provided with a non-blank value. + Check whether the from-tag-name input was explicitly provided (even if whitespace-only). - Reads the raw env var (before normalization) so that whitespace-only values are - still treated as provided, routing them to the fail-fast compare-mode validation - in validate_inputs() rather than silently skipping it. + Reads the raw env var before normalization, so a whitespace-only value is still + treated as provided and routed to the fail-fast compare-mode validation in + validate_inputs() rather than silently skipping it. """ return os.getenv(f'INPUT_{FROM_TAG_NAME.replace("-", "_").upper()}', "") != "" diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index af38a17d..b3efdfdd 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -82,13 +82,6 @@ OTHERS_NO_TOPIC, ] -# Compare mode tag validation -COMPARE_MODE_TAG_MISSING_ERROR: str = ( - "Compare mode requires both tags to exist. Tag '%s' not found in repository '%s'." - " Both 'tag-name' and 'from-tag-name' must exist as git tags. Ending!" -) -COMPARE_MODE_TAG_API_ERROR: str = "GitHub API error while checking tag '%s' in repository '%s': %s. Ending!" - LINKED_ISSUES_MAX = 10 ISSUES_FOR_PRS: str = """ query {{ From c835e38c91044eb6b00f1f046231cd4577d5dd3d Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 11:22:58 +0200 Subject: [PATCH 08/18] fix: enhance compare mode tag validation and error handling --- README.md | 5 +- docs/features/compare_mode.md | 4 +- release_notes_generator/data/SPEC.md | 0 release_notes_generator/data/miner.py | 21 +++++++- .../data/test_miner.py | 49 +++++++++++++++++++ 5 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 release_notes_generator/data/SPEC.md diff --git a/README.md b/README.md index e1c46f49..838d4233 100644 --- a/README.md +++ b/README.md @@ -123,8 +123,9 @@ For the full input and output reference, see [Configuration reference](docs/conf For how label → chapter mapping and aggregation works, see [Custom Chapters Behavior](docs/configuration_reference.md#custom-chapters-behavior). > **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 validates this -> before calling the compare API and exits with a clear error if either is absent. +> `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 diff --git a/docs/features/compare_mode.md b/docs/features/compare_mode.md index 7be53178..2c5fc10d 100644 --- a/docs/features/compare_mode.md +++ b/docs/features/compare_mode.md @@ -44,8 +44,8 @@ Compare mode is active **when `from-tag-name` is explicitly provided**. When it the existing timestamp path runs unchanged. > **Prerequisite — both tags must exist:** Before the compare API is called, the action -> validates that **both `from-tag-name` and `tag-name` exist as git tags** in the repository. -> If either tag is absent the action exits immediately with a clear error message. +> 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 diff --git a/release_notes_generator/data/SPEC.md b/release_notes_generator/data/SPEC.md new file mode 100644 index 00000000..e69de29b diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index dc32e271..028bd617 100644 --- a/release_notes_generator/data/miner.py +++ b/release_notes_generator/data/miner.py @@ -107,7 +107,26 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: 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() + + ref = self._safe_call(repo.get_git_ref)(f"tags/{from_tag}") + if ref is None: + logger.error( + "Tag '%s' does not exist in the repository. Ending!", + from_tag, + ) + sys.exit(1) + + ref = self._safe_call(repo.get_git_ref)(f"tags/{to_tag}") + if ref is None: + logger.error( + "Tag '%s' does not exist in the repository. Ending!", + to_tag, + ) + sys.exit(1) + + 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!", diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index c6831054..2ab112f6 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -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() @@ -788,4 +789,52 @@ def test_mine_data_timestamp_mode_compare_shas_empty(mocker, mock_repo): 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).""" + mock_repo.get_git_ref.side_effect = lambda ref: None if ref == "tags/v2.6.3" else mocker.Mock() + 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).""" + mock_repo.get_git_ref.side_effect = lambda ref: None if ref == "tags/v2.6.4" else mocker.Mock() + 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_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 = mocker.Mock() + + 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") + From 611685920e6dee418ca9530c14ca1bb178e6e48b Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 11:27:09 +0200 Subject: [PATCH 09/18] fix: update compare mode validation logic and improve documentation --- CONTRIBUTING.md | 3 ++- tests/unit/release_notes_generator/data/test_miner.py | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5418d96d..1a1ee143 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,7 +29,8 @@ When contributing changes that touch compare mode (the `from-tag-name` path): - Both `tag-name` and `from-tag-name` must exist as git tags before the compare API is called. -- The action validates this via `DataMiner._validate_compare_mode_tags()` and exits with code 1 if either tag is absent. +- Input-level check: `ActionInputs.validate_compare_mode_tag_names()` (called from `validate_inputs()`) exits with code 1 if either input string is empty. +- Tag-existence check: `DataMiner._handle_compare_mode()` calls `repo.get_git_ref("tags/")` for both tags and exits with code 1 naming the absent tag if either lookup fails. - Any change to this validation logic must update `docs/features/compare_mode.md` and the tests in `tests/unit/release_notes_generator/data/test_miner.py`. ## Branch Naming (PID:H-1) diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index 2ab112f6..bd70148a 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -836,5 +836,3 @@ def test_mine_data_compare_mode_both_tags_exist_calls_compare(mocker, mock_repo) miner.mine_data() mock_repo.compare.assert_called_once_with("v2.6.3", "v2.6.4") - - From 10cb26073cfa789e8b0b246dd444f50ab51fbfe5 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 11:37:31 +0200 Subject: [PATCH 10/18] fix: improve tag validation in compare mode and enhance error handling --- release_notes_generator/data/miner.py | 39 +++++++++++-------- .../data/test_miner.py | 27 +++++++++++-- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index 028bd617..315966bb 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 @@ -110,21 +110,8 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: from_tag = ActionInputs.get_from_tag_name() to_tag = ActionInputs.get_tag_name() - ref = self._safe_call(repo.get_git_ref)(f"tags/{from_tag}") - if ref is None: - logger.error( - "Tag '%s' does not exist in the repository. Ending!", - from_tag, - ) - sys.exit(1) - - ref = self._safe_call(repo.get_git_ref)(f"tags/{to_tag}") - if ref is None: - logger.error( - "Tag '%s' does not exist in the repository. Ending!", - to_tag, - ) - sys.exit(1) + 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: @@ -176,6 +163,26 @@ 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'. 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) + 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 bd70148a..8c66d6b1 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 @@ -794,7 +794,8 @@ def test_mine_data_timestamp_mode_compare_shas_empty(mocker, mock_repo): 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).""" - mock_repo.get_git_ref.side_effect = lambda ref: None if ref == "tags/v2.6.3" else mocker.Mock() + 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) @@ -810,7 +811,8 @@ def test_mine_data_compare_mode_from_tag_not_found_exits(mocker, mock_repo): 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).""" - mock_repo.get_git_ref.side_effect = lambda ref: None if ref == "tags/v2.6.4" else mocker.Mock() + 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) @@ -824,9 +826,26 @@ def test_mine_data_compare_mode_to_tag_not_found_exits(mocker, mock_repo): 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 = mocker.Mock() + mock_repo.get_git_ref.return_value = None # no exception = tag exists commit_mock = mocker.Mock() commit_mock.sha = "abc123" From 2f47802ab2cfbc5e3d0f2cb9fb5e3e76a573c4c4 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 11:38:23 +0200 Subject: [PATCH 11/18] fix: clarify fail-safe behavior in compare mode documentation --- docs/motivation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/motivation.md b/docs/motivation.md index 9192ea27..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. In compare mode, both tags must exist before any API call — the action exits immediately with a clear error rather than confusing 404 responses. | +| 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. | From f5e683014d74bca01fc724f0b1611788744cb6ff Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 12:04:16 +0200 Subject: [PATCH 12/18] fix: enhance error handling in compare mode for network exceptions and improve tag validation logging --- release_notes_generator/action_inputs.py | 2 +- release_notes_generator/data/miner.py | 8 ++++++++ .../release_notes_generator/data/test_miner.py | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 2f895acf..74edd80a 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -182,7 +182,7 @@ def is_from_tag_name_provided() -> bool: treated as provided and routed to the fail-fast compare-mode validation in validate_inputs() rather than silently skipping it. """ - return os.getenv(f'INPUT_{FROM_TAG_NAME.replace("-", "_").upper()}', "") != "" + return get_action_input(FROM_TAG_NAME, default="") != "" @staticmethod def validate_compare_mode_tag_names() -> None: diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index 315966bb..18059e03 100644 --- a/release_notes_generator/data/miner.py +++ b/release_notes_generator/data/miner.py @@ -182,6 +182,14 @@ def _validate_tag_exists(self, repo: Repository, tag: str) -> None: 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: """ diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index 8c66d6b1..a7007689 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -855,3 +855,19 @@ def test_mine_data_compare_mode_both_tags_exist_calls_compare(mocker, mock_repo) 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() From 3b311cfb43dcf6189b4d9259db51b187ef52025a Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 12:52:59 +0200 Subject: [PATCH 13/18] fix: remove obsolete SPEC.md file from release notes generator --- release_notes_generator/data/SPEC.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 release_notes_generator/data/SPEC.md diff --git a/release_notes_generator/data/SPEC.md b/release_notes_generator/data/SPEC.md deleted file mode 100644 index e69de29b..00000000 From 982128f67229cfb8e2c8e9069c506034de9b4e39 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Jun 2026 14:52:24 +0200 Subject: [PATCH 14/18] fix: enhance compare mode functionality by adding from-tag-name input and improving validation logic --- README.md | 15 +++- release_notes_generator/action_inputs.py | 50 ++++--------- release_notes_generator/data/miner.py | 14 ++-- .../test_action_inputs.py | 75 +++++++------------ 4 files changed, 60 insertions(+), 94 deletions(-) diff --git a/README.md b/README.md index 838d4233..89e8a444 100644 --- a/README.md +++ b/README.md @@ -139,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 @@ -155,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/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 74edd80a..ca176a90 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -166,41 +166,19 @@ def get_from_tag_name() -> str: return normalize_version_tag(raw) @staticmethod - def is_from_tag_name_defined() -> bool: + def is_from_tag_name_defined(*, use_raw: bool = False) -> bool: """ Check if the from-tag name is defined in the action inputs. - """ - value = ActionInputs.get_from_tag_name() - return value.strip() != "" - - @staticmethod - def is_from_tag_name_provided() -> bool: - """ - Check whether the from-tag-name input was explicitly provided (even if whitespace-only). - - Reads the raw env var before normalization, so a whitespace-only value is still - treated as provided and routed to the fail-fast compare-mode validation in - validate_inputs() rather than silently skipping it. - """ - return get_action_input(FROM_TAG_NAME, default="") != "" - - @staticmethod - def validate_compare_mode_tag_names() -> None: - """ - Validate that both tag-name and from-tag-name are non-empty strings. - Logs an error and exits if either is empty. Called before GitHub tag-existence - checks so callers receive a clear input-level error before any API call. - Both 'tag-name' and 'from-tag-name' must be non-empty when running in compare mode. + Parameters: + use_raw: When True, reads the raw env var before normalization so a + whitespace-only value is still treated as defined (used during + validation to route it to a fail-fast error). When False + (default), checks the normalized, stripped value. """ - tag = ActionInputs.get_tag_name() - if not tag.strip(): - logger.error("'tag-name' must not be empty when running in compare mode. Ending!") - sys.exit(1) - from_tag = ActionInputs.get_from_tag_name() - if not from_tag.strip(): - logger.error("'from-tag-name' must not be empty when running in compare mode. Ending!") - sys.exit(1) + if use_raw: + return get_action_input(FROM_TAG_NAME, default="") != "" + return ActionInputs.get_from_tag_name().strip() != "" @staticmethod def get_chapters() -> list[dict[str, str]]: @@ -633,6 +611,11 @@ def validate_inputs() -> None: from_tag_name = ActionInputs.get_from_tag_name() if not isinstance(from_tag_name, str): errors.append("From tag name must be a string.") + elif ActionInputs.is_from_tag_name_defined(use_raw=True) and not from_tag_name.strip(): + 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: @@ -698,11 +681,6 @@ def validate_inputs() -> None: print_empty_chapters = ActionInputs.get_print_empty_chapters() ActionInputs.validate_input(print_empty_chapters, bool, "Print empty chapters must be a boolean.", errors) - # Compare mode: validate both tag names are non-empty strings. - # Only runs when all prior validations passed to avoid cascading errors. - if not errors and ActionInputs.is_from_tag_name_provided(): - ActionInputs.validate_compare_mode_tag_names() - # Log errors if any if errors: for error in errors: diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index 18059e03..f3f236b1 100644 --- a/release_notes_generator/data/miner.py +++ b/release_notes_generator/data/miner.py @@ -102,14 +102,11 @@ 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(), - ) 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) @@ -117,8 +114,8 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: 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) @@ -169,7 +166,8 @@ def _validate_tag_exists(self, repo: Repository, tag: str) -> None: except GithubException as e: if e.status == 404: logger.error( - "Tag '%s' does not exist in repository '%s'. Ending!", + "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, ) diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 25292fc3..3e4d64a8 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -176,31 +176,31 @@ def test_get_from_tag_name_empty(mocker): assert ActionInputs.get_from_tag_name() == "" -# --- is_from_tag_name_provided --- +# --- is_from_tag_name_defined (use_raw=True) --- -def test_is_from_tag_name_provided_when_env_var_set(monkeypatch): +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_provided() is True + assert ActionInputs.is_from_tag_name_defined(use_raw=True) is True -def test_is_from_tag_name_provided_when_env_var_whitespace_only(monkeypatch): +def test_is_from_tag_name_defined_raw_when_env_var_whitespace_only(monkeypatch): """Env var present but whitespace-only → True (user provided a value, even if invalid).""" monkeypatch.setenv("INPUT_FROM_TAG_NAME", " ") - assert ActionInputs.is_from_tag_name_provided() is True + assert ActionInputs.is_from_tag_name_defined(use_raw=True) is True -def test_is_from_tag_name_provided_when_env_var_empty(monkeypatch): +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_provided() is False + assert ActionInputs.is_from_tag_name_defined(use_raw=True) is False -def test_is_from_tag_name_provided_when_env_var_absent(monkeypatch): +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_provided() is False + assert ActionInputs.is_from_tag_name_defined(use_raw=True) is False def test_get_from_tag_name_invalid_format(mocker): @@ -212,46 +212,23 @@ def test_get_from_tag_name_invalid_format(mocker): ) -# --- validate_compare_mode_tag_names --- - - -def test_validate_compare_mode_tag_names_both_set(mocker): - """Both tag-name and from-tag-name non-empty → no exit, no error.""" - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") - mock_exit = mocker.patch("sys.exit") - mock_error = mocker.patch("release_notes_generator.action_inputs.logger.error") - - ActionInputs.validate_compare_mode_tag_names() - - mock_exit.assert_not_called() - mock_error.assert_not_called() - - -def test_validate_compare_mode_tag_names_empty_tag_name_exits(mocker): - """Empty tag-name → sys.exit(1).""" - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="") - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v1.0.0") - mock_exit = mocker.patch("sys.exit") - error_mock = mocker.patch("release_notes_generator.action_inputs.logger.error") - - ActionInputs.validate_compare_mode_tag_names() - - mock_exit.assert_called_once_with(1) - assert any("tag-name" in str(c) for c in error_mock.call_args_list) - - -def test_validate_compare_mode_tag_names_empty_from_tag_name_exits(mocker): - """Empty from-tag-name → sys.exit(1).""" - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v1.1.0") - mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="") - mock_exit = mocker.patch("sys.exit") - error_mock = mocker.patch("release_notes_generator.action_inputs.logger.error") - - ActionInputs.validate_compare_mode_tag_names() - - mock_exit.assert_called_once_with(1) - assert any("from-tag-name" in str(c) for c in error_mock.call_args_list) +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) + mocker.patch( + "release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", + return_value=True, + ) + try: + 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): From cd2cfbc2dcc9827c020094c3fb0c7e153bed6d7d Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Tue, 30 Jun 2026 08:20:07 +0200 Subject: [PATCH 15/18] Fix review comments. --- CONTRIBUTING.md | 8 -------- tests/unit/release_notes_generator/test_action_inputs.py | 3 ++- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1a1ee143..ab7c3a5e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,14 +25,6 @@ * Ensure the Pull Request description clearly outlines your solution. * Link your PR to the relevant _Issue_. -## Compare Mode Contract - -When contributing changes that touch compare mode (the `from-tag-name` path): -- Both `tag-name` and `from-tag-name` must exist as git tags before the compare API is called. -- Input-level check: `ActionInputs.validate_compare_mode_tag_names()` (called from `validate_inputs()`) exits with code 1 if either input string is empty. -- Tag-existence check: `DataMiner._handle_compare_mode()` calls `repo.get_git_ref("tags/")` for both tags and exits with code 1 naming the absent tag if either lookup fails. -- Any change to this validation logic must update `docs/features/compare_mode.md` and the tests in `tests/unit/release_notes_generator/data/test_miner.py`. - ## Branch Naming (PID:H-1) Branches MUST start with one of the allowed prefixes: `feature/`, `fix/`, `docs/`, `chore/` Examples: diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index 3e4d64a8..ccc0b24f 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -217,7 +217,7 @@ def test_validate_inputs_compare_mode_whitespace_from_tag_name_fails(mocker): case = success_case.copy() case["get_from_tag_name"] = "" patchers = apply_mocks(case, mocker) - mocker.patch( + mock_is_from_tag_name_defined = mocker.patch( "release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True, ) @@ -225,6 +225,7 @@ def test_validate_inputs_compare_mode_whitespace_from_tag_name_fails(mocker): mock_error = mocker.patch("release_notes_generator.action_inputs.logger.error") mock_exit = mocker.patch("sys.exit") ActionInputs.validate_inputs() + mock_is_from_tag_name_defined.assert_called_once_with(use_raw=True) assert any("from-tag-name" in str(c) for c in mock_error.call_args_list) mock_exit.assert_called_once_with(1) finally: From 832242fd033f83df61090abab508a30866fcd475 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Tue, 30 Jun 2026 09:46:27 +0200 Subject: [PATCH 16/18] Simplified logic - based on review comments. --- release_notes_generator/action_inputs.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index ca176a90..74c1e80c 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -166,19 +166,17 @@ def get_from_tag_name() -> str: return normalize_version_tag(raw) @staticmethod - def is_from_tag_name_defined(*, use_raw: bool = False) -> bool: + def is_from_tag_name_defined(use_raw: bool = False) -> bool: """ - Check if the from-tag name is defined in the action inputs. + Check if the from-tag name is defined. Parameters: - use_raw: When True, reads the raw env var before normalization so a - whitespace-only value is still treated as defined (used during - validation to route it to a fail-fast error). When False - (default), checks the normalized, stripped value. + use_raw: When True, checks the raw env var value (non-empty, including whitespace). + When False (default), normalizes the tag first and checks if non-empty. """ if use_raw: return get_action_input(FROM_TAG_NAME, default="") != "" - return ActionInputs.get_from_tag_name().strip() != "" + return ActionInputs.get_from_tag_name() != "" @staticmethod def get_chapters() -> list[dict[str, str]]: @@ -611,7 +609,7 @@ def validate_inputs() -> None: from_tag_name = ActionInputs.get_from_tag_name() if not isinstance(from_tag_name, str): errors.append("From tag name must be a string.") - elif ActionInputs.is_from_tag_name_defined(use_raw=True) and not from_tag_name.strip(): + elif ActionInputs.is_from_tag_name_defined(use_raw=True) 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." From 960764e0ff7ccbee1cecaa22ce085699ec1e0603 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Tue, 30 Jun 2026 10:04:23 +0200 Subject: [PATCH 17/18] Fix review comments. --- release_notes_generator/action_inputs.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 74c1e80c..96464dae 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -166,16 +166,10 @@ def get_from_tag_name() -> str: return normalize_version_tag(raw) @staticmethod - def is_from_tag_name_defined(use_raw: bool = False) -> bool: + def is_from_tag_name_defined() -> bool: """ - Check if the from-tag name is defined. - - Parameters: - use_raw: When True, checks the raw env var value (non-empty, including whitespace). - When False (default), normalizes the tag first and checks if non-empty. + Check if the from-tag name is defined (normalizes to a non-empty tag). """ - if use_raw: - return get_action_input(FROM_TAG_NAME, default="") != "" return ActionInputs.get_from_tag_name() != "" @staticmethod @@ -607,9 +601,10 @@ 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 ActionInputs.is_from_tag_name_defined(use_raw=True) and from_tag_name == "": + 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." From fa1a6f171084f73f36e140c3f0bb42b0d3203c22 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Tue, 30 Jun 2026 10:09:10 +0200 Subject: [PATCH 18/18] Update of tests. --- .../test_action_inputs.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/unit/release_notes_generator/test_action_inputs.py b/tests/unit/release_notes_generator/test_action_inputs.py index ccc0b24f..da6220b6 100644 --- a/tests/unit/release_notes_generator/test_action_inputs.py +++ b/tests/unit/release_notes_generator/test_action_inputs.py @@ -176,31 +176,31 @@ def test_get_from_tag_name_empty(mocker): assert ActionInputs.get_from_tag_name() == "" -# --- is_from_tag_name_defined (use_raw=True) --- +# --- 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(use_raw=True) is True + 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 → True (user provided a value, even if invalid).""" + """Env var present but whitespace-only → False (normalizes to empty string).""" monkeypatch.setenv("INPUT_FROM_TAG_NAME", " ") - assert ActionInputs.is_from_tag_name_defined(use_raw=True) is True + 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(use_raw=True) is False + 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(use_raw=True) is False + assert ActionInputs.is_from_tag_name_defined() is False def test_get_from_tag_name_invalid_format(mocker): @@ -217,15 +217,11 @@ def test_validate_inputs_compare_mode_whitespace_from_tag_name_fails(mocker): case = success_case.copy() case["get_from_tag_name"] = "" patchers = apply_mocks(case, mocker) - mock_is_from_tag_name_defined = mocker.patch( - "release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", - return_value=True, - ) 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() - mock_is_from_tag_name_defined.assert_called_once_with(use_raw=True) assert any("from-tag-name" in str(c) for c in mock_error.call_args_list) mock_exit.assert_called_once_with(1) finally: