Merge pull request #854 from microsoft/bugfix/codeQL-suggestions

test: enhance mock specifications in various test files

Author: Roopan-Microsoft

src/tests/backend/test_app.py

@@ -13,8 +13,7 @@
 import pytest
 import sys
 import os
-from unittest.mock import Mock, AsyncMock, patch, MagicMock
-from types import ModuleType
+from unittest.mock import Mock, AsyncMock, patch, NonCallableMock
 
 # Environment variables are set by conftest.py, but ensure they're available
 os.environ.setdefault("APPLICATIONINSIGHTS_CONNECTION_STRING", "InstrumentationKey=test-key-12345")
@@ -36,7 +35,8 @@
 os.environ.setdefault("AZURE_OPENAI_RAI_DEPLOYMENT_NAME", "test-rai-deployment")
 
 # Check if v4 has been mocked by another test file (prevents import errors)
-_v4_is_mocked = 'v4' in sys.modules and isinstance(sys.modules['v4'], (Mock, MagicMock))
+# Use NonCallableMock to catch all mock subclasses (Mock, MagicMock, etc.)
+_v4_is_mocked = 'v4' in sys.modules and isinstance(sys.modules['v4'], NonCallableMock)
 if _v4_is_mocked:
     # Skip this module - v4 has been mocked by another test file
     pytest.skip(

src/tests/backend/v4/orchestration/helper/test_plan_to_mplan_converter.py

@@ -11,13 +11,14 @@
 
 import unittest
 import sys
-from unittest.mock import Mock, MagicMock
+from unittest.mock import NonCallableMock
 
 import pytest
 
 # Check if v4 has been mocked by another test file (prevents import errors)
-_v4_is_mocked = 'v4' in sys.modules and isinstance(sys.modules['v4'], (Mock, MagicMock))
-_v4_models_is_mocked = 'v4.models' in sys.modules and isinstance(sys.modules['v4.models'], (Mock, MagicMock))
+# Use NonCallableMock to catch all mock subclasses (Mock, MagicMock, etc.)
+_v4_is_mocked = 'v4' in sys.modules and isinstance(sys.modules['v4'], NonCallableMock)
+_v4_models_is_mocked = 'v4.models' in sys.modules and isinstance(sys.modules['v4.models'], NonCallableMock)
 if _v4_is_mocked or _v4_models_is_mocked:
     pytest.skip(
         "Skipping test_plan_to_mplan_converter.py: v4 module has been mocked by another test file. "

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

@@ -917,30 +917,30 @@ def test_extract_response_text_object_with_text_attr(self):
     def test_extract_response_text_object_with_empty_text(self):
         """Test extracting text from object with empty text attribute."""
         # Use spec to ensure only specified attributes exist
-        obj = Mock(spec=['text'])
+        obj = Mock(spec_set=['text'])
         obj.text = ""
         result = self.manager._extract_response_text(obj)
         self.assertEqual(result, "")
 
     def test_extract_response_text_agent_executor_response_with_agent_response(self):
         """Test extracting text from AgentExecutorResponse with agent_response.text."""
-        agent_resp = Mock()
+        agent_resp = Mock(spec_set=['text'])
         agent_resp.text = "Agent executor response"
 
-        executor_resp = Mock(spec=['agent_response'])
+        executor_resp = Mock(spec_set=['agent_response'])
         executor_resp.agent_response = agent_resp
 
         result = self.manager._extract_response_text(executor_resp)
         self.assertEqual(result, "Agent executor response")
 
     def test_extract_response_text_agent_executor_response_fallback_to_conversation(self):
         """Test extracting text from AgentExecutorResponse falling back to full_conversation."""
-        agent_resp = Mock()
+        agent_resp = Mock(spec_set=['text'])
         agent_resp.text = None
 
         last_msg = MockChatMessage("Last conversation message")
 
-        executor_resp = Mock(spec=['agent_response', 'full_conversation'])
+        executor_resp = Mock(spec_set=['agent_response', 'full_conversation'])
         executor_resp.agent_response = agent_resp
         executor_resp.full_conversation = [MockChatMessage("First"), last_msg]
 
@@ -949,10 +949,10 @@ def test_extract_response_text_agent_executor_response_fallback_to_conversation(
 
     def test_extract_response_text_agent_executor_response_empty_conversation(self):
         """Test extracting text from AgentExecutorResponse with empty conversation."""
-        agent_resp = Mock()
+        agent_resp = Mock(spec_set=['text'])
         agent_resp.text = None
 
-        executor_resp = Mock(spec=['agent_response', 'full_conversation'])
+        executor_resp = Mock(spec_set=['agent_response', 'full_conversation'])
         executor_resp.agent_response = agent_resp
         executor_resp.full_conversation = []
 
@@ -1091,7 +1091,7 @@ async def test_workflow_output_with_object_with_text(self):
         mock_workflow = Mock()
 
         # Create object with text attribute
-        obj_with_text = Mock(spec=['text'])
+        obj_with_text = Mock(spec_set=['text'])
         obj_with_text.text = "Object response"
         output_event = MockWorkflowOutputEvent(obj_with_text)