From 114f7a2501a0055a3e7f777d1c113ccff1048512 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Wed, 20 Nov 2024 11:21:38 +0100 Subject: [PATCH 1/5] #45 - skip-release-notes should allow multiple entires - Implemented the code. - Update of docs. --- README.md | 2 +- action.yml | 4 ++-- release_notes_generator/action_inputs.py | 13 ++++++------- release_notes_generator/record/record_factory.py | 16 +++++++++++++++- tests/test_action_inputs.py | 7 +++---- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 0418a35b..6703c340 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ Generate Release Notes action is dedicated to enhance the quality and organizati - **Default**: false ### `skip-release-notes-label` -- **Description**: Set to a label name to skip issues and PRs with this label from the release notes process generation. +- **Description**: List labels used for detection if issues or pull requests are ignored in the Release Notes generation process. Example: `skip-release-notes, question`. - **Required**: No - **Default**: `skip-release-notes` diff --git a/action.yml b/action.yml index 41c27ee9..a5ee5ed9 100644 --- a/action.yml +++ b/action.yml @@ -35,8 +35,8 @@ inputs: description: 'Use `published-at` timestamp instead of `created-at` as the reference point of previous Release.' required: false default: 'false' - skip-release-notes-label: - description: 'Skip label used for detection of issues or pull requests to ignore in Release Notes generation process.' + skip-release-notes-labels: + description: 'List labels used for detection if issues or pull requests are ignored in the Release Notes generation process.' required: false default: 'skip-release-notes' warnings: diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index a0fd326e..66a30e0a 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -109,11 +109,14 @@ def get_published_at() -> bool: return get_action_input(PUBLISHED_AT, "false").lower() == "true" @staticmethod - def get_skip_release_notes_label() -> str: + def get_skip_release_notes_labels() -> list[str]: """ Get the skip release notes label from the action inputs. """ - return get_action_input(SKIP_RELEASE_NOTES_LABEL) or "skip-release-notes" + user_choice = [item.strip() for item in get_action_input(SKIP_RELEASE_NOTES_LABEL, "").split(',')] + if len(user_choice) > 0: + return user_choice + return ["skip-release-notes"] @staticmethod def get_verbose() -> bool: @@ -212,10 +215,6 @@ def validate_inputs(): published_at = ActionInputs.get_published_at() ActionInputs.validate_input(published_at, bool, "Published at must be a boolean.", errors) - skip_release_notes_label = ActionInputs.get_skip_release_notes_label() - if not isinstance(skip_release_notes_label, str) or not skip_release_notes_label.strip(): - errors.append("Skip release notes label must be a non-empty string.") - verbose = ActionInputs.get_verbose() ActionInputs.validate_input(verbose, bool, "Verbose logging must be a boolean.", errors) @@ -248,7 +247,7 @@ def validate_inputs(): logger.debug("Tag name: %s", tag_name) logger.debug("Chapters JSON: %s", chapters_json) logger.debug("Published at: %s", published_at) - logger.debug("Skip release notes label: %s", skip_release_notes_label) + logger.debug("Skip release notes labels: %s", ActionInputs.get_skip_release_notes_labels()) logger.debug("Verbose logging: %s", verbose) logger.debug("Warnings: %s", warnings) logger.debug("Print empty chapters: %s", print_empty_chapters) diff --git a/release_notes_generator/record/record_factory.py b/release_notes_generator/record/record_factory.py index c6f6f4b4..7d452acb 100644 --- a/release_notes_generator/record/record_factory.py +++ b/release_notes_generator/record/record_factory.py @@ -26,6 +26,7 @@ from github.Repository import Repository from github.Commit import Commit +from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.model.record import Record from release_notes_generator.utils.decorators import safe_call_decorator @@ -66,10 +67,23 @@ def create_record_for_issue(r: Repository, i: Issue) -> None: @param i: Issue instance. @return: None """ + # check for skip labels presence and skip when detected + issue_labels = [label.name for label in issue.labels] + if bool(set(ActionInputs.get_skip_release_notes_labels()) - set(issue_labels)): + logger.debug("Issue %d: %s contains skip labels, skipping ...", issue.number, issue.title) + return + records[i.number] = Record(r, i) logger.debug("Created record for issue %d: %s", i.number, i.title) - def register_pull_request(pull: PullRequest): + def register_pull_request(pull: PullRequest) -> None: + # check for skip labels presence and skip when detected + pull_labels = [label.name for label in pull.labels] + skip_detected = bool(set(ActionInputs.get_skip_release_notes_labels()) - set(pull_labels)) + if skip_detected: + logger.debug("PR %d: %s contains skip labels, skipping ...", pull.number, pull.title) + return + for parent_issue_number in extract_issue_numbers_from_body(pull): if parent_issue_number not in records: logger.warning( diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py index fbe5a83b..827cee7a 100644 --- a/tests/test_action_inputs.py +++ b/tests/test_action_inputs.py @@ -27,7 +27,7 @@ "get_duplicity_icon": "🔁", "get_warnings": True, "get_published_at": False, - "get_skip_release_notes_label": "skip", + "get_skip_release_notes_labels": ["skip"], "get_print_empty_chapters": True, "get_verbose": True, } @@ -39,7 +39,6 @@ ("get_chapters_json", "invalid_json", "Chapters JSON must be a valid JSON string."), ("get_warnings", "not_bool", "Warnings must be a boolean."), ("get_published_at", "not_bool", "Published at must be a boolean."), - ("get_skip_release_notes_label", "", "Skip release notes label must be a non-empty string."), ("get_print_empty_chapters", "not_bool", "Print empty chapters must be a boolean."), ("get_verbose", "not_bool", "Verbose logging must be a boolean."), ("get_duplicity_icon", "", "Duplicity icon must be a non-empty string and have a length of 1."), @@ -118,8 +117,8 @@ def test_get_published_at(mocker): def test_get_skip_release_notes_label(mocker): - mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="") - assert ActionInputs.get_skip_release_notes_label() == "skip-release-notes" + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes") + assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes"] def test_get_print_empty_chapters(mocker): From 062635b77278f4712624d1b71de6f75b14344ed7 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Wed, 20 Nov 2024 14:47:23 +0100 Subject: [PATCH 2/5] - Fixed unit tests. --- release_notes_generator/record/record_factory.py | 5 ++--- tests/release_notes/test_record_factory.py | 8 ++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/release_notes_generator/record/record_factory.py b/release_notes_generator/record/record_factory.py index 7d452acb..1b9fe862 100644 --- a/release_notes_generator/record/record_factory.py +++ b/release_notes_generator/record/record_factory.py @@ -69,7 +69,7 @@ def create_record_for_issue(r: Repository, i: Issue) -> None: """ # check for skip labels presence and skip when detected issue_labels = [label.name for label in issue.labels] - if bool(set(ActionInputs.get_skip_release_notes_labels()) - set(issue_labels)): + if any(item in issue_labels for item in ActionInputs.get_skip_release_notes_labels()): logger.debug("Issue %d: %s contains skip labels, skipping ...", issue.number, issue.title) return @@ -79,8 +79,7 @@ def create_record_for_issue(r: Repository, i: Issue) -> None: def register_pull_request(pull: PullRequest) -> None: # check for skip labels presence and skip when detected pull_labels = [label.name for label in pull.labels] - skip_detected = bool(set(ActionInputs.get_skip_release_notes_labels()) - set(pull_labels)) - if skip_detected: + if any(item in pull_labels for item in ActionInputs.get_skip_release_notes_labels()): logger.debug("PR %d: %s contains skip labels, skipping ...", pull.number, pull.title) return diff --git a/tests/release_notes/test_record_factory.py b/tests/release_notes/test_record_factory.py index e7cdf23e..b3cc20be 100644 --- a/tests/release_notes/test_record_factory.py +++ b/tests/release_notes/test_record_factory.py @@ -78,6 +78,7 @@ def setup_issues_no_pulls_no_commits(mocker): mock_git_issue1.body = "Body of issue 1" mock_git_issue1.state = "open" mock_git_issue1.created_at = datetime.now() + mock_git_issue1.labels = [] mock_git_issue2 = mocker.Mock(spec=Issue) mock_git_issue2.id = 2 @@ -86,6 +87,7 @@ def setup_issues_no_pulls_no_commits(mocker): mock_git_issue2.body = "Body of issue 2" mock_git_issue2.state = "closed" mock_git_issue2.created_at = datetime.now() + mock_git_issue2.labels = [] return mock_git_issue1, mock_git_issue2 @@ -99,6 +101,7 @@ def setup_issues_pulls_commits(mocker): mock_git_issue1.body = "Body of issue 1" mock_git_issue1.state = "open" mock_git_issue1.created_at = datetime.now() + mock_git_issue1.labels = [] mock_git_issue2 = mocker.Mock(spec=Issue) mock_git_issue2.id = 2 @@ -107,6 +110,7 @@ def setup_issues_pulls_commits(mocker): mock_git_issue2.body = "Body of issue 2" mock_git_issue2.state = "closed" mock_git_issue2.created_at = datetime.now() + mock_git_issue2.labels = [] mock_git_pr1 = mocker.Mock(spec=PullRequest) mock_git_pr1.id = 101 @@ -120,7 +124,7 @@ def setup_issues_pulls_commits(mocker): mock_git_pr1.merged_at = None mock_git_pr1.assignee = None mock_git_pr1.merge_commit_sha = "abc123" - mock_git_pr1.get_labels = mocker.Mock(return_value=[]) + mock_git_pr1.labels = [] mock_git_pr2 = mocker.Mock(spec=PullRequest) mock_git_pr2.id = 102 @@ -134,7 +138,7 @@ def setup_issues_pulls_commits(mocker): mock_git_pr2.merged_at = None mock_git_pr2.assignee = None mock_git_pr2.merge_commit_sha = "def456" - mock_git_pr2.get_labels = mocker.Mock(return_value=[]) + mock_git_pr2.labels = [] mock_git_commit1 = mocker.Mock(spec=Commit) mock_git_commit1.sha = "abc123" From e951efc390e69e9959951086e754da900d8ef5e0 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 21 Nov 2024 08:29:14 +0100 Subject: [PATCH 3/5] - Applied black formatting. - Fixed and extended unit tests. --- README.md | 4 + release_notes_generator/action_inputs.py | 2 +- .../model/custom_chapters.py | 3 + release_notes_generator/model/record.py | 8 +- .../model/service_chapters.py | 3 + .../record/record_factory.py | 23 +++--- tests/conftest.py | 55 +++++++++++++ .../model/test_custom_chapters.py | 5 ++ tests/release_notes/test_record_factory.py | 80 ++++++++++++++++++- .../test_release_notes_builder.py | 39 +++++++++ 10 files changed, 203 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 6703c340..058da5da 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,10 @@ Generate Release Notes action is dedicated to enhance the quality and organizati - **Description**: List labels used for detection if issues or pull requests are ignored in the Release Notes generation process. Example: `skip-release-notes, question`. - **Required**: No - **Default**: `skip-release-notes` +- Notes: + - If used on issue then Issue will be skipped during Release Notes generation. + - If used on PR with issue then on PR it will be ignored and PR will show as part of issue's release notes. + - If used on PR without issue then PR will be skipped during Release Notes generation. ### `verbose` - **Description**: Set to true to enable verbose logging for detailed output during the action's execution. diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 66a30e0a..65807e7c 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -113,7 +113,7 @@ def get_skip_release_notes_labels() -> list[str]: """ Get the skip release notes label from the action inputs. """ - user_choice = [item.strip() for item in get_action_input(SKIP_RELEASE_NOTES_LABEL, "").split(',')] + user_choice = [item.strip() for item in get_action_input(SKIP_RELEASE_NOTES_LABEL, "").split(",")] if len(user_choice) > 0: return user_choice return ["skip-release-notes"] diff --git a/release_notes_generator/model/custom_chapters.py b/release_notes_generator/model/custom_chapters.py index a2582141..b3fc694c 100644 --- a/release_notes_generator/model/custom_chapters.py +++ b/release_notes_generator/model/custom_chapters.py @@ -41,6 +41,9 @@ def populate(self, records: dict[int, Record]) -> None: @return: None """ for nr in records: # iterate all records + if records[nr].skip: + continue + for ch in self.chapters.values(): # iterate all chapters if nr in self.populated_record_numbers_list and ActionInputs.get_duplicity_scope() not in ( DuplicityScopeEnum.CUSTOM, diff --git a/release_notes_generator/model/record.py b/release_notes_generator/model/record.py index e4780aca..1d81850e 100644 --- a/release_notes_generator/model/record.py +++ b/release_notes_generator/model/record.py @@ -46,7 +46,7 @@ class Record: A class used to represent a record in the release notes. """ - def __init__(self, repo: Repository, issue: Optional[Issue] = None): + def __init__(self, repo: Repository, issue: Optional[Issue] = None, skip: bool = False): self.__repo: Repository = repo self.__gh_issue: Issue = issue self.__pulls: list[PullRequest] = [] @@ -54,6 +54,7 @@ def __init__(self, repo: Repository, issue: Optional[Issue] = None): self.__is_release_note_detected: bool = False self.__present_in_chapters = 0 + self.__skip = skip @property def number(self) -> int: @@ -82,6 +83,11 @@ def is_present_in_chapters(self) -> bool: """Check if the record is present in chapters.""" return self.__present_in_chapters > 0 + @property + def skip(self) -> bool: + """Check if the record should be skipped during output generation process.""" + return self.__skip + @property def is_pr(self) -> bool: """Check if the record is a pull request.""" diff --git a/release_notes_generator/model/service_chapters.py b/release_notes_generator/model/service_chapters.py index c021d6c6..65007579 100644 --- a/release_notes_generator/model/service_chapters.py +++ b/release_notes_generator/model/service_chapters.py @@ -96,6 +96,9 @@ def populate(self, records: dict[int, Record]) -> None: """ # iterate all records for nr in records: + if records[nr].skip: + continue + # skip the record when used in used and not allowed to be duplicated in Service chapters if self.__is_row_present(nr) and not self.duplicity_allowed(): continue diff --git a/release_notes_generator/record/record_factory.py b/release_notes_generator/record/record_factory.py index 1b9fe862..99e460d9 100644 --- a/release_notes_generator/record/record_factory.py +++ b/release_notes_generator/record/record_factory.py @@ -69,20 +69,12 @@ def create_record_for_issue(r: Repository, i: Issue) -> None: """ # check for skip labels presence and skip when detected issue_labels = [label.name for label in issue.labels] - if any(item in issue_labels for item in ActionInputs.get_skip_release_notes_labels()): - logger.debug("Issue %d: %s contains skip labels, skipping ...", issue.number, issue.title) - return + skip_record = any(item in issue_labels for item in ActionInputs.get_skip_release_notes_labels()) + records[i.number] = Record(r, i, skip=skip_record) - records[i.number] = Record(r, i) logger.debug("Created record for issue %d: %s", i.number, i.title) - def register_pull_request(pull: PullRequest) -> None: - # check for skip labels presence and skip when detected - pull_labels = [label.name for label in pull.labels] - if any(item in pull_labels for item in ActionInputs.get_skip_release_notes_labels()): - logger.debug("PR %d: %s contains skip labels, skipping ...", pull.number, pull.title) - return - + def register_pull_request(pull: PullRequest, skip_record: bool) -> None: for parent_issue_number in extract_issue_numbers_from_body(pull): if parent_issue_number not in records: logger.warning( @@ -99,7 +91,7 @@ def register_pull_request(pull: PullRequest) -> None: records[parent_issue_number].register_pull_request(pull) logger.debug("Registering PR %d: %s to Issue %d", pull.number, pull.title, parent_issue_number) else: - records[pull.number] = Record(repo) + records[pull.number] = Record(repo, skip=skip_record) records[pull.number].register_pull_request(pull) logger.debug( "Registering stand-alone PR %d: %s as mentioned Issue %d not found.", @@ -129,12 +121,15 @@ def register_commit_to_record(commit: Commit) -> bool: create_record_for_issue(repo, issue) for pull in pulls: + pull_labels = [label.name for label in pull.labels] + skip_record: bool = any(item in pull_labels for item in ActionInputs.get_skip_release_notes_labels()) + if not extract_issue_numbers_from_body(pull): - records[pull.number] = Record(repo) + records[pull.number] = Record(repo, skip=skip_record) records[pull.number].register_pull_request(pull) logger.debug("Created record for PR %d: %s", pull.number, pull.title) else: - register_pull_request(pull) + register_pull_request(pull, skip_record) detected_prs_count = sum(register_commit_to_record(commit) for commit in commits) diff --git a/tests/conftest.py b/tests/conftest.py index a475eb8f..ca76dbe9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -169,6 +169,20 @@ def mock_issue_closed_i1_bug(mocker): return issue +@pytest.fixture +def mock_issue_closed_i1_bug_and_skip(mocker): + issue = mocker.Mock(spec=Issue) + issue.state = ISSUE_STATE_CLOSED + label1 = mocker.Mock(spec=MockLabel) + label1.name = "skip-release-notes" + label2 = mocker.Mock(spec=MockLabel) + label2.name = "bug" + issue.labels = [label1, label2] + issue.title = "I1+bug" + issue.number = 122 + return issue + + # Fixtures for GitHub Pull Request(s) @pytest.fixture def mock_pull_closed(mocker): @@ -189,6 +203,25 @@ def mock_pull_closed(mocker): return pull +@pytest.fixture +def mock_pull_closed_with_skip_label(mocker): + pull = mocker.Mock(spec=PullRequest) + pull.state = PR_STATE_CLOSED + pull.body = "Release Notes:\n- Fixed bug\n- Improved performance\n+ More nice code\n * Awesome architecture" + pull.url = "http://example.com/pull/123" + label1 = mocker.Mock(spec=MockLabel) + label1.name = "skip-release-notes" + pull.labels = [label1] + pull.number = 123 + pull.merge_commit_sha = "merge_commit_sha" + pull.title = "Fixed bug" + pull.created_at = datetime.now() + pull.updated_at = datetime.now() + pull.merged_at = None + pull.closed_at = datetime.now() + return pull + + @pytest.fixture def mock_pull_closed_with_rls_notes_101(mocker): pull = mocker.Mock(spec=PullRequest) @@ -364,6 +397,18 @@ def record_with_issue_closed_one_pull_merged(request): return rec +@pytest.fixture +def record_with_issue_closed_one_pull_merged_skip(request): + rec = Record( + repo=(mock_repo_fixture := request.getfixturevalue("mock_repo")), + issue=request.getfixturevalue("mock_issue_closed_i1_bug_and_skip"), + skip=True, + ) + rec.register_pull_request(request.getfixturevalue("mock_pull_merged")) + mock_repo_fixture.full_name = "org/repo" + return rec + + @pytest.fixture def record_with_issue_closed_two_pulls(request): rec = Record( @@ -466,6 +511,16 @@ def record_with_no_issue_one_pull_closed(request): return record +@pytest.fixture +def record_with_no_issue_one_pull_closed_with_skip_label(request): + record = Record(repo=(mock_repo_fixture := request.getfixturevalue("mock_repo")), skip=True) + mock_repo_fixture.full_name = "org/repo" + mock_repo_fixture.draft = False + record.register_pull_request(request.getfixturevalue("mock_pull_closed_with_skip_label")) + record.register_commit(request.getfixturevalue("mock_commit")) + return record + + @pytest.fixture def record_with_no_issue_one_pull_closed_no_rls_notes(request): record = Record(repo=request.getfixturevalue("mock_repo")) diff --git a/tests/release_notes/model/test_custom_chapters.py b/tests/release_notes/model/test_custom_chapters.py index 4a91d30c..e2d00dc9 100644 --- a/tests/release_notes/model/test_custom_chapters.py +++ b/tests/release_notes/model/test_custom_chapters.py @@ -38,18 +38,21 @@ def test_populate(custom_chapters, mocker): record1.pulls_count = 1 record1.is_present_in_chapters = False record1.to_chapter_row.return_value = "Record 1 Chapter Row" + record1.skip = False record2 = mocker.Mock(spec=Record) record2.labels = ["enhancement"] record2.pulls_count = 1 record2.is_present_in_chapters = False record2.to_chapter_row.return_value = "Record 2 Chapter Row" + record2.skip = False record3 = mocker.Mock(spec=Record) record3.labels = ["feature"] record3.pulls_count = 1 record3.is_present_in_chapters = False record3.to_chapter_row.return_value = "Record 3 Chapter Row" + record3.skip = False records = { 1: record1, @@ -102,6 +105,7 @@ def test_populate_service_duplicity_scope(custom_chapters, mocker): record1.pulls_count = 1 record1.is_present_in_chapters = False record1.to_chapter_row.return_value = "Record 1 Chapter Row" + record1.skip = False records = { 1: record1, @@ -125,6 +129,7 @@ def test_populate_none_duplicity_scope(custom_chapters, mocker): record1.pulls_count = 1 record1.is_present_in_chapters = False record1.to_chapter_row.return_value = "Record 1 Chapter Row" + record1.skip = False records = { 1: record1, diff --git a/tests/release_notes/test_record_factory.py b/tests/release_notes/test_record_factory.py index b3cc20be..b95f91d0 100644 --- a/tests/release_notes/test_record_factory.py +++ b/tests/release_notes/test_record_factory.py @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # - +import os import time from datetime import datetime @@ -25,6 +25,7 @@ from release_notes_generator.model.record import Record from release_notes_generator.record.record_factory import RecordFactory +from tests.conftest import MockLabel def setup_no_issues_pulls_commits(mocker): @@ -40,7 +41,7 @@ def setup_no_issues_pulls_commits(mocker): mock_git_pr1.merged_at = None mock_git_pr1.assignee = None mock_git_pr1.merge_commit_sha = "abc123" - mock_git_pr1.get_labels = mocker.Mock(return_value=[]) + mock_git_pr1.labels = [] mock_git_pr2 = mocker.Mock(spec=PullRequest) mock_git_pr2.id = 102 @@ -54,7 +55,7 @@ def setup_no_issues_pulls_commits(mocker): mock_git_pr2.merged_at = None mock_git_pr2.assignee = None mock_git_pr2.merge_commit_sha = "def456" - mock_git_pr2.get_labels = mocker.Mock(return_value=[]) + mock_git_pr2.labels = [] mock_git_commit1 = mocker.Mock(spec=Commit) mock_git_commit1.sha = "abc123" @@ -168,6 +169,9 @@ def test_generate_with_issues_and_pulls_and_commits(mocker, mock_repo): assert 1 in records assert 2 in records + assert not records[1].skip + assert not records[2].skip + # Verify the record creation assert isinstance(records[1], Record) assert isinstance(records[2], Record) @@ -184,6 +188,46 @@ def test_generate_with_issues_and_pulls_and_commits(mocker, mock_repo): assert 1 == records[2].pull_request_commit_count(102) +def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, mock_repo): + mock_github_client = mocker.Mock(spec=Github) + issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker) + mock_label = mocker.Mock(spec=MockLabel) + mock_label.name = "skip-release-notes" + issue1.labels = [mock_label] + pr2.labels = [mock_label] + issues = [issue1, issue2] + pulls = [pr1, pr2] + commit3 = mocker.Mock(spec=Commit) + commit3.sha = "ghi789" + commits = [commit1, commit2, commit3] + mocker.patch("release_notes_generator.builder.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes"]) + + records = RecordFactory.generate(mock_github_client, mock_repo, issues, pulls, commits) + + # Check if records for issues and PRs were created + assert 1 in records + assert 2 in records + + # Verify the record creation + assert isinstance(records[1], Record) + assert isinstance(records[2], Record) + + assert records[1].skip # skip label appied to issue as the record was created from issue + assert not records[2].skip # skip label is present only on inner PR but record create from issues (leading) + + # Verify that PRs are registered + # assert 1 == records[1].pulls_count + assert 1 == records[2].pulls_count + + # assert pr1 == records[1].pull_request(0) + assert pr2 == records[2].pull_request(0) + + # Verify that commits are registered + # assert 1 == records[1].pull_request_commit_count(101) + assert 1 == records[2].pull_request_commit_count(102) + + + def test_generate_with_no_commits(mocker, mock_repo): mock_github_client = mocker.Mock(spec=Github) # pylint: disable=unused-variable @@ -238,6 +282,36 @@ def test_generate_with_no_issues(mocker, request): assert 1 == records[101].pull_request_commit_count(101) assert 1 == records[102].pull_request_commit_count(102) +def test_generate_with_no_issues_skip_labels(mocker, request): + mock_github_client = mocker.Mock(spec=Github) + pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) + mock_label = mocker.Mock(spec=MockLabel) + mock_label.name = "skip-release-notes" + pr1.labels = [mock_label] + pulls = [pr1, pr2] + commits = [commit1, commit2] + mocker.patch("release_notes_generator.builder.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes"]) + + records = RecordFactory.generate(mock_github_client, request.getfixturevalue("mock_repo"), [], pulls, commits) + + # Verify the record creation + assert isinstance(records[101], Record) + assert isinstance(records[102], Record) + + assert records[101].skip + assert not records[102].skip + + # Verify that PRs are registered + assert 1 == records[101].pulls_count + assert 1 == records[102].pulls_count + + assert pr1 == records[101].pull_request(0) + assert pr2 == records[102].pull_request(0) + + # Verify that commits are registered + assert 1 == records[101].pull_request_commit_count(101) + assert 1 == records[102].pull_request_commit_count(102) + def test_generate_with_no_pulls(mocker, mock_repo): mock_github_client = mocker.Mock(spec=Github) diff --git a/tests/release_notes/test_release_notes_builder.py b/tests/release_notes/test_release_notes_builder.py index aeb57a55..a384849a 100644 --- a/tests/release_notes/test_release_notes_builder.py +++ b/tests/release_notes/test_release_notes_builder.py @@ -203,6 +203,8 @@ def __init__(self, name): http://example.com/changelog """ +RELEASE_NOTES_DATA_SERVICE_CHAPTERS_CLOSED_PR_NO_ISSUE_SKIP_USER_LABELS = RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS + RELEASE_NOTES_DATA_SERVICE_CHAPTERS_OPEN_ISSUE_AND_MERGED_PR_NO_USER_LABELS = """### Merged PRs Linked to 'Not Closed' Issue ⚠️ - #122 _I1 open_ in #101, #102 - PR 101 1st release note @@ -284,6 +286,8 @@ def __init__(self, name): http://example.com/changelog """ +RELEASE_NOTES_DATA_CLOSED_ISSUE_WITH_MERGED_PRS_WITH_USER_LABELS_WITH_SKIP_LABEL = RELEASE_NOTES_NO_DATA_NO_WARNING_NO_EMPTY_CHAPTERS + RELEASE_NOTES_DATA_CLOSED_ISSUE_WITH_MERGED_PRS_WITH_USER_LABELS = """### Chapter 1 🛠 - #122 _I1+bug_ in #123 - Fixed bug @@ -884,3 +888,38 @@ def test_merged_pr_with_closed_issue_mention_with_user_labels( actual_release_notes = builder.build() assert expected_release_notes == actual_release_notes + +def test_merged_pr_with_closed_issue_mention_with_user_labels_with_skip_label_on_issue( + custom_chapters_not_print_empty_chapters, record_with_issue_closed_one_pull_merged_skip, mocker +): + expected_release_notes = RELEASE_NOTES_DATA_CLOSED_ISSUE_WITH_MERGED_PRS_WITH_USER_LABELS_WITH_SKIP_LABEL + rec = record_with_issue_closed_one_pull_merged_skip + mocker.patch("release_notes_generator.builder.ActionInputs.get_print_empty_chapters", return_value=False) + + builder = ReleaseNotesBuilder( + records={rec.number: rec}, + changelog_url=DEFAULT_CHANGELOG_URL, + custom_chapters=custom_chapters_not_print_empty_chapters, + ) + + actual_release_notes = builder.build() + + assert expected_release_notes == actual_release_notes + + +def test_build_closed_pr_service_chapter_without_issue_with_skip_label_on_pr( + custom_chapters_not_print_empty_chapters, record_with_no_issue_one_pull_closed_with_skip_label, mocker +): + expected_release_notes = RELEASE_NOTES_DATA_SERVICE_CHAPTERS_CLOSED_PR_NO_ISSUE_SKIP_USER_LABELS + rec = record_with_no_issue_one_pull_closed_with_skip_label + mocker.patch("release_notes_generator.builder.ActionInputs.get_print_empty_chapters", return_value=False) + + builder = ReleaseNotesBuilder( + records={rec.number: rec}, + changelog_url=DEFAULT_CHANGELOG_URL, + custom_chapters=custom_chapters_not_print_empty_chapters, + ) + + actual_release_notes = builder.build() + + assert expected_release_notes == actual_release_notes From 9dbf2b96eed5e4f8e015451dc434260e583c3523 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 21 Nov 2024 08:44:12 +0100 Subject: [PATCH 4/5] - Adding more complexity ti the tests. --- tests/conftest.py | 3 +++ tests/release_notes/test_record_factory.py | 13 ++++++++----- tests/test_action_inputs.py | 7 +++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ca76dbe9..d8353e6c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -212,6 +212,9 @@ def mock_pull_closed_with_skip_label(mocker): label1 = mocker.Mock(spec=MockLabel) label1.name = "skip-release-notes" pull.labels = [label1] + label2 = mocker.Mock(spec=MockLabel) + label2.name = "another-skip-label" + pull.labels = [label2] pull.number = 123 pull.merge_commit_sha = "merge_commit_sha" pull.title = "Fixed bug" diff --git a/tests/release_notes/test_record_factory.py b/tests/release_notes/test_record_factory.py index b95f91d0..76528c4f 100644 --- a/tests/release_notes/test_record_factory.py +++ b/tests/release_notes/test_record_factory.py @@ -285,12 +285,15 @@ def test_generate_with_no_issues(mocker, request): def test_generate_with_no_issues_skip_labels(mocker, request): mock_github_client = mocker.Mock(spec=Github) pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) - mock_label = mocker.Mock(spec=MockLabel) - mock_label.name = "skip-release-notes" - pr1.labels = [mock_label] + mock_label1 = mocker.Mock(spec=MockLabel) + mock_label1.name = "skip-release-notes" + pr1.labels = [mock_label1] + mock_label2 = mocker.Mock(spec=MockLabel) + mock_label2.name = "another-skip-label" + pr2.labels = [mock_label2] pulls = [pr1, pr2] commits = [commit1, commit2] - mocker.patch("release_notes_generator.builder.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes"]) + mocker.patch("release_notes_generator.builder.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes", "another-skip-label"]) records = RecordFactory.generate(mock_github_client, request.getfixturevalue("mock_repo"), [], pulls, commits) @@ -299,7 +302,7 @@ def test_generate_with_no_issues_skip_labels(mocker, request): assert isinstance(records[102], Record) assert records[101].skip - assert not records[102].skip + assert records[102].skip # Verify that PRs are registered assert 1 == records[101].pulls_count diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py index 827cee7a..493afa78 100644 --- a/tests/test_action_inputs.py +++ b/tests/test_action_inputs.py @@ -120,6 +120,13 @@ def test_get_skip_release_notes_label(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes") assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes"] +def test_get_skip_release_notes_labels(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes, another-skip-label") + assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes", "another-skip-label"] + +def test_get_skip_release_notes_labels_no_space(mocker): + mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="skip-release-notes,another-skip-label") + assert ActionInputs.get_skip_release_notes_labels() == ["skip-release-notes", "another-skip-label"] def test_get_print_empty_chapters(mocker): mocker.patch("release_notes_generator.action_inputs.get_action_input", return_value="true") From b4eacda582e8811772da2a9f4633bb3de46c9ce6 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 25 Nov 2024 12:38:35 +0100 Subject: [PATCH 5/5] - Fix Review comments. --- README.md | 8 ++++---- action.yml | 2 +- release_notes_generator/action_inputs.py | 4 ++-- release_notes_generator/model/custom_chapters.py | 1 + release_notes_generator/utils/constants.py | 2 +- tests/release_notes/test_record_factory.py | 2 +- 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 058da5da..89d271cb 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ Generate Release Notes action is dedicated to enhance the quality and organizati - **Required**: No - **Default**: false -### `skip-release-notes-label` +### `skip-release-notes-labels` - **Description**: List labels used for detection if issues or pull requests are ignored in the Release Notes generation process. Example: `skip-release-notes, question`. - **Required**: No - **Default**: `skip-release-notes` @@ -161,7 +161,7 @@ Add the following step to your GitHub workflow (in example are used non-default duplicity-scope: 'service' duplicity-icon: '🔁' published-at: true - skip-release-notes-label: 'ignore-in-release' # changing default value of label + skip-release-notes-labels: 'ignore-in-release' # changing default value of label verbose: false warnings: false @@ -200,7 +200,7 @@ If an issue is linked to multiple PRs, the action fetches and aggregates contrib By set **published-at** to true the action will use the `published-at` timestamp of the latest release as the reference point for searching closed issues and PRs, instead of the `created-at` date. If first release, repository creation date is used. ### Enable skipping of release notes for specific issues using label -By set **skip-release-notes-label** to true the action will skip all issues and related PRs if they contain a label defined in configuration. This is useful for issues that are not relevant for release notes. +By defining the `skip-release-notes-labels` option, the action will skip all issues and related PRs if they contain a label defined in configuration. This is useful for issues that are not relevant for release notes. ### Enable Service Chapters If the `warnings` option is enabled in the action's configuration, the release notes will include sections that highlight possible issues. @@ -377,7 +377,7 @@ export INPUT_CHAPTERS='[ ]' export INPUT_WARNINGS="true" export INPUT_PUBLISHED_AT="true" -export INPUT_SKIP_RELEASE_NOTES_LABEL="ignore-in-release" +export INPUT_SKIP_RELEASE_NOTES_LABELS="ignore-in-release" export INPUT_PRINT_EMPTY_CHAPTERS="true" export INPUT_VERBOSE="true" diff --git a/action.yml b/action.yml index a5ee5ed9..eac09d62 100644 --- a/action.yml +++ b/action.yml @@ -115,7 +115,7 @@ runs: INPUT_DUPLICITY_ICON: ${{ inputs.duplicity-icon }} INPUT_WARNINGS: ${{ inputs.warnings }} INPUT_PUBLISHED_AT: ${{ inputs.published-at }} - INPUT_SKIP_RELEASE_NOTES_LABEL: ${{ inputs.skip-release-notes-label }} + INPUT_SKIP_RELEASE_NOTES_LABELS: ${{ inputs.skip-release-notes-labels }} INPUT_PRINT_EMPTY_CHAPTERS: ${{ inputs.print-empty-chapters }} INPUT_VERBOSE: ${{ inputs.verbose }} INPUT_GITHUB_REPOSITORY: ${{ github.repository }} diff --git a/release_notes_generator/action_inputs.py b/release_notes_generator/action_inputs.py index 65807e7c..d3774ae0 100644 --- a/release_notes_generator/action_inputs.py +++ b/release_notes_generator/action_inputs.py @@ -29,7 +29,6 @@ TAG_NAME, CHAPTERS, PUBLISHED_AT, - SKIP_RELEASE_NOTES_LABEL, VERBOSE, WARNINGS, RUNNER_DEBUG, @@ -39,6 +38,7 @@ ROW_FORMAT_LINK_PR, ROW_FORMAT_ISSUE, ROW_FORMAT_PR, + SKIP_RELEASE_NOTES_LABELS, ) from release_notes_generator.utils.enums import DuplicityScopeEnum from release_notes_generator.utils.gh_action import get_action_input @@ -113,7 +113,7 @@ def get_skip_release_notes_labels() -> list[str]: """ Get the skip release notes label from the action inputs. """ - user_choice = [item.strip() for item in get_action_input(SKIP_RELEASE_NOTES_LABEL, "").split(",")] + user_choice = [item.strip() for item in get_action_input(SKIP_RELEASE_NOTES_LABELS, "").split(",")] if len(user_choice) > 0: return user_choice return ["skip-release-notes"] diff --git a/release_notes_generator/model/custom_chapters.py b/release_notes_generator/model/custom_chapters.py index b3fc694c..ba9084e4 100644 --- a/release_notes_generator/model/custom_chapters.py +++ b/release_notes_generator/model/custom_chapters.py @@ -41,6 +41,7 @@ def populate(self, records: dict[int, Record]) -> None: @return: None """ for nr in records: # iterate all records + # check if the record should be skipped if records[nr].skip: continue diff --git a/release_notes_generator/utils/constants.py b/release_notes_generator/utils/constants.py index 5b4b3377..6d80c57a 100644 --- a/release_notes_generator/utils/constants.py +++ b/release_notes_generator/utils/constants.py @@ -26,7 +26,7 @@ DUPLICITY_SCOPE = "duplicity-scope" DUPLICITY_ICON = "duplicity-icon" PUBLISHED_AT = "published-at" -SKIP_RELEASE_NOTES_LABEL = "skip-release-notes-label" +SKIP_RELEASE_NOTES_LABELS = "skip-release-notes-labels" VERBOSE = "verbose" RUNNER_DEBUG = "RUNNER_DEBUG" ROW_FORMAT_ISSUE = "row-format-issue" diff --git a/tests/release_notes/test_record_factory.py b/tests/release_notes/test_record_factory.py index 76528c4f..7550b914 100644 --- a/tests/release_notes/test_record_factory.py +++ b/tests/release_notes/test_record_factory.py @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # -import os + import time from datetime import datetime