From 53588bc4557c1bf9e62f8ad8094fe6a803fc5ce2 Mon Sep 17 00:00:00 2001 From: Ben Hearsum Date: Thu, 20 Aug 2026 12:18:05 -0400 Subject: [PATCH] add support for running bugbug-selected tasks for autoland-based reviewbot pushes A large percentage of failures on autoland are caught by bugbug-selected tasks. I'd like to experiment with having bugbug-selected tasks run on reviewbot pushes to see if it helps developers notice and fix such problems earlier. Enabling this mostly requires tweaks to the parameters; it will also require a change on the Gecko side to ensure all of the right tasks are in target tasks (otherwise there's nothing bugbug to choose from). --- bot/code_review_bot/config.py | 6 ++++++ bot/code_review_bot/vcs.py | 16 ++++++++++++++ bot/tests/test_mercurial.py | 27 ++++++++++++++++++------ bot/tests/test_vcs.py | 39 +++++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 bot/tests/test_vcs.py diff --git a/bot/code_review_bot/config.py b/bot/code_review_bot/config.py index 3fa1cb239..fa1c9f912 100644 --- a/bot/code_review_bot/config.py +++ b/bot/code_review_bot/config.py @@ -67,6 +67,12 @@ def __init__(self): # Indexed by their Phabricator ID self.user_blacklist = {} + # bugbug test configuration + self.bugbug_enabled_repositories = ["firefox-autoland"] + # Because it's unclear how much load this will add, this is being rolled out gradually. + self.bugbug_enabled_percent = 0.1 + self.bugbug_optimize_strategy = "gecko_taskgraph.optimize:tryselect.bugbug_reduced_manifests_config_selection_medium" + # Always cleanup at the end of the execution atexit.register(self.cleanup) # caching the versions of the app diff --git a/bot/code_review_bot/vcs.py b/bot/code_review_bot/vcs.py index 3dd5d9d8e..f72772e2d 100644 --- a/bot/code_review_bot/vcs.py +++ b/bot/code_review_bot/vcs.py @@ -4,12 +4,14 @@ import json import os +import random import time import rs_parsepatch import structlog from libmozdata.phabricator import PhabricatorPatch +from code_review_bot.config import settings from code_review_bot.sources.phabricator import PhabricatorBuild logger = structlog.get_logger(__name__) @@ -22,6 +24,13 @@ PUSH_RETRY_EXPONENTIAL_DELAY = 6 +def bugbug_enabled(repository_name): + if repository_name not in settings.bugbug_enabled_repositories: + return False + + return random.random() < settings.bugbug_enabled_percent + + class RetryNeeded(Exception): """ Raised when retrying a build is needed @@ -176,6 +185,13 @@ def add_try_commit(self, build): "phabricator_diff": build.target_phid, }, } + + if bugbug_enabled(self.name): + config["parameters"]["test_manifest_loader"] = "bugbug" + config["parameters"]["optimize_strategies"] = ( + settings.bugbug_optimize_strategy + ) + diff_phid = build.stack[-1].phid if build.revision_url: diff --git a/bot/tests/test_mercurial.py b/bot/tests/test_mercurial.py index 443da766d..e0827679e 100644 --- a/bot/tests/test_mercurial.py +++ b/bot/tests/test_mercurial.py @@ -7,11 +7,13 @@ from unittest.mock import MagicMock import hglib +import pytest import responses from conftest import MockBuild from libmozdata.phabricator import PhabricatorPatch from code_review_bot import mercurial +from code_review_bot.config import settings class STDOutputMock: @@ -73,11 +75,20 @@ def test_robustcheckout(monkeypatch): ] -def test_push_to_try(PhabricatorMock, mock_mc, responses): +@pytest.mark.parametrize("bugbug", [False, True], ids=["bugbug-off", "bugbug-on"]) +def test_push_to_try(PhabricatorMock, mock_mc, responses, monkeypatch, bugbug): """ Run mercurial worker on a single diff with a push to try server + + The bugbug parameters only affect try_task_config.json: the commits + themselves are identical whether the repository is selected or not """ + if bugbug: + monkeypatch.setattr(settings, "bugbug_enabled_repositories", [mock_mc.name]) + monkeypatch.setattr(settings, "bugbug_enabled_percent", 1) + monkeypatch.setattr(settings, "bugbug_optimize_strategy", "test:strategy") + # Preload the build diff = { "phid": "PHID-DIFF-test123", @@ -128,14 +139,18 @@ def test_push_to_try(PhabricatorMock, mock_mc, responses): assert open(target).read() == "First Line\nSecond Line\n" # Check the try_task_config file + expected_parameters = { + "target_tasks_method": "codereview", + "optimize_target_tasks": True, + "phabricator_diff": "PHID-HMBT-deadbeef", + } + if bugbug: + expected_parameters["test_manifest_loader"] = "bugbug" + expected_parameters["optimize_strategies"] = "test:strategy" assert os.path.exists(config) assert json.load(open(config)) == { "version": 2, - "parameters": { - "target_tasks_method": "codereview", - "optimize_target_tasks": True, - "phabricator_diff": "PHID-HMBT-deadbeef", - }, + "parameters": expected_parameters, } # Get tip commit in repo diff --git a/bot/tests/test_vcs.py b/bot/tests/test_vcs.py new file mode 100644 index 000000000..85ff6ffa3 --- /dev/null +++ b/bot/tests/test_vcs.py @@ -0,0 +1,39 @@ +# 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/. +from code_review_bot import vcs +from code_review_bot.config import settings + + +def test_bugbug_default_configuration(): + """ + The shipped rollout configuration is usable: a percentage expressed as a + ratio, a non empty allow list and a non empty optimize strategy + """ + assert 0 <= settings.bugbug_enabled_percent <= 1 + assert settings.bugbug_enabled_repositories + assert settings.bugbug_optimize_strategy + + +def test_bugbug_disabled_for_unlisted_repository(monkeypatch): + """ + Repositories outside of the allow list are never selected, even when the + rollout percentage is at its maximum + """ + monkeypatch.setattr(settings, "bugbug_enabled_percent", 1) + + assert vcs.bugbug_enabled("mozilla-central") is False + assert vcs.bugbug_enabled("nss") is False + + +def test_bugbug_enabled_for_listed_repository(monkeypatch): + """ + Repositories from the allow list are selected by the rollout percentage + """ + monkeypatch.setattr(settings, "bugbug_enabled_repositories", ["autoland"]) + + monkeypatch.setattr(settings, "bugbug_enabled_percent", 1) + assert vcs.bugbug_enabled("autoland") is True + + monkeypatch.setattr(settings, "bugbug_enabled_percent", 0) + assert vcs.bugbug_enabled("autoland") is False