diff --git a/kernel_patches_daemon/branch_worker.py b/kernel_patches_daemon/branch_worker.py index f6a6a0a..1386544 100644 --- a/kernel_patches_daemon/branch_worker.py +++ b/kernel_patches_daemon/branch_worker.py @@ -979,6 +979,7 @@ async def checkout_and_patch( Patch in place and push. Returns true if whole series applied. Return None if at least one patch in series failed. + Raises NewPRWithNoChangeException if series would not result in any changes. If at least one patch in series failed nothing gets pushed. """ if await self._pr_closed(branch_name, series_to_apply): diff --git a/kernel_patches_daemon/github_sync.py b/kernel_patches_daemon/github_sync.py index e539992..7be23ee 100644 --- a/kernel_patches_daemon/github_sync.py +++ b/kernel_patches_daemon/github_sync.py @@ -168,6 +168,24 @@ def close_existing_prs_with_same_base( if pr_to_close.title in worker.prs: del worker.prs[pr_to_close.title] + async def checkout_and_patch_safe( + self, worker, branch_name: str, series_to_apply: Series + ) -> Optional[PullRequest]: + try: + self.increment_counter("all_known_subjects") + pr = await worker.checkout_and_patch(branch_name, series_to_apply) + if pr is None: + logging.info( + f"PR associated with branch {branch_name} for series {series_to_apply.id} is closed; ignoring" + ) + return pr + except NewPRWithNoChangeException as e: + self.increment_counter("empty_pr") + logger.exception( + f"Could not create PR for series {series_to_apply.id} merging {e.base_branch} into {e.target_branch} as PR would introduce no changes" + ) + return None + async def sync_patches(self) -> None: """ One subject = one branch @@ -239,17 +257,8 @@ async def sync_patches(self) -> None: else: logging.info(msg + "no more next, staying.") logging.info(f"Choosing branch {branch} to create/update PR.") - try: - self.increment_counter("all_known_subjects") - pr = await worker.checkout_and_patch(pr_branch_name, series) - if pr is None: - logging.info( - "PR associated with branch {pr_branch_name} for series {series.id} is closed; ignoring" - ) - continue - except NewPRWithNoChangeException: - self.increment_counter("empty_pr") - logger.exception("Could not create PR with no changes") + pr = await self.checkout_and_patch_safe(worker, pr_branch_name, series) + if pr is None: continue logging.info( @@ -283,8 +292,11 @@ async def sync_patches(self) -> None: pr.edit(title=subject.subject) branch_name = f"{await subject.branch}{HEAD_BASE_SEPARATOR}{worker.repo_branch}" latest_series = await subject.latest_series or series - self.increment_counter("all_known_subjects") - await worker.checkout_and_patch(branch_name, latest_series) + pr = await self.checkout_and_patch_safe( + worker, branch_name, latest_series + ) + if pr is None: + continue await worker.sync_checks(pr, latest_series) await loop.run_in_executor(None, worker.expire_branches) diff --git a/tests/test_github_sync.py b/tests/test_github_sync.py index 568d6a8..3690dc2 100644 --- a/tests/test_github_sync.py +++ b/tests/test_github_sync.py @@ -10,8 +10,9 @@ import unittest from dataclasses import dataclass from typing import Any, Dict, Optional -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch +from kernel_patches_daemon.branch_worker import NewPRWithNoChangeException from kernel_patches_daemon.config import KPDConfig from kernel_patches_daemon.github_sync import GithubSync @@ -54,7 +55,7 @@ def __init__( super().__init__(*args, **presets) -class TestGihubSync(unittest.TestCase): +class TestGihubSync(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: patcher = patch("kernel_patches_daemon.github_connector.Github") self._gh_mock = patcher.start() @@ -128,3 +129,37 @@ def test_close_existing_prs_with_same_base(self) -> None: self.assertTrue("matching" not in worker.prs) matching_pr_mock.edit.assert_called_with(state="close") + + async def test_checkout_and_patch_safe(self) -> None: + pr_branch_name = "fake_pr_branch" + series = MagicMock() + pr = MagicMock() + branch_worker_mock = MagicMock() + branch_worker_mock.checkout_and_patch = AsyncMock() + + # PR generated + branch_worker_mock.checkout_and_patch.return_value = pr + self.assertEqual( + await self._gh.checkout_and_patch_safe( + branch_worker_mock, pr_branch_name, series + ), + pr, + ) + + # One patch in series failed to apply + branch_worker_mock.checkout_and_patch.return_value = None + self.assertIsNone( + await self._gh.checkout_and_patch_safe( + branch_worker_mock, pr_branch_name, series + ) + ) + + # Series generates no changes vs target branch, likely already merged + branch_worker_mock.checkout_and_patch.side_effect = NewPRWithNoChangeException( + pr_branch_name, "target" + ) + self.assertIsNone( + await self._gh.checkout_and_patch_safe( + branch_worker_mock, pr_branch_name, series + ) + )