diff options
| -rw-r--r-- | scripts/pr_quality/check_pr.py | 11 | ||||
| -rw-r--r-- | scripts/pr_quality/errors.py | 11 | ||||
| -rw-r--r-- | scripts/pr_quality/tests/test_check_pr.py | 41 |
3 files changed, 36 insertions, 27 deletions
diff --git a/scripts/pr_quality/check_pr.py b/scripts/pr_quality/check_pr.py index 054d3e6adf..51d106e875 100644 --- a/scripts/pr_quality/check_pr.py +++ b/scripts/pr_quality/check_pr.py @@ -57,6 +57,8 @@ URLOPEN_TIMEOUT_SECONDS = 15 PR_TEMPLATE_DATE = date(2024, 3, 4) # 3fcef50 -- PR template introduced AI_DISCLOSURE_DATE = date(2026, 1, 8) # 4f580c4 -- AI disclosure added +ALLOWED_STAGES = ("Accepted", "Ready for checkin") + logger = logging.getLogger(__name__) @@ -215,7 +217,8 @@ def check_trac_ticket(pr_body, total_changes, threshold=LARGE_PR_THRESHOLD): def check_trac_status(ticket_id, ticket_data): - """The referenced Trac ticket must be Accepted, unresolved, and assigned. + """The referenced Trac ticket must be Accepted or Ready for checkin, + unresolved, and assigned. ticket_data is the dict returned by fetch_trac_ticket(). Passing None skips the check (non-fatal fetch error). Passing TICKET_NOT_FOUND fails @@ -232,10 +235,10 @@ def check_trac_status(ticket_id, ticket_data): stage = ticket_data.get("custom", {}).get("stage", "").strip() resolution = (ticket_data.get("resolution") or "").strip() status = ticket_data.get("status", "").strip() - if stage == "Accepted" and not resolution and status == "assigned": + if stage in ALLOWED_STAGES and not resolution and status == "assigned": return None current_state = [ - f"{stage=}" if stage != "Accepted" else "", + f"{stage=}" if stage not in ALLOWED_STAGES else "", f"{resolution=}" if resolution else "", f"{status=}" if status != "assigned" else "", ] @@ -480,7 +483,7 @@ def main( results = [ ("Trac ticket referenced", ticket_result, LEVEL_ERROR), - ("Trac ticket status is Accepted", ticket_status_result, LEVEL_ERROR), + ("Trac ticket is ready for work", ticket_status_result, LEVEL_ERROR), ("Trac ticket has_patch flag set", ticket_has_patch_result, LEVEL_WARNING), ("PR title includes ticket number", pr_title_result, LEVEL_WARNING), ("Branch description provided", check_branch_description(pr_body), LEVEL_ERROR), diff --git a/scripts/pr_quality/errors.py b/scripts/pr_quality/errors.py index 8b98873f6d..cd305868ac 100644 --- a/scripts/pr_quality/errors.py +++ b/scripts/pr_quality/errors.py @@ -56,18 +56,17 @@ INCOMPLETE_CHECKLIST = ( INVALID_TRAC_STATUS = ( "Trac Ticket Not Ready for a Pull Request", "The referenced ticket **ticket-{ticket_id}** is not ready for a pull request. " - "A ticket must be in the *Accepted* stage, *assigned* status, and have no " - "resolution.\n\n" + "A ticket must be in the *Accepted* or *Ready for checkin* stage, " + "*assigned* status, and have no resolution.\n\n" "**Current state:** {current_state}\n\n" "**What to do:**\n\n" f"1. Check the ticket at {TRAC_URL}/ticket/{{ticket_id}}.\n" "2. If *Unreviewed*, wait for a community member to accept it. " "A ticket cannot be accepted by its reporter.\n" - "3. If *Ready for Checkin*, there is already a solution for it.\n" - "4. If *Someday/Maybe*, discuss on the " + "3. If *Someday/Maybe*, discuss on the " f"[Django Forum]({FORUM_URL}/c/internals/5) before proceeding.\n" - "5. If resolved or closed, it is not eligible for a PR.\n" - "6. If not *assigned*, claim it by setting yourself as the owner.\n\n" + "4. If resolved or closed, it is not eligible for a PR.\n" + "5. If not *assigned*, claim it by setting yourself as the owner.\n\n" f"For more information on the Django triage process see {TRIAGING_URL}.", ) diff --git a/scripts/pr_quality/tests/test_check_pr.py b/scripts/pr_quality/tests/test_check_pr.py index f238ebb0c6..c7565df5ab 100644 --- a/scripts/pr_quality/tests/test_check_pr.py +++ b/scripts/pr_quality/tests/test_check_pr.py @@ -318,27 +318,34 @@ class TestCheckTracStatus(BaseTestCase): data = make_trac_json(stage="Accepted", status="assigned", resolution=None) self.assertIsNone(check_pr.check_trac_status("36991", data)) + def test_ready_for_checkin_assigned_unresolved_passes(self): + data = make_trac_json( + stage="Ready for checkin", status="assigned", resolution=None + ) + self.assertIsNone(check_pr.check_trac_status("36991", data)) + def test_non_accepted_stage_fails(self): - for stage in ["Unreviewed", "Ready for Checkin", "Someday/Maybe"]: + for stage in ["Unreviewed", "Someday/Maybe"]: with self.subTest(stage=stage): data = make_trac_json(stage=stage) self.assertIsNotNone(check_pr.check_trac_status("36991", data)) def test_resolved_ticket_fails(self): - for resolution in [ - "fixed", - "wontfix", - "duplicate", - "invalid", - "worksforme", - "needsinfo", - "needsnewfeatureprocess", - ]: - with self.subTest(resolution=resolution): - data = make_trac_json( - stage="Accepted", status="assigned", resolution=resolution - ) - self.assertIsNotNone(check_pr.check_trac_status("36991", data)) + for stage in ("Accepted", "Ready for checkin"): + for resolution in [ + "fixed", + "wontfix", + "duplicate", + "invalid", + "worksforme", + "needsinfo", + "needsnewfeatureprocess", + ]: + with self.subTest(stage=stage, resolution=resolution): + data = make_trac_json( + stage=stage, status="assigned", resolution=resolution + ) + self.assertIsNotNone(check_pr.check_trac_status("36991", data)) def test_unassigned_ticket_fails(self): for status in ["new", "closed", ""]: @@ -743,7 +750,7 @@ class TestIntegration(BaseTestCase): _, mock_summary, _ = self.call_main(pr_body=body) _, results, _ = mock_summary.call_args.args result_map = {name: result for name, result, _ in results} - self.assertIs(result_map["Trac ticket status is Accepted"], check_pr.SKIPPED) + self.assertIs(result_map["Trac ticket is ready for work"], check_pr.SKIPPED) self.assertIs(result_map["Trac ticket has_patch flag set"], check_pr.SKIPPED) self.assertIs(result_map["PR title includes ticket number"], check_pr.SKIPPED) @@ -754,7 +761,7 @@ class TestIntegration(BaseTestCase): ) _, results, _ = mock_summary.call_args.args result_map = {name: result for name, result, _ in results} - self.assertIsNotNone(result_map["Trac ticket status is Accepted"]) + self.assertIsNotNone(result_map["Trac ticket is ready for work"]) self.assertIs(result_map["Trac ticket has_patch flag set"], check_pr.SKIPPED) def test_established_author_skips_all_checks(self): |