Fix PR matching on cleanup

GithubSync.close_existing_prs_for_series() is supposed to close all
*other* PRs associated with a series being processed.  So far it only
looked at the *branch* name (`series/<id>=><base>`), when deciding
whether two PRs match.  This worked due to a bug, fixed in 88cb274,
which caused the oldest series id (branch name) to always be
associated with the series name (PR title).  Now that the bug is
fixed, we have to match by name during clean up, which is what happens
in this change.

Differential Revision: D79763361

Reviewed-by: Amery Hung <[email protected]>
Signed-off-by: Ihor Solodrai <[email protected]>

Author: theihor

kernel_patches_daemon/branch_worker.py

@@ -436,6 +436,12 @@ def same_series_different_target(ref: str, other_ref: str) -> bool:
     return ref1["series"] == ref2["series"] and ref1["target"] != ref2["target"]
 
 
+def prs_for_the_same_series(pr1: PullRequest, pr2: PullRequest) -> bool:
+    return pr1.title == pr2.title or same_series_different_target(
+        pr1.head.ref, pr2.head.ref
+    )
+
+
 def _reset_repo(repo, branch: str) -> None:
     """
     Reset the repository into a known good state, with `branch` checked out.

kernel_patches_daemon/github_sync.py

@@ -20,7 +20,7 @@
     parse_pr_ref,
     parsed_pr_ref_ok,
     pr_has_label,
-    same_series_different_target,
+    prs_for_the_same_series,
 )
 from kernel_patches_daemon.config import BranchConfig, KPDConfig
 from kernel_patches_daemon.github_logs import (
@@ -156,12 +156,19 @@ def close_existing_prs_for_series(
         the same series name, but different remote branch and close them.
         """
 
-        prs_to_close = [
+        existing_prs = [
             existing_pr
             for worker in workers
             for existing_pr in worker.prs.values()
-            if same_series_different_target(pr.head.ref, existing_pr.head.ref)
+            if existing_pr.number != pr.number
+        ]
+
+        prs_to_close = [
+            existing_pr
+            for existing_pr in existing_prs
+            if prs_for_the_same_series(pr, existing_pr)
         ]
+
         # Remove matching PRs from other workers
         for pr_to_close in prs_to_close:
             logging.info(

tests/test_branch_worker.py

@@ -6,15 +6,14 @@
 
 # pyre-unsafe
 
-import os
 import random
 import re
 import shutil
 import tempfile
 import unittest
 from dataclasses import dataclass, field
 from datetime import datetime
-from typing import Any, Dict, List
+from typing import Any, Dict, List, Optional
 from unittest.mock import MagicMock, patch
 
 import git
@@ -35,6 +34,7 @@
     EmailBodyContext,
     furnish_ci_email_body,
     parse_pr_ref,
+    prs_for_the_same_series,
     same_series_different_target,
     temporary_patch_file,
     UPSTREAM_REMOTE_NAME,
@@ -892,6 +892,43 @@ def test_same_series_different_target(self) -> None:
                 )
             )
 
+    def test_prs_for_the_same_series(self) -> None:
+        def create_mock_pr(title: Optional[str], head_ref: str) -> MagicMock:
+            pr = MagicMock()
+            pr.title = title
+            pr.head.ref = head_ref
+            return pr
+
+        with self.subTest("same_title_different_series"):
+            # Title match should return True regardless of series
+            pr1 = create_mock_pr("Fix memory leak", "series/123=>main")
+            pr2 = create_mock_pr("Fix memory leak", "series/456=>bpf-next")
+            self.assertTrue(prs_for_the_same_series(pr1, pr2))
+
+        with self.subTest("different_title_same_series_different_target"):
+            # Same series, different target should return True regardless of title
+            pr1 = create_mock_pr("Fix memory leak", "series/123=>main")
+            pr2 = create_mock_pr("Different title", "series/123=>bpf-next")
+            self.assertTrue(prs_for_the_same_series(pr1, pr2))
+
+        with self.subTest("different_title_different_series"):
+            # No match
+            pr1 = create_mock_pr("Fix memory leak", "series/123=>main")
+            pr2 = create_mock_pr("Different title", "series/456=>main")
+            self.assertFalse(prs_for_the_same_series(pr1, pr2))
+
+        with self.subTest("none_vs_string_title"):
+            # None vs string title should not match
+            pr1 = create_mock_pr(None, "series/123=>main")
+            pr2 = create_mock_pr("Fix memory leak", "series/456=>bpf-next")
+            self.assertFalse(prs_for_the_same_series(pr1, pr2))
+
+        with self.subTest("malformed_refs_no_match"):
+            # Malformed refs with different titles should return False
+            pr1 = create_mock_pr("Fix memory leak", "invalid-ref-format")
+            pr2 = create_mock_pr("Different title", "also-invalid")
+            self.assertFalse(prs_for_the_same_series(pr1, pr2))
+
 
 class TestGitSeriesAlreadyApplied(unittest.IsolatedAsyncioTestCase):
     def setUp(self):