From 442d9387843dce3ef5f0bd45f2bbdb64b37432aa Mon Sep 17 00:00:00 2001 From: Sean Lyons Date: Fri, 27 Mar 2026 16:23:09 -0400 Subject: [PATCH] SERVER-122295: Add check_for_todos to commit queue and PR aliases (#50102) GitOrigin-RevId: c2f09420181e9259b993542f124bc7fe771ad1bb --- buildscripts/tests/test_todo_check.py | 8 +++--- buildscripts/todo_check.py | 36 +++++++++++++++++---------- buildscripts/todo_linter.py | 3 ++- etc/evergreen.yml | 4 +-- evergreen/todos_check.sh | 14 ++--------- 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/buildscripts/tests/test_todo_check.py b/buildscripts/tests/test_todo_check.py index 61227f7a95c..cc678a3806a 100644 --- a/buildscripts/tests/test_todo_check.py +++ b/buildscripts/tests/test_todo_check.py @@ -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): diff --git a/buildscripts/todo_check.py b/buildscripts/todo_check.py index d27d1a2f989..5743aedeb6d 100755 --- a/buildscripts/todo_check.py +++ b/buildscripts/todo_check.py @@ -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: diff --git a/buildscripts/todo_linter.py b/buildscripts/todo_linter.py index 0cdce2d7d73..10503382d13 100644 --- a/buildscripts/todo_linter.py +++ b/buildscripts/todo_linter.py @@ -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 ) diff --git a/etc/evergreen.yml b/etc/evergreen.yml index 9488c5300a5..79e0f995844 100644 --- a/etc/evergreen.yml +++ b/etc/evergreen.yml @@ -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)$" diff --git a/evergreen/todos_check.sh b/evergreen/todos_check.sh index 6ee743fa25e..8cdf4565f52 100644 --- a/evergreen/todos_check.sh +++ b/evergreen/todos_check.sh @@ -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 <