Skip to content

Commit fb00119

Browse files
Merge pull request #848 from microsoft/psl-code-quality
fix: Improve the code quality
2 parents 58678ab + cc8865d commit fb00119

9 files changed

Lines changed: 35 additions & 15 deletions

File tree

src/tests/backend/common/config/test_app_config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import pytest
1313
import os
1414
import logging
15-
from unittest.mock import patch, MagicMock, AsyncMock
15+
from unittest.mock import patch, MagicMock
1616

1717
# Add the source root directory to the Python path for imports
1818
import sys

src/tests/backend/common/database/test_database_base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ async def get_team_agent(self, team_id, agent_name): return None
499499
assert database.initialized is True
500500
# Raise an exception to test cleanup
501501
raise ValueError("Test exception")
502-
502+
503503
# Even with exception, close should have been called
504504
assert database.closed is True
505505

src/tests/backend/common/utils/test_utils_date.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ def tearDown(self):
106106
else:
107107
locale.setlocale(locale.LC_TIME, "")
108108
except Exception:
109+
# Best-effort cleanup: if restoring the locale fails (e.g., unsupported locale),
110+
# do not fail tests because of the environment configuration.
109111
pass
110112

111113
def test_format_date_for_user_valid_iso_date(self):

src/tests/backend/v4/common/services/test_plan_service.py

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -529,18 +529,34 @@ async def test_static_method_properties(self):
529529
result = await PlanService.handle_plan_approval(mock_approval, "user")
530530
assert result is False
531531

532-
def test_event_tracking_calls(self):
532+
@pytest.mark.asyncio
533+
async def test_event_tracking_calls(self):
533534
"""Test that event tracking is called appropriately."""
534-
# This test verifies the event tracking integration
535-
with patch.object(mock_event_utils, 'track_event_if_configured') as mock_track:
535+
# Seed orchestration plan + memory store so approval path reaches event tracking.
536+
mock_mplan = MagicMock()
537+
mock_mplan.plan_id = None
538+
mock_mplan.team_id = None
539+
mock_mplan.model_dump.return_value = {"test": "plan"}
540+
mock_orchestration_config.plans = {"test-m-plan": mock_mplan}
541+
542+
mock_plan = MagicMock()
543+
mock_plan.team_id = "team-123"
544+
mock_db = MagicMock()
545+
mock_db.get_plan = AsyncMock(return_value=mock_plan)
546+
mock_db.update_plan = AsyncMock()
547+
mock_database_factory.DatabaseFactory.get_database = AsyncMock(return_value=mock_db)
548+
549+
with patch.object(plan_service_module, 'track_event_if_configured') as mock_track:
536550
mock_approval = MockPlanApprovalResponse(
537551
plan_id="test-plan",
538552
m_plan_id="test-m-plan",
539553
approved=True
540554
)
541-
542-
# The actual event tracking calls are tested indirectly through the service methods
543-
assert mock_track is not None
555+
556+
result = await PlanService.handle_plan_approval(mock_approval, "user")
557+
assert result is True
558+
# Verify that event tracking was invoked
559+
mock_track.assert_called_once()
544560

545561
def test_logging_integration(self):
546562
"""Test that logging is properly configured."""

src/tests/backend/v4/config/test_settings.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ def test_ad_token_provider(self, mock_config):
7575
self.assertEqual(token, "test-token-123")
7676
mock_credential.get_token.assert_called_once_with(mock_config.AZURE_COGNITIVE_SERVICES)
7777

78+
7879
class TestAzureConfigAsync(unittest.IsolatedAsyncioTestCase):
7980
"""Async test cases for AzureConfig class."""
8081

src/tests/backend/v4/magentic_agents/common/test_lifecycle.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
"""Unit tests for backend.v4.magentic_agents.common.lifecycle module."""
2-
import asyncio
32
import logging
43
import sys
54
from unittest.mock import Mock, patch, AsyncMock

src/tests/backend/v4/magentic_agents/test_proxy_agent.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,13 @@
5555
sys.modules['v4.models.messages'].TimeoutNotification = mock_timeout_notification
5656
sys.modules['v4.models.messages'].WebsocketMessageType = mock_websocket_message_type
5757

58-
5958
# Now import the module under test
60-
import backend.v4.magentic_agents.proxy_agent
59+
import backend.v4.magentic_agents.proxy_agent as proxy_agent_module
60+
61+
62+
def test_module_imports():
63+
"""Ensure the proxy_agent module imports correctly and is referenced in tests."""
64+
assert proxy_agent_module is not None
6165

6266

6367
class TestProxyAgentComplexScenarios:

src/tests/backend/v4/orchestration/test_orchestration_manager.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import logging
88
import os
99
import sys
10-
from unittest import IsolatedAsyncioTestCase
10+
from unittest import IsolatedAsyncioTestCase, main
1111
from unittest.mock import AsyncMock, Mock, patch
1212

1313
# Add the backend directory to the Python path
@@ -1159,5 +1159,4 @@ async def test_workflow_output_with_empty_list(self):
11591159

11601160

11611161
if __name__ == '__main__':
1162-
import unittest
1163-
unittest.main()
1162+
main()

tests/e2e-test/tests/test_MACAE_Smoke_test.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ def test_macae_v4_gp_workflow(login_logout, request):
106106
logger.info("STEP 5: Approving Retail Task Plan")
107107
logger.info("=" * 80)
108108
step5_start = time.time()
109-
step5_retry_attempted = False
110109
try:
111110
biab_page.approve_retail_task_plan()
112111
step5_end = time.time()

0 commit comments

Comments
 (0)