diff --git a/webhook_server_container/libs/github_api.py b/webhook_server_container/libs/github_api.py index 8419e580..f22a2150 100644 --- a/webhook_server_container/libs/github_api.py +++ b/webhook_server_container/libs/github_api.py @@ -15,7 +15,6 @@ from stringcolor import cs from github.Branch import Branch -from github.ContentFile import ContentFile import requests import shortuuid from starlette.datastructures import Headers @@ -203,7 +202,7 @@ def process(self) -> None: self.last_committer = getattr(self.last_commit.committer, "login", self.parent_committer) self.changed_files = self.list_changed_commit_files() self.pull_request_branch = self.pull_request.base.ref - self.approvers_and_reviewers = self.get_approvers_and_reviewers() + self.all_approvers_and_reviewers = self.get_all_approvers_and_reviewers() self.all_approvers = self.get_all_approvers() self.all_reviewers = self.get_all_reviewers() @@ -581,43 +580,15 @@ def _error(_out: str, _err: str) -> None: """ self.send_slack_message(message=message, webhook_url=self.slack_webhook_url) - def get_owners_content(self, folder_path: str = "") -> Dict[str, Any]: - if folder_path: - # Normalize path and check for directory traversal - norm_path = os.path.normpath(folder_path) - if ( - norm_path.startswith("/") - or norm_path.startswith("\\") - or ".." in norm_path - or not all(part.isalnum() or part in "-_" for part in norm_path.split(os.path.sep)) - ): - self.logger.error(f"{self.log_prefix} Invalid folder path: {folder_path}") - return {} - - try: - owners_path = f"{folder_path}/OWNERS" if folder_path else "OWNERS" - owners_content: list[ContentFile] | ContentFile = self.repository.get_contents(owners_path) - if isinstance(owners_content, list): - self.logger.debug(f"{self.log_prefix} Found more than one OWNERS file, using the first one") - owners_content = owners_content[0] - - _content: Dict[str, Any] = yaml.safe_load(owners_content.decoded_content) - self.logger.debug(f"{self.log_prefix} OWNERS file content: {_content}") - return _content - - except UnknownObjectException: - self.logger.error(f"{self.log_prefix} OWNERS file not found") - return {} - @property def root_reviewers(self) -> List[str]: - _reviewers = self.approvers_and_reviewers.get(".", {}).get("reviewers", []) + _reviewers = self.all_approvers_and_reviewers.get(".", {}).get("reviewers", []) self.logger.debug(f"{self.log_prefix} ROOT Reviewers: {_reviewers}") return _reviewers @property def root_approvers(self) -> List[str]: - _approvers = self.approvers_and_reviewers.get(".", {}).get("approvers", []) + _approvers = self.all_approvers_and_reviewers.get(".", {}).get("approvers", []) self.logger.debug(f"{self.log_prefix} ROOT Approvers: {_approvers}") return _approvers @@ -1327,8 +1298,9 @@ def check_if_can_be_merged(self) -> None: _labels = self.pull_request_labels_names() self.logger.debug(f"{self.log_prefix} check if can be merged. PR labels are: {_labels}") - if not self.pull_request.mergeable: - failure_output += "PR is not mergeable: {self.pull_request.mergeable_state}\n" + is_pr_mergable = self.pull_request.mergeable + if not is_pr_mergable: + failure_output += f"PR is not mergeable: {is_pr_mergable}\n" required_check_in_progress_failure_output, check_runs_in_progress = self._required_check_in_progress( last_commit_check_runs=last_commit_check_runs @@ -2027,9 +1999,9 @@ def run_podman_command(self, command: str, pipe: bool = False) -> Tuple[bool, st return rc, out, err - def get_approvers_and_reviewers(self) -> dict[str, dict[str, list[str]]]: + def get_all_approvers_and_reviewers(self) -> dict[str, dict[str, Any]]: # Dictionary mapping OWNERS file paths to their approvers and reviewers - _owners: dict[str, dict[str, list[str]]] = {} + _owners: dict[str, dict[str, Any]] = {} max_owners_files = 1000 # Configurable limit owners_count = 0 @@ -2050,7 +2022,6 @@ def get_approvers_and_reviewers(self) -> dict[str, dict[str, list[str]]]: try: content = yaml.safe_load(_path.decoded_content) if self._validate_owners_content(content, content_path): - # Use Path for consistent path handling parent_path = str(Path(content_path).parent) if not parent_path: parent_path = "." @@ -2065,44 +2036,47 @@ def get_approvers_and_reviewers(self) -> dict[str, dict[str, list[str]]]: def get_all_approvers(self) -> list[str]: _approvers: list[str] = [] - for list_of_approvers in self.owners_data_for_changed_files()["approvers"]: - for _approver in list_of_approvers: + for list_of_approvers in self.owners_data_for_changed_files().values(): + for _approver in list_of_approvers.get("approvers", []): _approvers.append(_approver) - reviewers = list(set(self.root_approvers + _approvers)) - reviewers.sort() - return reviewers + _approvers.sort() + return _approvers def get_all_reviewers(self) -> list[str]: _reviewers: list[str] = [] - for list_of_reviewers in self.owners_data_for_changed_files()["reviewers"]: - for _approver in list_of_reviewers: + for list_of_reviewers in self.owners_data_for_changed_files().values(): + for _approver in list_of_reviewers.get("reviewers", []): _reviewers.append(_approver) - approvers = list(set(self.root_reviewers + _reviewers)) - approvers.sort() - return approvers + _reviewers.sort() + return _reviewers - def owners_data_for_changed_files(self) -> dict[str, list[list[str]]]: - data: dict[str, list[list[str]]] = {"approvers": [], "reviewers": []} + def owners_data_for_changed_files(self) -> dict[str, dict[str, Any]]: + data: dict[str, dict[str, Any]] = {} changed_folders = {Path(cf).parent for cf in self.changed_files} - for changed_folder_path in changed_folders: - for owners_dir, owners_data in self.approvers_and_reviewers.items(): - _owners_dir = Path(owners_dir) + changed_folder_match: list[str] = [] + + require_root_approvers: bool = False - if _owners_dir == changed_folder_path or _owners_dir in changed_folder_path.parents: - _reviewers = owners_data.get("reviewers", []) - self.logger.debug(f"{self.log_prefix} Found reviewers for {owners_dir}: {_reviewers}") - data["reviewers"].append(_reviewers) + for owners_dir, owners_data in self.all_approvers_and_reviewers.items(): + _owners_dir = Path(owners_dir) - _approvers = owners_data.get("approvers", []) - self.logger.debug(f"{self.log_prefix} Found approvers for {owners_dir}: {_approvers}") - data["approvers"].append(_approvers) + for changed_folder in changed_folders: + if _owners_dir == changed_folder or _owners_dir.parents == changed_folders: + data[owners_dir] = owners_data + changed_folder_match.append(owners_dir) + if not require_root_approvers: + require_root_approvers = owners_data.get("root-approvers", True) + + if [_folder for _folder in changed_folders if str(_folder) not in changed_folder_match]: + data["."] = self.all_approvers_and_reviewers.get(".", {}) + + if require_root_approvers: + data["."] = self.all_approvers_and_reviewers.get(".", {}) - data["reviewers"].sort() - data["approvers"].sort() return data def _validate_owners_content(self, content: Any, path: str) -> bool: @@ -2201,14 +2175,6 @@ def _check_lables_for_can_be_merged(self, labels: list[str]) -> str: return failure_output def _check_if_pr_approved(self, labels: list[str]) -> str: - _pr_approvers: list[str] = [] - all_needed_approvers = [] - for approvers_list in self.owners_data_for_changed_files()["approvers"]: - if approvers_list not in all_needed_approvers: - all_needed_approvers.append(approvers_list) - - # all_needed_approvers is [['approver1', 'approver2'], ['approver3', 'approver4']] - # To mark PR as approved we need at least one lgtm/approved from each nested list inside all_needed_approvers approved_by = [] for _label in labels: if APPROVED_BY_LABEL_PREFIX.lower() in _label.lower(): @@ -2220,15 +2186,20 @@ def _check_if_pr_approved(self, labels: list[str]) -> str: missing_approvers = self.all_approvers.copy() - for owners_data in self.approvers_and_reviewers.values(): - _approvers = owners_data.get("approvers", []) - for approver in _approvers: - if approver in approved_by: - _pr_approvers.append(approver) + for data in self.owners_data_for_changed_files().values(): + required_pr_approvers = data.get("approvers", []) + for required_pr_approver in required_pr_approvers: + if required_pr_approver in approved_by: # Once we found approver in approved_by list, we remove all approvers from missing_approvers list for this owners file - {missing_approvers.remove(_approver) for _approver in _approvers if _approver in missing_approvers} # type: ignore + { + missing_approvers.remove(_approver) # type: ignore + for _approver in required_pr_approvers + if _approver in missing_approvers + } break + missing_approvers = list(set(missing_approvers)) + if missing_approvers: return f"Missing lgtm/approved from approvers: {', '.join(missing_approvers)}\n" diff --git a/webhook_server_container/tests/test_github_api.py b/webhook_server_container/tests/test_github_api.py index 392f9c4d..c8fd2234 100644 --- a/webhook_server_container/tests/test_github_api.py +++ b/webhook_server_container/tests/test_github_api.py @@ -7,6 +7,16 @@ from webhook_server_container.libs.github_api import ProcessGithubWehook from webhook_server_container.utils.constants import APPROVED_BY_LABEL_PREFIX, SIZE_LABEL_PREFIX +ALL_CHANGED_FILES = [ + "OWNERS", + "folder1/OWNERS", + "code/file.py", + "README.md", + "folder2/lib.py", + "folder/folder4/another_file.txt", + "folder5/file", +] + class Label: def __init__(self, name: str): @@ -21,7 +31,13 @@ def __init__(self, path: str): @property def tree(self): trees = [] - for _path in ["OWNERS", "test1/OWNERS", "code/file.py", "README.md"]: + for _path in [ + "OWNERS", + "folder1/OWNERS", + "folder2/OWNERS", + "folder/folder4/OWNERS", + "folder5/OWNERS", + ]: trees.append(Tree(_path)) return trees @@ -43,17 +59,43 @@ def get_git_tree(self, sha: str, recursive: bool): return Tree("") def get_contents(self, path: str): - owners_data = yaml.dump({"approvers": ["approver1", "approver2"], "reviewers": ["reviewer1", "reviewer2"]}) + owners_data = yaml.dump({ + "approvers": ["root_approver1", "root_approver2"], + "reviewers": ["root_reviewer1", "root_reviewer2"], + }) + + folder1_owners_data = yaml.dump({ + "approvers": ["folder1_approver1", "folder1_approver2"], + "reviewers": ["folder1_reviewer1", "folder1_reviewer2"], + }) - test1_owners_data = yaml.dump({ - "approvers": ["approver3", "approver4"], - "reviewers": ["reviewer3", "reviewer4"], + folder4_owners_data = yaml.dump({ + "approvers": ["folder4_approver1", "folder4_approver2"], + "reviewers": ["folder4_reviewer1", "folder4_reviewer2"], }) + folder5_owners_data = yaml.dump({ + "root-approvers": False, + "approvers": ["folder5_approver1", "folder5_approver2"], + "reviewers": ["folder5_reviewer1", "folder5_reviewer2"], + }) if path == "OWNERS": return ContentFile(owners_data) - elif path == "test1/OWNERS": - return ContentFile(test1_owners_data) + + elif path == "folder1/OWNERS": + return ContentFile(folder1_owners_data) + + elif path == "folder2/OWNERS": + return ContentFile(yaml.dump({})) + + elif path == "folder/folder4/OWNERS": + return ContentFile(folder4_owners_data) + + elif path == "folder": + return ContentFile(yaml.dump({})) + + elif path == "folder5/OWNERS": + return ContentFile(folder5_owners_data) class PullRequest: @@ -72,7 +114,7 @@ def lables(self) -> list[Label]: @pytest.fixture(scope="function") -def process_github_webhook(mocker): +def process_github_webhook(mocker, request): base_import_path = "webhook_server_container.libs.github_api" os.environ["WEBHOOK_SERVER_DATA_DIR"] = "webhook_server_container/tests/manifests" @@ -85,22 +127,62 @@ def process_github_webhook(mocker): {"repository": {"name": Repository().name}}, Headers({"X-GitHub-Event": "test-event"}), logging.getLogger() ) process_github_webhook.pull_request_branch = "main" - process_github_webhook.changed_files = ["OWNERS", "test1/OWNERS", "code/file.py", "README.md"] + if hasattr(request, "param") and request.param: + process_github_webhook.changed_files = request.param[0] + else: + process_github_webhook.changed_files = ALL_CHANGED_FILES + return process_github_webhook @pytest.fixture(scope="function") -def approvers_and_reviewers(process_github_webhook): - process_github_webhook.approvers_and_reviewers = { - ".": {"approvers": ["approver1", "approver2"], "reviewers": ["reviewer1", "reviewer2"]}, - "test1": {"approvers": ["approver3", "approver4"], "reviewers": ["reviewer3", "reviewer4"]}, +def all_approvers_and_reviewers(process_github_webhook): + process_github_webhook.all_approvers_and_reviewers = { + ".": {"approvers": ["root_approver1", "root_approver2"], "reviewers": ["root_reviewer1", "root_reviewer2"]}, + "folder1": { + "approvers": ["folder1_approver1", "folder1_approver2"], + "reviewers": ["folder1_reviewer1", "folder1_reviewer2"], + }, + "folder2": {}, + "folder/folder4": { + "approvers": ["folder4_approver1", "folder4_approver2"], + "reviewers": ["folder4_reviewer1", "folder4_reviewer2"], + }, + "folder5": { + "approvers": ["folder5_approver1", "folder5_approver2"], + "reviewers": ["folder5_reviewer1", "folder5_reviewer2"], + "root-approvers": False, + }, } @pytest.fixture(scope="function") def all_approvers_reviewers(process_github_webhook): - process_github_webhook.all_approvers = ["approver1", "approver2", "approver3", "approver4"] - process_github_webhook.all_reviewers = ["reviewer1", "reviewer2", "reviewer3", "reviewer4"] + process_github_webhook.all_approvers = [ + "folder1_approver1", + "folder1_approver2", + "folder4_approver1", + "folder4_approver2", + "folder5_approver1", + "folder5_approver2", + "root_approver1", + "root_approver2", + ] + + process_github_webhook.all_approvers.sort() + + process_github_webhook.all_reviewers = [ + "folder1_reviewer1", + "folder1_reviewer2", + "folder4_reviewer1", + "folder4_reviewer2", + "folder5_reviewer1", + "folder5_reviewer2", + "root_reviewer1", + "root_reviewer2", + ] + + process_github_webhook.all_reviewers.sort() @pytest.mark.parametrize( @@ -122,34 +204,36 @@ def test_get_size_thresholds(process_github_webhook, additions, deletions, expec assert result == f"{SIZE_LABEL_PREFIX}{expected_label}" -def test_get_approvers_and_reviewers(process_github_webhook, approvers_and_reviewers): +def test_get_all_approvers_and_reviewers(process_github_webhook, all_approvers_and_reviewers): process_github_webhook.repository = Repository() - read_owners_result = process_github_webhook.get_approvers_and_reviewers() - assert read_owners_result == process_github_webhook.approvers_and_reviewers + read_owners_result = process_github_webhook.get_all_approvers_and_reviewers() + assert read_owners_result == process_github_webhook.all_approvers_and_reviewers -def test_owners_data_for_changed_files(process_github_webhook, approvers_and_reviewers): +def test_owners_data_for_changed_files(process_github_webhook, all_approvers_and_reviewers): owners_data_chaged_files_result = process_github_webhook.owners_data_for_changed_files() owners_data_chaged_files_expected = { - "approvers": [ - ["approver1", "approver2"], - ["approver1", "approver2"], - ["approver3", "approver4"], - ["approver1", "approver2"], - ], - "reviewers": [ - ["reviewer1", "reviewer2"], - ["reviewer1", "reviewer2"], - ["reviewer3", "reviewer4"], - ["reviewer1", "reviewer2"], - ], + "folder5": { + "approvers": ["folder5_approver1", "folder5_approver2"], + "reviewers": ["folder5_reviewer1", "folder5_reviewer2"], + "root-approvers": False, + }, + "folder1": { + "approvers": ["folder1_approver1", "folder1_approver2"], + "reviewers": ["folder1_reviewer1", "folder1_reviewer2"], + }, + ".": {"approvers": ["root_approver1", "root_approver2"], "reviewers": ["root_reviewer1", "root_reviewer2"]}, + "folder2": {}, + "folder/folder4": { + "approvers": ["folder4_approver1", "folder4_approver2"], + "reviewers": ["folder4_reviewer1", "folder4_reviewer2"], + }, } - owners_data_chaged_files_expected["approvers"].sort() - owners_data_chaged_files_expected["reviewers"].sort() + assert owners_data_chaged_files_result == owners_data_chaged_files_expected -def test_all_approvers_reviewers(process_github_webhook, approvers_and_reviewers, all_approvers_reviewers): +def test_all_approvers_reviewers(process_github_webhook, all_approvers_and_reviewers, all_approvers_reviewers): all_approvers = process_github_webhook.get_all_approvers() assert all_approvers == process_github_webhook.all_approvers @@ -157,36 +241,276 @@ def test_all_approvers_reviewers(process_github_webhook, approvers_and_reviewers assert all_reviewers == process_github_webhook.all_reviewers -def test_check_if_pr_approved(process_github_webhook, approvers_and_reviewers, all_approvers_reviewers): - pr_approved_all_result = process_github_webhook._check_if_pr_approved( +def test_check_pr_approved(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + f"{APPROVED_BY_LABEL_PREFIX}root_approver2", + f"{APPROVED_BY_LABEL_PREFIX}folder1_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder1_approver2", + f"{APPROVED_BY_LABEL_PREFIX}folder4_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder4_approver2", + f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder5_approver2", + ] + ) + assert check_if_pr_approved == "" + + +def test_check_pr_minimum_approved(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder1_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder4_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", + ] + ) + assert check_if_pr_approved == "" + + +def test_check_pr_not_approved(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + ] + ) + missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] + missing_approvers.sort() + expected_approvers = [ + "folder1_approver1", + "folder1_approver2", + "folder4_approver1", + "folder4_approver2", + "folder5_approver1", + "folder5_approver2", + ] + expected_approvers.sort() + assert missing_approvers == expected_approvers + + +def test_check_pr_partial_approved(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + f"{APPROVED_BY_LABEL_PREFIX}root_approver2", + ] + ) + missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] + missing_approvers.sort() + expected_approvers = [ + "folder1_approver1", + "folder1_approver2", + "folder4_approver1", + "folder4_approver2", + "folder5_approver1", + "folder5_approver2", + ] + expected_approvers.sort() + assert missing_approvers == expected_approvers + + +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "file_in_root", + "folder2", + "folder/folder4/another_file.txt", + ] + ]) + ], + indirect=True, +) +def test_check_pr_approved_specific_folder(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder4_approver1", + ] + ) + assert check_if_pr_approved == "" + + +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "file_in_root", + "folder2", + "folder/another_file.txt", + ] + ]) + ], + indirect=True, +) +def test_check_pr_approved_nested_folder_no_owners(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + ] + ) + assert check_if_pr_approved == "" + + +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "folder1/file", + ] + ]) + ], + indirect=True, +) +def test_check_pr_approved_specific_folder_with_root_approvers(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( labels=[ - f"{APPROVED_BY_LABEL_PREFIX}approver1", - f"{APPROVED_BY_LABEL_PREFIX}approver2", - f"{APPROVED_BY_LABEL_PREFIX}approver3", - f"{APPROVED_BY_LABEL_PREFIX}approver4", + f"{APPROVED_BY_LABEL_PREFIX}folder1_approver1", ] ) - assert pr_approved_all_result == "" + missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] + missing_approvers.sort() + expected_approvers = [ + "root_approver1", + "root_approver2", + ] + expected_approvers.sort() + assert missing_approvers == expected_approvers + - pr_approved_minimum_result = process_github_webhook._check_if_pr_approved( +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "folder5/file", + ] + ]) + ], + indirect=True, +) +def test_check_pr_approved_specific_folder_no_root_approvers(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( labels=[ - f"{APPROVED_BY_LABEL_PREFIX}approver1", - f"{APPROVED_BY_LABEL_PREFIX}approver3", + f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", ] ) - assert pr_approved_minimum_result == "" + assert check_if_pr_approved == "" + - pr_not_approved_result = process_github_webhook._check_if_pr_approved( +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "folder_with_no_owners/file", + ] + ]) + ], + indirect=True, +) +def test_check_pr_not_approved_specific_folder_without_owners(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( labels=[ - f"{APPROVED_BY_LABEL_PREFIX}approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", ] ) - assert pr_not_approved_result == "Missing lgtm/approved from approvers: approver3, approver4\n" + missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] + missing_approvers.sort() + expected_approvers = [ + "root_approver1", + "root_approver2", + ] + expected_approvers.sort() + assert missing_approvers == expected_approvers - pr_partial_approved_result = process_github_webhook._check_if_pr_approved( + +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "folder_with_no_owners/file", + ] + ]) + ], + indirect=True, +) +def test_check_pr_approved_specific_folder_without_owners(process_github_webhook, all_approvers_and_reviewers): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + ] + ) + assert check_if_pr_approved == "" + + +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "folder_with_no_owners/file", + "folder5/file", + ] + ]) + ], + indirect=True, +) +def test_check_pr_approved_folder_with_no_owners_and_folder_without_root_approvers( + process_github_webhook, all_approvers_and_reviewers +): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( + labels=[ + f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", + ] + ) + assert check_if_pr_approved == "" + + +@pytest.mark.parametrize( + "process_github_webhook", + [ + pytest.param([ + [ + "folder_with_no_owners/file", + "folder5/file", + ] + ]) + ], + indirect=True, +) +def test_check_pr_not_approved_folder_with_no_owners_and_folder_without_root_approvers( + process_github_webhook, all_approvers_and_reviewers +): + process_github_webhook.all_approvers = process_github_webhook.get_all_approvers() + check_if_pr_approved = process_github_webhook._check_if_pr_approved( labels=[ - f"{APPROVED_BY_LABEL_PREFIX}approver1", - f"{APPROVED_BY_LABEL_PREFIX}approver2", + f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", ] ) - assert pr_partial_approved_result == "Missing lgtm/approved from approvers: approver3, approver4\n" + missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] + missing_approvers.sort() + expected_approvers = [ + "root_approver1", + "root_approver2", + ] + expected_approvers.sort() + assert missing_approvers == expected_approvers