From 92497811d2f7ba83e8c751d89a5c3452dd993578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Kone=C4=8Dn=C3=BD?= Date: Wed, 16 Mar 2022 16:55:58 +0100 Subject: [PATCH] Add unit tests for scm_request_processor module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add unit tests for verify_slas method and continue adding tests for create_new_repo method. Signed-off-by: Michal Konečný --- tests/plugins/test_scm_request_processor.py | 418 ++++++++++++++++++++ toddlers/plugins/scm_request_processor.py | 87 ++-- 2 files changed, 459 insertions(+), 46 deletions(-) diff --git a/tests/plugins/test_scm_request_processor.py b/tests/plugins/test_scm_request_processor.py index 1fc40e8..e353f76 100644 --- a/tests/plugins/test_scm_request_processor.py +++ b/tests/plugins/test_scm_request_processor.py @@ -348,3 +348,421 @@ class TestProcessTicket: mock_new_branch.assert_called_with( issue, content, ) + + +class TestVerifySLAs: + """ + Test class for `toddlers.plugins.scm_request_processor.SCMRequestProcessor.verify_slas` method. + """ + + def setup(self): + """ + Initialize toddler. + """ + self.toddler = scm_request_processor.SCMRequestProcessor() + self.toddler.pagure_io = Mock() + self.toddler.dist_git = Mock() + + def test_verify_slas_not_dict(self): + """ + Assert that SLA verification will fail if SLA isn't dict. + """ + sla = [] + + with pytest.raises(ValueError, match="The object provided is not a dict"): + self.toddler.verify_slas("", sla) + + def test_verify_slas_no_branch(self): + """ + Assert that the validation will not fail if no branch is provided. + This will fail later in create method, but the verification passes. + """ + sla = {} + branch = None + + self.toddler.verify_slas(branch, sla) + + def test_verify_slas_branch_sla_correct(self): + """ + Assert that the validation will not fail if provided branch SLAs are + correct. + """ + branch = "branch" + sla = { "rawhide": "2022-06-01"} + + self.toddler.branch_slas = { + branch: { + "rawhide": "2022-06-01" + } + } + + self.toddler.verify_slas(branch, sla) + + def test_verify_slas_branch_sla_incorrect(self): + """ + Assert that the validation will fail if provided branch SLAs are + incorrect. + """ + branch = "branch" + sla = { "rawhide": "2022-01-01"} + + self.toddler.branch_slas = { + branch: { + "rawhide": "2022-06-01" + } + } + + error = 'The SLAs for the branch "{0}" are incorrect'.format(branch) + + with pytest.raises(ValidationError, match=error): + self.toddler.verify_slas(branch, sla) + + def test_verify_slas_eol_not_string(self): + """ + Assert that the validation will fail if provided SLA EOL is + not string. + """ + sla = { "rawhide": 100 } + + error = 'The SL\'s EOL is not a string. It was type "{0}"'.format(type(100).__name__) + + with pytest.raises(ValidationError, match=error): + self.toddler.verify_slas("", sla) + + def test_verify_slas_eol_invalid_format(self): + """ + Assert that the validation will fail if provided SLA EOL date is in invalid format. + """ + sla = { "rawhide": "01-01-2022" } + + error = 'The EOL date "{0}" is in an invalid format'.format("01-01-2022") + + with pytest.raises(ValidationError, match=error): + self.toddler.verify_slas("", sla) + + def test_verify_slas_eol_expired(self): + """ + Assert that the validation will fail if provided SLA EOL date is already expired. + """ + sla = { "rawhide": "2022-01-01" } + + error = 'The SL "{0}" is already expired'.format("2022-01-01") + + with pytest.raises(ValidationError, match=error): + self.toddler.verify_slas("", sla) + + def test_verify_slas_eol_date_invalid(self): + """ + Assert that the validation will fail if provided SLA EOL date is invalid. + """ + sla = { "rawhide": "2050-01-01" } + + error = 'The SL "{0}" must expire on June 1st or December 1st'.format("2050-01-01") + + with pytest.raises(ValidationError, match=error): + self.toddler.verify_slas("", sla) + + @patch('toddlers.utils.pdc.get_sla') + def test_verify_slas_not_in_pdc(self, mock_pdc): + """ + Assert that the validation will fail if SLA is not in PDC. + """ + sla = { "rawhide": "2050-06-01" } + + error = 'The SL "{0}" is not in PDC'.format("rawhide") + + mock_pdc.return_value = None + + with pytest.raises(ValidationError, match=error): + self.toddler.verify_slas("", sla) + + mock_pdc.assert_called_with("rawhide") + + @patch('toddlers.utils.pdc.get_sla') + def test_verify_slas_in_pdc(self, mock_pdc): + """ + Assert that the validation will pass if SLA is in PDC. + """ + sla = { "rawhide": "2050-06-01" } + + mock_pdc.return_value = sla + + self.toddler.verify_slas("", sla) + + mock_pdc.assert_called_with("rawhide") + + +class TestCreateNewRepo: + """ + Test class for `toddlers.plugins.scm_request_processor.SCMRequestProcessor.create_new_repo` method. + """ + + def setup(self): + """ + Initialize toddler. + """ + self.toddler = scm_request_processor.SCMRequestProcessor() + self.toddler.pagure_io = Mock() + self.toddler.dist_git = Mock() + + def test_create_new_repo_missing_required_key(self): + """ + Assert that ticket will be closed if required key is missing in request. + """ + issue = { + "id": 100, + } + self.toddler.create_new_repo(issue, {}) + + self.toddler.pagure_io.close_issue.assert_called_with( + 100, namespace=scm_request_processor.PROJECT_NAMESPACE, + message="Invalid body, missing required field: repo", + reason="Invalid" + ) + + def test_create_new_repo_invalid_repo_name(self): + """ + Assert that ticket will be closed if provided repository name is invalid. + """ + issue = { + "id": 100, + "user": { + "name": "zlopez" + } + } + + json = { + "repo": "+a", + "branch": "rawhide", + "namespace": "namespace", + "bug_id": "123", + "action": "new_repo", + "sls": { + "rawhide": "2050-06-01" + }, + "monitor": "monitor" + } + + self.toddler.create_new_repo(issue, json) + + error = ('The repository name is invalid. It must be at least two ' + 'characters long with only letters, numbers, hyphens, ' + 'underscores, plus signs, and/or periods. Please note that ' + 'the project cannot start with a period or a plus sign.') + + self.toddler.pagure_io.close_issue.assert_called_with( + 100, namespace=scm_request_processor.PROJECT_NAMESPACE, + message=error, + reason="Invalid" + ) + + def test_create_new_repo_missing_bug_id(self): + """ + Assert that ticket will be closed if Bugzilla bug id is not provided. + """ + issue = { + "id": 100, + "user": { + "name": "zlopez" + } + } + + json = { + "repo": "repo", + "branch": "rawhide", + "namespace": "namespace", + "bug_id": "", + "action": "new_repo", + "sls": { + "rawhide": "2050-06-01" + }, + "monitor": "monitor" + } + + self.toddler.create_new_repo(issue, json) + + self.toddler.pagure_io.close_issue.assert_called_with( + 100, namespace=scm_request_processor.PROJECT_NAMESPACE, + message="An invalid Bugzilla bug was provided", + reason="Invalid" + ) + + @patch("toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug") + def test_create_new_repo_invalid_review_bug(self, mock_validate_review_bug): + """ + Assert that ticket will be closed if Bugzilla bug is not valid. + """ + issue = { + "id": 100, + "user": { + "name": "zlopez" + } + } + + json = { + "repo": "repo", + "branch": "rawhide", + "namespace": "namespace", + "bug_id": "123", + "action": "new_repo", + "sls": { + "rawhide": "2050-06-01" + }, + "monitor": "monitor" + } + + mock_validate_review_bug.side_effect = ValidationError("error") + + self.toddler.create_new_repo(issue, json) + + mock_validate_review_bug.assert_called_with( + "123", "repo", "rawhide", + namespace="namespace", pagure_user="zlopez" + ) + + self.toddler.pagure_io.close_issue.assert_called_with( + 100, namespace=scm_request_processor.PROJECT_NAMESPACE, + message="error", + reason="Invalid" + ) + + @patch("toddlers.plugins.scm_request_processor.SCMRequestProcessor.valid_epel_package") + def test_create_new_repo_invalid_epel(self, mock_valid_epel_package): + """ + Assert that ticket will be closed if repo is invalid EPEL repo. + """ + issue = { + "id": 100, + "user": { + "name": "zlopez" + } + } + + repo = "repo" + branch = "epel8" + namespace = "rpms" + bug_id = "123" + action = "new_repo" + sls = { + "rawhide": "2050-06-01" + } + monitor = "monitor" + exception = True + json = { + "repo": repo, + "branch": branch, + "namespace": namespace, + "bug_id": bug_id, + "action": action, + "sls": sls, + "monitor": monitor, + "exception": exception + } + + mock_valid_epel_package.return_value = False + + self.toddler.create_new_repo(issue, json) + + mock_valid_epel_package.assert_called_with( + repo, branch + ) + + self.toddler.pagure_io.close_issue.assert_called_with( + 100, namespace=scm_request_processor.PROJECT_NAMESPACE, + message=scm_request_processor.INVALID_EPEL_ERROR, + reason="Invalid" + ) + + def test_create_new_repo_requester_not_in_dist_git(self): + """ + Assert that ticket will be commented on when requester doesn't have a valid dist git account. + """ + issue = { + "id": 100, + "user": { + "name": "zlopez" + } + } + + repo = "repo" + branch = "rawhide" + namespace = "rpms" + bug_id = "123" + action = "new_repo" + sls = { + "rawhide": "2050-06-01" + } + monitor = "monitor" + exception = True + json = { + "repo": repo, + "branch": branch, + "namespace": namespace, + "bug_id": bug_id, + "action": action, + "sls": sls, + "monitor": monitor, + "exception": exception + } + + self.toddler.dist_git.user_exists.return_value = False + self.toddler.dist_git._pagure_url = "https://src.fedoraproject.org" + + self.toddler.create_new_repo(issue, json) + + self.toddler.dist_git.user_exists.assert_called_with( + "zlopez" + ) + + message = "@zlopez needs to login to {0} to sync accounts before we can proceed.".format(self.toddler.dist_git._pagure_url) + + self.toddler.pagure_io.add_comment_to_issue.assert_called_with( + 100, namespace=scm_request_processor.PROJECT_NAMESPACE, + comment=message, + ) + + def test_create_new_repo_project_exists(self): + """ + Assert that ticket will be closed when repo already exists in dist git. + """ + issue = { + "id": 100, + "user": { + "name": "zlopez" + } + } + + repo = "repo" + branch = "rawhide" + namespace = "rpms" + bug_id = "123" + action = "new_repo" + sls = { + "rawhide": "2050-06-01" + } + monitor = "monitor" + exception = True + json = { + "repo": repo, + "branch": branch, + "namespace": namespace, + "bug_id": bug_id, + "action": action, + "sls": sls, + "monitor": monitor, + "exception": exception + } + + self.toddler.dist_git.get_project.return_value = "project" + + self.toddler.create_new_repo(issue, json) + + self.toddler.dist_git.get_project.assert_called_with( + namespace, repo + ) + + self.toddler.pagure_io.close_issue.assert_called_with( + 100, namespace=scm_request_processor.PROJECT_NAMESPACE, + message="The Pagure project already exists", + reason="Invalid" + ) diff --git a/toddlers/plugins/scm_request_processor.py b/toddlers/plugins/scm_request_processor.py index fdc16dc..5f3f013 100644 --- a/toddlers/plugins/scm_request_processor.py +++ b/toddlers/plugins/scm_request_processor.py @@ -268,7 +268,7 @@ class SCMRequestProcessor(ToddlerBase): 'incorrect'.format(branch)) for sla, eol in sla_dict.items(): - if not isinstance(eol, string_types): + if not isinstance(eol, str): raise ValidationError( 'The SL\'s EOL is not a string. It was type "{0}".'.format(type(eol).__name__)) try: @@ -345,11 +345,12 @@ class SCMRequestProcessor(ToddlerBase): message='An invalid Bugzilla bug was provided', reason="Invalid" ) + return try: _log.info('- Checking that #{0} is a valid RHBZ.'.format(bug_id)) self.validate_review_bug( - bug_id, repo, branch_name, require_auth=True, - check_fas=True, namespace=namespace, pagure_user=requester) + bug_id, repo, branch_name, + namespace=namespace, pagure_user=requester) except ValidationError as error: # pragma: no cover self.pagure_io.close_issue( issue["id"], @@ -374,9 +375,9 @@ class SCMRequestProcessor(ToddlerBase): issue_id = issue['id'] _log.info('- Checking if user {0} has an account in dist-git.'.format( requester)) - if not pagure.user_exists(requester): + if not self.dist_git.user_exists(requester): sync_comment = ('@{0} needs to login to {1} to sync accounts ' - 'before we can proceed.'.format(requester, pagure._PAGURE_URL)) + 'before we can proceed.'.format(requester, self.dist_git._pagure_url)) self.pagure_io.add_comment_to_issue( issue["id"], namespace=PROJECT_NAMESPACE, @@ -646,7 +647,6 @@ class SCMRequestProcessor(ToddlerBase): bugzilla_system.comment_on_bug(bug_id, new_repo_comment) def validate_review_bug(self, bug_id: str, pkg: str, branch: str, - require_auth: bool = False, check_fas: bool = False, namespace: str = 'rpms', pagure_user: Optional[str] = None ): """ @@ -659,9 +659,6 @@ class SCMRequestProcessor(ToddlerBase): bug_id: ID of the bugzillla bug pkg: string of the package name branch: string of the branch name - require_auth: a boolean determining if an exception is thrown - if the client is not authenticated - check_fas: a boolean determining if the approver is a packager namespace: a string of the requested repo's namespace pagure_user: a string of the requesting user's Pagure username """ @@ -696,7 +693,7 @@ class SCMRequestProcessor(ToddlerBase): elif bug.assigned_to in ['', None, 'nobody@fedoraproject.org']: raise ValidationError( 'The Bugzilla bug provided is not assigned to anyone') - if check_fas and require_auth and pagure_user: + if pagure_user: fas_user = fedora_account.get_user_by_email(bug.creator) if not fas_user: raise ValidationError( @@ -712,47 +709,45 @@ class SCMRequestProcessor(ToddlerBase): if flag.get('name') == 'fedora-review': if flag.get('status') == '+': flag_set = True - if check_fas and require_auth: - fas_reviewer = fedora_account.get_user_by_email(bug.assigned_to) - if not fas_reviewer: - raise ValidationError( - 'The email address "{0}" of the Bugzilla reviewer ' - 'is not tied to a user in FAS or FAS check failed. ' - 'Group membership can\'t be validated.'.format(bug.assigned_to)) - if not fedora_account.user_member_of(fas_reviewer, 'packager'): - raise ValidationError('The Bugzilla bug\'s review ' - 'is approved by a user "{0}" that is ' - 'not a packager or FAS check failed'.format( - bug.assigned_to)) - fas_submitter = self.get_fas_user_by_bz_email(bug.creator) - if not fas_submitter: - raise ValidationError( - 'The email address "{0}" of the Bugzilla submitter ' - 'is not tied to a user in FAS. Group membership ' - 'can\'t be validated.'.format(bug.creator)) - if not fedora_acount.user_member_of(fas_submitter, 'packager'): - raise ValidationError('The Bugzilla reporter "{0}"' - 'is not a packager'.format(bug.creator)) + fas_reviewer = fedora_account.get_user_by_email(bug.assigned_to) + if not fas_reviewer: + raise ValidationError( + 'The email address "{0}" of the Bugzilla reviewer ' + 'is not tied to a user in FAS or FAS check failed. ' + 'Group membership can\'t be validated.'.format(bug.assigned_to)) + if not fedora_account.user_member_of(fas_reviewer, 'packager'): + raise ValidationError('The Bugzilla bug\'s review ' + 'is approved by a user "{0}" that is ' + 'not a packager or FAS check failed'.format( + bug.assigned_to)) + fas_submitter = self.get_fas_user_by_bz_email(bug.creator) + if not fas_submitter: + raise ValidationError( + 'The email address "{0}" of the Bugzilla submitter ' + 'is not tied to a user in FAS. Group membership ' + 'can\'t be validated.'.format(bug.creator)) + if not fedora_acount.user_member_of(fas_submitter, 'packager'): + raise ValidationError('The Bugzilla reporter "{0}"' + 'is not a packager'.format(bug.creator)) # Setter will be an empty string and emails will not be shown # if the user is not logged in. This is why we check for # authentication here. - if require_auth: - if flag['setter'] == bug.creator: - error = ('The Bugzilla bug\'s review is approved ' - 'by the person creating the bug. This is ' - 'not allowed.') - raise ValidationError(error) + if flag['setter'] == bug.creator: + error = ('The Bugzilla bug\'s review is approved ' + 'by the person creating the bug. This is ' + 'not allowed.') + raise ValidationError(error) - assigned_to_emails = [bug.assigned_to] + assigned_to_emails = [bug.assigned_to] - assigned_to_user = fedora_account.get_user_by_username(bug.assigned_to) - if assigned_to_user: - assigned_to_emails.append( - assigned_to_user["emails"]) - if flag['setter'] not in assigned_to_emails: - raise ValidationError('The review is not approved by ' - 'the assignee of the Bugzilla ' - 'bug') + assigned_to_user = fedora_account.get_user_by_username(bug.assigned_to) + if assigned_to_user: + assigned_to_emails.append( + assigned_to_user["emails"]) + if flag['setter'] not in assigned_to_emails: + raise ValidationError('The review is not approved by ' + 'the assignee of the Bugzilla ' + 'bug') update_dt = flag.get('modification_date') if update_dt: