branch_worker: helper renames

A couple of helper functions refered to "series/123" part of a branch
name ("series/123=>target") as "base" which is quite confusing.

Rename relevant functions and vars to make them more clear.

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

Author: theihor

kernel_patches_daemon/branch_worker.py

@@ -398,21 +398,22 @@ def create_color_labels(labels_cfg: Dict[str, str], repo: Repository) -> None:
             repo.create_label(name=label, color=color)
 
 
-def get_base_branch_from_ref(ref: str) -> str:
-    return ref.split(HEAD_BASE_SEPARATOR)[0]
+def parse_pr_ref(ref: str) -> List[str]:
+    # "series123=>target" -> ["series123", "target"]
+    return ref.split(HEAD_BASE_SEPARATOR)
 
 
-def has_same_base_different_remote(ref: str, other_ref: str) -> bool:
+def same_series_different_remote(ref: str, other_ref: str) -> bool:
     if ref == other_ref:
         return False
+    series = parse_pr_ref(ref)[0]
+    other_series = parse_pr_ref(other_ref)[0]
+    return series == other_series
 
-    base = get_base_branch_from_ref(ref)
-    other_base = get_base_branch_from_ref(other_ref)
 
-    if base != other_base:
-        return False
-
-    return True
+def series_id_from_ref(ref: str) -> int:
+    series_str = parse_pr_ref(ref)[0]
+    return int(series_str.split("/")[1])
 
 
 def _reset_repo(repo, branch: str) -> None:

kernel_patches_daemon/github_sync.py

@@ -15,8 +15,8 @@
 from github.PullRequest import PullRequest
 from kernel_patches_daemon.branch_worker import (
     BranchWorker,
-    get_base_branch_from_ref,
-    has_same_base_different_remote,
+    same_series_different_remote,
+    series_id_from_ref,
     HEAD_BASE_SEPARATOR,
     NewPRWithNoChangeException,
 )
@@ -143,20 +143,20 @@ async def get_mapped_branches(self, series: Series) -> List[str]:
         logging.info(f"Mapped to default branch order: {mapped_branches}")
         return mapped_branches
 
-    def close_existing_prs_with_same_base(
+    def close_existing_prs_for_series(
         self, workers: Sequence["BranchWorker"], pr: PullRequest
     ) -> None:
-        """Close existing pull requests with the same base, but different remote branch
+        """Close existing pull requests for the same series, but different target branch
 
         For given pull request, find all other pull requests with
-        the same base, but different remote branch and close them.
+        the same series name, but different remote branch and close them.
         """
 
         prs_to_close = [
             existing_pr
             for worker in workers
             for existing_pr in worker.prs.values()
-            if has_same_base_different_remote(pr.head.ref, existing_pr.head.ref)
+            if same_series_different_remote(pr.head.ref, existing_pr.head.ref)
         ]
         # Remove matching PRs from other workers
         for pr_to_close in prs_to_close:
@@ -170,7 +170,7 @@ def close_existing_prs_with_same_base(
                     del worker.prs[pr_to_close.title]
 
     async def checkout_and_patch_safe(
-        self, worker, branch_name: str, series_to_apply: Series
+        self, worker: BranchWorker, branch_name: str, series_to_apply: Series
     ) -> Optional[PullRequest]:
         try:
             self.increment_counter("all_known_subjects")
@@ -221,6 +221,7 @@ async def sync_relevant_subject(self, subject: Subject) -> None:
                     continue
                 else:
                     logging.info(msg + "no more next, staying.")
+
             logging.info(f"Choosing branch {branch} to create/update PR.")
             pr = await self.checkout_and_patch_safe(worker, pr_branch_name, series)
             if pr is None:
@@ -231,7 +232,7 @@ async def sync_relevant_subject(self, subject: Subject) -> None:
             )
             await worker.sync_checks(pr, series)
             # Close out other PRs if exists
-            self.close_existing_prs_with_same_base(list(self.workers.values()), pr)
+            self.close_existing_prs_for_series(list(self.workers.values()), pr)
 
             break
         pass
@@ -289,7 +290,7 @@ async def sync_patches(self) -> None:
                     if "/" not in pr.head.ref or HEAD_BASE_SEPARATOR not in pr.head.ref:
                         continue
 
-                    series_id = int(get_base_branch_from_ref(pr.head.ref.split("/")[1]))
+                    series_id = series_id_from_ref(pr.head.ref)
                     series = await self.pw.get_series_by_id(series_id)
                     subject = self.pw.get_subject_by_series(series)
                     if subject_name != subject.subject:

tests/test_branch_worker.py

@@ -34,7 +34,7 @@
     email_in_submitter_allowlist,
     EmailBodyContext,
     furnish_ci_email_body,
-    has_same_base_different_remote,
+    same_series_different_remote,
     HEAD_BASE_SEPARATOR,
     temporary_patch_file,
     UPSTREAM_REMOTE_NAME,
@@ -869,21 +869,23 @@ def test_create_color_labels(self) -> None:
             repo_mock.create_label.assert_not_called()
             repo_label_mock.edit.assert_called_once_with(name="label", color="00000")
 
-    def test_has_same_base_different_remote(self) -> None:
-        with self.subTest("same_base_different_remote"):
+    def same_series_different_remote(self) -> None:
+        with self.subTest("same_series_different_remote"):
             self.assertTrue(
-                has_same_base_different_remote("base=>remote", "base=>other_remote")
+                same_series_different_remote(
+                    "series/1=>remote", "series/1=>other_remote"
+                )
             )
 
-        with self.subTest("different_base_same_remote"):
+        with self.subTest("different_series_same_remote"):
             self.assertFalse(
-                has_same_base_different_remote("base=>remote", "other_base=>remote")
+                same_series_different_remote("series/1=>remote", "series/2=>remote")
             )
 
-        with self.subTest("different_base_different_remote"):
+        with self.subTest("different_series_different_remote"):
             self.assertFalse(
-                has_same_base_different_remote(
-                    "base=>remote", "other_base=>other_remote"
+                same_series_different_remote(
+                    "series/1=>remote", "series/2=>other_remote"
                 )
             )
 

tests/test_github_sync.py

@@ -118,7 +118,7 @@ class TestCase:
                     gh.workers[TEST_BRANCH].ci_repo_dir.startswith(case.prefix),
                 )
 
-    def test_close_existing_prs_with_same_base(self) -> None:
+    def test_close_existing_prs_for_series(self) -> None:
         matching_pr_mock = MagicMock()
         matching_pr_mock.title = "matching"
         matching_pr_mock.head.ref = "test_branch=>remote_branch"
@@ -137,7 +137,7 @@ def test_close_existing_prs_with_same_base(self) -> None:
         input_pr_mock.head.ref = "test_branch=>other_remote_branch"
 
         workers = [copy.copy(branch_worker_mock) for _ in range(2)]
-        self._gh.close_existing_prs_with_same_base(workers, input_pr_mock)
+        self._gh.close_existing_prs_for_series(workers, input_pr_mock)
         for worker in workers:
             self.assertEqual(len(worker.prs), 1)
             self.assertTrue("irrelevant" in worker.prs)
@@ -212,7 +212,7 @@ async def test_sync_relevant_subject_success_first_branch(self) -> None:
         pr_mock.head.ref = series_branch_name
 
         self._gh.checkout_and_patch_safe = AsyncMock(return_value=pr_mock)
-        self._gh.close_existing_prs_with_same_base = MagicMock()
+        self._gh.close_existing_prs_for_series = MagicMock()
 
         worker_mock = self._gh.workers[TEST_BRANCH]
         worker_mock.sync_checks = AsyncMock()
@@ -229,7 +229,7 @@ async def test_sync_relevant_subject_success_first_branch(self) -> None:
             worker_mock, series_branch_name, series_mock
         )
         worker_mock.sync_checks.assert_called_once_with(pr_mock, series_mock)
-        self._gh.close_existing_prs_with_same_base.assert_called_once_with(
+        self._gh.close_existing_prs_for_series.assert_called_once_with(
             list(self._gh.workers.values()), pr_mock
         )
 
@@ -246,7 +246,7 @@ async def test_sync_relevant_subject_success_second_branch(self) -> None:
         pr_mock.head.ref = good_branch_name
 
         self._gh.checkout_and_patch_safe = AsyncMock(return_value=pr_mock)
-        self._gh.close_existing_prs_with_same_base = MagicMock()
+        self._gh.close_existing_prs_for_series = MagicMock()
 
         bad_worker_mock = self._gh.workers[TEST_BRANCH]
         bad_worker_mock.sync_checks = AsyncMock()
@@ -274,7 +274,7 @@ async def test_sync_relevant_subject_success_second_branch(self) -> None:
             good_worker_mock, good_branch_name, series_mock
         )
         good_worker_mock.sync_checks.assert_called_once_with(pr_mock, series_mock)
-        self._gh.close_existing_prs_with_same_base.assert_called_once_with(
+        self._gh.close_existing_prs_for_series.assert_called_once_with(
             list(self._gh.workers.values()), pr_mock
         )