Skip to content

Remove needs-testing-tag if present when we add a testing tag #3665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marco-c merged 1 commit into from
Sep 21, 2026
+30 −4
Merged

Remove needs-testing-tag if present when we add a testing tag #3665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions bot/code_review_bot/testing_policy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
[
Expand Down Expand Up @@ -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(
Expand Down
18 changes: 18 additions & 0 deletions bot/tests/test_testing_policy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"]
Expand Down