From f8c94b2fa0ed50eb1dfb3c4db7e64886d85259df Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Wed, 11 Mar 2026 15:35:29 +0100 Subject: [PATCH 1/5] Update linting --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2d01bc445..ca8922215 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -33,7 +33,7 @@ repos: rev: v2.2.4 hooks: - id: codespell - exclude_types: [json] + exclude_types: [json, pem] - repo: https://github.com/marco-c/taskcluster_yml_validator rev: v0.0.11 hooks: From 9020d8b1ab06b38deedbcc59d96c5c73f2b105d2 Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Wed, 11 Mar 2026 15:55:26 +0100 Subject: [PATCH 2/5] Github reporter --- bot/code_review_bot/report/__init__.py | 2 + bot/code_review_bot/report/github.py | 62 ++++++++++++++++ bot/code_review_bot/revisions/github.py | 26 ++++++- bot/code_review_bot/sources/github.py | 97 +++++++++++++++++++++++++ bot/requirements.txt | 1 + 5 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 bot/code_review_bot/report/github.py create mode 100644 bot/code_review_bot/sources/github.py diff --git a/bot/code_review_bot/report/__init__.py b/bot/code_review_bot/report/__init__.py index f1d5294ee..ed79bb1ec 100644 --- a/bot/code_review_bot/report/__init__.py +++ b/bot/code_review_bot/report/__init__.py @@ -4,6 +4,7 @@ import structlog +from code_review_bot.report.github import GithubReporter from code_review_bot.report.lando import LandoReporter from code_review_bot.report.mail import MailReporter from code_review_bot.report.mail_builderrors import BuildErrorsReporter @@ -22,6 +23,7 @@ def get_reporters(configuration): "mail": MailReporter, "build_error": BuildErrorsReporter, "phabricator": PhabricatorReporter, + "github": GithubReporter, } out = {} diff --git a/bot/code_review_bot/report/github.py b/bot/code_review_bot/report/github.py new file mode 100644 index 000000000..25db93bf6 --- /dev/null +++ b/bot/code_review_bot/report/github.py @@ -0,0 +1,62 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import structlog + +from code_review_bot.report.base import Reporter +from code_review_bot.sources.github import GithubClient, ReviewEvent + +logger = structlog.get_logger(__name__) + + +class GithubReporter(Reporter): + # Auth to Github using a configuration (from Taskcluster secret) + + def __init__(self, configuration={}, *args, **kwargs): + for key in ("client_id", "private_key_pem", "installation_id"): + if not configuration.get(key): + raise Exception(f"Missing github reporter configuration key {key}") + + # Setup github App secret from the configuration + self.github_client = GithubClient( + client_id=configuration["client_id"], + private_key=configuration["private_key_pem"], + installation_id=configuration["installation_id"], + ) + + self.analyzers_skipped = configuration.get("analyzers_skipped", []) + assert isinstance( + self.analyzers_skipped, list + ), "analyzers_skipped must be a list" + + def publish(self, issues, revision, task_failures, notices, reviewers): + """ + Publish issues on a Github pull request. + """ + if reviewers: + raise NotImplementedError + # Avoid publishing a patch from a de-activated analyzer + publishable_issues = [ + issue + for issue in issues + if issue.is_publishable() + and issue.analyzer.name not in self.analyzers_skipped + ] + + if publishable_issues: + # Publish a review summarizing detected, unresolved and closed issues + message = f"{len(issues)} issues have been found in this revision" + event = ReviewEvent.RequestChanges + else: + # Simply approve the pull request + logger.info("No publishable issue, approving the pull request") + message = None + event = ReviewEvent.Approved + + self.github_client.publish_review( + issues=publishable_issues, + revision=revision, + message=message, + event=event, + ) diff --git a/bot/code_review_bot/revisions/github.py b/bot/code_review_bot/revisions/github.py index 58953a0c8..82369dbb4 100644 --- a/bot/code_review_bot/revisions/github.py +++ b/bot/code_review_bot/revisions/github.py @@ -2,11 +2,13 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +from functools import cached_property from urllib.parse import urlparse import requests import structlog +from code_review_bot import taskcluster from code_review_bot.git import build_repo_slug from code_review_bot.revisions import Revision @@ -77,6 +79,27 @@ def as_dict(self): "pull_number": self.pull_number, } + @cached_property + def pull_request(self): + from code_review_bot.sources.github import GithubClient + + reporter_conf = next( + ( + reporter + for reporter in taskcluster.secrets["REPORTERS"] + if reporter["reporter"] == "github" + ), + None, + ) + # A github reporter configuration is required to perform a github Pull Request analysis + assert reporter_conf, "Github reporter secrets must be set to access information about the pull request" + client = GithubClient( + client_id=reporter_conf["client_id"], + private_key=reporter_conf["private_key_pem"], + installation_id=reporter_conf["installation_id"], + ) + return client.get_pull_request(self) + def serialize(self): """ Outputs a tuple of dicts for revision and diff (empty for Github) sent to backend @@ -84,8 +107,7 @@ def serialize(self): revision = { "provider": "github", "provider_id": self.pull_number, - # TODO: Use the pull request information from the API - "title": f"Issue {self.pull_number}", + "title": self.pull_request.title, "bugzilla_id": None, "base_repository": self.base_repository, "base_changeset": self.base_changeset, diff --git a/bot/code_review_bot/sources/github.py b/bot/code_review_bot/sources/github.py new file mode 100644 index 000000000..b45f527df --- /dev/null +++ b/bot/code_review_bot/sources/github.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import enum + +import structlog +from github import Auth, GithubIntegration +from github.PullRequest import ReviewComment + +from code_review_bot import Issue +from code_review_bot.revisions import GithubRevision + +logger = structlog.get_logger(__name__) + + +class ReviewEvent(enum.Enum): + """ + Review action you want to perform. + https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28#create-a-review-for-a-pull-request--parameters + """ + + Pending = "PENDING" + Approved = "APPROVE" + RequestChanges = "REQUEST_CHANGES" + Comment = "COMMENT" + + +class GithubClient: + def __init__(self, client_id: str, private_key: str, installation_id: str): + self.client_id = client_id + + # Setup auth + self.auth = Auth.AppAuth(self.client_id, private_key) + self.github_integration = GithubIntegration(auth=self.auth) + + installations = self.github_integration.get_installations() + self.installation = next( + (i for i in installations if i.id == installation_id), None + ) + if not self.installation: + raise ValueError( + f"Installation ID is not available. Available installations are {list(installations)}" + ) + # setup API + self.api = self.installation.get_github_for_installation() + + self.review_comments = [] + + def get_pull_request(self, revision: GithubRevision): + repo = self.api.get_repo(revision.repo_name) + return repo.get_pull(revision.pull_number) + + def _build_review_comment(self, issue): + return ReviewComment( + path=issue.path, + line=issue.line, + body=issue.message, + ) + + def publish_review( + self, + issues: list[Issue], + revision: GithubRevision, + event: ReviewEvent, + message: str | None = None, + ): + """ + Publish a review from a list of publishable issues, requesting changes to the author. + """ + + if not isinstance(revision, GithubRevision): + logger.warning( + f"Revision must originate from Github in order to publish a review, skipping {revision}." + ) + return + + repo = self.api.get_repo(revision.repo_name) + pull_request = repo.get_pull(revision.pull_number) + + attrs = {} + if message is None: + assert ( + event == ReviewEvent.Approved + ), "Body can be left null only when approving a pull request" + else: + attrs["body"] = message + + pull_request.create_review( + commit=repo.get_commit(revision.pull_head_sha), + comments=[self._build_review_comment(issue) for issue in issues], + # https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28#create-a-review-for-a-pull-request + event=event.value, + **attrs, + ) diff --git a/bot/requirements.txt b/bot/requirements.txt index 206d59720..2e8d197e7 100644 --- a/bot/requirements.txt +++ b/bot/requirements.txt @@ -2,6 +2,7 @@ aiohttp<4 gitpython==3.1.47 influxdb==5.3.2 libmozdata==0.2.12 +PyGithub==2.8.1 python-hglib==2.6.2 pyyaml==6.0.3 rs_parsepatch==0.4.4 From 76f4a718f5f8b0b381da131e25315058e9179562 Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Wed, 11 Mar 2026 15:56:26 +0100 Subject: [PATCH 3/5] Update tests --- bot/tests/conftest.py | 60 +++++++++++ bot/tests/fixtures/private_key.pem | 28 +++++ bot/tests/test_reporter_github.py | 163 +++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+) create mode 100644 bot/tests/fixtures/private_key.pem create mode 100644 bot/tests/test_reporter_github.py diff --git a/bot/tests/conftest.py b/bot/tests/conftest.py index caa4ae7fa..0f931b3ab 100644 --- a/bot/tests/conftest.py +++ b/bot/tests/conftest.py @@ -13,6 +13,8 @@ from collections import defaultdict, namedtuple from configparser import ConfigParser from contextlib import contextmanager +from datetime import UTC, datetime, timedelta +from textwrap import dedent from unittest.mock import MagicMock import hglib @@ -282,6 +284,64 @@ def diff_search(request): yield PhabricatorAPI(url="http://phabricator.test/api/", api_key="deadbeef") +@pytest.fixture +def mock_github(mock_config): + """ + Mock default github API calls made by the client + """ + diff = dedent( + """diff --git a/path/to/test.cpp b/path/to/test.cpp + index c57eff55..980a0d5f 100644 + --- a/path/to/test.cpp + +++ b/path/to/test.cpp + @@ -1 +1 @@ + -#include + +Hello World! + """ + ) + + responses.add( + responses.GET, + "https://github.tests.com/owner/repo-name/pull/1.diff", + json=diff, + ) + responses.add( + responses.GET, + "https://api.github.com:443/app/installations", + json=[ + { + "id": 123456789, + "access_tokens_url": "https://github.tests.com/app/installations/123456789/access_tokens", + } + ], + ) + responses.add( + responses.POST, + "https://api.github.com:443/app/installations/123456789/access_tokens", + json={ + "token": "auth_token", + "expires_at": (datetime.now(UTC) + timedelta(1)).strftime( + "%Y-%m-%dT%H:%M:%S.%fZ" + ), + }, + ) + responses.add( + responses.GET, + "https://api.github.com:443/repos/owner/repo-name", + json={"url": "https://api.github.com/repos/owner/repo-name"}, + ) + responses.add( + responses.GET, + "https://api.github.com:443/repos/owner/repo-name/pulls/1", + json={"url": "https://api.github.com/repos/owner/repo-name/pulls/1"}, + ) + responses.add( + responses.GET, + "https://api.github.com:443/repos/owner/repo-name/commits/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + json={"sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + ) + + @pytest.fixture def mock_try_task(): """ diff --git a/bot/tests/fixtures/private_key.pem b/bot/tests/fixtures/private_key.pem new file mode 100644 index 000000000..8f33755ca --- /dev/null +++ b/bot/tests/fixtures/private_key.pem @@ -0,0 +1,28 @@ +# THIS IS A FAKE PRIVATE KEY USED FOR TESTS +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAjIf0Q38ga5TF9CbNXewI/duyPJgz/TJAvdHvexwtp3qIIWfH +1CNK0NvcmaLWcvgyVn4nj8aLexQiJZQVQYII/YMwXAg2tK75dWkP56cWL0odb4Zs +o2GU14xRGdonixVb8COC8CxiLzFGBXpY2gMfru/9di6/0hRY1o5Qd2aYrXZVHDpe +0ATuIF0gR8MbMz8y8Azqvs+89epjAmKo+6+sU/7yITm9aJ685CEiug3127Kowk07 +TYeebXlzw2eqxcIw35loAs/oqyOIEX607KtVjCBl73FoEfuLHhsi1hFxBOg+HvXn +TzdFiLN/99ZGRRMlJhZCw/DWpZ9uip5MrohneQIDAQABAoIBAD9TVELGGnngBIPM +qGZWYobiZSLhAyxpZLskyuGTBQ+fK5DCD04MyT3slS+2LSSJq0VGe9VSBrBjli+Q +1zM5wYtbfoM6QEyTPF4oBb7BkEGnCDSlQnctFcE7vaAEqiUGbvN7TRmlJmlVrtPx +GfDDz5cpFfIXhuDHwnCMmL31QX+ITV8pmrYYwhpOp+js+25wA7PFzXhgqQtqgGbM +L0O1Tn/POqTbEUBi/S95KwMnNYVEAx6lwmcpLb0KD3x2g0bkHFyeIc+pMeJa+3Kn +OpY1DTlJP3nkpiLmpWIpKk4HMo6oZhcRo9DDLQtJzE+rllEwjZBrSKW3l4d3Eh6t +itsGj7ECgYEA+9OcFcUuMVwFlknTVSoykGv4DQ2gcN/W+H70bYHgAAvjolYxnsQX +hYucxJqTBaG8l5LXjlSynoqMnbm5FY1FLOu8lzt8YhPfc0f/N1jTVL5v52mxyUNr +HjPcbbv9+vihcrhEFkhgRN/lzimv7Eweo5x4wlm8aLOIb/Eg8Y8Wth8CgYEAjtwq +hsPdf0NP/tkJFSrB7Y2I1zBruNvTuJarcv1/w61XLEBR42Odq+Y11/crY8qkwK7z +A5SOxsI6o/si2RDlSZJ5w8t+7Kz/yr5PSFcQHGsokIadQgXCP/hetS2eiNNH94vM +YvwHvSl0ey47qK0h3ugqIZJ1pBSRc0NlfJz8v2cCgYEA0hXdd0QCn2cXuiNozPnh +KR8J10nw+XmkC7dODzV0PFWu2DV0O/F3dg/c/x+9W8tsXD9C2RjL0vvfB45zXAl5 +FlqsALa9s8zEc5Yy0memFmKxVKuWiENYT+AQGvPklMVrWxtiofxLY+ot+2pHu6hd +Pz1AeVMHnYl5X3oYc61d0x0CgYBBYT9RJ8hx2rN8lYVTm5rfBdwvZ2iVVH2jx8i1 +OpDDU8xGYzVW1JsvNY9ExEimRfJ6gFaVN+LT0cYWj/OV1eapchCp67KtzErQVaJh +H/8uklghNIo50frhXeCyGCuqwM752o/yaRd9mcBGM5V4D6wloKjPboDKU+NxFdIX +Yp1FVwKBgGgG4RA3UqAY51E7zA7k3WR3sj49c6oktXVi2n7FuO3PPVTg5LAZ/c81 +vVrip+dOQD53APtrwnFpDeM+AJ03RsIfjVVfB822xRpcy7jDA04bOmJ1Skouoptx +CyIV/PUVbtmNdxJ6T1dCzAvhmK6895FK+xCwBnpaN213Nx/eG49+ +-----END RSA PRIVATE KEY----- diff --git a/bot/tests/test_reporter_github.py b/bot/tests/test_reporter_github.py new file mode 100644 index 000000000..e28d94d4c --- /dev/null +++ b/bot/tests/test_reporter_github.py @@ -0,0 +1,163 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + + +import json +from pathlib import Path + +import responses +from conftest import FIXTURES_DIR + +from code_review_bot.report.github import GithubReporter +from code_review_bot.revisions import GithubRevision, Revision +from code_review_bot.tasks.clang_tidy import ClangTidyIssue, ClangTidyTask +from code_review_bot.tasks.coverage import CoverageIssue, ZeroCoverageTask + + +def test_github_review( + monkeypatch, + mock_github, + mock_config, + phab, + mock_try_task, + mock_github_decision_task, + mock_task, + mock_backend_secret, +): + """ + Report 2 cland tidy issues by pushing a review to a Github pull request + """ + revision = Revision.from_try_task(mock_try_task, mock_github_decision_task, None) + assert isinstance(revision, GithubRevision) + revision.lines = { + # Add dummy lines diff + "test.txt": [0], + "path/to/test.cpp": [0], + "another_test.cpp": [41, 42, 43], + } + revision.files = ["test.txt", "test.cpp", "another_test.cpp"] + revision.id = 52 + monkeypatch.setattr(revision, "load_file", lambda x: "some_content") + + reporter = GithubReporter( + { + "client_id": "client_id", + "private_key_pem": (Path(FIXTURES_DIR) / "private_key.pem").read_text(), + "installation_id": 123456789, + } + ) + + issue_clang_tidy = ClangTidyIssue( + mock_task(ClangTidyTask, "source-test-clang-tidy"), + revision, + "another_test.cpp", + "42", + "51", + "modernize-use-nullptr", + "dummy message", + ) + assert issue_clang_tidy.is_publishable() + + issue_coverage = CoverageIssue( + mock_task(ZeroCoverageTask, "coverage"), + "path/to/test.cpp", + "1", + "This file is uncovered", + revision, + ) + assert issue_coverage.is_publishable() + + responses.add( + responses.POST, + "https://api.github.com:443/repos/owner/repo-name/pulls/1/reviews", + json={}, + ) + + reporter.publish([issue_clang_tidy, issue_coverage], revision, [], [], []) + assert [(call.request.method, call.request.url) for call in responses.calls] == [ + ("GET", "https://github.tests.com/owner/repo-name/pull/1.diff"), + ("GET", "https://api.github.com:443/app/installations"), + ( + "POST", + "https://api.github.com:443/app/installations/123456789/access_tokens", + ), + ("GET", "https://api.github.com:443/repos/owner/repo-name"), + ("GET", "https://api.github.com:443/repos/owner/repo-name/pulls/1"), + ( + "GET", + "https://api.github.com:443/repos/owner/repo-name/commits/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + ), + ("POST", "https://api.github.com:443/repos/owner/repo-name/pulls/1/reviews"), + ] + review_creation = responses.calls[-1] + assert json.loads(review_creation.request.body) == { + "body": "2 issues have been found in this revision", + "comments": [ + { + "body": "dummy message", + "path": "another_test.cpp", + "line": 42, + }, + { + "body": "This file is uncovered", + "path": "path/to/test.cpp", + "line": 1, + }, + ], + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "event": "REQUEST_CHANGES", + } + + +def test_github_review_approve( + monkeypatch, + mock_github, + mock_config, + phab, + mock_try_task, + mock_github_decision_task, + mock_task, + mock_backend_secret, +): + """In case no issue is found, the pull request is approved""" + revision = Revision.from_try_task(mock_try_task, mock_github_decision_task, None) + revision.lines = {} + revision.files = ["test.txt", "test.cpp", "another_test.cpp"] + revision.id = 52 + reporter = GithubReporter( + { + "client_id": "client_id", + "private_key_pem": (Path(FIXTURES_DIR) / "private_key.pem").read_text(), + "installation_id": 123456789, + } + ) + + responses.add( + responses.POST, + "https://api.github.com:443/repos/owner/repo-name/pulls/1/reviews", + json={}, + ) + + reporter.publish([], revision, [], [], []) + assert [(call.request.method, call.request.url) for call in responses.calls] == [ + ("GET", "https://github.tests.com/owner/repo-name/pull/1.diff"), + ("GET", "https://api.github.com:443/app/installations"), + ( + "POST", + "https://api.github.com:443/app/installations/123456789/access_tokens", + ), + ("GET", "https://api.github.com:443/repos/owner/repo-name"), + ("GET", "https://api.github.com:443/repos/owner/repo-name/pulls/1"), + ( + "GET", + "https://api.github.com:443/repos/owner/repo-name/commits/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + ), + ("POST", "https://api.github.com:443/repos/owner/repo-name/pulls/1/reviews"), + ] + review_creation = responses.calls[-1] + assert json.loads(review_creation.request.body) == { + "comments": [], + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "event": "APPROVE", + } From 51bb1167a1eac351c687135dbfdfaa4e86d89cf6 Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Thu, 23 Apr 2026 10:52:04 +0200 Subject: [PATCH 4/5] Rebase on top of github-bot-support --- bot/code_review_bot/sources/github.py | 2 +- bot/tests/conftest.py | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/bot/code_review_bot/sources/github.py b/bot/code_review_bot/sources/github.py index b45f527df..84e2d0130 100644 --- a/bot/code_review_bot/sources/github.py +++ b/bot/code_review_bot/sources/github.py @@ -89,7 +89,7 @@ def publish_review( attrs["body"] = message pull_request.create_review( - commit=repo.get_commit(revision.pull_head_sha), + commit=repo.get_commit(revision.head_changeset), comments=[self._build_review_comment(issue) for issue in issues], # https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28#create-a-review-for-a-pull-request event=event.value, diff --git a/bot/tests/conftest.py b/bot/tests/conftest.py index 0f931b3ab..35f6a2552 100644 --- a/bot/tests/conftest.py +++ b/bot/tests/conftest.py @@ -350,6 +350,25 @@ def mock_try_task(): return {"extra": {"code-review": {"phabricator-diff": "PHID-HMBT-test"}}} +@pytest.fixture +def mock_github_decision_task(): + """ + Mock a decision task definition from a github revision + """ + return { + "payload": { + "env": { + "GECKO_REPOSITORY_TYPE": "git", + "GECKO_HEAD_REPOSITORY": "https://github.tests.com/owner/repo-name", + "GECKO_HEAD_REV": "a" * 40, + "GECKO_BASE_REPOSITORY": "https://github.tests.com/owner/repo-name", + "GECKO_BASE_REV": "b" * 40, + "GECKO_PULL_REQUEST_NUMBER": 1, + } + } + } + + @pytest.fixture def mock_decision_task(): """ From 6831b811f08a078da4bd0db4aef0319567967f99 Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Mon, 11 May 2026 18:54:43 +0200 Subject: [PATCH 5/5] Fix typo: 'cland' -> 'clang' --- bot/tests/test_reporter_github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/tests/test_reporter_github.py b/bot/tests/test_reporter_github.py index e28d94d4c..2c2df9142 100644 --- a/bot/tests/test_reporter_github.py +++ b/bot/tests/test_reporter_github.py @@ -26,7 +26,7 @@ def test_github_review( mock_backend_secret, ): """ - Report 2 cland tidy issues by pushing a review to a Github pull request + Report 2 clang tidy issues by pushing a review to a Github pull request """ revision = Revision.from_try_task(mock_try_task, mock_github_decision_task, None) assert isinstance(revision, GithubRevision)