patchwork: remove @Property from Subject getters

Using @Property and async on the same function is confusing and makes
mocking for unit tests difficult and sometimes impossible.

Remove @Property decorator from Subject getters and make them simple
async functions. Fix usages accordingly.

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

Author: theihor

kernel_patches_daemon/branch_worker.py

@@ -1100,7 +1100,8 @@ def filter_closed_pr(self, head: str) -> Optional[PullRequest]:
         return res
 
     async def subject_to_branch(self, subject: Subject) -> str:
-        return f"{await subject.branch}{SERIES_TARGET_SEPARATOR}{self.repo_branch}"
+        subj_branch = await subject.branch()
+        return f"{subj_branch}{SERIES_TARGET_SEPARATOR}{self.repo_branch}"
 
     async def sync_checks(self, pr: PullRequest, series: Series) -> None:
         # Make sure that we are working with up-to-date data (as opposed to

kernel_patches_daemon/github_sync.py

@@ -199,7 +199,7 @@ async def sync_relevant_subject(self, subject: Subject) -> None:
         4. Start from first branch, try to apply and generate PR,
            if fails continue to next branch, if no more branches, generate a merge-conflict PR
         """
-        series = none_throws(await subject.latest_series)
+        series = none_throws(await subject.latest_series())
         tags = await series.all_tags()
         logging.info(f"Processing {series.id}: {subject.subject} (tags: {tags})")
 
@@ -303,8 +303,8 @@ async def sync_patches(self) -> None:
                             f"Renaming {pr} from {subject_name} to {subject.subject} according to {series.id}"
                         )
                         pr.edit(title=subject.subject)
-                    branch_name = f"{await subject.branch}{SERIES_TARGET_SEPARATOR}{worker.repo_branch}"
-                    latest_series = await subject.latest_series or series
+                    branch_name = await worker.subject_to_branch(subject)
+                    latest_series = await subject.latest_series() or series
                     pr = await self.checkout_and_patch_safe(
                         worker, branch_name, latest_series
                     )

kernel_patches_daemon/patchwork.py

@@ -289,21 +289,18 @@ def __init__(self, subject: str, pw_client: "Patchwork") -> None:
         self.pw_client = pw_client
         self.subject = subject
 
-    @property
     async def branch(self) -> Optional[str]:
-        relevant_series = await self.relevant_series
+        relevant_series = await self.relevant_series()
         if len(relevant_series) == 0:
             return None
         return f"series{SERIES_ID_SEPARATOR}{relevant_series[0].id}"
 
-    @property
     async def latest_series(self) -> Optional["Series"]:
-        relevant_series = await self.relevant_series
+        relevant_series = await self.relevant_series()
         if len(relevant_series) == 0:
             return None
         return relevant_series[-1]
 
-    @property
     @cached(cache=TTLCache(maxsize=1, ttl=600))
     async def relevant_series(self) -> List["Series"]:
         """
@@ -768,7 +765,7 @@ async def get_relevant_subjects(self) -> Sequence[Subject]:
             async def fetch_latest_series(
                 subject_name, subject_obj
             ) -> Tuple[str, Series, Optional[Series]]:
-                return (subject_name, subject_obj, await subject_obj.latest_series)
+                return (subject_name, subject_obj, await subject_obj.latest_series())
 
             tasks = [fetch_latest_series(k, v) for k, v in subjects.items()]
             tasks = await asyncio.gather(*tasks)

tests/test_github_sync.py

@@ -186,7 +186,7 @@ def _setup_sync_relevant_subject_mocks(self):
         series_mock.all_tags = AsyncMock(return_value=["tag"])
         subject_mock = MagicMock()
         subject_mock.subject = "Test subject"
-        subject_mock.latest_series = AsyncMock(return_value=series_mock)()
+        subject_mock.latest_series = AsyncMock(return_value=series_mock)
 
         return subject_mock, series_mock
 
@@ -206,7 +206,7 @@ async def test_sync_relevant_subject_success_first_branch(self) -> None:
 
         subject_mock, series_mock = self._setup_sync_relevant_subject_mocks()
         series_mock.all_tags = AsyncMock(return_value=["multibranch-tag"])
-        subject_mock.branch = AsyncMock(return_value=series_prefix)()
+        subject_mock.branch = AsyncMock(return_value=series_prefix)
 
         pr_mock = MagicMock()
         pr_mock.head.ref = series_branch_name

tests/test_patchwork.py

@@ -636,11 +636,13 @@ async def test_relevant_series(self, m: aioresponses) -> None:
         init_pw_responses(m, DEFAULT_TEST_RESPONSES)
 
         s = Subject("foo", self._pw)
+
         # There is 2 relevant series
-        self.assertEqual(len(await s.relevant_series), 4)
+        relevant_series = await s.relevant_series()
+        self.assertEqual(len(relevant_series), 4)
 
         # Series are ordered from oldest to newest
-        series = (await s.relevant_series)[0]
+        series = relevant_series[0]
         self.assertEqual(series.id, FOO_SERIES_FIRST)
         # It has only 1 diff, diff 11
         self.assertEqual(len(await series.get_patches()), 1)
@@ -655,7 +657,7 @@ async def test_latest_series(self, m: aioresponses) -> None:
         init_pw_responses(m, DEFAULT_TEST_RESPONSES)
 
         s = Subject("foo", self._pw)
-        latest_series = none_throws(await s.latest_series)
+        latest_series = none_throws(await s.latest_series())
         # It is Series with ID FOO_SERIES_LAST
         self.assertEqual(latest_series.id, FOO_SERIES_LAST)
         # and has 3 diffs
@@ -670,6 +672,6 @@ async def test_branch_name(self, m: aioresponses) -> None:
         init_pw_responses(m, DEFAULT_TEST_RESPONSES)
 
         s = Subject("foo", self._pw)
-        branch = await s.branch
+        branch = await s.branch()
         # It is Series with ID 4
         self.assertEqual(branch, f"series/{FOO_SERIES_FIRST}")