Skip to content

fix: welcome_msg approvers and reviewers duplicate names #857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
myakove merged 2 commits into from
Sep 3, 2025
Merged

fix: welcome_msg approvers and reviewers duplicate names #857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b8ba10
fix: welcome_msg approvers and reviewers duplicate names
myakove Sep 3, 2025
7d2dda0
fix: test_get_logs_page
myakove Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions webhook_server/libs/owners_files_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ async def get_all_pull_request_approvers(self) -> list[str]:
for _approver in list_of_approvers.get("approvers", []):
_approvers.append(_approver)

_approvers = list(set(_approvers))
_approvers.sort()
self.logger.debug(f"{self.log_prefix} All pull request approvers: {_approvers}")
return _approvers
Expand All @@ -186,6 +187,7 @@ async def get_all_pull_request_reviewers(self) -> list[str]:
for _reviewer in list_of_reviewers.get("reviewers", []):
_reviewers.append(_reviewer)

_reviewers = list(set(_reviewers))
_reviewers.sort()
self.logger.debug(f"Pull request reviewers are: {_reviewers}")
return _reviewers
Expand Down
78 changes: 53 additions & 25 deletions webhook_server/tests/test_log_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
"""Tests for log viewer API endpoints and WebSocket functionality."""

import asyncio
import os
import datetime
import json
import os
import tempfile
from pathlib import Path
from unittest.mock import AsyncMock, Mock, patch
Expand Down Expand Up @@ -496,12 +496,27 @@ def test_get_logs_page(self) -> None:
mock_instance.get_log_page.return_value = HTMLResponse(content="<html><body>Log Viewer</body></html>")
mock_instance.shutdown = AsyncMock() # Add async shutdown method

from webhook_server.app import FASTAPI_APP
# Mock httpx.AsyncClient to prevent SSL errors during lifespan startup
mock_http_client = AsyncMock()
mock_http_client.aclose = AsyncMock()

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.httpx.AsyncClient", return_value=mock_http_client):
# Mock external HTTP dependencies
with patch(
"webhook_server.utils.app_utils.get_github_allowlist", new_callable=AsyncMock
) as mock_github:
with patch(
"webhook_server.utils.app_utils.get_cloudflare_allowlist", new_callable=AsyncMock
) as mock_cloudflare:
mock_github.return_value = []
mock_cloudflare.return_value = []

from webhook_server.app import FASTAPI_APP

with TestClient(FASTAPI_APP) as client:
response = client.get("/logs")
assert response.status_code == 200
assert "Log Viewer" in response.text

Comment thread
myakove marked this conversation as resolved.
def test_get_log_entries_no_filters(self, sample_log_entries: list[LogEntry]) -> None:
"""Test retrieving log entries without filters."""
Expand Down Expand Up @@ -820,9 +835,10 @@ async def mock_handle_websocket_error(websocket):
@pytest.mark.asyncio
async def test_websocket_handle_real_implementation(self):
"""Test actual WebSocket handler implementation."""
from webhook_server.web.log_viewer import LogViewerController
from unittest.mock import Mock

from webhook_server.web.log_viewer import LogViewerController

mock_logger = Mock()
controller = LogViewerController(logger=mock_logger)

Expand Down Expand Up @@ -1101,7 +1117,7 @@ class TestWorkflowStepsAPI:
def test_get_workflow_steps_success(self) -> None:
"""Test successful workflow steps retrieval."""
# Import modules and patch before creating test client
from unittest.mock import Mock, AsyncMock
from unittest.mock import AsyncMock, Mock

# Mock workflow steps data
mock_workflow_data = {
Expand Down Expand Up @@ -1135,17 +1151,23 @@ def test_get_workflow_steps_success(self) -> None:
mock_instance.get_workflow_steps.return_value = mock_workflow_data
mock_instance.shutdown = AsyncMock() # Add async shutdown method

# Mock httpx.AsyncClient to prevent SSL errors during lifespan startup
mock_http_client = AsyncMock()
mock_http_client.aclose = AsyncMock()

# Patch using setattr to directly set the singleton instance
with patch("webhook_server.app.get_log_viewer_controller", return_value=mock_instance):
# Also patch the singleton variable itself
with patch("webhook_server.app._log_viewer_controller_singleton", mock_instance):
from webhook_server.app import FASTAPI_APP
from fastapi.testclient import TestClient
with patch("webhook_server.app.httpx.AsyncClient", return_value=mock_http_client):
with patch("webhook_server.app.get_log_viewer_controller", return_value=mock_instance):
# Also patch the singleton variable itself
with patch("webhook_server.app._log_viewer_controller_singleton", mock_instance):
from fastapi.testclient import TestClient

client = TestClient(FASTAPI_APP)
from webhook_server.app import FASTAPI_APP

# Make the request
response = client.get("/logs/api/workflow-steps/test-hook-123")
client = TestClient(FASTAPI_APP)

# Make the request
response = client.get("/logs/api/workflow-steps/test-hook-123")

# Assertions
assert response.status_code == 200
Expand All @@ -1162,7 +1184,7 @@ def test_get_workflow_steps_success(self) -> None:
def test_get_workflow_steps_no_steps_found(self) -> None:
"""Test workflow steps when no steps are found."""
# Import modules and patch before creating test client
from unittest.mock import Mock, AsyncMock
from unittest.mock import AsyncMock, Mock

# Mock empty workflow data
mock_workflow_data = {
Expand All @@ -1177,17 +1199,23 @@ def test_get_workflow_steps_no_steps_found(self) -> None:
mock_instance.get_workflow_steps.return_value = mock_workflow_data
mock_instance.shutdown = AsyncMock() # Add async shutdown method

# Mock httpx.AsyncClient to prevent SSL errors during lifespan startup
mock_http_client = AsyncMock()
mock_http_client.aclose = AsyncMock()

# Patch using setattr to directly set the singleton instance
with patch("webhook_server.app.get_log_viewer_controller", return_value=mock_instance):
# Also patch the singleton variable itself
with patch("webhook_server.app._log_viewer_controller_singleton", mock_instance):
from webhook_server.app import FASTAPI_APP
from fastapi.testclient import TestClient
with patch("webhook_server.app.httpx.AsyncClient", return_value=mock_http_client):
with patch("webhook_server.app.get_log_viewer_controller", return_value=mock_instance):
# Also patch the singleton variable itself
with patch("webhook_server.app._log_viewer_controller_singleton", mock_instance):
from fastapi.testclient import TestClient

from webhook_server.app import FASTAPI_APP

client = TestClient(FASTAPI_APP)
client = TestClient(FASTAPI_APP)

# Make the request
response = client.get("/logs/api/workflow-steps/test-hook-456")
# Make the request
response = client.get("/logs/api/workflow-steps/test-hook-456")

# Assertions
assert response.status_code == 200
Expand Down
3 changes: 2 additions & 1 deletion webhook_server/tests/test_owners_files_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest.mock import AsyncMock, Mock, call, patch

import pytest
import yaml
from unittest.mock import AsyncMock, Mock, patch, call

from webhook_server.libs.owners_files_handler import OwnersFileHandler
from webhook_server.tests.conftest import ContentFile
Expand Down