diff --git a/webhook_server/libs/handlers/check_run_handler.py b/webhook_server/libs/handlers/check_run_handler.py index d3f0c52a..ab189c62 100644 --- a/webhook_server/libs/handlers/check_run_handler.py +++ b/webhook_server/libs/handlers/check_run_handler.py @@ -2,6 +2,7 @@ from typing import TYPE_CHECKING, Any from github.CheckRun import CheckRun +from github.CommitStatus import CommitStatus from github.PullRequest import PullRequest from github.Repository import Repository @@ -38,6 +39,7 @@ def __init__(self, github_webhook: "GithubWebhook", owners_file_handler: OwnersF self.repository: Repository = self.github_webhook.repository self._repository_private: bool | None = None self._branch_required_status_checks: list[str] | None = None + self._all_required_status_checks: list[str] | None = None if isinstance(self.owners_file_handler, OwnersFileHandler): self.labels_handler = LabelsHandler( github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler @@ -360,26 +362,84 @@ async def is_check_run_in_progress(self, check_run: str) -> bool: return False async def required_check_failed_or_no_status( - self, pull_request: PullRequest, last_commit_check_runs: list[CheckRun], check_runs_in_progress: list[str] + self, + pull_request: PullRequest, + last_commit_check_runs: list[CheckRun], + last_commit_statuses: list[CommitStatus], + check_runs_in_progress: list[str], ) -> str: failed_check_runs: list[str] = [] - no_status_check_runs: list[str] = [] + + # Find required checks that are missing entirely from check runs list + required_checks = set(await self.all_required_status_checks(pull_request=pull_request)) + existing_check_names = {cr.name for cr in last_commit_check_runs} + missing_required_checks = required_checks - existing_check_names + + # Add missing checks to no_status list (these haven't been created yet) + no_status_check_runs: list[str] = list(missing_required_checks) + + # Add commit statuses (legacy API) to existing checks + status_check_names = {status.context for status in last_commit_statuses} + existing_check_names = existing_check_names | status_check_names + + # Recalculate missing checks after adding statuses + missing_required_checks = required_checks - existing_check_names + no_status_check_runs = list(missing_required_checks) + + # Check commit statuses for failures/pending + self.logger.debug(f"{self.log_prefix} Status details: {[(s.context, s.state) for s in last_commit_statuses]}") + + # Filter to latest status per context (highest ID = most recent) + status_by_context: dict[str, CommitStatus] = {} + for status in last_commit_statuses: + if status.context not in status_by_context or status.id > status_by_context[status.context].id: + status_by_context[status.context] = status + + latest_statuses = list(status_by_context.values()) + self.logger.debug( + f"{self.log_prefix} Filtered {len(last_commit_statuses)} statuses to {len(latest_statuses)} latest statuses" + ) + + for status in latest_statuses: + if status.context not in required_checks: + continue # Not a required check + + if status.state == "success": + continue # Passed + + if status.state == "pending": + # Skip if already marked as in-progress (to avoid duplicate reporting) + if status.context in check_runs_in_progress: + continue + if status.context not in no_status_check_runs: + no_status_check_runs.append(status.context) + elif status.state in ("failure", "error"): + if status.context not in failed_check_runs: + failed_check_runs.append(status.context) for check_run in last_commit_check_runs: - self.logger.debug(f"{self.log_prefix} Check if {check_run.name} failed or do not have status.") + # Skip check runs that have a corresponding success status + status_contexts = {status.context for status in latest_statuses if status.state == "success"} + if check_run.name in status_contexts: + continue + if ( check_run.name == CAN_BE_MERGED_STR or check_run.conclusion == SUCCESS_STR or check_run.name not in await self.all_required_status_checks(pull_request=pull_request) ): - self.logger.debug(f"{self.log_prefix} {check_run.name} is success or not required, skipping.") continue if check_run.conclusion is None: - no_status_check_runs.append(check_run.name) + if check_run.name not in no_status_check_runs: + no_status_check_runs.append(check_run.name) else: - failed_check_runs.append(check_run.name) + if check_run.name not in failed_check_runs: + failed_check_runs.append(check_run.name) + + self.logger.debug(f"{self.log_prefix} no_status_check_runs after processing check runs: {no_status_check_runs}") + self.logger.debug(f"{self.log_prefix} failed_check_runs after processing check runs: {failed_check_runs}") msg = "" @@ -399,6 +459,10 @@ async def required_check_failed_or_no_status( return msg async def all_required_status_checks(self, pull_request: PullRequest) -> list[str]: + # Cache to avoid repeated processing + if self._all_required_status_checks is not None: + return self._all_required_status_checks + all_required_status_checks: list[str] = [] branch_required_status_checks = await self.get_branch_required_status_checks(pull_request=pull_request) @@ -419,12 +483,13 @@ async def all_required_status_checks(self, pull_request: PullRequest) -> list[st _all_required_status_checks = branch_required_status_checks + all_required_status_checks self.logger.debug(f"{self.log_prefix} All required status checks: {_all_required_status_checks}") + self._all_required_status_checks = _all_required_status_checks return _all_required_status_checks async def get_branch_required_status_checks(self, pull_request: PullRequest) -> list[str]: # Check if private repo first (cache to avoid repeated API calls) if self._repository_private is None: - self._repository_private = self.repository.private + self._repository_private = await asyncio.to_thread(lambda: self.repository.private) if self._repository_private: self.logger.info( @@ -438,13 +503,17 @@ async def get_branch_required_status_checks(self, pull_request: PullRequest) -> pull_request_branch = await asyncio.to_thread(self.repository.get_branch, pull_request.base.ref) branch_protection = await asyncio.to_thread(pull_request_branch.get_protection) - branch_required_status_checks = branch_protection.required_status_checks.contexts + branch_required_status_checks = await asyncio.to_thread( + lambda: branch_protection.required_status_checks.contexts + ) self.logger.debug(f"{self.log_prefix} branch_required_status_checks: {branch_required_status_checks}") self._branch_required_status_checks = branch_required_status_checks return self._branch_required_status_checks async def required_check_in_progress( - self, pull_request: PullRequest, last_commit_check_runs: list[CheckRun] + self, + pull_request: PullRequest, + last_commit_check_runs: list[CheckRun], ) -> tuple[str, list[str]]: self.logger.debug(f"{self.log_prefix} Check if any required check runs in progress.") @@ -455,6 +524,10 @@ async def required_check_in_progress( and check_run.name != CAN_BE_MERGED_STR and check_run.name in await self.all_required_status_checks(pull_request=pull_request) ] + + # Note: Status API doesn't have an "in_progress" state - only pending (queued), + # success, failure, and error. We only check Check Runs for in-progress status. + if check_runs_in_progress: self.logger.debug( f"{self.log_prefix} Some required check runs in progress {check_runs_in_progress}, " diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index fd249a83..ef7f81c8 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -956,8 +956,20 @@ async def check_if_can_be_merged(self, pull_request: PullRequest) -> None: try: self.logger.info(f"{self.log_prefix} Check if {CAN_BE_MERGED_STR}.") await self.check_run_handler.set_merge_check_in_progress() - _last_commit_check_runs = await asyncio.to_thread(self.github_webhook.last_commit.get_check_runs) - last_commit_check_runs = list(_last_commit_check_runs) + # Fetch check runs and statuses in parallel (2 API calls → 1 concurrent operation) + _check_runs, _statuses = await asyncio.gather( + asyncio.to_thread(lambda: list(self.github_webhook.last_commit.get_check_runs())), + asyncio.to_thread(lambda: list(self.github_webhook.last_commit.get_statuses())), + ) + last_commit_check_runs = _check_runs + last_commit_statuses = _statuses + self.logger.debug( + f"{self.log_prefix} Fetched {len(last_commit_check_runs)} check runs " + f"and {len(last_commit_statuses)} statuses" + ) + if last_commit_statuses: + status_names = [s.context for s in last_commit_statuses] + self.logger.debug(f"{self.log_prefix} Commit statuses: {status_names}") _labels = await self.labels_handler.pull_request_labels_names(pull_request=pull_request) self.logger.debug(f"{self.log_prefix} check if can be merged. PR labels are: {_labels}") @@ -970,7 +982,8 @@ async def check_if_can_be_merged(self, pull_request: PullRequest) -> None: required_check_in_progress_failure_output, check_runs_in_progress, ) = await self.check_run_handler.required_check_in_progress( - pull_request=pull_request, last_commit_check_runs=last_commit_check_runs + pull_request=pull_request, + last_commit_check_runs=last_commit_check_runs, ) if required_check_in_progress_failure_output: failure_output += required_check_in_progress_failure_output @@ -984,6 +997,7 @@ async def check_if_can_be_merged(self, pull_request: PullRequest) -> None: required_check_failed_failure_output = await self.check_run_handler.required_check_failed_or_no_status( pull_request=pull_request, last_commit_check_runs=last_commit_check_runs, + last_commit_statuses=last_commit_statuses, check_runs_in_progress=check_runs_in_progress, ) if required_check_failed_failure_output: diff --git a/webhook_server/tests/test_check_run_handler.py b/webhook_server/tests/test_check_run_handler.py index 879202c5..fc226793 100644 --- a/webhook_server/tests/test_check_run_handler.py +++ b/webhook_server/tests/test_check_run_handler.py @@ -1,6 +1,8 @@ -from unittest.mock import Mock, patch +from unittest.mock import AsyncMock, Mock, patch import pytest +from github.CheckRun import CheckRun +from github.CommitStatus import CommitStatus from webhook_server.libs.handlers.check_run_handler import CheckRunHandler from webhook_server.utils.constants import ( @@ -49,6 +51,13 @@ def check_run_handler(self, mock_github_webhook: Mock) -> CheckRunHandler: """Create a CheckRunHandler instance with mocked dependencies.""" return CheckRunHandler(mock_github_webhook) + @pytest.fixture + def mock_pull_request(self) -> Mock: + """Create a mock PullRequest instance.""" + mock_pr = Mock() + mock_pr.base.ref = "main" + return mock_pr + @pytest.mark.asyncio async def test_process_pull_request_check_run_webhook_data_completed( self, check_run_handler: CheckRunHandler @@ -551,7 +560,9 @@ async def test_required_check_failed_or_no_status(self, check_run_handler: Check mock_check_run.conclusion = FAILURE_STR with patch.object(check_run_handler, "all_required_status_checks", return_value=["test-check"]): - result = await check_run_handler.required_check_failed_or_no_status(mock_pull_request, [mock_check_run], []) + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_check_run], [], [] + ) assert "test-check" in result @@ -636,3 +647,333 @@ async def test_required_check_in_progress_can_be_merged(self, check_run_handler: assert msg == "" assert in_progress_checks == [] + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_missing_required_check( + self, check_run_handler: CheckRunHandler + ) -> None: + """Test that missing required checks are detected and appear in error message.""" + mock_pull_request = Mock() + + # Setup: "tox" is required but completely missing from check runs + required_checks = [TOX_STR, VERIFIED_LABEL_STR] + + # Only verified check exists, tox is missing + mock_verified_check = Mock() + mock_verified_check.name = VERIFIED_LABEL_STR + mock_verified_check.conclusion = SUCCESS_STR + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_verified_check], [], [] + ) + + assert TOX_STR in result + assert "not started" in result + # verified check is successful, should not appear in error + assert VERIFIED_LABEL_STR not in result + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_queued_required_check( + self, check_run_handler: CheckRunHandler + ) -> None: + """Test that queued required checks (status=queued, conclusion=None) appear in error message.""" + mock_pull_request = Mock() + + # Setup: "tox" is required and queued (conclusion=None) + required_checks = [TOX_STR, VERIFIED_LABEL_STR] + + mock_tox_check = Mock() + mock_tox_check.name = TOX_STR + mock_tox_check.status = QUEUED_STR + mock_tox_check.conclusion = None + + mock_verified_check = Mock() + mock_verified_check.name = VERIFIED_LABEL_STR + mock_verified_check.conclusion = SUCCESS_STR + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_tox_check, mock_verified_check], [], [] + ) + + assert TOX_STR in result + assert "not started" in result + # verified check is successful, should not appear in error + assert VERIFIED_LABEL_STR not in result + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_failed_required_check( + self, check_run_handler: CheckRunHandler + ) -> None: + """Test that failed required checks (conclusion=failure) appear in error message.""" + mock_pull_request = Mock() + + # Setup: "tox" is required and failed + required_checks = [TOX_STR, VERIFIED_LABEL_STR] + + mock_tox_check = Mock() + mock_tox_check.name = TOX_STR + mock_tox_check.conclusion = FAILURE_STR + + mock_verified_check = Mock() + mock_verified_check.name = VERIFIED_LABEL_STR + mock_verified_check.conclusion = SUCCESS_STR + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_tox_check, mock_verified_check], [], [] + ) + + assert TOX_STR in result + assert "failed" in result + # verified check is successful, should not appear in error + assert VERIFIED_LABEL_STR not in result + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_all_successful(self, check_run_handler: CheckRunHandler) -> None: + """Test that all successful required checks return empty string (no error).""" + mock_pull_request = Mock() + + # Setup: All required checks are successful + required_checks = [TOX_STR, VERIFIED_LABEL_STR, BUILD_CONTAINER_STR] + + mock_tox_check = Mock() + mock_tox_check.name = TOX_STR + mock_tox_check.conclusion = SUCCESS_STR + + mock_verified_check = Mock() + mock_verified_check.name = VERIFIED_LABEL_STR + mock_verified_check.conclusion = SUCCESS_STR + + mock_container_check = Mock() + mock_container_check.name = BUILD_CONTAINER_STR + mock_container_check.conclusion = SUCCESS_STR + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_tox_check, mock_verified_check, mock_container_check], [], [] + ) + + # No errors expected - all checks successful + assert result == "" + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_missing_non_required_check( + self, check_run_handler: CheckRunHandler + ) -> None: + """Test that missing non-required checks do NOT cause an error.""" + mock_pull_request = Mock() + + # Setup: Only TOX_STR is required, other checks are not + required_checks = [TOX_STR] + + mock_tox_check = Mock() + mock_tox_check.name = TOX_STR + mock_tox_check.conclusion = SUCCESS_STR + + # BUILD_CONTAINER_STR exists but is not required + mock_container_check = Mock() + mock_container_check.name = BUILD_CONTAINER_STR + mock_container_check.conclusion = SUCCESS_STR + + # VERIFIED_LABEL_STR is missing but not required + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_tox_check, mock_container_check], [], [] + ) + + # No errors expected - only required check (tox) is successful + assert result == "" + # Missing non-required check should not appear in error + assert VERIFIED_LABEL_STR not in result + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_multiple_missing_checks( + self, check_run_handler: CheckRunHandler + ) -> None: + """Test that multiple missing required checks all appear in error message.""" + mock_pull_request = Mock() + + # Setup: Multiple required checks, some missing + required_checks = [TOX_STR, VERIFIED_LABEL_STR, BUILD_CONTAINER_STR, PYTHON_MODULE_INSTALL_STR] + + # Only verified check exists + mock_verified_check = Mock() + mock_verified_check.name = VERIFIED_LABEL_STR + mock_verified_check.conclusion = SUCCESS_STR + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_verified_check], [], [] + ) + + # All missing checks should appear in error + assert TOX_STR in result + assert BUILD_CONTAINER_STR in result + assert PYTHON_MODULE_INSTALL_STR in result + assert "not started" in result + # Successful check should not appear + assert VERIFIED_LABEL_STR not in result + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_mixed_states(self, check_run_handler: CheckRunHandler) -> None: + """Test mixed check states: missing, queued, failed, and successful.""" + mock_pull_request = Mock() + + # Setup: Multiple required checks in different states + required_checks = [TOX_STR, VERIFIED_LABEL_STR, BUILD_CONTAINER_STR, PYTHON_MODULE_INSTALL_STR] + + mock_tox_check = Mock() + mock_tox_check.name = TOX_STR + mock_tox_check.conclusion = FAILURE_STR # Failed + + mock_verified_check = Mock() + mock_verified_check.name = VERIFIED_LABEL_STR + mock_verified_check.conclusion = None # Queued/In progress + mock_verified_check.status = QUEUED_STR + + mock_container_check = Mock() + mock_container_check.name = BUILD_CONTAINER_STR + mock_container_check.conclusion = SUCCESS_STR # Successful + + # PYTHON_MODULE_INSTALL_STR is missing entirely + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_tox_check, mock_verified_check, mock_container_check], [], [] + ) + + # Failed check should appear + assert TOX_STR in result + assert "failed" in result + + # Queued check should appear as not started + assert VERIFIED_LABEL_STR in result + assert "not started" in result + + # Missing check should appear as not started + assert PYTHON_MODULE_INSTALL_STR in result + + # Successful check should not appear + # BUILD_CONTAINER_STR should not be in the error portion + assert result.count(BUILD_CONTAINER_STR) == 0 + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_in_progress_excluded( + self, check_run_handler: CheckRunHandler + ) -> None: + """Test that checks in progress are excluded from failed checks list.""" + mock_pull_request = Mock() + + # Setup: Failed check that is also in progress (being rerun) + required_checks = [TOX_STR, VERIFIED_LABEL_STR] + + mock_tox_check = Mock() + mock_tox_check.name = TOX_STR + mock_tox_check.conclusion = FAILURE_STR # Failed but in progress + + mock_verified_check = Mock() + mock_verified_check.name = VERIFIED_LABEL_STR + mock_verified_check.conclusion = SUCCESS_STR + + # TOX_STR is in the in_progress list (being rerun) + check_runs_in_progress = [TOX_STR] + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_tox_check, mock_verified_check], [], check_runs_in_progress + ) + + # Since tox is in progress, it should not appear in failed checks + # (it's being rerun, so we wait for new result) + assert result == "" or TOX_STR not in result + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_can_be_merged_ignored( + self, check_run_handler: CheckRunHandler + ) -> None: + """Test that can-be-merged check is ignored even if it fails.""" + mock_pull_request = Mock() + + # Setup: can-be-merged is in required checks but should be ignored + required_checks = [TOX_STR, CAN_BE_MERGED_STR] + + mock_tox_check = Mock() + mock_tox_check.name = TOX_STR + mock_tox_check.conclusion = SUCCESS_STR + + mock_merge_check = Mock() + mock_merge_check.name = CAN_BE_MERGED_STR + mock_merge_check.conclusion = FAILURE_STR # Failed, but should be ignored + + with patch.object(check_run_handler, "all_required_status_checks", return_value=required_checks): + result = await check_run_handler.required_check_failed_or_no_status( + mock_pull_request, [mock_tox_check, mock_merge_check], [], [] + ) + + # No errors expected - can-be-merged is ignored + assert result == "" + assert CAN_BE_MERGED_STR not in result + + @pytest.mark.asyncio + async def test_required_check_failed_or_no_status_with_commit_statuses( + self, check_run_handler: CheckRunHandler, mock_pull_request: Mock + ) -> None: + """Test that commit statuses (legacy API) are validated alongside check runs. + + Simulates PR #928 where pre-commit.ci uses GitHub Statuses API (not Check Runs API). + This test verifies: + 1. External services like pre-commit.ci use Statuses API, not Check Runs API + 2. Both Check Runs and Statuses are validated together + 3. Status state mapping (success/pending/failure/error) to check outcomes + 4. Non-required statuses are ignored + 5. Queued check runs are still reported correctly + 6. Combined check runs + statuses validation works + """ + # Mock required checks including a status-based check + required_checks = [TOX_STR, VERIFIED_LABEL_STR, "pre-commit.ci - pr", BUILD_CONTAINER_STR] + + # Create mock check runs (webhook server checks) + mock_check_run_tox = Mock(spec=CheckRun) + mock_check_run_tox.name = TOX_STR + mock_check_run_tox.conclusion = SUCCESS_STR + + mock_check_run_verified = Mock(spec=CheckRun) + mock_check_run_verified.name = VERIFIED_LABEL_STR + mock_check_run_verified.conclusion = None # Queued + + mock_check_run_container = Mock(spec=CheckRun) + mock_check_run_container.name = BUILD_CONTAINER_STR + mock_check_run_container.conclusion = SUCCESS_STR + + check_runs = [mock_check_run_tox, mock_check_run_verified, mock_check_run_container] + + # Create mock commit statuses (external service checks) + mock_status_precommit = Mock(spec=CommitStatus) + mock_status_precommit.context = "pre-commit.ci - pr" + mock_status_precommit.state = "success" + mock_status_precommit.id = 1 + + mock_status_coderabbit = Mock(spec=CommitStatus) + mock_status_coderabbit.context = "CodeRabbit" + mock_status_coderabbit.state = "success" + mock_status_coderabbit.id = 2 + + statuses = [mock_status_precommit, mock_status_coderabbit] + + with patch.object(check_run_handler, "all_required_status_checks", new=AsyncMock(return_value=required_checks)): + result = await check_run_handler.required_check_failed_or_no_status( + pull_request=mock_pull_request, + last_commit_check_runs=check_runs, + last_commit_statuses=statuses, + check_runs_in_progress=[], + ) + + # Verify: Should only report 'verified' as not started, NOT 'pre-commit.ci - pr' + assert VERIFIED_LABEL_STR in result + assert "pre-commit.ci - pr" not in result + assert "Some check runs not started" in result + assert VERIFIED_LABEL_STR in result + assert "CodeRabbit" not in result # Not required, should be ignored diff --git a/webhook_server/tests/test_log_api.py b/webhook_server/tests/test_log_api.py index d69d427a..f8b71bf2 100644 --- a/webhook_server/tests/test_log_api.py +++ b/webhook_server/tests/test_log_api.py @@ -641,6 +641,14 @@ def test_get_logs_page(self) -> None: mock_http_client = AsyncMock() mock_http_client.aclose = AsyncMock() + # Mock asyncio.wait to return immediately (prevents 30-second shutdown timeout) + async def mock_wait(tasks, timeout=None, return_when=None): + # Cancel all tasks immediately to prevent blocking + for task in tasks: + task.cancel() + # Return empty done set and the tasks as pending + return set(), tasks + with patch("webhook_server.app.httpx.AsyncClient", return_value=mock_http_client): # Mock external HTTP dependencies with patch( @@ -649,13 +657,14 @@ def test_get_logs_page(self) -> None: with patch( "webhook_server.utils.app_utils.get_cloudflare_allowlist", new_callable=AsyncMock ) as mock_cloudflare: - mock_github.return_value = [] - mock_cloudflare.return_value = [] - - with TestClient(FASTAPI_APP) as client: - response = client.get("/logs") - assert response.status_code == 200 - assert "Log Viewer" in response.text + with patch("webhook_server.app.asyncio.wait", side_effect=mock_wait): + mock_github.return_value = [] + mock_cloudflare.return_value = [] + + with TestClient(FASTAPI_APP) as client: + response = client.get("/logs") + assert response.status_code == 200 + assert "Log Viewer" in response.text def test_get_log_entries_no_filters(self, sample_log_entries: list[LogEntry]) -> None: """Test retrieving log entries without filters.""" diff --git a/webhook_server/tests/test_logging_separation.py b/webhook_server/tests/test_logging_separation.py index 91060d73..b677bc9f 100644 --- a/webhook_server/tests/test_logging_separation.py +++ b/webhook_server/tests/test_logging_separation.py @@ -36,6 +36,14 @@ async def test_mcp_logging_configuration(): # Mock dependencies mock_app = MagicMock() + # Mock asyncio.wait to return immediately (prevents 30-second shutdown timeout) + async def mock_wait(tasks, timeout=None, return_when=None): + # Cancel all tasks immediately to prevent blocking + for task in tasks: + task.cancel() + # Return empty done set and the tasks as pending + return set(), tasks + with ( patch("webhook_server.app.Config") as MockConfig, patch("webhook_server.app.MCP_SERVER_ENABLED", True), @@ -45,6 +53,7 @@ async def test_mcp_logging_configuration(): patch("webhook_server.app.get_cloudflare_allowlist", new_callable=AsyncMock), patch("webhook_server.app.httpx.AsyncClient") as MockClient, patch("webhook_server.app.LOGGER"), + patch("webhook_server.app.asyncio.wait", side_effect=mock_wait), ): # Setup mocks mock_config_instance = MockConfig.return_value diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index 02287505..93e1a3bd 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -695,7 +695,9 @@ async def test_check_if_can_be_merged_approved( pull_request_handler.labels_handler, "pull_request_labels_names", new=AsyncMock(return_value=[]) ), patch.object( - pull_request_handler.github_webhook, "last_commit", Mock(get_check_runs=Mock(return_value=[])) + pull_request_handler.github_webhook, + "last_commit", + Mock(get_check_runs=Mock(return_value=[]), get_statuses=Mock(return_value=[])), ), ): await pull_request_handler.check_if_can_be_merged(pull_request=mock_pull_request)