From bb9ec2e4be3c3ef815b552815806501648281c3d Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Fri, 18 Sep 2026 17:51:19 +0200 Subject: [PATCH] Remove needs-testing-tag if present when we add a testing tag --- bot/code_review_bot/testing_policy.py | 16 ++++++++++++---- bot/tests/test_testing_policy.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/bot/code_review_bot/testing_policy.py b/bot/code_review_bot/testing_policy.py index f5d807404..24f2eb882 100644 --- a/bot/code_review_bot/testing_policy.py +++ b/bot/code_review_bot/testing_policy.py @@ -26,6 +26,9 @@ # Phabricator project tagging commits that come with their own tests TESTING_APPROVED_PHID = "PHID-PROJ-h7y4cs7m2o67iczw62pp" +# Phabricator project flagging revisions still missing a testing policy tag +NEEDS_TESTING_TAG_PHID = "PHID-PROJ-j3au2u2ypmko4ndmzmcu" + # All the testing policy projects on Phabricator, a revision should only have one of them TESTING_POLICY_TAG_PHIDS = frozenset( [ @@ -254,10 +257,15 @@ def apply_testing_policy_tag(api, revision): ) return None - api.edit_revision( - revision.phabricator_id, - [{"type": "projects.add", "value": [tag_phid]}], - ) + transactions = [{"type": "projects.add", "value": [tag_phid]}] + + # The revision is not missing a tag anymore + if NEEDS_TESTING_TAG_PHID in project_phids: + transactions.append( + {"type": "projects.remove", "value": [NEEDS_TESTING_TAG_PHID]} + ) + + api.edit_revision(revision.phabricator_id, transactions) except Exception as e: # Tagging is a best effort feature that should never block the publication logger.warning( diff --git a/bot/tests/test_testing_policy.py b/bot/tests/test_testing_policy.py index a3c67dfbe..f69cbb8cb 100644 --- a/bot/tests/test_testing_policy.py +++ b/bot/tests/test_testing_policy.py @@ -7,6 +7,7 @@ import pytest from code_review_bot.testing_policy import ( + NEEDS_TESTING_TAG_PHID, TESTING_APPROVED_PHID, TESTING_EXCEPTION_UI_PHID, TESTING_EXCEPTION_UNCHANGED_PHID, @@ -289,6 +290,23 @@ def test_apply_tag_no_projects(api, revision): api.edit_revision.assert_called_once() +def test_apply_tag_removes_needs_testing_tag(api, revision): + """The needs-testing-tag flag is removed along with adding the tag""" + api.load_revision.return_value = _revision_data( + [OTHER_PROJECT_PHID, NEEDS_TESTING_TAG_PHID] + ) + + assert apply_testing_policy_tag(api, revision) == TESTING_EXCEPTION_UNCHANGED_PHID + + api.edit_revision.assert_called_once_with( + REVISION_ID, + [ + {"type": "projects.add", "value": [TESTING_EXCEPTION_UNCHANGED_PHID]}, + {"type": "projects.remove", "value": [NEEDS_TESTING_TAG_PHID]}, + ], + ) + + def test_no_tag_for_code_changes(api, revision): """Nothing happens when the patch touches code""" revision.files = ["docs/index.rst", "dom/base/nsDocument.cpp"]