mirror of https://github.com/mongodb/mongo
SERVER-101443 Get PR title/description feedback on push (#32907)
GitOrigin-RevId: 8488bf74340cf02347d551f9bc3ecbcec51f2391
This commit is contained in:
parent
71bf33ab43
commit
f0e28cbc5f
|
|
@ -1,10 +1,14 @@
|
||||||
"""Unit tests for the evergreen_task_timeout script."""
|
"""Unit tests for the evergreen_task_timeout script."""
|
||||||
|
|
||||||
import unittest
|
import unittest
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
from git import Commit, Repo
|
from git import Commit, Repo
|
||||||
|
|
||||||
from buildscripts.validate_commit_message import is_valid_commit
|
from buildscripts.validate_commit_message import (
|
||||||
|
get_non_merge_queue_squashed_commits,
|
||||||
|
is_valid_commit,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class ValidateCommitMessageTest(unittest.TestCase):
|
class ValidateCommitMessageTest(unittest.TestCase):
|
||||||
|
|
@ -68,3 +72,28 @@ class ValidateCommitMessageTest(unittest.TestCase):
|
||||||
]
|
]
|
||||||
|
|
||||||
self.assertTrue(all(not is_valid_commit(message) for message in messages))
|
self.assertTrue(all(not is_valid_commit(message) for message in messages))
|
||||||
|
|
||||||
|
@patch("requests.post")
|
||||||
|
def test_squashed_commit(self, mock_request):
|
||||||
|
class FakeResponse:
|
||||||
|
def json(self):
|
||||||
|
return {
|
||||||
|
"data": {
|
||||||
|
"repository": {
|
||||||
|
"pullRequest": {
|
||||||
|
"viewerMergeHeadlineText": "SERVER-1234 Add a ton of great support (#32823)",
|
||||||
|
"viewerMergeBodyText": "This PR adds back support for a lot of things\nMany great things!",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
mock_request.return_value = FakeResponse()
|
||||||
|
commits = get_non_merge_queue_squashed_commits(
|
||||||
|
github_org="fun_org", github_repo="fun_repo", pr_number=1024, github_token="fun_token"
|
||||||
|
)
|
||||||
|
self.assertEqual(len(commits), 1)
|
||||||
|
self.assertEqual(
|
||||||
|
commits[0].message,
|
||||||
|
"SERVER-1234 Add a ton of great support (#32823)\nThis PR adds back support for a lot of things\nMany great things!",
|
||||||
|
)
|
||||||
|
|
|
||||||
|
|
@ -32,6 +32,7 @@ import pathlib
|
||||||
import re
|
import re
|
||||||
import subprocess
|
import subprocess
|
||||||
|
|
||||||
|
import requests
|
||||||
import structlog
|
import structlog
|
||||||
import typer
|
import typer
|
||||||
from git import Commit, Repo
|
from git import Commit, Repo
|
||||||
|
|
@ -65,7 +66,10 @@ def is_valid_commit(commit: Commit) -> bool:
|
||||||
# 4. Revert "Import wiredtiger
|
# 4. Revert "Import wiredtiger
|
||||||
if not VALID_SUMMARY.match(commit.summary):
|
if not VALID_SUMMARY.match(commit.summary):
|
||||||
LOGGER.error(
|
LOGGER.error(
|
||||||
"Commit did not contain a valid summary",
|
f"""Commit did not contain a valid summary.
|
||||||
|
Please update the PR title and description to match the following regular expressions {VALID_SUMMARY}.
|
||||||
|
If you are seeing this on a PR, after changing the title/description you will need to rerun this check before being able to submit your PR.
|
||||||
|
The decision to add this check was made in SERVER-101443, please feel free to leave comments/feedback on that ticket.""",
|
||||||
commit_hexsha=commit.hexsha,
|
commit_hexsha=commit.hexsha,
|
||||||
commit_summary=commit.summary,
|
commit_summary=commit.summary,
|
||||||
)
|
)
|
||||||
|
|
@ -77,7 +81,10 @@ def is_valid_commit(commit: Commit) -> bool:
|
||||||
for banned_string in BANNED_STRINGS:
|
for banned_string in BANNED_STRINGS:
|
||||||
if "".join(banned_string.split()) in stripped_message:
|
if "".join(banned_string.split()) in stripped_message:
|
||||||
LOGGER.error(
|
LOGGER.error(
|
||||||
"Commit contains banned string (ignoring whitespace)",
|
"""Commit contains banned string (ignoring whitespace).
|
||||||
|
Please update the PR title and description to not contain the following banned string.
|
||||||
|
If you are seeing this on a PR, after changing the title/description you will need to rerun this check before being able to submit your PR.
|
||||||
|
The decision to add this check was made in SERVER-101443, please feel free to leave comments/feedback on that ticket.""",
|
||||||
banned_string=banned_string,
|
banned_string=banned_string,
|
||||||
commit_hexsha=commit.hexsha,
|
commit_hexsha=commit.hexsha,
|
||||||
commit_message=commit.message,
|
commit_message=commit.message,
|
||||||
|
|
@ -87,28 +94,57 @@ def is_valid_commit(commit: Commit) -> bool:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
def main(
|
def get_non_merge_queue_squashed_commits(
|
||||||
branch_name: Annotated[
|
github_org: str,
|
||||||
str,
|
github_repo: str,
|
||||||
typer.Option(envvar="BRANCH_NAME", help="Name of the branch to compare against HEAD"),
|
pr_number: int,
|
||||||
],
|
github_token: str,
|
||||||
is_commit_queue: Annotated[
|
) -> list[Commit]:
|
||||||
str,
|
assert github_org
|
||||||
typer.Option(
|
assert github_repo
|
||||||
envvar="IS_COMMIT_QUEUE",
|
assert pr_number >= 0
|
||||||
help="If this is being run in the commit/merge queue. Set to anything to be considered part of the commit/merge queue.",
|
assert github_token
|
||||||
),
|
|
||||||
] = "",
|
|
||||||
):
|
|
||||||
"""
|
|
||||||
Validate the commit message.
|
|
||||||
|
|
||||||
It validates the latest message when no arguments are provided.
|
pr_merge_info_query = {
|
||||||
"""
|
"query": f"""{{
|
||||||
|
repository(owner: "{github_org}", name: "{github_repo}") {{
|
||||||
|
pullRequest(number: {pr_number}) {{
|
||||||
|
viewerMergeHeadlineText(mergeType: SQUASH)
|
||||||
|
viewerMergeBodyText(mergeType: SQUASH)
|
||||||
|
}}
|
||||||
|
}}
|
||||||
|
}}"""
|
||||||
|
}
|
||||||
|
headers = {"Authorization": f"token {github_token}"}
|
||||||
|
|
||||||
if not is_commit_queue:
|
LOGGER.info("Sending request", request=pr_merge_info_query)
|
||||||
LOGGER.info("Exiting early since this is not running in the commit/merge queue")
|
req = requests.post(
|
||||||
raise typer.Exit(code=STATUS_OK)
|
url="https://api.github.com/graphql",
|
||||||
|
json=pr_merge_info_query,
|
||||||
|
headers=headers,
|
||||||
|
timeout=60, # 60s
|
||||||
|
)
|
||||||
|
resp = req.json()
|
||||||
|
# Response will look like
|
||||||
|
# {'data': {'repository': {'pullRequest':
|
||||||
|
# {
|
||||||
|
# 'viewerMergeHeadlineText': 'SERVER-1234 Add a ton of great support (#32823)',
|
||||||
|
# 'viewerMergeBodyText': 'This PR adds back support for a lot of things\nMany great things!'
|
||||||
|
# }}}}
|
||||||
|
LOGGER.info("Squashed content", content=resp)
|
||||||
|
pr_info = resp["data"]["repository"]["pullRequest"]
|
||||||
|
fake_repo = Repo()
|
||||||
|
return [
|
||||||
|
Commit(
|
||||||
|
repo=fake_repo,
|
||||||
|
binsha=b"00000000000000000000",
|
||||||
|
message="\n".join([pr_info["viewerMergeHeadlineText"], pr_info["viewerMergeBodyText"]]),
|
||||||
|
)
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def get_merge_queue_commits(branch_name: str) -> list[Commit]:
|
||||||
|
assert branch_name
|
||||||
|
|
||||||
diff_commits = subprocess.run(
|
diff_commits = subprocess.run(
|
||||||
["git", "log", '--pretty=format:"%H"', f"{branch_name}...HEAD"],
|
["git", "log", '--pretty=format:"%H"', f"{branch_name}...HEAD"],
|
||||||
|
|
@ -121,8 +157,56 @@ def main(
|
||||||
LOGGER.info("Diff commit hashes", commit_hashs=commit_hashs)
|
LOGGER.info("Diff commit hashes", commit_hashs=commit_hashs)
|
||||||
repo = Repo(repo_root)
|
repo = Repo(repo_root)
|
||||||
|
|
||||||
for commit_hash in commit_hashs:
|
return [repo.commit(commit_hash) for commit_hash in commit_hashs]
|
||||||
commit = repo.commit(commit_hash)
|
|
||||||
|
|
||||||
|
def main(
|
||||||
|
github_org: Annotated[
|
||||||
|
str,
|
||||||
|
typer.Option(envvar="GITHUB_ORG", help="Name of the github organization (e.g. 10gen)"),
|
||||||
|
] = "",
|
||||||
|
github_repo: Annotated[
|
||||||
|
str,
|
||||||
|
typer.Option(envvar="GITHUB_REPO", help="Name of the repo (e.g. mongo)"),
|
||||||
|
] = "",
|
||||||
|
branch_name: Annotated[
|
||||||
|
str,
|
||||||
|
typer.Option(envvar="BRANCH_NAME", help="Name of the branch to compare against HEAD"),
|
||||||
|
] = "",
|
||||||
|
pr_number: Annotated[
|
||||||
|
int,
|
||||||
|
typer.Option(envvar="PR_NUMBER", help="PR Number to compare with"),
|
||||||
|
] = -1,
|
||||||
|
github_token: Annotated[
|
||||||
|
str,
|
||||||
|
typer.Option(envvar="GITHUB_TOKEN", help="Github token with pr read access"),
|
||||||
|
] = "",
|
||||||
|
requester: Annotated[
|
||||||
|
str,
|
||||||
|
typer.Option(
|
||||||
|
envvar="REQUESTER",
|
||||||
|
help="What is requested this task. Defined https://docs.devprod.prod.corp.mongodb.com/evergreen/Project-Configuration/Project-Configuration-Files#expansions.",
|
||||||
|
),
|
||||||
|
] = "",
|
||||||
|
):
|
||||||
|
"""
|
||||||
|
Validate the commit message.
|
||||||
|
|
||||||
|
It validates the latest message when no arguments are provided.
|
||||||
|
"""
|
||||||
|
|
||||||
|
commits: list[Commit] = []
|
||||||
|
if requester == "github_merge_queue":
|
||||||
|
commits = get_merge_queue_commits(branch_name)
|
||||||
|
elif requester == "github_pr":
|
||||||
|
commits = get_non_merge_queue_squashed_commits(
|
||||||
|
github_org, github_repo, pr_number, github_token
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
LOGGER.error("Running with an invalid requester", requester=requester)
|
||||||
|
raise typer.Exit(code=STATUS_ERROR)
|
||||||
|
|
||||||
|
for commit in commits:
|
||||||
if not is_valid_commit(commit):
|
if not is_valid_commit(commit):
|
||||||
LOGGER.error("Found an invalid commit", commit=commit)
|
LOGGER.error("Found an invalid commit", commit=commit)
|
||||||
raise typer.Exit(code=STATUS_ERROR)
|
raise typer.Exit(code=STATUS_ERROR)
|
||||||
|
|
|
||||||
|
|
@ -1516,6 +1516,7 @@ tasks:
|
||||||
docker_password: ${dockerhub_password}
|
docker_password: ${dockerhub_password}
|
||||||
|
|
||||||
- name: validate_commit_message
|
- name: validate_commit_message
|
||||||
|
allowed_requesters: ["github_merge_queue", "github_pr"]
|
||||||
tags:
|
tags:
|
||||||
[
|
[
|
||||||
"assigned_to_jira_team_devprod_correctness",
|
"assigned_to_jira_team_devprod_correctness",
|
||||||
|
|
@ -1532,10 +1533,20 @@ tasks:
|
||||||
- func: "upload pip requirements"
|
- func: "upload pip requirements"
|
||||||
- func: "configure evergreen api credentials"
|
- func: "configure evergreen api credentials"
|
||||||
- func: "f_expansions_write"
|
- func: "f_expansions_write"
|
||||||
|
- command: github.generate_token
|
||||||
|
params:
|
||||||
|
owner: ${github_org}
|
||||||
|
repo: ${github_repo}
|
||||||
|
expansion_name: github_token
|
||||||
|
permissions:
|
||||||
|
pull_requests: read
|
||||||
|
metadata: read
|
||||||
- command: subprocess.exec
|
- command: subprocess.exec
|
||||||
type: test
|
type: test
|
||||||
params:
|
params:
|
||||||
binary: bash
|
binary: bash
|
||||||
|
include_expansions_in_env:
|
||||||
|
- github_token
|
||||||
args:
|
args:
|
||||||
- "./src/evergreen/run_python_script_with_report.sh"
|
- "./src/evergreen/run_python_script_with_report.sh"
|
||||||
- "validate_commit_message"
|
- "validate_commit_message"
|
||||||
|
|
@ -1545,8 +1556,12 @@ tasks:
|
||||||
JIRA_AUTH_ACCESS_TOKEN_SECRET: ${jira_auth_access_token_secret}
|
JIRA_AUTH_ACCESS_TOKEN_SECRET: ${jira_auth_access_token_secret}
|
||||||
JIRA_AUTH_CONSUMER_KEY: ${jira_auth_consumer_key}
|
JIRA_AUTH_CONSUMER_KEY: ${jira_auth_consumer_key}
|
||||||
JIRA_AUTH_KEY_CERT: ${jira_auth_key_cert}
|
JIRA_AUTH_KEY_CERT: ${jira_auth_key_cert}
|
||||||
IS_COMMIT_QUEUE: ${is_commit_queue}
|
REQUESTER: ${requester}
|
||||||
BRANCH_NAME: ${branch_name}
|
BRANCH_NAME: ${branch_name}
|
||||||
|
PR_NUMBER: ${github_pr_number}
|
||||||
|
GITHUB_ORG: ${github_org}
|
||||||
|
GITHUB_REPO: ${github_repo}
|
||||||
|
GITHUB_TOKEN: ${github_token}
|
||||||
|
|
||||||
- name: version_burn_in_gen
|
- name: version_burn_in_gen
|
||||||
run_on: ubuntu2004-medium
|
run_on: ubuntu2004-medium
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue