SERVER-122295: Add check_for_todos to commit queue and PR aliases (#50102)
GitOrigin-RevId: c2f09420181e9259b993542f124bc7fe771ad1bb
This commit is contained in:
parent
62d4aa1239
commit
442d938784
@ -181,7 +181,7 @@ class TestValidateCommitQueue(unittest.TestCase):
|
||||
todos = under_test.TodoChecker()
|
||||
todos.check_file("my file", create_file_iterator(file_contents))
|
||||
|
||||
self.assertFalse(todos.validate_commit_queue(commit_message))
|
||||
self.assertFalse(todos.validate_from_commit(commit_message))
|
||||
|
||||
def test_todos_associated_with_commit_message_should_be_found(self):
|
||||
commit_message = "SERVER-1234 making a commit"
|
||||
@ -196,7 +196,7 @@ class TestValidateCommitQueue(unittest.TestCase):
|
||||
todos = under_test.TodoChecker()
|
||||
todos.check_file("my file", create_file_iterator(file_contents))
|
||||
|
||||
self.assertTrue(todos.validate_commit_queue(commit_message))
|
||||
self.assertTrue(todos.validate_from_commit(commit_message))
|
||||
|
||||
def test_commit_messages_with_multiple_commits_search_all_of_them(self):
|
||||
commit_message = """
|
||||
@ -217,7 +217,7 @@ class TestValidateCommitQueue(unittest.TestCase):
|
||||
todos = under_test.TodoChecker()
|
||||
todos.check_file("my file", create_file_iterator(file_contents))
|
||||
|
||||
self.assertTrue(todos.validate_commit_queue(commit_message))
|
||||
self.assertTrue(todos.validate_from_commit(commit_message))
|
||||
|
||||
def test_commit_messages_with_no_tickets_doesnt_cause_issues(self):
|
||||
commit_message = "A random commit"
|
||||
@ -232,7 +232,7 @@ class TestValidateCommitQueue(unittest.TestCase):
|
||||
todos = under_test.TodoChecker()
|
||||
todos.check_file("my file", create_file_iterator(file_contents))
|
||||
|
||||
self.assertFalse(todos.validate_commit_queue(commit_message))
|
||||
self.assertFalse(todos.validate_from_commit(commit_message))
|
||||
|
||||
|
||||
def write_file(path: str, contents: str):
|
||||
|
||||
@ -10,6 +10,10 @@ from typing import Callable, Iterable, NamedTuple, Optional
|
||||
|
||||
import click
|
||||
|
||||
sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(os.path.realpath(__file__)))))
|
||||
|
||||
|
||||
from buildscripts.validate_commit_message import get_merge_queue_commits
|
||||
from evergreen import RetryingEvergreenApi
|
||||
|
||||
EVG_CONFIG_FILE = "./.evergreen.yml"
|
||||
@ -151,7 +155,7 @@ class TodoChecker:
|
||||
|
||||
return True
|
||||
|
||||
def validate_commit_queue(self, commit_message: str) -> bool:
|
||||
def validate_from_commit(self, commit_message: str) -> bool:
|
||||
"""
|
||||
Check that the given commit message does not reference TODO comments.
|
||||
|
||||
@ -213,16 +217,21 @@ def get_summary_for_patch(version_id: str) -> str:
|
||||
@click.command()
|
||||
@click.option("--ticket", help="Only report on TODOs associated with given Jira ticket.")
|
||||
@click.option("--base-dir", default=BASE_SEARCH_DIR, help="Base directory to search in.")
|
||||
@click.option(
|
||||
"--commit-message", help="For commit-queue execution only, ensure no TODOs for this commit"
|
||||
)
|
||||
@click.option(
|
||||
"--patch-build",
|
||||
type=str,
|
||||
help="For patch build execution only, check for any TODOs from patch description",
|
||||
)
|
||||
@click.option(
|
||||
"--merge-queue-branch",
|
||||
type=str,
|
||||
help="For merge queue execution only, branch name to compare against HEAD",
|
||||
)
|
||||
def main(
|
||||
ticket: Optional[str], base_dir: str, commit_message: Optional[str], patch_build: Optional[str]
|
||||
ticket: Optional[str],
|
||||
base_dir: str,
|
||||
patch_build: Optional[str],
|
||||
merge_queue_branch: Optional[str],
|
||||
):
|
||||
"""
|
||||
Search for and report on TODO comments in the code base.
|
||||
@ -266,23 +275,24 @@ def main(
|
||||
\f
|
||||
:param ticket: Only report on TODOs associated with this jira ticket.
|
||||
:param base_dir: Search files in this base directory.
|
||||
:param commit_message: Commit message if running in the commit-queue.
|
||||
:param merge_queue_branch: Branch name to compare against HEAD if running in the merge queue.
|
||||
:param patch_build: Version ID of patch build to check.
|
||||
|
||||
"""
|
||||
if commit_message and ticket is not None:
|
||||
if merge_queue_branch and ticket is not None:
|
||||
raise click.UsageError("--ticket cannot be used in commit queue.")
|
||||
|
||||
todo_checker = TodoChecker()
|
||||
todo_checker.check_all_files(base_dir)
|
||||
|
||||
if patch_build:
|
||||
if ticket is not None:
|
||||
raise click.UsageError("--ticket cannot be used in patch builds.")
|
||||
commit_message = get_summary_for_patch(patch_build)
|
||||
|
||||
todo_checker = TodoChecker()
|
||||
todo_checker.check_all_files(base_dir)
|
||||
|
||||
if commit_message:
|
||||
found_todos = todo_checker.validate_commit_queue(commit_message)
|
||||
found_todos = todo_checker.validate_from_commit(commit_message)
|
||||
elif merge_queue_branch:
|
||||
commits = get_merge_queue_commits(merge_queue_branch)
|
||||
found_todos = any(todo_checker.validate_from_commit(commit.message) for commit in commits)
|
||||
elif ticket:
|
||||
found_todos = todo_checker.report_on_ticket(ticket)
|
||||
else:
|
||||
|
||||
@ -33,8 +33,9 @@ def is_interesting_file(file_name: str) -> bool:
|
||||
and not file_name.startswith("src/mongo/gotools/")
|
||||
and not file_name.startswith("src/streams/third_party")
|
||||
and not file_name.startswith("src/mongo/db/modules/enterprise/src/streams/third_party")
|
||||
# Exclude this file because it's filled with TODO SERVER-XXXXX patterns
|
||||
# Exclude these two files because they are filled with TODO SERVER-XXXXX patterns
|
||||
and not file_name == "buildscripts/todo_linter.py"
|
||||
and not file_name == "buildscripts/todo_check.py"
|
||||
and FILES_RE.search(file_name) is not None
|
||||
)
|
||||
|
||||
|
||||
@ -112,7 +112,7 @@ parameters:
|
||||
|
||||
commit_queue_aliases:
|
||||
- variant: "^(commit-queue)$"
|
||||
task: "^(bazel_.*|run_.*|unit_test.*|compile_.*|lint_.*|resmoke_tests|check_for_noexcept|version_gen_validation|validate_commit_message|resmoke_validation_tests|buildscripts_test)$"
|
||||
task: "^(bazel_.*|run_.*|unit_test.*|compile_.*|lint_.*|resmoke_tests|check_for_noexcept|version_gen_validation|validate_commit_message|resmoke_validation_tests|buildscripts_test|check_for_todos)$"
|
||||
variant_tags: []
|
||||
task_tags: []
|
||||
- variant: "^(amazon-linux2023-arm64-static-compile)$"
|
||||
@ -130,7 +130,7 @@ commit_queue_aliases:
|
||||
|
||||
github_pr_aliases:
|
||||
- variant: "^(commit-queue)$"
|
||||
task: "^(bazel_.*|run_.*|unit_test.*|compile_.*|lint_.*|resmoke_tests|check_for_noexcept|version_gen_validation|validate_commit_message|resmoke_validation_tests|buildscripts_test)$"
|
||||
task: "^(bazel_.*|run_.*|unit_test.*|compile_.*|lint_.*|resmoke_tests|check_for_noexcept|version_gen_validation|validate_commit_message|resmoke_validation_tests|buildscripts_test|check_for_todos)$"
|
||||
variant_tags: []
|
||||
task_tags: []
|
||||
- variant: "^(amazon-linux2023-arm64-static-compile)$"
|
||||
|
||||
@ -8,18 +8,8 @@ activate_venv
|
||||
set -o verbose
|
||||
set -o errexit
|
||||
|
||||
# Since `commit_message` is an evergreen expansion, we need a way to ensure we
|
||||
# properly deal with any special characters that could cause issues (like "). To
|
||||
# do this, we will write it out to a file, then read that file into a variable.
|
||||
if [ "${is_commit_queue}" == "true" ]; then
|
||||
cat >commit_message.txt <<END_OF_COMMIT_MSG
|
||||
${commit_message}
|
||||
END_OF_COMMIT_MSG
|
||||
|
||||
commit_message_content=$(cat commit_message.txt)
|
||||
rm commit_message.txt
|
||||
|
||||
$python buildscripts/todo_check.py --commit-message "$commit_message_content"
|
||||
if [ "${requester}" == "github_merge_queue" ]; then
|
||||
$python buildscripts/todo_check.py --merge-queue-branch "${branch_name}"
|
||||
elif [ "${is_patch}" == "true" ]; then
|
||||
$python buildscripts/todo_check.py --patch-build ${version_id}
|
||||
fi
|
||||
|
||||
Loading…
Reference in New Issue
Block a user