diff --git a/reusable_workflows/check_cla/check_cla_issue.py b/reusable_workflows/check_cla/check_cla_issue.py index 3a9abbf..1994659 100644 --- a/reusable_workflows/check_cla/check_cla_issue.py +++ b/reusable_workflows/check_cla/check_cla_issue.py @@ -20,7 +20,7 @@ def main() -> None: cla_signed = cla.check_if_cla_signed(issue, user) if not cla_signed: - cla.comment_on_issue(issue) + cla.leave_failed_comment_on_issue(issue) else: cla.handle_cla_signed(issue, user) diff --git a/reusable_workflows/check_cla/check_cla_pr.py b/reusable_workflows/check_cla/check_cla_pr.py index f6f9533..cf9d4e1 100644 --- a/reusable_workflows/check_cla/check_cla_pr.py +++ b/reusable_workflows/check_cla/check_cla_pr.py @@ -12,33 +12,27 @@ APPROVED_LABEL = "cla:agreed" GH_WORKFLOW_LABEL = "cla:gh-wf-pending" -# keep all old bot names for backwards compatibility -CLA_BOT_NAMES = ["cla-idx-bot[bot]", "sa-github-api", "dfinity-droid-prod[bot]"] - class CLAHandler: def __init__(self, gh: github3.login) -> None: self.cla_repo = gh.repository(owner="dfinity", repository="cla") self.cla_link = f"{self.cla_repo.html_url}/blob/main/CLA.md" - def check_comment_already_exists( - self, comments: github3.structs.GitHubIterator + @staticmethod + def check_if_comment_already_exists( + search_comment: str, comments: github3.structs.GitHubIterator ) -> bool: - for comment in comments: - if comment.user.login in CLA_BOT_NAMES: - return True - return False + return any(search_comment == comment.body for comment in comments) - def comment_on_issue(self, issue: GHIssue): + def leave_failed_comment_on_issue(self, issue: GHIssue) -> None: # check if bot has already left a message to avoid spam issue_comments = issue.comments() - bot_comment = self.check_comment_already_exists(issue_comments) - if not bot_comment: + if not self.check_if_comment_already_exists(messages.FAILED_COMMENT, issue_comments): issue.create_comment(messages.FAILED_COMMENT) - def comment_on_pr(self, pr: GHPullRequest, pr_comment): - bot_comment = self.check_comment_already_exists(pr.issue_comments()) - if not bot_comment: + def comment_on_pr(self, pr: GHPullRequest, pr_comment: str) -> None: + pr_comments = pr.issue_comments() + if not self.check_if_comment_already_exists(pr_comment, pr_comments): pr.create_comment(pr_comment) def check_if_cla_signed(self, issue: GHIssue, user: str) -> bool: @@ -56,7 +50,7 @@ def check_if_cla_signed(self, issue: GHIssue, user: str) -> bool: def get_cla_issue(self, user: str) -> Optional[GHIssue]: for issue in self.cla_repo.issues(): - if issue.title == f"cla: @{user}" and issue.user.login in CLA_BOT_NAMES: + if issue.title == f"cla: @{user}": return issue print(f"No CLA issue for {user}") return None # to make linter happy diff --git a/reusable_workflows/tests/test_cla_issue.py b/reusable_workflows/tests/test_cla_issue.py index 5ce390b..2ac4150 100644 --- a/reusable_workflows/tests/test_cla_issue.py +++ b/reusable_workflows/tests/test_cla_issue.py @@ -33,7 +33,7 @@ def test_end_to_end_cla_signed(cla_mock, gh_login_mock): main() cla.check_if_cla_signed.assert_called_with(issue, "username") - cla.comment_on_issue.assert_not_called() + cla.leave_failed_comment_on_issue.assert_not_called() cla.handle_cla_signed.assert_called_once() @@ -56,7 +56,7 @@ def test_end_to_end_cla_not_signed(cla_mock, gh_login_mock): main() cla.check_if_cla_signed.assert_called_with(issue, "username") - cla.comment_on_issue.assert_called_once() + cla.leave_failed_comment_on_issue.assert_called_once() cla.handle_cla_signed.assert_not_called() diff --git a/reusable_workflows/tests/test_cla_pr.py b/reusable_workflows/tests/test_cla_pr.py index 65bff83..02621b3 100644 --- a/reusable_workflows/tests/test_cla_pr.py +++ b/reusable_workflows/tests/test_cla_pr.py @@ -1,11 +1,13 @@ import os from unittest import mock +import github3 import pytest from shared.messages import ( AGREED_MESSAGE, CLA_AGREEMENT_MESSAGE, + FAILED_COMMENT, USER_AGREEMENT_MESSAGE, ) from check_cla.check_cla_pr import CLAHandler, main @@ -28,15 +30,14 @@ def test_bot_comment_exists(): cla = CLAHandler(mock.Mock()) comments_iterator = mock.Mock() comment1 = mock.Mock() - comment1.user.login = "username" + comment1.body = "comment1" comment2 = mock.Mock() - comment2.user.login = "sa-github-api" + comment2.body = "comment2" comments_iterator.__iter__ = mock.Mock( - return_value=iter([comment1, comment2, comment1]) + return_value=iter([comment1, comment2]) ) - # comments_iterator.return_value = [comment1, comment2, comment1] - bot_comment = cla.check_comment_already_exists(comments_iterator) + bot_comment = cla.check_if_comment_already_exists("comment1", comments_iterator) assert bot_comment is True @@ -45,14 +46,25 @@ def test_no_bot_comment(): cla = CLAHandler(mock.Mock()) issue_comments = mock.Mock() comment1 = mock.Mock() - comment1.user.login = "username" - issue_comments.__iter__ = mock.Mock(return_value=iter([comment1, comment1])) + comment1.body = "comment" + issue_comments.__iter__ = mock.Mock(return_value=iter([comment1])) - bot_comment = cla.check_comment_already_exists(issue_comments) + bot_comment = cla.check_if_comment_already_exists("comment2", issue_comments) assert bot_comment is False +def test_leave_failed_comment_on_issue(): + cla = CLAHandler(mock.Mock()) + issue = mock.Mock() + issue.comments.return_value = mock.Mock() + cla.check_if_comment_already_exists = mock.Mock(return_value=False) + + cla.leave_failed_comment_on_issue(issue) + + issue.create_comment.assert_called_once_with(FAILED_COMMENT) + + def test_cla_is_signed(capfd): cla = CLAHandler(mock.Mock()) issue = mock.Mock() @@ -88,7 +100,6 @@ def test_cla_is_not_signed(capfd): cla = CLAHandler(mock.Mock()) issue = mock.Mock() comment = mock.Mock() - comment.user.login = "bot" issue.comments.return_value = [mock.Mock(), comment] response = cla.check_if_cla_signed(issue, "username") @@ -103,7 +114,6 @@ def test_get_cla_issue_success(): cla_repo = mock.Mock() issue = mock.Mock() issue.title = "cla: @username" - issue.user.login = "sa-github-api" cla_repo.issues.return_value = [mock.Mock(), issue] cla.cla_repo = cla_repo @@ -299,3 +309,29 @@ def test_github_token_not_passed_in(github_login_mock): assert ( str(exc.value) == "github login failed - maybe GH_TOKEN was not correctly set" ) + +@pytest.mark.integration +def test_cla_signed(): + gh = github3.login(token=os.getenv("GH_TOKEN")) + cla = CLAHandler(gh) + issue = cla.get_cla_issue("droid-uexternal") + + # because the issue can change states, we don't check the status, just that it throws no errors + cla.check_if_cla_signed(issue, "droid-uexternal") + +@pytest.mark.integration +def get_cla_issue(): + gh = github3.login(token=os.getenv("GH_TOKEN")) + cla = CLAHandler(gh) + issue = cla.get_cla_issue("droid-uexternal") + + assert issue is not None + assert issue.title == "cla: @droid-uexternal" + +@pytest.mark.integration +def test_pr_comments_accessible(): + gh = github3.login(token=os.getenv("GH_TOKEN")) + pr = gh.pull_request("dfinity", "test-compliant-repository-public", 4) + comments = pr.issue_comments() + + assert len(list(comments)) > 0