diff --git a/.pylintrc b/.pylintrc index 693e6f9e..ec9a1731 100644 --- a/.pylintrc +++ b/.pylintrc @@ -304,7 +304,7 @@ max-attributes=7 max-bool-expr=5 # Maximum number of branch for function / method body. -max-branches=13 +max-branches=15 # Maximum number of locals for function / method body. max-locals=20 diff --git a/release_notes_generator/builder/builder.py b/release_notes_generator/builder/builder.py index 4da143c6..f7b0b7c2 100644 --- a/release_notes_generator/builder/builder.py +++ b/release_notes_generator/builder/builder.py @@ -38,7 +38,7 @@ class ReleaseNotesBuilder: def __init__( self, - records: dict[int | str, Record], + records: dict[str, Record], changelog_url: str, custom_chapters: CustomChapters, ): diff --git a/release_notes_generator/chapters/base_chapters.py b/release_notes_generator/chapters/base_chapters.py index e9f861fc..bb9a0ee9 100644 --- a/release_notes_generator/chapters/base_chapters.py +++ b/release_notes_generator/chapters/base_chapters.py @@ -104,9 +104,13 @@ def titles(self) -> list[str]: return [chapter.title for chapter in self.chapters.values()] @abstractmethod - def populate(self, records: dict[int | str, Record]) -> None: + def populate(self, records: dict[str, Record]) -> None: """ Populates the chapters with records. - @param records: A dictionary of records where the key is an integer and the value is a Record object. + Parameters: + records (dict[str, Record]): A dictionary of records to populate the chapters with. + + Returns: + None """ diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index b90c65d2..491b52b8 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -39,12 +39,15 @@ class CustomChapters(BaseChapters): A class used to represent the custom chapters in the release notes. """ - def populate(self, records: dict[int | str, Record]) -> None: + def populate(self, records: dict[str, Record]) -> None: """ Populates the custom chapters with records. - @param records: A dictionary of records where the key is an integer and the value is a Record object. - @return: None + Parameters: + @param records: A dictionary of records keyed by 'owner/repo#number' and values are Record objects. + + Returns: + None """ for record_id, record in records.items(): # iterate all records # check if the record should be skipped @@ -68,9 +71,19 @@ def populate(self, records: dict[int | str, Record]) -> None: for record_label in records[record_id].labels: # iterate all labels of the record (issue, or 1st PR) if record_label in ch.labels and pulls_count > 0: - if not records[record_id].is_present_in_chapters: - ch.add_row(record_id, records[record_id].to_chapter_row(True)) - self.populated_record_numbers_list.append(record_id) + if ( + not records[record_id].is_present_in_chapters + and records[record_id].contains_change_increment() + ): + allow_dup = ActionInputs.get_duplicity_scope() in ( + DuplicityScopeEnum.CUSTOM, + DuplicityScopeEnum.BOTH, + ) + if (allow_dup or not records[record_id].is_present_in_chapters) and records[ + record_id + ].contains_change_increment(): + ch.add_row(record_id, records[record_id].to_chapter_row(True)) + self.populated_record_numbers_list.append(record_id) def from_yaml_array(self, chapters: list[dict[str, str]]) -> "CustomChapters": """ diff --git a/release_notes_generator/chapters/service_chapters.py b/release_notes_generator/chapters/service_chapters.py index 78d236b6..ca7cb266 100644 --- a/release_notes_generator/chapters/service_chapters.py +++ b/release_notes_generator/chapters/service_chapters.py @@ -93,7 +93,7 @@ def __init__( self.show_chapter_merged_prs_linked_to_open_issues = True - def populate(self, records: dict[int | str, Record]) -> None: + def populate(self, records: dict[str, Record]) -> None: """ Populates the service chapters with records. diff --git a/release_notes_generator/filter.py b/release_notes_generator/filter.py index 7afe69fd..a8491935 100644 --- a/release_notes_generator/filter.py +++ b/release_notes_generator/filter.py @@ -61,8 +61,7 @@ def filter(self, data: MinedData) -> MinedData: Returns: MinedData: The filtered mined data. """ - md = MinedData() - md.repository = data.repository + md = MinedData(data.repository) md.release = data.release md.since = data.since diff --git a/release_notes_generator/generator.py b/release_notes_generator/generator.py index 25f3a7ad..c0d483e1 100644 --- a/release_notes_generator/generator.py +++ b/release_notes_generator/generator.py @@ -24,6 +24,7 @@ from typing import Optional from github import Github +from github.Repository import Repository from release_notes_generator.filter import FilterByRelease from release_notes_generator.miner import DataMiner @@ -76,9 +77,14 @@ def generate(self) -> Optional[str]: @return: The generated release notes as a string, or None if the repository could not be found. """ miner = DataMiner(self._github_instance, self._rate_limiter) + if not miner.check_repository_exists(): + return None + data = miner.mine_data() + if data.is_empty(): return None + self.custom_chapters.since = data.since filterer = FilterByRelease() @@ -92,9 +98,10 @@ def generate(self) -> Optional[str]: assert data_filtered_by_release.repository is not None, "Repository must not be None" - rls_notes_records: dict[int | str, Record] = self._get_record_factory(github=self._github_instance).generate( - data=data_filtered_by_release - ) + rls_notes_records: dict[str, Record] = self._get_record_factory( + github=self._github_instance, + home_repository=data_filtered_by_release.repository, + ).generate(data=data_filtered_by_release) return ReleaseNotesBuilder( records=rls_notes_records, @@ -102,16 +109,20 @@ def generate(self) -> Optional[str]: changelog_url=changelog_url, ).build() - def _get_record_factory(self, github: Github) -> DefaultRecordFactory: + def _get_record_factory(self, github: Github, home_repository: Repository) -> DefaultRecordFactory: """ Determines and returns the appropriate RecordFactory instance based on the action inputs. + Parameters: + github (Github): An instance of the Github class. + home_repository (Repository): The home repository for which records are to be generated. + Returns: DefaultRecordFactory: An instance of either IssueHierarchyRecordFactory or RecordFactory. """ if ActionInputs.get_hierarchy(): logger.info("Using IssueHierarchyRecordFactory based on action inputs.") - return IssueHierarchyRecordFactory(github) + return IssueHierarchyRecordFactory(github, home_repository) logger.info("Using default RecordFactory based on action inputs.") - return DefaultRecordFactory(github) + return DefaultRecordFactory(github, home_repository) diff --git a/release_notes_generator/miner.py b/release_notes_generator/miner.py index 5576626f..aca2a5ec 100644 --- a/release_notes_generator/miner.py +++ b/release_notes_generator/miner.py @@ -47,18 +47,30 @@ def __init__(self, github_instance: Github, rate_limiter: GithubRateLimiter): self.github_instance = github_instance self._safe_call = safe_call_decorator(rate_limiter) + def check_repository_exists(self) -> bool: + """ + Checks if the specified GitHub repository exists. + + Returns: + bool: True if the repository exists, False otherwise. + """ + repo: Repository = self._safe_call(self.github_instance.get_repo)(ActionInputs.get_github_repository()) + if repo is None: + logger.error("Repository not found: %s", ActionInputs.get_github_repository()) + return False + return True + def mine_data(self) -> MinedData: """ Mines data from GitHub, including repository information, issues, pull requests, commits, and releases. """ logger.info("Starting data mining from GitHub...") - data = MinedData() - - data.repository = self._safe_call(self.github_instance.get_repo)(ActionInputs.get_github_repository()) - if data.repository is None: + repo: Repository = self._safe_call(self.github_instance.get_repo)(ActionInputs.get_github_repository()) + if repo is None: logger.error("Repository not found: %s", ActionInputs.get_github_repository()) - return data + raise ValueError("Repository not found") + data = MinedData(repo) data.release = self.get_latest_release(data.repository) self._get_issues(data) diff --git a/release_notes_generator/model/commit_record.py b/release_notes_generator/model/commit_record.py index 0dd4949c..98dfbcc5 100644 --- a/release_notes_generator/model/commit_record.py +++ b/release_notes_generator/model/commit_record.py @@ -65,6 +65,9 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: return row + def contains_change_increment(self) -> bool: + return True + def get_rls_notes(self, line_marks: Optional[list[str]] = None) -> str: # Hint: direct commits does not support release notes return "" diff --git a/release_notes_generator/model/hierarchy_issue_record.py b/release_notes_generator/model/hierarchy_issue_record.py index b59fcc2f..889b2764 100644 --- a/release_notes_generator/model/hierarchy_issue_record.py +++ b/release_notes_generator/model/hierarchy_issue_record.py @@ -23,8 +23,8 @@ def __init__(self, issue: Issue, issue_labels: Optional[list[str]] = None, skip: super().__init__(issue, issue_labels, skip) self._level: int = 0 - self._sub_issues: dict[int, SubIssueRecord] = {} - self._sub_hierarchy_issues: dict[int, "HierarchyIssueRecord"] = {} + self._sub_issues: dict[str, SubIssueRecord] = {} + self._sub_hierarchy_issues: dict[str, "HierarchyIssueRecord"] = {} @property def level(self) -> int: @@ -120,7 +120,8 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: # add sub-hierarchy issues for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): - row = f"{row}\n{sub_hierarchy_issue.to_chapter_row()}" + if sub_hierarchy_issue.contains_change_increment(): + row = f"{row}\n{sub_hierarchy_issue.to_chapter_row()}" # add sub-issues if len(self._sub_issues) > 0: @@ -129,6 +130,9 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: if sub_issue.is_open: continue # only closed issues are reported in release notes + if not sub_issue.contains_change_increment(): + continue # skip sub-issues without change increment + sub_issue_block = "- " + sub_issue.to_chapter_row() ind_child_block = "\n".join( f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines() diff --git a/release_notes_generator/model/issue_record.py b/release_notes_generator/model/issue_record.py index 375028d0..fca5b0ce 100644 --- a/release_notes_generator/model/issue_record.py +++ b/release_notes_generator/model/issue_record.py @@ -122,6 +122,9 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: return row + def contains_change_increment(self) -> bool: + return self.pull_requests_count() > 0 + def get_rls_notes(self, line_marks: Optional[list[str]] = None) -> str: release_notes = "" detection_pattern = ActionInputs.get_release_notes_title() diff --git a/release_notes_generator/model/mined_data.py b/release_notes_generator/model/mined_data.py index 9df82a37..2306128d 100644 --- a/release_notes_generator/model/mined_data.py +++ b/release_notes_generator/model/mined_data.py @@ -38,8 +38,8 @@ class MinedData: """Class for keeping track of mined GitHub data.""" - def __init__(self): - self.repository: Optional[Repository] = None + def __init__(self, repository: Repository): + self.repository: Repository = repository self.release: Optional[GitRelease] = None self.issues: list[Issue] = [] self.pull_requests: list[PullRequest] = [] @@ -48,9 +48,9 @@ def __init__(self): def is_empty(self): """ - Checks if the mined data is empty, meaning no repository has been set. + Check if the mined data is empty (no issues, pull requests, or commits). Returns: - bool: True if the repository is None, False otherwise. + bool: True if empty, False otherwise. """ - return self.repository is None + return self.issues == [] and self.pull_requests == [] and self.commits == [] diff --git a/release_notes_generator/model/pull_request_record.py b/release_notes_generator/model/pull_request_record.py index 3bf53f08..44eabe6e 100644 --- a/release_notes_generator/model/pull_request_record.py +++ b/release_notes_generator/model/pull_request_record.py @@ -128,6 +128,9 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: row = f"{row}\n{self.get_rls_notes()}" return row + def contains_change_increment(self) -> bool: + return True + def get_rls_notes(self, line_marks: Optional[list[str]] = None) -> str: release_notes = "" detection_pattern = ActionInputs.get_release_notes_title() diff --git a/release_notes_generator/model/record.py b/release_notes_generator/model/record.py index 28367bee..14d5c8e5 100644 --- a/release_notes_generator/model/record.py +++ b/release_notes_generator/model/record.py @@ -117,6 +117,15 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: str: The string representation of the record in a chapter row. """ + @abstractmethod + def contains_change_increment(self) -> bool: + """ + Checks if the record contains a change increment. + + Returns: + bool: True if the record contains a change increment, False otherwise. + """ + @abstractmethod def get_labels(self) -> list[str]: """ diff --git a/release_notes_generator/record/factory/default_record_factory.py b/release_notes_generator/record/factory/default_record_factory.py index a3420610..3d98842e 100644 --- a/release_notes_generator/record/factory/default_record_factory.py +++ b/release_notes_generator/record/factory/default_record_factory.py @@ -19,12 +19,14 @@ """ import logging +from functools import singledispatchmethod, lru_cache from typing import cast, Optional from github import Github from github.Issue import Issue from github.PullRequest import PullRequest from github.Commit import Commit +from github.Repository import Repository from release_notes_generator.model.commit_record import CommitRecord from release_notes_generator.model.issue_record import IssueRecord @@ -46,71 +48,114 @@ class DefaultRecordFactory(RecordFactory): A class used to generate records for release notes. """ - def __init__(self, github: Github) -> None: + def __init__(self, github: Github, home_repository: Repository) -> None: rate_limiter = GithubRateLimiter(github) self._safe_call = safe_call_decorator(rate_limiter) + self._home_repository = home_repository - self._records: dict[int | str, Record] = {} + self._records: dict[str, Record] = {} - def generate(self, data: MinedData) -> dict[int | str, Record]: + @singledispatchmethod # pylint: disable=abstract-method + def get_id(self, obj) -> str: + """ + Get the ID of an object. + + Parameters: + obj: The object to get the ID of. + + Returns: + str: The ID of the object. + """ + raise NotImplementedError(f"Unsupported type: {type(obj)}") + + @get_id.register + def _(self, issue: Issue) -> str: + # delegate to a cached, hashable-only helper + return self._issue_id(issue.repository.full_name, issue.number) + + @staticmethod + @lru_cache(maxsize=2048) + def _issue_id(repo_full_name: str, number: int) -> str: + return f"{repo_full_name}#{number}" + + @get_id.register + def _(self, pull_request: PullRequest) -> str: + return f"{self._home_repository.full_name}#{pull_request.number}" + + @get_id.register + def _(self, commit: Commit) -> str: + return f"{commit.repository.full_name}@{commit.sha}" + + def generate(self, data: MinedData) -> dict[str, Record]: """ Generate records for release notes. Parameters: data (MinedData): The MinedData instance containing repository, issues, pull requests, and commits. Returns: - dict[int|str, Record]: A dictionary of records where the key is the issue or pull request number. + dict[str, Record]: A dictionary of records keyed by 'owner/repo#number' (or commit SHA for commits). """ - def register_pull_request(pull: PullRequest, skip_rec: bool) -> None: - detected_issues = extract_issue_numbers_from_body(pull) + def register_pull_request(pr: PullRequest, skip_rec: bool) -> None: + pid = self.get_id(pr) + pull_labels = [label.name for label in pr.get_labels()] + attached_any = False + detected_issues = extract_issue_numbers_from_body(pr, repository=data.repository) logger.debug("Detected issues - from body: %s", detected_issues) - linked = self._safe_call(get_issues_for_pr)(pull_number=pull.number) + linked = self._safe_call(get_issues_for_pr)(pull_number=pr.number) if linked: detected_issues.update(linked) logger.debug("Detected issues - merged: %s", detected_issues) - for parent_issue_number in detected_issues: + for parent_issue_id in detected_issues: # create an issue record if not present for PR parent - if parent_issue_number not in self._records: + if parent_issue_id not in self._records: logger.warning( - "Detected PR %d linked to issue %d which is not in the list of received issues. " + "Detected PR %d linked to issue %s which is not in the list of received issues. " "Fetching ...", - pull.number, - parent_issue_number, + pr.number, + parent_issue_id, ) + # dev note: here we expect that PR links to an issue in the same repository !!! + parent_issue_number = int(parent_issue_id.split("#")[1]) parent_issue = ( self._safe_call(data.repository.get_issue)(parent_issue_number) if data.repository else None ) if parent_issue is not None: self._create_record_for_issue(parent_issue) - if parent_issue_number in self._records: - cast(IssueRecord, self._records[parent_issue_number]).register_pull_request(pull) - logger.debug("Registering PR %d: %s to Issue %d", pull.number, pull.title, parent_issue_number) + if parent_issue_id in self._records: + cast(IssueRecord, self._records[parent_issue_id]).register_pull_request(pr) + logger.debug("Registering PR %d: %s to Issue %s", pr.number, pr.title, parent_issue_id) + attached_any = True else: logger.debug( - "Registering stand-alone PR %d: %s as mentioned Issue %d not found.", - pull.number, - pull.title, - parent_issue_number, + "Registering stand-alone PR %d: %s as mentioned Issue %s not found.", + pr.number, + pr.title, + parent_issue_id, ) + if not attached_any: + self._records[pid] = PullRequestRecord(pr, pull_labels, skip=skip_rec) + logger.debug("Created stand-alone PR record %s: %s (fallback)", pid, pr.title) + logger.debug("Registering issues to records...") for issue in data.issues: self._create_record_for_issue(issue) logger.debug("Registering pull requests to records...") for pull in data.pull_requests: + pid = self.get_id(pull) pull_labels = [label.name for label in pull.get_labels()] skip_record: bool = any(item in pull_labels for item in ActionInputs.get_skip_release_notes_labels()) linked_from_api = self._safe_call(get_issues_for_pr)(pull_number=pull.number) or set() - linked_from_body = extract_issue_numbers_from_body(pull) + linked_from_body = extract_issue_numbers_from_body(pull, data.repository) if not linked_from_api and not linked_from_body: - self._records[pull.number] = PullRequestRecord(pull, pull_labels, skip=skip_record) - logger.debug("Created record for PR %d: %s", pull.number, pull.title) + self._records[pid] = PullRequestRecord(pull, pull_labels, skip=skip_record) + logger.debug("Created record for PR %s: %s", pid, pull.title) else: - logger.debug("Registering pull number: %s, title : %s", pull.number, pull.title) + logger.debug("Registering pull number: %s, title : %s", pid, pull.title) register_pull_request(pull, skip_record) logger.debug("Registering commits to records...") @@ -147,7 +192,7 @@ def register_commit_to_record(self, commit: Commit) -> bool: rec_pr.register_commit(commit) return True - self._records[commit.sha] = CommitRecord(commit=commit) + self._records[self.get_id(commit)] = CommitRecord(commit=commit) logger.debug("Created record for direct commit %s: %s", commit.sha, commit.commit.message) return False @@ -166,5 +211,5 @@ def _create_record_for_issue(self, issue: Issue, issue_labels: Optional[list[str if issue_labels is None: issue_labels = [label.name for label in issue.get_labels()] skip_record = any(item in issue_labels for item in ActionInputs.get_skip_release_notes_labels()) - self._records[issue.number] = IssueRecord(issue=issue, skip=skip_record) - logger.debug("Created record for non hierarchy issue %d: %s", issue.number, issue.title) + self._records[iid := self.get_id(issue)] = IssueRecord(issue=issue, skip=skip_record, issue_labels=issue_labels) + logger.debug("Created record for non hierarchy issue '%s': %s", iid, issue.title) diff --git a/release_notes_generator/record/factory/issue_hierarchy_record_factory.py b/release_notes_generator/record/factory/issue_hierarchy_record_factory.py index 3de18fa2..b78c7707 100644 --- a/release_notes_generator/record/factory/issue_hierarchy_record_factory.py +++ b/release_notes_generator/record/factory/issue_hierarchy_record_factory.py @@ -22,8 +22,9 @@ from typing import cast, Optional from github import Github -from github.Issue import Issue +from github.Issue import Issue, SubIssue from github.PullRequest import PullRequest +from github.Repository import Repository from release_notes_generator.model.commit_record import CommitRecord from release_notes_generator.model.hierarchy_issue_record import HierarchyIssueRecord @@ -45,32 +46,65 @@ class IssueHierarchyRecordFactory(DefaultRecordFactory): A class used to generate records for release notes. """ - def __init__(self, github: Github) -> None: - super().__init__(github) + def __init__(self, github: Github, home_repository: Repository) -> None: + super().__init__(github, home_repository) - self.__registered_issues: set[int] = set() - self.__sub_issue_parents: dict[int, int] = {} + self.__registered_issues: set[str] = set() + self.__sub_issue_parents: dict[str, str] = {} self.__registered_commits: set[str] = set() - def generate(self, data: MinedData) -> dict[int | str, Record]: + self.__external_sub_issues: list[SubIssue] = [] + self.__local_sub_issues_checked: list[str] = [] + + def generate(self, data: MinedData) -> dict[str, Record]: """ Generate records for release notes. + Parameters: data (MinedData): The MinedData instance containing repository, issues, pull requests, and commits. + Returns: - dict[int|str, Record]: A dictionary of records where the key is the issue or pull request number. + dict[str, Record]: A dictionary of records indexed by their IDs. """ logger.debug("Creation of records started...") # First register all issues with sub-issues + issues_expansion: list[SubIssue] = [] for issue in data.issues: - if issue.number in self.__registered_issues: + if self.get_id(issue) in self.__registered_issues: continue - self._create_issue_record_using_sub_issues_existence(issue) + issues_expansion.extend(self._create_issue_record_using_sub_issues_existence(issue, data)) + + data.issues.extend(issues_expansion) + + # Second register all hierarchy issues from sub-issues + registered_before = -1 + while registered_before < len(self.__registered_issues): + registered_before = len(self.__registered_issues) + logger.debug("Looking for hierarchical issue among sub-issues...") + + issues_expansion = [] + for issue in data.issues: + iid = self.get_id(issue) + if iid in self.__registered_issues: + continue + + if iid in self.__sub_issue_parents and iid not in self.__local_sub_issues_checked: + issues_expansion.extend(self._create_issue_record_using_sub_issues_existence(issue, data)) + self.__local_sub_issues_checked.append(iid) + + data.issues.extend(issues_expansion) + + # Third register all external sub-issues + for ext_sub_issue in self.__external_sub_issues: + if self.get_id(ext_sub_issue) in self.__registered_issues: + continue + + self._create_record_for_sub_issue(ext_sub_issue) # Now register all issues without sub-issues for issue in data.issues: - if issue.number in self.__registered_issues: + if self.get_id(issue) in self.__registered_issues: continue self._create_issue_record_using_sub_issues_not_existence(issue) @@ -85,7 +119,7 @@ def generate(self, data: MinedData) -> dict[int | str, Record]: logger.debug("Registering direct commits to records...") for commit in data.commits: if commit.sha not in self.__registered_commits: - self._records[commit.sha] = CommitRecord(commit) + self._records[self.get_id(commit)] = CommitRecord(commit) # dev note: now we have all PRs and commits registered to issues or as stand-alone records # let build hierarchy @@ -109,26 +143,30 @@ def _register_pull_and_its_commits_to_issue(self, pull: PullRequest, data: Mined self.__registered_commits.update(c.sha for c in related_commits) linked_from_api = self._safe_call(get_issues_for_pr)(pull_number=pull.number) or set() - linked_from_body = extract_issue_numbers_from_body(pull) - pull_issues: list[int] = list(linked_from_api.union(linked_from_body)) + linked_from_body = extract_issue_numbers_from_body(pull, data.repository) + pull_issues: list[str] = list(linked_from_api.union(linked_from_body)) attached_any = False if len(pull_issues) > 0: - for issue_number in pull_issues: - if issue_number not in self._records: + for issue_id in pull_issues: + if issue_id not in self._records: logger.warning( - "Detected PR %d linked to issue %d which is not in the list of received issues. " + "Detected PR %d linked to issue %s which is not in the list of received issues. " "Fetching ...", pull.number, - issue_number, + issue_id, ) + # dev note: here we expect that PR links to an issue in the same repository !!! + # TODO - handle cross-repo issue linking - the split[0] can contain different repo name + # this problem is present on multiple places + issue_number = int(issue_id.split("#")[1]) parent_issue = self._safe_call(data.repository.get_issue)(issue_number) if data.repository else None if parent_issue is not None: - self._create_issue_record_using_sub_issues_existence(parent_issue) + self._create_issue_record_using_sub_issues_existence(parent_issue, data) - if issue_number in self._records and isinstance( - self._records[issue_number], (SubIssueRecord, HierarchyIssueRecord, IssueRecord) + if issue_id in self._records and isinstance( + self._records[issue_id], (SubIssueRecord, HierarchyIssueRecord, IssueRecord) ): - rec = cast(IssueRecord, self._records[issue_number]) + rec = cast(IssueRecord, self._records[issue_id]) rec.register_pull_request(pull) logger.debug("Registering pull number: %s, title : %s", pull.number, pull.title) @@ -142,22 +180,72 @@ def _register_pull_and_its_commits_to_issue(self, pull: PullRequest, data: Mined pr_rec = PullRequestRecord(pull, pull_labels, skip_record) for c in related_commits: # register commits to the PR record pr_rec.register_commit(c) - self._records[pull.number] = pr_rec - logger.debug("Created record for PR %d: %s", pull.number, pull.title) + pid = self.get_id(pull) + self._records[pid] = pr_rec + logger.debug("Created record for PR %s: %s", pid, pull.title) - def _create_issue_record_using_sub_issues_existence(self, issue: Issue) -> None: + def _create_issue_record_using_sub_issues_existence(self, issue: Issue, data: MinedData) -> list[SubIssue]: # use presence of sub-issues as a hint for hierarchy issue or non hierarchy issue sub_issues = list(issue.get_sub_issues()) + logger.debug("Found %d sub-issues for %d", len(sub_issues), issue.number) + new_local_issues: list[SubIssue] = [] if len(sub_issues) > 0: self._create_record_for_hierarchy_issue(issue) for si in sub_issues: - # register sub-issue and its parent for later hierarchy building - self.__sub_issue_parents[si.number] = issue.number # Note: GitHub now allows only 1 parent + siid = self.get_id(si) + + # check if sub-issue is from current repository + if si.repository.full_name != issue.repository.full_name: + # register sub-issue and its parent for later hierarchy building + # Note: GitHub now allows only 1 parent + self.__sub_issue_parents[siid] = self.get_id(issue) + + self.__external_sub_issues.append(si) + logger.debug( + "Detected sub-issue %d from different repository %s - adding as external sub-issue" + " for later processing", + si.number, + si.repository.full_name, + ) + + else: + use_issue = False + if ( + data.since + and si.state == IssueRecord.ISSUE_STATE_CLOSED + and si.closed_at + and data.since > si.closed_at + ): + logger.debug("Detected sub-issue %d closed in previous release.", si.number) + if len(list(si.get_sub_issues())) > 0: + use_issue = True + else: + self.__registered_issues.add(siid) + + elif si.state == IssueRecord.ISSUE_STATE_OPEN: + logger.debug("Detected sub-issue %d is still open.", si.number) + if len(list(si.get_sub_issues())) > 0: + use_issue = True + else: + self.__registered_issues.add(siid) + + elif si.state == IssueRecord.ISSUE_STATE_CLOSED: # issue is valid + use_issue = True + + else: + logger.warning("Detected unexpected sub-issue %d with parent %d", si.number, issue.number) + + if use_issue: + self.__sub_issue_parents[siid] = self.get_id(issue) + if si not in data.issues: + new_local_issues.append(si) + + return new_local_issues def _create_issue_record_using_sub_issues_not_existence(self, issue: Issue) -> None: # Expected to run after all issue with sub-issues are registered - if issue.number in self.__sub_issue_parents.keys(): # pylint: disable=consider-iterating-dictionary + if self.get_id(issue) in self.__sub_issue_parents.keys(): # pylint: disable=consider-iterating-dictionary self._create_record_for_sub_issue(issue) else: self._create_record_for_issue(issue) @@ -174,13 +262,14 @@ def _create_record_for_hierarchy_issue(self, i: Issue, issue_labels: Optional[li None """ # check for skip labels presence and skip when detected + iid = self.get_id(i) if issue_labels is None: issue_labels = self._get_issue_labels_mix_with_type(i) skip_record = any(item in issue_labels for item in ActionInputs.get_skip_release_notes_labels()) - self._records[i.number] = HierarchyIssueRecord(issue=i, skip=skip_record) - self.__registered_issues.add(i.number) - logger.debug("Created record for hierarchy issue %d: %s", i.number, i.title) + self._records[iid] = HierarchyIssueRecord(issue=i, skip=skip_record, issue_labels=issue_labels) + self.__registered_issues.add(iid) + logger.debug("Created record for hierarchy issue %s: %s", iid, i.title) def _get_issue_labels_mix_with_type(self, issue: Issue) -> list[str]: labels: list[str] = [label.name for label in issue.get_labels()] @@ -197,51 +286,52 @@ def _create_record_for_issue(self, issue: Issue, issue_labels: Optional[list[str issue_labels = self._get_issue_labels_mix_with_type(issue) super()._create_record_for_issue(issue, issue_labels) - self.__registered_issues.add(issue.number) + self.__registered_issues.add(self.get_id(issue)) def _create_record_for_sub_issue(self, issue: Issue, issue_labels: Optional[list[str]] = None) -> None: if issue_labels is None: issue_labels = self._get_issue_labels_mix_with_type(issue) + iid: str = self.get_id(issue) skip_record = any(item in issue_labels for item in ActionInputs.get_skip_release_notes_labels()) - logger.debug("Created record for sub issue %d: %s", issue.number, issue.title) - self.__registered_issues.add(issue.number) - self._records[issue.number] = SubIssueRecord(issue, issue_labels, skip_record) + logger.debug("Created record for sub issue %s: %s", iid, issue.title) + self.__registered_issues.add(iid) + self._records[iid] = SubIssueRecord(issue, issue_labels, skip_record) def _re_register_hierarchy_issues(self): - sub_issues_numbers = list(self.__sub_issue_parents.keys()) + logger.debug("Re-registering hierarchy issues ...") + + sub_issues_ids: list[str] = list(self.__sub_issue_parents.keys()) made_progress = False - for sub_issue_number in sub_issues_numbers: - # remove issue(sub_issue_number) from current records and add it to parent + for sub_issue_id in sub_issues_ids: + # remove issue(sub_issue_id) from current records and add it to parent # as sub-issue or sub-hierarchy-issue # but do it only for issue where parent issue number is not in _sub_issue_parents keys # Why? We building hierarchy from bottom. Access in records is very easy. - if sub_issue_number in self.__sub_issue_parents.values(): + if sub_issue_id in self.__sub_issue_parents.values(): continue - parent_issue_nr: int = self.__sub_issue_parents[sub_issue_number] - parent_rec = cast(HierarchyIssueRecord, self._records[parent_issue_nr]) - sub_rec = self._records[sub_issue_number] + parent_issue_id: str = self.__sub_issue_parents[sub_issue_id] + parent_rec = cast(HierarchyIssueRecord, self._records[parent_issue_id]) + sub_rec = self._records[sub_issue_id] if isinstance(sub_rec, SubIssueRecord): - parent_rec.sub_issues[sub_issue_number] = sub_rec # add to parent as SubIssueRecord - self._records.pop(sub_issue_number) # remove from main records as it is sub-one - self.__sub_issue_parents.pop(sub_issue_number) # remove from sub-parents as it is now sub-one + parent_rec.sub_issues[sub_issue_id] = sub_rec # add to parent as SubIssueRecord + self._records.pop(sub_issue_id) # remove from main records as it is sub-one + self.__sub_issue_parents.pop(sub_issue_id) # remove from sub-parents as it is now sub-one made_progress = True elif isinstance(sub_rec, HierarchyIssueRecord): - parent_rec.sub_hierarchy_issues[sub_issue_number] = ( - sub_rec # add to parent as 'Sub' HierarchyIssueRecord - ) - self._records.pop(sub_issue_number) # remove from main records as it is sub-one - self.__sub_issue_parents.pop(sub_issue_number) # remove from sub-parents as it is now sub-one + parent_rec.sub_hierarchy_issues[sub_issue_id] = sub_rec # add to parent as 'Sub' HierarchyIssueRecord + self._records.pop(sub_issue_id) # remove from main records as it is sub-one + self.__sub_issue_parents.pop(sub_issue_id) # remove from sub-parents as it is now sub-one made_progress = True else: logger.error( - "Detected IssueRecord in position of SubIssueRecord - leaving as standalone" " and dropping mapping" + "Detected IssueRecord in position of SubIssueRecord - leaving as standalone and dropping mapping" ) # Avoid infinite recursion by removing the unresolved mapping - self.__sub_issue_parents.pop(sub_issue_number, None) + self.__sub_issue_parents.pop(sub_issue_id, None) if self.__sub_issue_parents and made_progress: self._re_register_hierarchy_issues() diff --git a/release_notes_generator/record/factory/record_factory.py b/release_notes_generator/record/factory/record_factory.py index 62fdbd00..ea3ed70f 100644 --- a/release_notes_generator/record/factory/record_factory.py +++ b/release_notes_generator/record/factory/record_factory.py @@ -32,14 +32,11 @@ class RecordFactory(metaclass=abc.ABCMeta): """ @abc.abstractmethod - def generate(self, data: MinedData) -> dict[int | str, Record]: + def generate(self, data: MinedData) -> dict[str, Record]: """ Generate records for release notes. Parameters: data (MinedData): The MinedData instance containing repository, issues, pull requests, and commits. Returns: - dict[int|str, Record]: A dictionary of records where the key is the issue or pull request number. + dict[str, Record]: A dictionary of records where the key is 'org_name/repo_name#number'. """ - - # TODO - do review of children and decide if more useful method could be defined here for inheritation - # fix unit test first to detect breaking changes diff --git a/release_notes_generator/utils/pull_request_utils.py b/release_notes_generator/utils/pull_request_utils.py index 956797d3..17c5789d 100644 --- a/release_notes_generator/utils/pull_request_utils.py +++ b/release_notes_generator/utils/pull_request_utils.py @@ -23,12 +23,13 @@ import requests from github.PullRequest import PullRequest +from github.Repository import Repository from release_notes_generator.action_inputs import ActionInputs from release_notes_generator.utils.constants import ISSUES_FOR_PRS, LINKED_ISSUES_MAX -def extract_issue_numbers_from_body(pr: PullRequest) -> set[int]: +def extract_issue_numbers_from_body(pr: PullRequest, repository: Repository) -> set[str]: """ Extracts the numbers of the issues mentioned in the body of the pull request. @@ -46,18 +47,21 @@ def extract_issue_numbers_from_body(pr: PullRequest) -> set[int]: # Extract the issue numbers from the matches issue_numbers = {int(match[-1]) for match in issue_matches} + issue_ids = {f"{repository.full_name}#{number}" for number in issue_numbers} - return issue_numbers + return issue_ids @lru_cache(maxsize=1024) -def get_issues_for_pr(pull_number: int) -> set[int]: +def get_issues_for_pr(pull_number: int) -> set[str]: """Fetch closing issue numbers for a PR via GitHub GraphQL and return them as a set.""" github_api_url = "https://api.github.com/graphql" + owner = ActionInputs.get_github_owner() + name = ActionInputs.get_github_repo_name() query = ISSUES_FOR_PRS.format( number=pull_number, - owner=ActionInputs.get_github_owner(), - name=ActionInputs.get_github_repo_name(), + owner=owner, + name=name, first=LINKED_ISSUES_MAX, ) headers = { @@ -71,4 +75,8 @@ def get_issues_for_pr(pull_number: int) -> set[int]: if data.get("errors"): raise RuntimeError(f"GitHub GraphQL errors: {data['errors']}") - return {node["number"] for node in data["data"]["repository"]["pullRequest"]["closingIssuesReferences"]["nodes"]} + # TODO - mine owner and use it in ID + return { + f"{owner}/{name}#{node['number']}" + for node in data["data"]["repository"]["pullRequest"]["closingIssuesReferences"]["nodes"] + } diff --git a/tests/conftest.py b/tests/conftest.py index 0bcd44cc..c6c89ccc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,7 @@ import pytest from github import Github, IssueType +from github.Commit import Commit from github.GitRelease import GitRelease from github.Issue import Issue from github.PullRequest import PullRequest @@ -146,6 +147,7 @@ def mock_issue_open(mocker): issue.title = "I1 open" issue.state_reason = None issue.body = "I1 open" + issue.repository.full_name = "org/repo" label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -164,6 +166,7 @@ def mock_issue_open_2(mocker): issue.title = "I2 open" issue.state_reason = None issue.body = "I2 open" + issue.repository.full_name = "org/repo" label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -182,6 +185,8 @@ def mock_issue_closed(mocker): issue.number = 121 issue.body = "Some issue body text" issue.get_sub_issues.return_value = [] + issue.repository.full_name = "org/repo" + issue.closed_at = datetime.now() label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -199,6 +204,8 @@ def mock_issue_closed_i1_bug(mocker): issue.title = "I1+bug" issue.number = 122 issue.body = "Some issue body text\nRelease Notes:\n- Fixed bug\n- Improved performance\n+ More nice code\n * Awesome architecture" + issue.repository.full_name = "org/repo" + issue.closed_at = datetime.now() label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -215,6 +222,8 @@ def mock_issue_closed_i1_bug_and_skip(mocker): issue.state = IssueRecord.ISSUE_STATE_CLOSED issue.title = "I1+bug" issue.number = 122 + issue.repository.full_name = "org/repo" + issue.closed_at = datetime.now() label1 = mocker.Mock(spec=MockLabel) label1.name = "skip-release-notes" @@ -235,6 +244,8 @@ def mock_open_sub_issue(mocker): issue.body = "I400 open\nRelease Notes:\n- Hierarchy level release note" issue.type = None issue.created_at = datetime.now() + issue.closed_at = None + issue.repository.full_name = "org/repo" issue.get_labels.return_value = [] issue.get_sub_issues.return_value = [] @@ -252,6 +263,8 @@ def mock_closed_sub_issue(mocker): issue.body = "I450 closed\nRelease Notes:\n- Hierarchy level release note" issue.type = None issue.created_at = datetime.now() + issue.closed_at = datetime.now() + issue.repository.full_name = "org/repo" issue.get_labels.return_value = [] issue.get_sub_issues.return_value = [] @@ -269,6 +282,8 @@ def mock_open_hierarchy_issue(mocker): issue.body = "I300 open\nRelease Notes:\n- Hierarchy level release note" issue.type = None issue.created_at = datetime.now() + issue.closed_at = None + issue.repository.full_name = "org/repo" issue.get_labels.return_value = [] issue.get_sub_issues.return_value = [] @@ -288,6 +303,8 @@ def mock_open_hierarchy_issue_epic(mocker): issue.body = "I200 open\nRelease Notes:\n- Epic level release note" issue.type = issue_type issue.created_at = datetime.now() + issue.closed_at = None + issue.repository.full_name = "org/repo" label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -311,6 +328,8 @@ def mock_open_hierarchy_issue_feature(mocker): issue.body = "HI201 open\nRelease Notes:\n- Feature level release note" issue.type = issue_type issue.created_at = datetime.now() + issue.closed_at = None + issue.repository.full_name = "org/repo" label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -333,6 +352,8 @@ def mock_closed_issue_type_task(mocker): issue.body = "Some issue body text" issue.type = issue_type issue.created_at = datetime.now() + issue.closed_at = datetime.now() + issue.repository.full_name = "org/repo" label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -352,6 +373,8 @@ def mock_closed_issue_type_none(mocker): issue.body = "Some sub issue body text" issue.type = None issue.created_at = datetime.now() + issue.closed_at = datetime.now() + issue.repository.full_name = "org/repo" label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -374,6 +397,8 @@ def mock_closed_issue_type_bug(mocker): issue.body = "Some issue body text\nRelease Notes:\n- Fixed bug\n- Improved performance\n+ More nice code\n * Awesome architecture" issue.type = issue_type issue.created_at = datetime.now() + issue.closed_at = datetime.now() + issue.repository.full_name = "org/repo" label1 = mocker.Mock(spec=MockLabel) label1.name = "label1" @@ -573,16 +598,17 @@ def mock_pull_no_rls_notes(mocker): # Fixtures for GitHub Commit(s) @pytest.fixture def mock_commit(mocker): - commit = mocker.Mock() + commit = mocker.Mock(spec=Commit) commit.author.login = "author" commit.sha = "merge_commit_sha" commit.commit.message = "Fixed bug" + commit.repository.full_name = "org/repo" return commit @pytest.fixture def mined_data_isolated_record_types_no_labels_no_type_defined( - mock_issue_closed, mock_pull_closed, mock_pull_merged, mock_commit, + mock_repo, mock_issue_closed, mock_pull_closed, mock_pull_merged, mock_commit, mock_open_hierarchy_issue, mock_open_sub_issue, mock_closed_sub_issue ): # - single issue record (closed) @@ -592,7 +618,7 @@ def mined_data_isolated_record_types_no_labels_no_type_defined( # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record - data = MinedData() + data = MinedData(mock_repo) # single issue record (closed) solo_closed_issue = copy.deepcopy(mock_issue_closed) # 121 @@ -810,7 +836,7 @@ def mined_data_isolated_record_types_with_labels_with_type_defined(mocker, mined t_epic = mocker.Mock(spec=IssueType) t_epic.name = "Epic" t_feature = mocker.Mock(spec=IssueType) - t_feature.name = "feature" + t_feature.name = "Feature" t_task = mocker.Mock(spec=IssueType) t_task.name = "Task" t_bug = mocker.Mock(spec=IssueType) diff --git a/tests/release_notes/builder/test_release_notes_builder.py b/tests/release_notes/builder/test_release_notes_builder.py index cbb47ec0..50f7f127 100644 --- a/tests/release_notes/builder/test_release_notes_builder.py +++ b/tests/release_notes/builder/test_release_notes_builder.py @@ -426,7 +426,7 @@ - Epic: _HI304 open_ #304 - _Release Notes_: - Hierarchy level release note - - feature: _HI350 open_ #350 + - Feature: _HI350 open_ #350 - _Release Notes_: - Sub-hierarchy level release note - #453 _SI453 closed_ in #152 @@ -480,7 +480,7 @@ - 🔔 Epic: _HI304 open_ #304 - _Release Notes_: - Hierarchy level release note - - 🔔 feature: _HI350 open_ #350 + - 🔔 Feature: _HI350 open_ #350 - _Release Notes_: - Sub-hierarchy level release note - 🔔 #453 _SI453 closed_ in #152 @@ -1585,7 +1585,9 @@ def test_build_closed_pr_service_chapter_without_issue_with_skip_label_on_pr( def test_build_hierarchy_rls_notes_no_labels_no_type( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_no_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, + mined_data_isolated_record_types_no_labels_no_type_defined ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_NO_LABELS_NO_TYPE @@ -1601,7 +1603,7 @@ def test_build_hierarchy_rls_notes_no_labels_no_type( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_no_labels_no_type_defined) builder = ReleaseNotesBuilder( @@ -1616,7 +1618,8 @@ def test_build_hierarchy_rls_notes_no_labels_no_type( def test_build_hierarchy_rls_notes_with_labels_no_type( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_no_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_no_type_defined ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_WITH_LABELS_NO_TYPE @@ -1632,7 +1635,7 @@ def test_build_hierarchy_rls_notes_with_labels_no_type( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_with_labels_no_type_defined) builder = ReleaseNotesBuilder( @@ -1643,15 +1646,12 @@ def test_build_hierarchy_rls_notes_with_labels_no_type( actual_release_notes = builder.build() - print("XXX") - print(actual_release_notes) - print("YYY") - assert expected_release_notes == actual_release_notes def test_build_hierarchy_rls_notes_no_labels_with_type( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_with_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_with_type_defined ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_NO_LABELS_WITH_TYPE @@ -1667,7 +1667,7 @@ def test_build_hierarchy_rls_notes_no_labels_with_type( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_no_labels_with_type_defined) builder = ReleaseNotesBuilder( @@ -1681,7 +1681,8 @@ def test_build_hierarchy_rls_notes_no_labels_with_type( assert expected_release_notes == actual_release_notes def test_build_hierarchy_rls_notes_with_labels_with_type( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_with_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_with_type_defined ): expected_release_notes = RELEASE_NOTES_DATA_HIERARCHY_WITH_LABELS_WITH_TYPE @@ -1697,7 +1698,7 @@ def test_build_hierarchy_rls_notes_with_labels_with_type( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_with_labels_with_type_defined) builder = ReleaseNotesBuilder( @@ -1712,7 +1713,8 @@ def test_build_hierarchy_rls_notes_with_labels_with_type( def test_build_no_hierarchy_rls_notes_no_labels_no_type_with_hierarchy_data( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_no_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_no_type_defined ): expected_release_notes = RELEASE_NOTES_NO_DATA_HIERARCHY_NO_LABELS_NO_TYPE @@ -1728,7 +1730,7 @@ def test_build_no_hierarchy_rls_notes_no_labels_no_type_with_hierarchy_data( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = DefaultRecordFactory(github=mock_github_client) + factory = DefaultRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_no_labels_no_type_defined) builder = ReleaseNotesBuilder( @@ -1742,7 +1744,8 @@ def test_build_no_hierarchy_rls_notes_no_labels_no_type_with_hierarchy_data( assert expected_release_notes == actual_release_notes def test_build_no_hierarchy_rls_notes_with_labels_no_type_with_hierarchy_data( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_no_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_no_type_defined ): expected_release_notes = RELEASE_NOTES_NO_DATA_HIERARCHY_WITH_LABELS_NO_TYPE @@ -1757,7 +1760,7 @@ def test_build_no_hierarchy_rls_notes_with_labels_no_type_with_hierarchy_data( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = DefaultRecordFactory(github=mock_github_client) + factory = DefaultRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_with_labels_no_type_defined) builder = ReleaseNotesBuilder( @@ -1772,7 +1775,8 @@ def test_build_no_hierarchy_rls_notes_with_labels_no_type_with_hierarchy_data( def test_build_no_hierarchy_rls_notes_no_labels_with_type_with_hierarchy_data( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_with_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_no_labels_with_type_defined ): expected_release_notes = RELEASE_NOTES_NO_DATA_HIERARCHY_NO_LABELS_WITH_TYPE @@ -1788,7 +1792,7 @@ def test_build_no_hierarchy_rls_notes_no_labels_with_type_with_hierarchy_data( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = DefaultRecordFactory(github=mock_github_client) + factory = DefaultRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_no_labels_with_type_defined) builder = ReleaseNotesBuilder( @@ -1803,7 +1807,8 @@ def test_build_no_hierarchy_rls_notes_no_labels_with_type_with_hierarchy_data( def test_build_no_hierarchy_rls_notes_with_labels_with_type_with_hierarchy_data( - mocker, custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_with_type_defined + mocker, mock_repo, + custom_chapters_not_print_empty_chapters, mined_data_isolated_record_types_with_labels_with_type_defined ): expected_release_notes = RELEASE_NOTES_NO_DATA_HIERARCHY_WITH_LABELS_WITH_TYPE @@ -1819,7 +1824,7 @@ def test_build_no_hierarchy_rls_notes_with_labels_with_type_with_hierarchy_data( mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = DefaultRecordFactory(github=mock_github_client) + factory = DefaultRecordFactory(github=mock_github_client, home_repository=mock_repo) records = factory.generate(mined_data_isolated_record_types_with_labels_with_type_defined) builder = ReleaseNotesBuilder( diff --git a/tests/release_notes/model/test_record.py b/tests/release_notes/model/test_record.py index 95dc0dd3..a1f17145 100644 --- a/tests/release_notes/model/test_record.py +++ b/tests/release_notes/model/test_record.py @@ -55,6 +55,9 @@ def get_rls_notes(self, line_marks=None): def get_labels(self) -> list[str]: return self._labels + def contains_change_increment(self) -> bool: + return True + def test_is_present_in_chapters(): rec = DummyRecord() assert not rec.is_present_in_chapters diff --git a/tests/release_notes/record/factory/test_default_record_factory.py b/tests/release_notes/record/factory/test_default_record_factory.py index 51a7fcdb..7cf9250b 100644 --- a/tests/release_notes/record/factory/test_default_record_factory.py +++ b/tests/release_notes/record/factory/test_default_record_factory.py @@ -83,6 +83,7 @@ def setup_issues_no_pulls_no_commits(mocker): mock_git_issue1.state = "open" mock_git_issue1.created_at = datetime.now() mock_git_issue1.get_labels.return_value = [] + mock_git_issue1.repository.full_name = "org/repo" mock_git_issue2 = mocker.Mock(spec=Issue) mock_git_issue2.id = 2 @@ -92,11 +93,12 @@ def setup_issues_no_pulls_no_commits(mocker): mock_git_issue2.state = "closed" mock_git_issue2.created_at = datetime.now() mock_git_issue2.get_labels.return_value = [] + mock_git_issue2.repository.full_name = "org/repo" return mock_git_issue1, mock_git_issue2 -def setup_issues_pulls_commits(mocker): +def setup_issues_pulls_commits(mocker, mock_repo): # Mock GitHub API objects mock_git_issue1 = mocker.Mock(spec=Issue) mock_git_issue1.id = 1 @@ -106,6 +108,7 @@ def setup_issues_pulls_commits(mocker): mock_git_issue1.state = "open" mock_git_issue1.created_at = datetime.now() mock_git_issue1.get_labels.return_value = [] + mock_git_issue1.repository.full_name = "org/repo" mock_git_issue2 = mocker.Mock(spec=Issue) mock_git_issue2.id = 2 @@ -115,6 +118,7 @@ def setup_issues_pulls_commits(mocker): mock_git_issue2.state = "closed" mock_git_issue2.created_at = datetime.now() mock_git_issue2.get_labels.return_value = [] + mock_git_issue2.repository.full_name = "org/repo" mock_git_pr1 = mocker.Mock(spec=PullRequest) mock_git_pr1.id = 101 @@ -148,11 +152,13 @@ def setup_issues_pulls_commits(mocker): mock_git_commit1.sha = "abc123" mock_git_commit1.commit.message = "Commit message 1" mock_git_commit1.author.login = "author1" + mock_git_commit1.repository = mock_repo mock_git_commit2 = mocker.Mock(spec=Commit) mock_git_commit2.sha = "def456" mock_git_commit2.commit.message = "Commit message 2" mock_git_commit2.author.login = "author2" + mock_git_commit2.repository = mock_repo return mock_git_issue1, mock_git_issue2, mock_git_pr1, mock_git_pr2, mock_git_commit1, mock_git_commit2 @@ -163,42 +169,42 @@ def wrapper(fn): return fn return wrapper -def mock_get_issues_for_pr(pull_number: int) -> list[int]: +def mock_get_issues_for_pr(pull_number: int) -> list[str]: if pull_number == 101: - return [1] + return ['org/repo#1'] elif pull_number == 102: - return [2] + return ['org/repo#2'] return [] def test_generate_with_issues_and_pulls_and_commits(mocker, mock_repo): mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) mock_github_client = mocker.Mock(spec=Github) - issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker) + issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker, mock_repo) mock_rate_limit = mocker.Mock() mock_rate_limit.rate.remaining = 10 mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - data = MinedData() + data = MinedData(mock_repo) data.issues = [issue1] data.pull_requests = [pr1] commit3 = mocker.Mock(spec=Commit) commit3.sha = "ghi789" + commit3.repository = mock_repo data.commits = [commit1, commit2, commit3] - data.repository = mock_repo - records = DefaultRecordFactory(mock_github_client).generate(data) + records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) # Check if records for issues and PRs were created assert len(records) == 3 - assert 1 in records - assert not records[1].skip + assert 'org/repo#1' in records + assert not records['org/repo#1'].skip # Verify the record creation - assert records[1].__class__ is IssueRecord - rec_i1 = cast(IssueRecord, records[1]) + assert records['org/repo#1'].__class__ is IssueRecord + rec_i1 = cast(IssueRecord, records['org/repo#1']) # Verify that PRs are registered assert 1 == rec_i1.pull_requests_count() @@ -211,7 +217,7 @@ def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, moc mocker.patch("release_notes_generator.record.factory.default_record_factory.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes"]) mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) mock_github_client = mocker.Mock(spec=Github) - issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker) + issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker, mock_repo) # Define mock labels mock_label = mocker.Mock() @@ -221,32 +227,32 @@ def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, moc commit3 = mocker.Mock(spec=Commit) commit3.sha = "ghi789" + commit3.repository.full_name = "org/repo" - data = MinedData() - data.repository = mock_repo + data = MinedData(mock_repo) data.issues = [issue1, issue2] data.pull_requests = [pr1, pr2] data.commits = [commit1, commit2, commit3] - records = DefaultRecordFactory(mock_github_client).generate(data) + records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) # Check if records for issues and PRs were created assert len(records) == 3 - assert 1 in records - assert 2 in records - assert 'ghi789' in records + assert 'org/repo#1' in records + assert 'org/repo#2' in records + assert 'org/repo@ghi789' in records # Verify the record creation - assert isinstance(records[1], IssueRecord) - assert isinstance(records[2], IssueRecord) - assert isinstance(records['ghi789'], CommitRecord) + assert isinstance(records['org/repo#1'], IssueRecord) + assert isinstance(records['org/repo#2'], IssueRecord) + assert isinstance(records['org/repo@ghi789'], CommitRecord) - assert records[1].skip # skip label applied 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) + assert records['org/repo#1'].skip # skip label applied to issue as the record was created from issue + assert not records['org/repo#2'].skip # skip label is present only on inner PR but record create from issues (leading) - rec_i1 = cast(IssueRecord, records[1]) - rec_i2 = cast(IssueRecord, records[2]) + rec_i1 = cast(IssueRecord, records['org/repo#1']) + rec_i2 = cast(IssueRecord, records['org/repo#2']) # Verify that PRs are registered assert 1 == rec_i1.pull_requests_count() @@ -258,14 +264,14 @@ def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, moc # Verify that commits are registered assert commit1 == rec_i1.get_commit(101, 'abc123') assert commit2 == rec_i2.get_commit(102, 'def456') - assert commit3 == cast(CommitRecord, records['ghi789']).commit + assert commit3 == cast(CommitRecord, records['org/repo@ghi789']).commit def test_generate_with_no_commits(mocker, mock_repo): mock_github_client = mocker.Mock(spec=Github) - data = MinedData() + data = MinedData(mock_repo) # pylint: disable=unused-variable - issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker) + issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker, mock_repo) data.issues = [issue1] data.pull_requests = [pr1] # PR linked to a non-fetched issues (due to since condition) @@ -276,18 +282,17 @@ def test_generate_with_no_commits(mocker, mock_repo): mock_repo.get_issue.return_value = issue2 data.commits = [] # No commits - data.repository = mock_repo - mocker.patch("release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value=[2]) - records = DefaultRecordFactory(mock_github_client).generate(data) + mocker.patch("release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value=['org/repo#2']) + records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) assert 2 == len(records) # Verify the record creation - assert isinstance(records[1], IssueRecord) - assert isinstance(records[2], IssueRecord) + assert isinstance(records['org/repo#1'], IssueRecord) + assert isinstance(records['org/repo#2'], IssueRecord) - rec_issue1 = cast(IssueRecord,records[1]) - rec_issue2 = cast(IssueRecord,records[2]) + rec_issue1 = cast(IssueRecord,records['org/repo#1']) + rec_issue2 = cast(IssueRecord,records['org/repo#2']) # Verify that PRs are registered assert 1 == rec_issue1.pull_requests_count() @@ -297,9 +302,9 @@ def test_generate_with_no_commits(mocker, mock_repo): def test_generate_with_no_commits_with_wrong_issue_number_in_pull_body_mention(mocker, mock_repo): mock_github_client = mocker.Mock(spec=Github) - data = MinedData() + data = MinedData(mock_repo) # pylint: disable=unused-variable - issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker) + issue1, issue2, pr1, pr2, commit1, commit2 = setup_issues_pulls_commits(mocker, mock_repo) pr1.body = "Closes #2" data.issues = [issue1] data.pull_requests = [pr1] # PR linked to a non-fetched issues (due to since condition) @@ -311,18 +316,17 @@ def test_generate_with_no_commits_with_wrong_issue_number_in_pull_body_mention(m mock_repo.get_issue.return_value = issue2 data.commits = [] # No commits - data.repository = mock_repo - mocker.patch("release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value=[2]) - records = DefaultRecordFactory(mock_github_client).generate(data) + mocker.patch("release_notes_generator.record.factory.default_record_factory.get_issues_for_pr", return_value=['org/repo#2']) + records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) assert 2 == len(records) # Verify the record creation - assert isinstance(records[1], IssueRecord) - assert isinstance(records[2], IssueRecord) + assert isinstance(records['org/repo#1'], IssueRecord) + assert isinstance(records['org/repo#2'], IssueRecord) - rec_issue1 = cast(IssueRecord, records[1]) - rec_issue2 = cast(IssueRecord, records[2]) + rec_issue1 = cast(IssueRecord, records['org/repo#1']) + rec_issue2 = cast(IssueRecord, records['org/repo#2']) # Verify that PRs are registered assert 0 == rec_issue1.pull_requests_count() @@ -332,35 +336,35 @@ def test_generate_with_no_commits_with_wrong_issue_number_in_pull_body_mention(m def mock_safe_call_decorator_no_issues(_rate_limiter): def wrapper(fn): - if fn.__name__ == "get_issues_for_pr": + if getattr(fn, "__name__", None) == "get_issues_for_pr": return mock_get_issues_for_pr_no_issues return fn return wrapper -def mock_get_issues_for_pr_no_issues(pull_number: int) -> list[int]: +def mock_get_issues_for_pr_no_issues(pull_number: int) -> list[str]: return [] -def test_generate_with_no_issues(mocker, request): +def test_generate_with_no_issues(mocker, mock_repo, request): mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator_no_issues) mock_github_client = mocker.Mock(spec=Github) - data = MinedData() + data = MinedData(mock_repo) pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) data.pull_requests = [pr1, pr2] data.commits = [commit1, commit2] data.repository = request.getfixturevalue("mock_repo") data.issues = [] # No issues - records = DefaultRecordFactory(mock_github_client).generate(data) + records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) # Verify the record creation assert 2 == len(records) - assert isinstance(records[101], PullRequestRecord) - assert isinstance(records[102], PullRequestRecord) + assert isinstance(records['org/repo#101'], PullRequestRecord) + assert isinstance(records['org/repo#102'], PullRequestRecord) - rec_pr1 = cast(PullRequestRecord, records[101]) - rec_pr2 = cast(PullRequestRecord, records[102]) + rec_pr1 = cast(PullRequestRecord, records['org/repo#101']) + rec_pr2 = cast(PullRequestRecord, records['org/repo#102']) # Verify that PRs are registered assert pr1 == rec_pr1.pull_request @@ -372,11 +376,11 @@ def test_generate_with_no_issues(mocker, request): assert commit1 == rec_pr1.get_commit('abc123') assert commit2 == rec_pr2.get_commit('def456') -def test_generate_with_no_issues_skip_labels(mocker, request): +def test_generate_with_no_issues_skip_labels(mocker, mock_repo, request): mocker.patch("release_notes_generator.record.factory.default_record_factory.ActionInputs.get_skip_release_notes_labels", return_value=["skip-release-notes", "another-skip-label"]) mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator_no_issues) mock_github_client = mocker.Mock(spec=Github) - data = MinedData() + data = MinedData(mock_repo) pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) # Define mock labels @@ -393,19 +397,19 @@ def test_generate_with_no_issues_skip_labels(mocker, request): data.repository = request.getfixturevalue("mock_repo") data.issues = [] # No issues - records = DefaultRecordFactory(mock_github_client).generate(data) + records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) # Verify the record creation assert 2 == len(records) - assert isinstance(records[101], PullRequestRecord) - assert isinstance(records[102], PullRequestRecord) + assert isinstance(records['org/repo#101'], PullRequestRecord) + assert isinstance(records['org/repo#102'], PullRequestRecord) - assert records[101].skip - assert records[102].skip + assert records['org/repo#101'].skip + assert records['org/repo#102'].skip - rec_pr1 = cast(PullRequestRecord, records[101]) - rec_pr2 = cast(PullRequestRecord, records[102]) + rec_pr1 = cast(PullRequestRecord, records['org/repo#101']) + rec_pr2 = cast(PullRequestRecord, records['org/repo#102']) # Verify that PRs are registered assert 1 == rec_pr1.commits_count() @@ -417,22 +421,21 @@ def test_generate_with_no_issues_skip_labels(mocker, request): def test_generate_with_no_pulls(mocker, mock_repo): mock_github_client = mocker.Mock(spec=Github) - data = MinedData() + data = MinedData(mock_repo) issue1, issue2 = setup_issues_no_pulls_no_commits(mocker) data.issues = [issue1, issue2] - data.repository = mock_repo data.pull_requests = [] # No pull requests data.commits = [] # No commits - records = DefaultRecordFactory(mock_github_client).generate(data) + records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) # Verify the record creation assert 2 == len(records) - assert isinstance(records[1], IssueRecord) - assert isinstance(records[2], IssueRecord) + assert isinstance(records['org/repo#1'], IssueRecord) + assert isinstance(records['org/repo#2'], IssueRecord) # Verify that PRs are registered - assert 0 == cast(IssueRecord, records[1]).pull_requests_count() - assert 0 == cast(IssueRecord, records[2]).pull_requests_count() + assert 0 == cast(IssueRecord, records['org/repo#1']).pull_requests_count() + assert 0 == cast(IssueRecord, records['org/repo#2']).pull_requests_count() def mock_safe_call_decorator_wrong_issue_number(_rate_limiter): diff --git a/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py b/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py index b3447239..3aa659f6 100644 --- a/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py +++ b/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py @@ -29,10 +29,10 @@ # generate -def test_generate_no_input_data(mocker): +def test_generate_no_input_data(mocker, mock_repo): mock_github_client = mocker.Mock(spec=Github) - factory = IssueHierarchyRecordFactory(github=mock_github_client) - data = MinedData() + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) + data = MinedData(mock_repo) result = factory.generate(data) @@ -45,7 +45,8 @@ def test_generate_no_input_data(mocker): # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mined_data_isolated_record_types_no_labels_no_type_defined): +def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mock_repo, + mined_data_isolated_record_types_no_labels_no_type_defined): mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) mock_github_client = mocker.Mock(spec=Github) @@ -54,57 +55,57 @@ def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mined_ mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) result = factory.generate(mined_data_isolated_record_types_no_labels_no_type_defined) assert 8 == len(result) - assert {121, 301, 302, 303, 304, 123, 124, "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result[121], IssueRecord) - assert isinstance(result[301], HierarchyIssueRecord) - assert isinstance(result[302], HierarchyIssueRecord) - assert isinstance(result[303], HierarchyIssueRecord) - assert isinstance(result[304], HierarchyIssueRecord) - assert isinstance(result[123], PullRequestRecord) - assert isinstance(result[124], PullRequestRecord) - assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - - rec_i = cast(IssueRecord, result[121]) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + + assert isinstance(result['org/repo#121'], IssueRecord) + assert isinstance(result['org/repo#301'], HierarchyIssueRecord) + assert isinstance(result['org/repo#302'], HierarchyIssueRecord) + assert isinstance(result['org/repo#303'], HierarchyIssueRecord) + assert isinstance(result['org/repo#304'], HierarchyIssueRecord) + assert isinstance(result['org/repo#123'], PullRequestRecord) + assert isinstance(result['org/repo#124'], PullRequestRecord) + assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + + rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result[301]) + rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues[450].pull_requests_count() + assert 1 == len(rec_hi_1.sub_issues.values()) + assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() assert 0 == rec_hi_1.level - rec_hi_2 = cast(HierarchyIssueRecord, result[302]) + rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues[451].pull_requests_count() + assert 1 == len(rec_hi_2.sub_issues.values()) + assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() assert 0 == rec_hi_2.level - rec_hi_3 = cast(HierarchyIssueRecord, result[303]) + rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues[452].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues[452].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == len(rec_hi_3.sub_issues.values()) + assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() + assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message assert 0 == rec_hi_3.level - rec_hi_4 = cast(HierarchyIssueRecord, result[304]) + rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues[350].sub_issues[453].get_commit(152, "merge_commit_sha_152").commit.message + assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message assert 0 == rec_hi_4.level - rec_hi_5 = cast(HierarchyIssueRecord, result[304]) - assert 1 == rec_hi_5.sub_hierarchy_issues[350].level + rec_hi_5 = cast(HierarchyIssueRecord, result['org/repo#304']) + assert 1 == rec_hi_5.sub_hierarchy_issues['org/repo#350'].level # - single issue record (closed) @@ -114,7 +115,8 @@ def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mined_ # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mined_data_isolated_record_types_with_labels_no_type_defined): +def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mock_repo, + mined_data_isolated_record_types_with_labels_no_type_defined): mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) mock_github_client = mocker.Mock(spec=Github) @@ -123,50 +125,50 @@ def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mine mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) result = factory.generate(mined_data_isolated_record_types_with_labels_no_type_defined) assert 8 == len(result) - assert {121, 301, 302, 303, 304, 123, 124, "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result[121], IssueRecord) - assert isinstance(result[301], HierarchyIssueRecord) - assert isinstance(result[302], HierarchyIssueRecord) - assert isinstance(result[303], HierarchyIssueRecord) - assert isinstance(result[304], HierarchyIssueRecord) - assert isinstance(result[123], PullRequestRecord) - assert isinstance(result[124], PullRequestRecord) - assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - - rec_i = cast(IssueRecord, result[121]) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + + assert isinstance(result['org/repo#121'], IssueRecord) + assert isinstance(result['org/repo#301'], HierarchyIssueRecord) + assert isinstance(result['org/repo#302'], HierarchyIssueRecord) + assert isinstance(result['org/repo#303'], HierarchyIssueRecord) + assert isinstance(result['org/repo#304'], HierarchyIssueRecord) + assert isinstance(result['org/repo#123'], PullRequestRecord) + assert isinstance(result['org/repo#124'], PullRequestRecord) + assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + + rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result[301]) + rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues[450].pull_requests_count() + assert 1 == len(rec_hi_1.sub_issues.values()) + assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() - rec_hi_2 = cast(HierarchyIssueRecord, result[302]) + rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues[451].pull_requests_count() + assert 1 == len(rec_hi_2.sub_issues.values()) + assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() - rec_hi_3 = cast(HierarchyIssueRecord, result[303]) + rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues[452].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues[452].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == len(rec_hi_3.sub_issues.values()) + assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() + assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message - rec_hi_4 = cast(HierarchyIssueRecord, result[304]) + rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues[350].sub_issues[453].get_commit(152, "merge_commit_sha_152").commit.message + assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message # - single issue record (closed) @@ -176,7 +178,8 @@ def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mine # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mined_data_isolated_record_types_no_labels_with_type_defined): +def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mock_repo, + mined_data_isolated_record_types_no_labels_with_type_defined): mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) mock_github_client = mocker.Mock(spec=Github) @@ -185,50 +188,50 @@ def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mine mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) result = factory.generate(mined_data_isolated_record_types_no_labels_with_type_defined) assert 8 == len(result) - assert {121, 301, 302, 303, 304, 123, 124, "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result[121], IssueRecord) - assert isinstance(result[301], HierarchyIssueRecord) - assert isinstance(result[302], HierarchyIssueRecord) - assert isinstance(result[303], HierarchyIssueRecord) - assert isinstance(result[304], HierarchyIssueRecord) - assert isinstance(result[123], PullRequestRecord) - assert isinstance(result[124], PullRequestRecord) - assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - - rec_i = cast(IssueRecord, result[121]) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + + assert isinstance(result['org/repo#121'], IssueRecord) + assert isinstance(result['org/repo#301'], HierarchyIssueRecord) + assert isinstance(result['org/repo#302'], HierarchyIssueRecord) + assert isinstance(result['org/repo#303'], HierarchyIssueRecord) + assert isinstance(result['org/repo#304'], HierarchyIssueRecord) + assert isinstance(result['org/repo#123'], PullRequestRecord) + assert isinstance(result['org/repo#124'], PullRequestRecord) + assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + + rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result[301]) + rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues[450].pull_requests_count() + assert 1 == len(rec_hi_1.sub_issues.values()) + assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() - rec_hi_2 = cast(HierarchyIssueRecord, result[302]) + rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues[451].pull_requests_count() + assert 1 == len(rec_hi_2.sub_issues.values()) + assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() - rec_hi_3 = cast(HierarchyIssueRecord, result[303]) + rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues[452].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues[452].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == len(rec_hi_3.sub_issues.values()) + assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() + assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message - rec_hi_4 = cast(HierarchyIssueRecord, result[304]) + rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues[350].sub_issues[453].get_commit(152, "merge_commit_sha_152").commit.message + assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message # - single issue record (closed) @@ -238,7 +241,8 @@ def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mine # - single hierarchy issue record - one sub hierarchy issues - two sub-issues with PRs - with commits # - single pull request record (closed, merged) # - single direct commit record -def test_generate_isolated_record_types_with_labels_with_type_defined(mocker, mined_data_isolated_record_types_with_labels_with_type_defined): +def test_generate_isolated_record_types_with_labels_with_type_defined(mocker, mock_repo, + mined_data_isolated_record_types_with_labels_with_type_defined): mocker.patch("release_notes_generator.record.factory.default_record_factory.safe_call_decorator", side_effect=mock_safe_call_decorator) mock_github_client = mocker.Mock(spec=Github) @@ -247,47 +251,47 @@ def test_generate_isolated_record_types_with_labels_with_type_defined(mocker, mi mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 mock_github_client.get_rate_limit.return_value = mock_rate_limit - factory = IssueHierarchyRecordFactory(github=mock_github_client) + factory = IssueHierarchyRecordFactory(github=mock_github_client, home_repository=mock_repo) result = factory.generate(mined_data_isolated_record_types_with_labels_with_type_defined) assert 8 == len(result) - assert {121, 301, 302, 303, 304, 123, 124, "merge_commit_sha_direct"}.issubset(result.keys()) - - assert isinstance(result[121], IssueRecord) - assert isinstance(result[301], HierarchyIssueRecord) - assert isinstance(result[302], HierarchyIssueRecord) - assert isinstance(result[303], HierarchyIssueRecord) - assert isinstance(result[304], HierarchyIssueRecord) - assert isinstance(result[123], PullRequestRecord) - assert isinstance(result[124], PullRequestRecord) - assert isinstance(result["merge_commit_sha_direct"], CommitRecord) - - rec_i = cast(IssueRecord, result[121]) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + + assert isinstance(result['org/repo#121'], IssueRecord) + assert isinstance(result['org/repo#301'], HierarchyIssueRecord) + assert isinstance(result['org/repo#302'], HierarchyIssueRecord) + assert isinstance(result['org/repo#303'], HierarchyIssueRecord) + assert isinstance(result['org/repo#304'], HierarchyIssueRecord) + assert isinstance(result['org/repo#123'], PullRequestRecord) + assert isinstance(result['org/repo#124'], PullRequestRecord) + assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + + rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() - rec_hi_1 = cast(HierarchyIssueRecord, result[301]) + rec_hi_1 = cast(HierarchyIssueRecord, result['org/repo#301']) assert 0 == rec_hi_1.pull_requests_count() assert 0 == len(rec_hi_1.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_1.sub_issues.values()) - assert 0 == rec_hi_1.sub_issues[450].pull_requests_count() + assert 1 == len(rec_hi_1.sub_issues.values()) + assert 0 == rec_hi_1.sub_issues['org/repo#450'].pull_requests_count() - rec_hi_2 = cast(HierarchyIssueRecord, result[302]) + rec_hi_2 = cast(HierarchyIssueRecord, result['org/repo#302']) assert 1 == rec_hi_2.pull_requests_count() assert 0 == len(rec_hi_2.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_2.sub_issues.values()) - assert 1 == rec_hi_2.sub_issues[451].pull_requests_count() + assert 1 == len(rec_hi_2.sub_issues.values()) + assert 1 == rec_hi_2.sub_issues['org/repo#451'].pull_requests_count() - rec_hi_3 = cast(HierarchyIssueRecord, result[303]) + rec_hi_3 = cast(HierarchyIssueRecord, result['org/repo#303']) assert 1 == rec_hi_3.pull_requests_count() assert 0 == len(rec_hi_3.sub_hierarchy_issues.values()) - assert 2 == len(rec_hi_3.sub_issues.values()) - assert 1 == rec_hi_3.sub_issues[452].pull_requests_count() - assert "Fixed bug in PR 151" == rec_hi_3.sub_issues[452].get_commit(151, "merge_commit_sha_151").commit.message + assert 1 == len(rec_hi_3.sub_issues.values()) + assert 1 == rec_hi_3.sub_issues['org/repo#452'].pull_requests_count() + assert "Fixed bug in PR 151" == rec_hi_3.sub_issues['org/repo#452'].get_commit(151, "merge_commit_sha_151").commit.message - rec_hi_4 = cast(HierarchyIssueRecord, result[304]) + rec_hi_4 = cast(HierarchyIssueRecord, result['org/repo#304']) assert 1 == rec_hi_4.pull_requests_count() assert 1 == len(rec_hi_4.sub_hierarchy_issues.values()) assert 0 == len(rec_hi_4.sub_issues.values()) assert 1 == rec_hi_4.pull_requests_count() - assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues[350].sub_issues[453].get_commit(152, "merge_commit_sha_152").commit.message + assert "Fixed bug in PR 152" == rec_hi_4.sub_hierarchy_issues['org/repo#350'].sub_issues['org/repo#453'].get_commit(152, "merge_commit_sha_152").commit.message diff --git a/tests/test_miner.py b/tests/test_miner.py index c94c988f..c446a6a2 100644 --- a/tests/test_miner.py +++ b/tests/test_miner.py @@ -39,8 +39,7 @@ class MinedDataMock(MinedData): """ def __init__(self, mocker, rls_mock: Optional[GitRelease], mock_repo: Repository): - super().__init__() - self.repository = mock_repo + super().__init__(mock_repo) self.release = rls_mock if rls_mock is not None else mocker.Mock(spec=GitRelease) self.issues = [ mocker.Mock(spec=Issue, title="Mock Issue 1", number=1), diff --git a/tests/utils/test_pull_request_utils.py b/tests/utils/test_pull_request_utils.py index c183d54b..88f2020f 100644 --- a/tests/utils/test_pull_request_utils.py +++ b/tests/utils/test_pull_request_utils.py @@ -20,7 +20,6 @@ from release_notes_generator.utils import pull_request_utils as pru from release_notes_generator.utils.pull_request_utils import ( extract_issue_numbers_from_body, - get_issues_for_pr, ) @@ -44,42 +43,42 @@ def _patch_issues_template(monkeypatch, template="Q {number} {owner} {name} {fir # extract_issue_numbers_from_body -def test_extract_issue_numbers_from_body_no_issues(mocker): +def test_extract_issue_numbers_from_body_no_issues(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = "This PR does not fix any issues." - issue_numbers = extract_issue_numbers_from_body(mock_pr) - assert issue_numbers == set() + issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) + assert issue_ids == set() -def test_extract_issue_numbers_from_body_single_issue(mocker): +def test_extract_issue_numbers_from_body_single_issue(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = "This PR closes #123." - issue_numbers = extract_issue_numbers_from_body(mock_pr) - assert issue_numbers == {123} + issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) + assert issue_ids == {'org/repo#123'} -def test_extract_issue_numbers_from_body_multiple_issues(mocker): +def test_extract_issue_numbers_from_body_multiple_issues(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = "This PR fixes #123 and resolves #456." - issue_numbers = extract_issue_numbers_from_body(mock_pr) - assert issue_numbers == {123, 456} + issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) + assert issue_ids == {'org/repo#123', 'org/repo#456'} -def test_extract_issue_numbers_from_body_mixed_case_keywords(mocker): +def test_extract_issue_numbers_from_body_mixed_case_keywords(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = "This PR Fixes #123 and Resolves #456." - issue_numbers = extract_issue_numbers_from_body(mock_pr) - assert issue_numbers == {123, 456} + issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) + assert issue_ids == {'org/repo#123', 'org/repo#456'} -def test_extract_issue_numbers_from_body_no_body(mocker): +def test_extract_issue_numbers_from_body_no_body(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = None - issue_numbers = extract_issue_numbers_from_body(mock_pr) - assert issue_numbers == set() + issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) + assert issue_ids == set() -def test_extract_issue_numbers_from_body_complex_text_with_wrong_syntax(mocker): +def test_extract_issue_numbers_from_body_complex_text_with_wrong_syntax(mocker, mock_repo): mock_pr = mocker.Mock(spec=PullRequest) mock_pr.body = """ This PR does a lot: @@ -87,8 +86,8 @@ def test_extract_issue_numbers_from_body_complex_text_with_wrong_syntax(mocker): - fixes issue #456 - resolves the bug in #789 """ - issue_numbers = extract_issue_numbers_from_body(mock_pr) - assert issue_numbers == {123} + issue_ids = extract_issue_numbers_from_body(mock_pr, mock_repo) + assert issue_ids == {'org/repo#123'} # get_issues_for_pr @@ -125,7 +124,7 @@ def fake_post(url, json=None, headers=None, verify=None, timeout=None): monkeypatch.setattr(pru.requests, "post", fake_post) result = pru.get_issues_for_pr(123) - assert result == {11, 22} + assert result == {'OWN/REPO#11', 'OWN/REPO#22'} assert captured["url"] == "https://api.github.com/graphql" # Query string correctly formatted assert captured["json"]["query"] == "Q 123 OWN REPO 10" @@ -210,7 +209,7 @@ def fake_post(*a, **k): first = pru.get_issues_for_pr(900) second = pru.get_issues_for_pr(900) # should use cache - assert first == {9} and second == {9} + assert first == {'OWN/REPO#9'} and second == {'OWN/REPO#9'} assert calls["count"] == 1 # only one network call def test_get_issues_for_pr_different_numbers_not_cached(monkeypatch): @@ -242,6 +241,6 @@ def fake_post(url, json=None, **k): r1 = pru.get_issues_for_pr(1) r2 = pru.get_issues_for_pr(2) - assert r1 == {1} - assert r2 == {2} + assert r1 == {'OWN/REPO#1'} + assert r2 == {'OWN/REPO#2'} assert calls["nums"] == [1, 2]