From e7db663e4c87939256ad04dea98eee098c1061c6 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Tue, 25 Feb 2025 14:49:16 -0300 Subject: [PATCH 01/18] adding tests for bedrock agent channel --- .../test_bedrock_agent_channel.py | 366 ++++++++++++++++++ 1 file changed, 366 insertions(+) diff --git a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py index 66e203d93065..bdc60da9cf23 100644 --- a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py +++ b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py @@ -1,8 +1,17 @@ # Copyright (c) Microsoft. All rights reserved. +from typing import AsyncIterable +from unittest.mock import MagicMock, patch + import pytest +from semantic_kernel.agents.agent import Agent +from semantic_kernel.agents.bedrock.models.bedrock_agent_model import BedrockAgentModel +from semantic_kernel.agents.channels.bedrock_agent_channel import BedrockAgentChannel from semantic_kernel.contents.chat_message_content import ChatMessageContent +from semantic_kernel.contents.streaming_chat_message_content import StreamingChatMessageContent +from semantic_kernel.contents.utils.author_role import AuthorRole +from semantic_kernel.exceptions.agent_exceptions import AgentChatException @pytest.fixture @@ -32,6 +41,30 @@ def chat_history_not_alternate_role() -> list[ChatMessageContent]: ] +@pytest.fixture +def channel_and_agent(): + """ + Fixture that creates a BedrockAgentChannel instance and a mock BedrockAgent. + """ + from semantic_kernel.agents.bedrock.bedrock_agent import BedrockAgent + + # Create mocks + mock_agent = MagicMock(spec=BedrockAgent) + # Set the name and agent_model properties + mock_agent.name = "MockBedrockAgent" + mock_agent.agent_model = MagicMock(spec=BedrockAgentModel) + mock_agent.agent_model.foundation_model = "mock-foundation-model" + + # Mock create_session_id to return a fixed session id + mock_agent.create_session_id.return_value = "test-session-id" + + # Initialize the BedrockAgentChannel with no messages initially + channel = BedrockAgentChannel() + + # Return both + return channel, mock_agent + + async def test_receive_message(mock_channel, chat_history): # Test to verify the receive_message functionality await mock_channel.receive(chat_history) @@ -61,3 +94,336 @@ async def test_channel_reset(mock_channel, chat_history): assert len(mock_channel) == len(chat_history) await mock_channel.reset() assert len(mock_channel) == 0 + + +async def test_receive_appends_history_correctly(channel_and_agent): + """Test that the receive method appends messages while ensuring they alternate in author role.""" + channel, _ = channel_and_agent + + # Provide a list of messages with identical roles to see if placeholders are inserted + incoming_messages = [ + ChatMessageContent(role=AuthorRole.USER, content="User message 1"), + ChatMessageContent(role=AuthorRole.USER, content="User message 2"), + ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant message 1"), + ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant message 2"), + ] + + await channel.receive(incoming_messages) + + # The final channel.messages should be: + # user message 1, user placeholder, user message 2, assistant placeholder, assistant message 1, + # assistant placeholder, assistant message 2 + expected_roles = [ + AuthorRole.USER, + AuthorRole.ASSISTANT, # placeholder + AuthorRole.USER, + AuthorRole.ASSISTANT, + AuthorRole.USER, # placeholder + AuthorRole.ASSISTANT, + ] + expected_contents = [ + "User message 1", + BedrockAgentChannel.MESSAGE_PLACEHOLDER, + "User message 2", + "Assistant message 1", + BedrockAgentChannel.MESSAGE_PLACEHOLDER, + "Assistant message 2", + ] + + a = channel.messages + + print(a) + + assert len(channel.messages) == len(expected_roles) + for i, (msg, exp_role, exp_content) in enumerate(zip(channel.messages, expected_roles, expected_contents)): + assert msg.role == exp_role, f"Role mismatch at index {i}" + assert msg.content == exp_content, f"Content mismatch at index {i}" + + +async def test_invoke_raises_exception_for_non_bedrock_agent(channel_and_agent): + """Test invoke method raises AgentChatException if the agent provided is not a BedrockAgent.""" + channel, _ = channel_and_agent + + # Place a message in the channel so it's not empty + channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) + + # Create a dummy agent that is not BedrockAgent + non_bedrock_agent = Agent() + + with pytest.raises(AgentChatException) as exc_info: + _ = [msg async for msg in channel.invoke(non_bedrock_agent)] + + assert "Agent is not of the expected type" in str(exc_info.value) + + +async def test_invoke_raises_exception_if_no_history(channel_and_agent): + """Test invoke method raises AgentChatException if no chat history is available.""" + channel, agent = channel_and_agent + + with pytest.raises(AgentChatException) as exc_info: + _ = [msg async for msg in channel.invoke(agent)] + + assert "No chat history available" in str(exc_info.value) + + +@patch("semantic_kernel.agents.bedrock.bedrock_agent.BedrockAgent.create_session_id", return_value="session-id") +async def test_invoke_inserts_placeholders_when_history_needs_to_alternate(mock_session, channel_and_agent): + """Test invoke ensures _ensure_history_alternates and _ensure_last_message_is_user are called.""" + channel, agent = channel_and_agent + + # Put messages in the channel such that the last message is an assistant's + channel.messages.append(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant 1")) + + # Mock agent.invoke to return an async generator + async def mock_invoke(session_id: str, input_text: str, sessionState=None, **kwargs): + # We just yield one message as if the agent responded + yield ChatMessageContent(role=AuthorRole.ASSISTANT, content="Mock Agent Response") + + agent.invoke = mock_invoke + + # Because the last message is from the assistant, we expect a placeholder user message to be appended + # also the history might need to alternate. + # But since there's only one message, there's nothing to fix except the last message is user. + + # We will now add a user message so we do not get the "No chat history available" error + channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User 1")) + + # Now we do invoke + outputs = [msg async for msg in channel.invoke(agent)] + + # We'll check if the response is appended to channel.messages + assert len(outputs) == 1 + assert outputs[0][0] is True, "Expected a user-facing response" + agent_response = outputs[0][1] + assert agent_response.content == "Mock Agent Response" + + # The channel messages should now have 3 messages: the assistant, the user, and the new agent message + assert len(channel.messages) == 3 + assert channel.messages[-1].role == AuthorRole.ASSISTANT + assert channel.messages[-1].content == "Mock Agent Response" + + +async def test_invoke_stream_raises_error_for_non_bedrock_agent(channel_and_agent): + """Test invoke_stream raises AgentChatException if the agent provided is not a BedrockAgent.""" + channel, _ = channel_and_agent + channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) + + non_bedrock_agent = Agent() + with pytest.raises(AgentChatException) as exc_info: + _ = [msg async for msg in channel.invoke_stream(non_bedrock_agent, [])] + + assert "Agent is not of the expected type" in str(exc_info.value) + + +async def test_invoke_stream_raises_no_chat_history(channel_and_agent): + """Test invoke_stream raises AgentChatException if no messages in the channel.""" + channel, agent = channel_and_agent + + with pytest.raises(AgentChatException) as exc_info: + _ = [msg async for msg in channel.invoke_stream(agent, [])] + + assert "No chat history available." in str(exc_info.value) + + +@patch("semantic_kernel.agents.bedrock.bedrock_agent.BedrockAgent.create_session_id", return_value="session-id") +async def test_invoke_stream_appends_response_message(mock_session, channel_and_agent): + """Test invoke_stream properly yields streaming content and appends an aggregated message at the end.""" + channel, agent = channel_and_agent + + # Put a user message in the channel so it won't raise No chat history + channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="Last user message")) + + async def mock_invoke_stream( + session_id: str, input_text: str, sessionState=None, **kwargs + ) -> AsyncIterable[StreamingChatMessageContent]: + yield StreamingChatMessageContent( + role=AuthorRole.ASSISTANT, + choice_index=0, + content="Hello", + ) + yield StreamingChatMessageContent( + role=AuthorRole.ASSISTANT, + choice_index=0, + content=" World", + ) + + agent.invoke_stream = mock_invoke_stream + + # Check that we get the streamed messages and that the summarized message is appended afterward + messages_param = [ChatMessageContent(role=AuthorRole.USER, content="Last user message")] # just to pass the param + streamed_content = [msg async for msg in channel.invoke_stream(agent, messages_param)] + + # We expect two streamed chunks: "Hello" and " World" + assert len(streamed_content) == 2 + assert streamed_content[0].content == "Hello" + assert streamed_content[1].content == " World" + + # Then we expect the channel to append an aggregated ChatMessageContent with "Hello World" + assert len(messages_param) == 2 + appended = messages_param[1] + assert appended.role == AuthorRole.ASSISTANT + assert appended.content == "Hello World" + + +async def test_get_history(channel_and_agent): + """Test get_history yields messages in reverse order.""" + channel, _ = channel_and_agent + + channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User1")) + channel.messages.append(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant1")) + channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User2")) + + reversed_history = [msg async for msg in channel.get_history()] + + # Should be reversed + assert reversed_history[0].content == "User2" + assert reversed_history[1].content == "Assistant1" + assert reversed_history[2].content == "User1" + + +async def test_reset(channel_and_agent): + """Test that reset clears all messages from the channel.""" + channel, _ = channel_and_agent + + channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User1")) + channel.messages.append(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant1")) + + assert len(channel.messages) == 2 + + await channel.reset() + + assert len(channel.messages) == 0 + + +async def test_ensure_history_alternates_handles_multiple_consecutive_roles(): + """Direct test for _ensure_history_alternates by simulating various consecutive roles.""" + channel = BedrockAgentChannel() + + channel.messages = [ + ChatMessageContent(role=AuthorRole.USER, content="User1"), + ChatMessageContent(role=AuthorRole.USER, content="User2"), + ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist1"), + ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist2"), + ChatMessageContent(role=AuthorRole.USER, content="User3"), + ChatMessageContent(role=AuthorRole.USER, content="User4"), + ] + + channel._ensure_history_alternates() + + # Expect placeholders inserted: + # User1, placeholder assistant, User2, Assistant(Assist1), placeholder user, Assistant(Assist2), + # placeholder assistant, User3, placeholder assistant, User4 + + # Let's verify the roles: + expected_roles = [ + AuthorRole.USER, + AuthorRole.ASSISTANT, # placeholder for consecutive user + AuthorRole.USER, + AuthorRole.ASSISTANT, + AuthorRole.USER, # placeholder for consecutive assistant + AuthorRole.ASSISTANT, + AuthorRole.ASSISTANT, # placeholder for consecutive user (Wait carefully: we had Assist1 -> Assist2? We'll see) + AuthorRole.USER, + AuthorRole.ASSISTANT, # placeholder after we see user -> user + AuthorRole.USER, + ] + + # Actually let's systematically figure out the inserted placeholders. + # The original had: Index 0: U1, Index 1: U2 -> consecutive user, so after index 1 we insert placeholder assistant. + # Then index 2: Assist1. Now index 3: Assist2 -> consecutive assistant, so insert placeholder user. + # Then index 4: U3, index 5: U4 -> consecutive user, insert placeholder assistant. + + # Final roles: + # [U1, placeholder(A), U2, A, placeholder(U), A, U3, placeholder(A), U4] + # Wait, let's carefully track insertion: + # messages before insertion: + # 0: U1 + # 1: U2 + # 2: A1 + # 3: A2 + # 4: U3 + # 5: U4 + # We'll walk with current_index starting at 1: + # 1: role=U2 same as role=U1 => insert placeholder A at index=1 => increment index=3 + # now 2-> new index=3 => role= A2 same as A1 => insert placeholder user at index=3 => increment index=5 + # now 4-> new index=5 => role=U4 same as U3 => insert placeholder assistant => increment index=7 => done + # So final messages: + # index 0: U1 + # index 1: placeholder(A) + # index 2: U2 + # index 3: A1 + # index 4: placeholder(U) + # index 5: A2 + # index 6: U3 + # index 7: placeholder(A) + # index 8: U4 + + # let's define expected roles from that final structure: + expected_roles = [ + AuthorRole.USER, + AuthorRole.ASSISTANT, # placeholder + AuthorRole.USER, + AuthorRole.ASSISTANT, + AuthorRole.USER, # placeholder + AuthorRole.ASSISTANT, + AuthorRole.USER, + AuthorRole.ASSISTANT, # placeholder + AuthorRole.USER, + ] + expected_contents = [ + "User1", + BedrockAgentChannel.MESSAGE_PLACEHOLDER, + "User2", + "Assist1", + BedrockAgentChannel.MESSAGE_PLACEHOLDER, + "Assist2", + "User3", + BedrockAgentChannel.MESSAGE_PLACEHOLDER, + "User4", + ] + + assert len(channel.messages) == len(expected_roles) + for i, (msg, exp_role, exp_content) in enumerate(zip(channel.messages, expected_roles, expected_contents)): + assert msg.role == exp_role, f"Role mismatch at index {i}. Got {msg.role}, expected {exp_role}" + assert msg.content == exp_content, f"Content mismatch at index {i}. Got {msg.content}, expected {exp_content}" + + +async def test_ensure_last_message_is_user_appends_placeholder_if_last_assistant(): + """Test that _ensure_last_message_is_user appends a user placeholder if the last message is an assistant one.""" + channel = BedrockAgentChannel() + + channel.messages = [ + ChatMessageContent(role=AuthorRole.USER, content="User1"), + ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist1"), + ] + + channel._ensure_last_message_is_user() + + assert len(channel.messages) == 3 + last_msg = channel.messages[-1] + assert last_msg.role == AuthorRole.USER + assert last_msg.content == BedrockAgentChannel.MESSAGE_PLACEHOLDER + + +async def test_parse_chat_history_to_session_state_returns_valid_structure(): + """Test that _parse_chat_history_to_session_state() returns expected structure ignoring the last message.""" + channel = BedrockAgentChannel() + + channel.messages = [ + ChatMessageContent(role=AuthorRole.USER, content="User1"), + ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist1"), + ChatMessageContent(role=AuthorRole.USER, content="User2"), + ] + + session_state = channel._parse_chat_history_to_session_state() + + # The last message is not taken, so we only see the first two messages + assert "conversationHistory" in session_state + assert "messages" in session_state["conversationHistory"] + + msg_list = session_state["conversationHistory"]["messages"] + assert len(msg_list) == 2 + assert msg_list[0]["role"] == "user" + assert msg_list[0]["content"] == [{"text": "User1"}] + assert msg_list[1]["role"] == "assistant" + assert msg_list[1]["content"] == [{"text": "Assist1"}] From de3d42f7bfb6a2a783365b7bac21b3da7045048a Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Tue, 25 Feb 2025 14:49:25 -0300 Subject: [PATCH 02/18] adding tests for bedrock model provider utils --- .../test_bedrock_model_provider_utils.py | 329 ++++++++++++++++++ 1 file changed, 329 insertions(+) diff --git a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py index 4a5728be554c..5ca6370ea7af 100644 --- a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py +++ b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py @@ -1,5 +1,7 @@ # Copyright (c) Microsoft. All rights reserved. +from unittest.mock import AsyncMock, MagicMock, patch + import pytest from semantic_kernel.connectors.ai.bedrock.bedrock_prompt_execution_settings import BedrockChatPromptExecutionSettings @@ -7,10 +9,27 @@ BedrockModelProvider, ) from semantic_kernel.connectors.ai.bedrock.services.model_provider.utils import ( + _format_assistant_message, + _format_system_message, + _format_tool_message, + _format_user_message, + finish_reason_from_bedrock_to_semantic_kernel, + format_bedrock_function_name_to_kernel_function_fully_qualified_name, remove_none_recursively, + run_in_executor, update_settings_from_function_choice_configuration, ) +from semantic_kernel.connectors.ai.function_call_choice_configuration import FunctionCallChoiceConfiguration from semantic_kernel.connectors.ai.function_choice_behavior import FunctionChoiceBehavior +from semantic_kernel.connectors.ai.function_choice_type import FunctionChoiceType +from semantic_kernel.contents.chat_message_content import ChatMessageContent +from semantic_kernel.contents.function_call_content import FunctionCallContent +from semantic_kernel.contents.function_result_content import FunctionResultContent +from semantic_kernel.contents.image_content import ImageContent +from semantic_kernel.contents.text_content import TextContent +from semantic_kernel.contents.utils.author_role import AuthorRole +from semantic_kernel.contents.utils.finish_reason import FinishReason +from semantic_kernel.exceptions.service_exceptions import ServiceInvalidRequestError from semantic_kernel.kernel import Kernel @@ -146,3 +165,313 @@ def test_inference_profile_with_bedrock_model() -> None: unknown_inference_profile = "unknown" with pytest.raises(ValueError, match="Model ID unknown does not contain a valid model provider name."): BedrockModelProvider.to_model_provider(unknown_inference_profile) + + +@pytest.mark.asyncio +async def test_run_in_executor_calls_executor_properly() -> None: + """Test that run_in_executor properly calls an executor and function.""" + mock_executor = MagicMock() + mock_func = MagicMock(return_value="test_result") + + with patch( + "asyncio.get_event_loop", return_value=MagicMock(run_in_executor=AsyncMock(return_value="test_result")) + ) as evt: + result = await run_in_executor(mock_executor, mock_func, "arg1", key="value") + + evt.assert_called_once() + assert result == "test_result" + + +def test_remove_none_recursively_empty_dict() -> None: + """Test that an empty dict returns an empty dict.""" + assert remove_none_recursively({}) == {} + + +def test_remove_none_recursively_no_none() -> None: + """Test that a dict with no None values remains the same.""" + original = {"a": 1, "b": 2} + result = remove_none_recursively(original) + assert result == {"a": 1, "b": 2} + + +def test_remove_none_recursively_with_none() -> None: + """Test that dict values of None are removed.""" + original = {"a": 1, "b": None, "c": {"d": None, "e": 3}} + result = remove_none_recursively(original) + # 'b' should be removed and 'd' inside nested dict should be removed + assert result == {"a": 1, "c": {"e": 3}} + + +def test_remove_none_recursively_max_depth() -> None: + """Test that the function respects max_depth.""" + original = {"a": {"b": {"c": None}}} + # If max_depth=1, it won't go deep enough to remove 'c'. + result = remove_none_recursively(original, max_depth=1) + assert result == {"a": {"b": {"c": None}}} + + # If max_depth=3, it should remove 'c'. + result = remove_none_recursively(original, max_depth=3) + assert result == {"a": {"b": {}}} + + +def test_format_system_message() -> None: + """Test that system message is formatted correctly.""" + content = ChatMessageContent(role=AuthorRole.SYSTEM, content="System message") + formatted = _format_system_message(content) + assert formatted == {"text": "System message"} + + +def test_format_user_message_text_only() -> None: + """Test user message with only text content.""" + text_item = TextContent(text="Hello!") + user_message = ChatMessageContent(role=AuthorRole.USER, items=[text_item]) + + formatted = _format_user_message(user_message) + assert formatted["role"] == "user" + assert len(formatted["content"]) == 1 + assert formatted["content"][0] == {"text": "Hello!"} + + +def test_format_user_message_image_only() -> None: + """Test user message with only image content.""" + img_item = ImageContent(data=b"abc", mime_type="image/png") + user_message = ChatMessageContent(role=AuthorRole.USER, items=[img_item]) + + formatted = _format_user_message(user_message) + assert formatted["role"] == "user" + assert len(formatted["content"]) == 1 + image_section = formatted["content"][0].get("image") + assert image_section["format"] == "png" + assert image_section["source"]["bytes"] == b"abc" + + +def test_format_user_message_unsupported_content() -> None: + """Test user message raises error with unsupported content type.""" + # We can simulate an unsupported content type by using FunctionCallContent. + func_call_item = FunctionCallContent(id="123", function_name="test_function", arguments="{}") + user_message = ChatMessageContent(role=AuthorRole.USER, items=[func_call_item]) + + with pytest.raises(ServiceInvalidRequestError) as exc: + _format_user_message(user_message) + + assert "Only text and image content are supported in a user message." in str(exc.value) + + +def test_format_assistant_message_text_content() -> None: + """Test assistant message with text content.""" + text_item = TextContent(text="Assistant response") + assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[text_item]) + + formatted = _format_assistant_message(assistant_message) + assert formatted["role"] == "assistant" + assert formatted["content"] == [{"text": "Assistant response"}] + + +def test_format_assistant_message_function_call_content() -> None: + """Test assistant message with function call content.""" + func_item = FunctionCallContent( + id="fc1", plugin_name="plugin", function_name="function", arguments='{"param": "value"}' + ) + assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[func_item]) + + formatted = _format_assistant_message(assistant_message) + assert len(formatted["content"]) == 1 + tool_use = formatted["content"][0].get("toolUse") + assert tool_use + assert tool_use["toolUseId"] == "fc1" + assert tool_use["name"] == "plugin_function" + assert tool_use["input"] == {"param": "value"} + + +def test_format_assistant_message_image_content_raises() -> None: + """Test assistant message with image raises error.""" + img_item = ImageContent(data=b"abc", mime_type="image/jpeg") + assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[img_item]) + + with pytest.raises(ServiceInvalidRequestError) as exc: + _format_assistant_message(assistant_message) + + assert "Image content is not supported in an assistant message." in str(exc.value) + + +def test_format_assistant_message_unsupported_type() -> None: + """Test assistant message with unsupported item content type.""" + func_res_item = FunctionResultContent(id="res1", function_name="some_function", result="some_result") + assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[func_res_item]) + + with pytest.raises(ServiceInvalidRequestError) as exc: + _format_assistant_message(assistant_message) + assert "Unsupported content type in an assistant message:" in str(exc.value) + + +def test_format_tool_message_text() -> None: + """Test tool message with text content.""" + text_item = TextContent(text="Some text") + tool_message = ChatMessageContent(role=AuthorRole.TOOL, items=[text_item]) + + formatted = _format_tool_message(tool_message) + assert formatted["role"] == "user" # note that for a tool message, role set to 'user' + assert formatted["content"] == [{"text": "Some text"}] + + +def test_format_tool_message_function_result() -> None: + """Test tool message with function result content.""" + func_result_item = FunctionResultContent(id="res_id", function_name="test_function", result="some result") + tool_message = ChatMessageContent(role=AuthorRole.TOOL, items=[func_result_item]) + + formatted = _format_tool_message(tool_message) + assert formatted["role"] == "user" + content = formatted["content"][0] + assert content.get("toolResult") + assert content["toolResult"]["toolUseId"] == "res_id" + assert content["toolResult"]["content"] == [{"text": "some result"}] + + +def test_format_tool_message_image_raises() -> None: + """Test tool message with image content raises an error.""" + img_item = ImageContent(data=b"xyz", mime_type="image/jpeg") + tool_message = ChatMessageContent(role=AuthorRole.TOOL, items=[img_item]) + + with pytest.raises(ServiceInvalidRequestError) as exc: + _format_tool_message(tool_message) + assert "Image content is not supported in a tool message." in str(exc.value) + + +def test_format_bedrock_function_name_to_kernel_function_fully_qualified_name_no_sep() -> None: + """Test function names without underscore remain unchanged.""" + name = "SinglePartName" + result = format_bedrock_function_name_to_kernel_function_fully_qualified_name(name) + assert result == name + + +def test_format_bedrock_function_name_to_kernel_function_fully_qualified_name_with_sep() -> None: + """Test function names with underscore are split into plugin/function with the default sep.""" + name = "plugin_function" + result = format_bedrock_function_name_to_kernel_function_fully_qualified_name(name) + assert result == "plugin-function" + + +def test_finish_reason_from_bedrock_to_semantic_kernel_stop() -> None: + """Test that 'stop_sequence' maps to FinishReason.STOP""" + reason = finish_reason_from_bedrock_to_semantic_kernel("stop_sequence") + assert reason == FinishReason.STOP + + reason = finish_reason_from_bedrock_to_semantic_kernel("end_turn") + assert reason == FinishReason.STOP + + +def test_finish_reason_from_bedrock_to_semantic_kernel_length() -> None: + """Test that 'max_tokens' maps to FinishReason.LENGTH""" + reason = finish_reason_from_bedrock_to_semantic_kernel("max_tokens") + assert reason == FinishReason.LENGTH + + +def test_finish_reason_from_bedrock_to_semantic_kernel_content_filtered() -> None: + """Test that 'content_filtered' maps to FinishReason.CONTENT_FILTER""" + reason = finish_reason_from_bedrock_to_semantic_kernel("content_filtered") + assert reason == FinishReason.CONTENT_FILTER + + +def test_finish_reason_from_bedrock_to_semantic_kernel_tool_use() -> None: + """Test that 'tool_use' maps to FinishReason.TOOL_CALLS""" + reason = finish_reason_from_bedrock_to_semantic_kernel("tool_use") + assert reason == FinishReason.TOOL_CALLS + + +def test_finish_reason_from_bedrock_to_semantic_kernel_unknown() -> None: + """Test that unknown finish reason returns None""" + reason = finish_reason_from_bedrock_to_semantic_kernel("something_unknown") + assert reason is None + + +@pytest.fixture +def mock_bedrock_settings() -> BedrockChatPromptExecutionSettings: + """Helper fixture for BedrockChatPromptExecutionSettings.""" + return BedrockChatPromptExecutionSettings() + + +@pytest.fixture +def mock_function_choice_config() -> FunctionCallChoiceConfiguration: + """Helper fixture for a sample FunctionCallChoiceConfiguration.""" + + # We'll create mock kernel functions with metadata + mock_func_1 = MagicMock() + mock_func_1.custom_fully_qualified_name.return_value = "plugin_function1" + mock_func_1.description = "Function 1 description" + + param1 = MagicMock() + param1.name = "param1" + param1.schema_data = {"type": "string"} + param1.is_required = True + + param2 = MagicMock() + param2.name = "param2" + param2.schema_data = {"type": "integer"} + param2.is_required = False + + mock_func_1.parameters = [ + param1, + param2, + ] + mock_func_2 = MagicMock() + mock_func_2.custom_fully_qualified_name.return_value = "plugin_function2" + mock_func_2.description = "Function 2 description" + mock_func_2.parameters = [] + + config = FunctionCallChoiceConfiguration() + config.available_functions = [mock_func_1, mock_func_2] + + return config + + +def test_update_settings_from_function_choice_configuration_none_type( + mock_function_choice_config, mock_bedrock_settings +) -> None: + """Test that if the FunctionChoiceType is NONE it doesn't modify settings.""" + update_settings_from_function_choice_configuration( + mock_function_choice_config, mock_bedrock_settings, FunctionChoiceType.NONE + ) + assert mock_bedrock_settings.tool_choice is None + assert mock_bedrock_settings.tools is None + + +def test_update_settings_from_function_choice_configuration_auto_two_tools( + mock_function_choice_config, mock_bedrock_settings +) -> None: + """Test that AUTO sets tool_choice to {"auto": {}} and sets tools list""" + update_settings_from_function_choice_configuration( + mock_function_choice_config, mock_bedrock_settings, FunctionChoiceType.AUTO + ) + assert mock_bedrock_settings.tool_choice == {"auto": {}} + assert len(mock_bedrock_settings.tools) == 2 + # Validate structure of first tool + tool_spec_1 = mock_bedrock_settings.tools[0].get("toolSpec") + assert tool_spec_1["name"] == "plugin_function1" + assert tool_spec_1["description"] == "Function 1 description" + + +def test_update_settings_from_function_choice_configuration_required_many( + mock_function_choice_config, mock_bedrock_settings +) -> None: + """Test that REQUIRED with more than one function sets tool_choice to {"any": {}}.""" + update_settings_from_function_choice_configuration( + mock_function_choice_config, mock_bedrock_settings, FunctionChoiceType.REQUIRED + ) + assert mock_bedrock_settings.tool_choice == {"any": {}} + assert len(mock_bedrock_settings.tools) == 2 + + +def test_update_settings_from_function_choice_configuration_required_one(mock_bedrock_settings) -> None: + """Test that REQUIRED with a single function picks "tool" with that function name.""" + single_func = MagicMock() + single_func.custom_fully_qualified_name.return_value = "plugin_function" + single_func.description = "Only function" + single_func.parameters = [] + + config = FunctionCallChoiceConfiguration() + config.available_functions = [single_func] + + update_settings_from_function_choice_configuration(config, mock_bedrock_settings, FunctionChoiceType.REQUIRED) + assert mock_bedrock_settings.tool_choice == {"tool": {"name": "plugin_function"}} + assert len(mock_bedrock_settings.tools) == 1 + assert mock_bedrock_settings.tools[0]["toolSpec"]["name"] == "plugin_function" From 63a508a9fb1dc9e3b003c1e75753d0d19c71b4b8 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Tue, 25 Feb 2025 14:49:34 -0300 Subject: [PATCH 03/18] adding tests for mistral ai chat completion --- .../test_mistralai_chat_completion.py | 383 +++++++++++++++++- 1 file changed, 379 insertions(+), 4 deletions(-) diff --git a/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py b/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py index 8db675b627bc..1a8fa18bf40a 100644 --- a/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py +++ b/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py @@ -1,23 +1,35 @@ # Copyright (c) Microsoft. All rights reserved. + from unittest.mock import AsyncMock, MagicMock, patch import pytest -from mistralai import Mistral +from mistralai import CompletionEvent, FunctionCall, Mistral +from mistralai.models import ( + AssistantMessage, + ChatCompletionChoice, + ChatCompletionResponse, + CompletionChunk, + CompletionResponseStreamChoice, + DeltaMessage, + ToolCall, + UsageInfo, +) from semantic_kernel.connectors.ai.chat_completion_client_base import ChatCompletionClientBase -from semantic_kernel.connectors.ai.function_choice_behavior import FunctionChoiceBehavior +from semantic_kernel.connectors.ai.function_call_choice_configuration import FunctionCallChoiceConfiguration +from semantic_kernel.connectors.ai.function_choice_behavior import FunctionChoiceBehavior, FunctionChoiceType from semantic_kernel.connectors.ai.mistral_ai.prompt_execution_settings.mistral_ai_prompt_execution_settings import ( MistralAIChatPromptExecutionSettings, ) from semantic_kernel.connectors.ai.mistral_ai.services.mistral_ai_chat_completion import MistralAIChatCompletion +from semantic_kernel.connectors.ai.mistral_ai.settings.mistral_ai_settings import MistralAISettings from semantic_kernel.connectors.ai.open_ai.prompt_execution_settings.open_ai_prompt_execution_settings import ( OpenAIChatPromptExecutionSettings, ) +from semantic_kernel.contents import FunctionCallContent, StreamingTextContent, TextContent from semantic_kernel.contents.chat_history import ChatHistory from semantic_kernel.contents.chat_message_content import ( ChatMessageContent, - FunctionCallContent, - TextContent, ) from semantic_kernel.contents.streaming_chat_message_content import StreamingChatMessageContent from semantic_kernel.contents.utils.author_role import AuthorRole @@ -27,6 +39,7 @@ ServiceResponseException, ) from semantic_kernel.functions.kernel_arguments import KernelArguments +from semantic_kernel.functions.kernel_function import KernelFunction from semantic_kernel.functions.kernel_function_decorator import kernel_function from semantic_kernel.kernel import Kernel @@ -372,3 +385,365 @@ async def test_with_different_execution_settings_stream( continue assert mock_mistral_ai_client_completion_stream.chat.stream_async.call_args.kwargs["temperature"] == 0.2 assert mock_mistral_ai_client_completion_stream.chat.stream_async.call_args.kwargs["seed"] == 2 + + +async def test_mistral_ai_chat_completion_initialization_valid(): + """Test MistralAIChatCompletion initialization should succeed with valid params.""" + mistralai_settings = MagicMock() + mistralai_settings.chat_model_id = "chat_model_id" + + with ( + patch( + "semantic_kernel.connectors.ai.mistral_ai.settings.mistral_ai_settings.MistralAISettings.create", + return_value=mistralai_settings, + ) as mock_settings_create, + patch( + "semantic_kernel.connectors.ai.mistral_ai.services.mistral_ai_base.MistralAIBase.__init__" + ) as mistral_base, + ): + # We gave an api_key to ensure it doesn't raise an exception, but the rest can be None. + ai_completion = MistralAIChatCompletion( + ai_model_id="some-model-id", + api_key="some-key", + ) + assert ai_completion is not None + + mock_settings_create.assert_called_once() + mistral_base.assert_called_once() + + +async def test_mistral_ai_chat_completion_initialization_missing_model_id(): + """Test MistralAIChatCompletion initialization should raise ServiceInitializationError when model id is missing.""" + with patch("mistralai.Mistral", return_value=MagicMock()): + with pytest.raises(ServiceInitializationError) as exc_info: + MistralAIChatCompletion( + ai_model_id=None, + api_key="some-key", + ) + assert "The MistralAI chat model ID is required." in str(exc_info.value) + + +async def test_mistral_ai_chat_completion_initialization_validation_error(): + """Test MistralAIChatCompletion initialization when MistralAISettings creation fails and raises ValidationError.""" + # About ValidationError, we can patch the MistralAISettings create method to raise it. + from pydantic import ValidationError + + with patch.object( + MistralAISettings, + "create", + side_effect=ValidationError("Validation error", []), + ): + with pytest.raises(ServiceInitializationError) as exc_info: + MistralAIChatCompletion( + ai_model_id="test", + api_key="test", + ) + assert "Failed to create MistralAI settings." in str(exc_info.value) + + +async def test_mistral_ai_chat_completion_inner_get_chat_message_contents_success(): + """Test _inner_get_chat_message_contents with a successful ChatCompletionResponse.""" + + # Mock the response from the Mistral chat complete_async. + mock_response = ChatCompletionResponse( + id="some_id", + object="object", + created=12345, + usage=UsageInfo(prompt_tokens=10, completion_tokens=20, total_tokens=30), + model="test-model", + choices=[ + ChatCompletionChoice( + index=0, + message=AssistantMessage(role="assistant", content="Hello!"), + finish_reason="stop", + ) + ], + ) + + async_mock_client = MagicMock(spec=Mistral) + async_mock_client.chat = MagicMock() + async_mock_client.chat.complete_async = AsyncMock(return_value=mock_response) + + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + async_client=async_mock_client, + ) + + # We create a ChatHistory. + chat_history = ChatHistory() + settings = MistralAIChatPromptExecutionSettings() + + results = await chat_completion._inner_get_chat_message_contents(chat_history, settings) + + # We should have exactly one ChatMessageContent. + assert len(results) == 1 + assert results[0].role.value == "assistant" + assert results[0].finish_reason is not None + assert results[0].finish_reason.value == "stop" + assert "Hello!" in results[0].content + async_mock_client.chat.complete_async.assert_awaited_once() + + +async def test_mistral_ai_chat_completion_inner_get_chat_message_contents_failure(): + """Test _inner_get_chat_message_contents should raise ServiceResponseException if Mistral call fails.""" + async_mock_client = MagicMock(spec=Mistral) + async_mock_client.chat = MagicMock() + async_mock_client.chat.complete_async = AsyncMock(side_effect=Exception("API error")) + + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + async_client=async_mock_client, + ) + + chat_history = ChatHistory() + settings = MistralAIChatPromptExecutionSettings() + + with pytest.raises(ServiceResponseException) as exc: + await chat_completion._inner_get_chat_message_contents(chat_history, settings) + assert "service failed to complete the prompt" in str(exc.value) + + +async def test_mistral_ai_chat_completion_inner_get_streaming_chat_message_contents_success(): + """Test _inner_get_streaming_chat_message_contents when streaming successfully.""" + + # We'll yield multiple chunks to simulate streaming. + mock_chunk1 = CompletionEvent( + data=CompletionChunk( + id="chunk1", + created=1, + model="test-model", + choices=[ + CompletionResponseStreamChoice( + index=0, + delta=DeltaMessage(role="assistant", content="Hello "), + finish_reason=None, + ) + ], + ) + ) + mock_chunk2 = CompletionEvent( + data=CompletionChunk( + id="chunk1", + created=1, + model="test-model", + choices=[ + CompletionResponseStreamChoice( + index=0, + delta=DeltaMessage(content="World!"), + finish_reason="stop", + ) + ], + ) + ) + + async def mock_stream_async(**kwargs): + yield mock_chunk1 + yield mock_chunk2 + + async_mock_client = MagicMock(spec=Mistral) + async_mock_client.chat = MagicMock() + async_mock_client.chat.stream_async = AsyncMock(return_value=mock_stream_async()) + + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + async_client=async_mock_client, + ) + + chat_history = ChatHistory() + settings = MistralAIChatPromptExecutionSettings() + + collected_chunks = [] + async for chunk_list in chat_completion._inner_get_streaming_chat_message_contents(chat_history, settings): + collected_chunks.append(chunk_list) + + # We expect two sets of chunk_list yields. + assert len(collected_chunks) == 2 + assert len(collected_chunks[0]) == 1 + assert len(collected_chunks[1]) == 1 + + # First chunk contains "Hello ", second chunk "World!". + assert collected_chunks[0][0].items[0].text == "Hello " + assert collected_chunks[1][0].items[0].text == "World!" + + +async def test_mistral_ai_chat_completion_inner_get_streaming_chat_message_contents_failure(): + """Test _inner_get_streaming_chat_message_contents raising a ServiceResponseException on failure.""" + async_mock_client = MagicMock(spec=Mistral) + async_mock_client.chat = MagicMock() + async_mock_client.chat.stream_async = AsyncMock(side_effect=Exception("Streaming error")) + + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + async_client=async_mock_client, + ) + + chat_history = ChatHistory() + settings = MistralAIChatPromptExecutionSettings() + + with pytest.raises(ServiceResponseException) as exc: + async for _ in chat_completion._inner_get_streaming_chat_message_contents(chat_history, settings): + pass + assert "service failed to complete the prompt" in str(exc.value) + + +async def test_mistral_ai_chat_completion_create_chat_message_content_with_tool_calls(): + """Test _create_chat_message_content to ensure it picks up tool calls if present.""" + + # We mock a choice that has message with content and some tool calls. + mock_tool = ToolCall( + id="tool_1", + function=FunctionCall(name="function_name", arguments="function_args"), + ) + mock_message = AssistantMessage( + role="assistant", + content="Hello with tool!", + tool_calls=[mock_tool], + ) + + mock_choice = ChatCompletionChoice( + index=0, + message=mock_message, + finish_reason="stop", + ) + mock_response = ChatCompletionResponse( + id="resp_id", + object="object", + created=999, + usage=UsageInfo(prompt_tokens=10, completion_tokens=20, total_tokens=30), + model="some-model", + choices=[mock_choice], + ) + + async_mock_client = MagicMock(spec=Mistral) + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + async_client=async_mock_client, + ) + + # We'll directly call the underlying method. + meta = {"metadata": "test"} + result = chat_completion._create_chat_message_content(mock_response, mock_choice, meta) + + # We expect to see one text item and one function call item. + assert len(result.items) == 2 + assert any(isinstance(i, TextContent) for i in result.items) + assert any(isinstance(i, FunctionCallContent) for i in result.items) + + func_call = [i for i in result.items if isinstance(i, FunctionCallContent)][0] + assert func_call.id == "tool_1" + assert func_call.name == "function_name" + assert func_call.arguments == "function_args" + + +async def test_mistral_ai_chat_completion_create_streaming_chat_message_content_with_tool_calls(): + """Test _create_streaming_chat_message_content to ensure it picks up tool calls if present.""" + + mock_tool = ToolCall( + id="tool_2", + function=FunctionCall(name="function_name_2", arguments="function_args_2"), + ) + + mock_chunk = CompletionChunk( + id="chunk2", + created=222, + model="model-2", + choices=[], + ) + + mock_choice = CompletionResponseStreamChoice( + index=1, + delta=DeltaMessage(role="assistant", content="Hello from stream!", tool_calls=[mock_tool]), + finish_reason=None, + ) + chunk_metadata = {"chunk": "meta"} + + async_mock_client = MagicMock(spec=Mistral) + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + async_client=async_mock_client, + ) + + result = chat_completion._create_streaming_chat_message_content( + chunk=mock_chunk, + choice=mock_choice, + chunk_metadata=chunk_metadata, + function_invoke_attempt=0, + ) + + # We expect to see one StreamingTextContent item and one FunctionCallContent. + assert len(result.items) == 2 + stc = [i for i in result.items if isinstance(i, StreamingTextContent)][0] + assert stc.text == "Hello from stream!" + + fcc = [i for i in result.items if isinstance(i, FunctionCallContent)][0] + assert fcc.id == "tool_2" + assert fcc.name == "function_name_2" + assert fcc.arguments == "function_args_2" + + +async def test_mistral_ai_chat_completion_update_settings_from_function_call_configuration_mistral(): + """Test update_settings_from_function_call_configuration_mistral sets tools etc.""" + + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + ) + + # Create a mock settings object. + settings = MistralAIChatPromptExecutionSettings() + # Create a function choice config with some available functions. + config = FunctionCallChoiceConfiguration() + mock_func = MagicMock( + spec=KernelFunction, + ) + mock_func.name = "my_func" + mock_func.description = "some desc" + mock_func.fully_qualified_name = "mod.my_func" + mock_func.parameters = [] + config.available_functions = [mock_func] + + # Call the update_settings_from_function_call_configuration_mistral with type=ANY. + chat_completion.update_settings_from_function_call_configuration_mistral( + function_choice_configuration=config, + settings=settings, + type=FunctionChoiceType.AUTO, + ) + + assert settings.tool_choice == FunctionChoiceType.AUTO.value + assert settings.tools is not None + assert len(settings.tools) == 1 + assert settings.tools[0]["function"]["name"] == "mod.my_func" + + +async def test_mistral_ai_chat_completion_reset_function_choice_settings(): + """Test that _reset_function_choice_settings resets specific attributes.""" + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + ) + settings = MistralAIChatPromptExecutionSettings(tool_choice="any", tools=[{"name": "func1"}]) + + chat_completion._reset_function_choice_settings(settings) + assert settings.tool_choice is None + assert settings.tools is None + + +async def test_mistral_ai_chat_completion_service_url(): + """Test that service_url attempts to use _endpoint from the async_client.""" + async_mock_client = MagicMock(spec=Mistral) + async_mock_client._endpoint = "mistral" + + chat_completion = MistralAIChatCompletion( + ai_model_id="test-model", + api_key="test_key", + async_client=async_mock_client, + ) + + url = chat_completion.service_url() + assert url == "mistral" From e6f0eb42c69b673f0f56dc819225aa797c149428 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Tue, 25 Feb 2025 14:49:41 -0300 Subject: [PATCH 04/18] tests for bing search --- .../search/bing/test_bing_search.py | 293 ++++++++++++++++++ 1 file changed, 293 insertions(+) create mode 100644 python/tests/unit/connectors/search/bing/test_bing_search.py diff --git a/python/tests/unit/connectors/search/bing/test_bing_search.py b/python/tests/unit/connectors/search/bing/test_bing_search.py new file mode 100644 index 000000000000..afce9435be97 --- /dev/null +++ b/python/tests/unit/connectors/search/bing/test_bing_search.py @@ -0,0 +1,293 @@ +# Copyright (c) Microsoft. All rights reserved. + +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest +from pydantic import ValidationError + +from semantic_kernel.connectors.search.bing.bing_search import BingSearch +from semantic_kernel.connectors.search.bing.bing_search_response import BingSearchResponse, BingWebPages +from semantic_kernel.connectors.search.bing.bing_search_settings import BingSettings +from semantic_kernel.connectors.search.bing.bing_web_page import BingWebPage +from semantic_kernel.data.kernel_search_results import KernelSearchResults +from semantic_kernel.data.text_search.text_search_filter import TextSearchFilter +from semantic_kernel.data.text_search.text_search_options import TextSearchOptions +from semantic_kernel.data.text_search.text_search_result import TextSearchResult +from semantic_kernel.exceptions import ServiceInitializationError, ServiceInvalidRequestError + + +async def test_bing_search_init_success(): + """Test that BingSearch initializes successfully with valid parameters.""" + # Arrange/Act + with patch.object(BingSettings, "create", return_value=MagicMock(spec=BingSettings)) as mock_settings_create: + search_instance = BingSearch(api_key="fake_api_key", custom_config="fake_config") + + # Assert + mock_settings_create.assert_called_once() + assert search_instance is not None + + +async def test_bing_search_init_validation_error(): + """Test that BingSearch raises ServiceInitializationError if BingSettings creation fails.""" + # Arrange + with patch.object(BingSettings, "create", side_effect=ValidationError("error", [])): + # Act / Assert + with pytest.raises(ServiceInitializationError) as exc_info: + _ = BingSearch(api_key="invalid_api_key") + assert "Failed to create Bing settings." in str(exc_info.value) + + +async def test_search_success(): + """Test that search method returns KernelSearchResults successfully on valid response.""" + # Arrange + mock_web_pages = BingWebPage(snippet="Test snippet") + mock_response = BingSearchResponse( + webPages=MagicMock(spec=BingWebPages, value=[mock_web_pages], total_estimated_matches=10), + query_context={"alteredQuery": "altered something"}, + ) + + async_client_mock = AsyncMock() + mock_result = MagicMock() + mock_result.text = """ +{"webPages": { + "value": [{"snippet": "Test snippet"}], + "totalEstimatedMatches": 10}, + "queryContext": {"alteredQuery": "altered something"} +}""" + async_client_mock.get.return_value = mock_result + + # Act + with ( + patch("semantic_kernel.connectors.search.bing.bing_search.AsyncClient", return_value=async_client_mock), + patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), + ): + search_instance = BingSearch(api_key="fake_api_key") + options = TextSearchOptions(include_total_count=True) + kernel_results: KernelSearchResults[str] = await search_instance.search("Test query", options) + + # Assert + results_list = [] + async for res in kernel_results.results: + results_list.append(res) + + assert len(results_list) == 1 + assert results_list[0] == "Test snippet" + assert kernel_results.total_count == 10 + assert kernel_results.metadata == {"altered_query": "altered something"} + + +async def test_search_http_status_error(): + """Test that search method raises ServiceInvalidRequestError on HTTPStatusError.""" + # Arrange + async_client_mock = AsyncMock() + mock_response = MagicMock() + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "Error", request=MagicMock(), response=MagicMock() + ) + async_client_mock.get.return_value = mock_response + + # Act + with patch( + "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock + ): + search_instance = BingSearch(api_key="fake_api_key") + # Assert + with pytest.raises(ServiceInvalidRequestError) as exc_info: + await search_instance.search("Test query") + assert "Failed to get search results." in str(exc_info.value) + + +async def test_search_request_error(): + """Test that search method raises ServiceInvalidRequestError on RequestError.""" + # Arrange + async_client_mock = AsyncMock() + async_client_mock.get.side_effect = httpx.RequestError("Client error") + + # Act + with patch( + "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock + ): + search_instance = BingSearch(api_key="fake_api_key") + # Assert + with pytest.raises(ServiceInvalidRequestError) as exc_info: + await search_instance.search("Test query") + assert "A client error occurred while getting search results." in str(exc_info.value) + + +async def test_search_generic_exception(): + """Test that search method raises ServiceInvalidRequestError on unexpected exception.""" + # Arrange + async_client_mock = AsyncMock() + async_client_mock.get.side_effect = Exception("Something unexpected") + + # Act + with patch( + "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock + ): + search_instance = BingSearch(api_key="fake_api_key") + # Assert + with pytest.raises(ServiceInvalidRequestError) as exc_info: + await search_instance.search("Test query") + assert "An unexpected error occurred while getting search results." in str(exc_info.value) + + +async def test_validate_options_raises_error_for_large_top(): + """Test that _validate_options raises ServiceInvalidRequestError when top >= 50.""" + # Arrange + search_instance = BingSearch(api_key="fake_api_key") + options = TextSearchOptions(top=50) + + # Act / Assert + with pytest.raises(ServiceInvalidRequestError) as exc_info: + await search_instance._inner_search("test", options) + assert "count value must be less than 50." in str(exc_info.value) + + +async def test_get_text_search_results_success(): + """Test that get_text_search_results returns KernelSearchResults[TextSearchResult].""" + # Arrange + mock_web_pages = BingWebPage(name="Result Name", snippet="Snippet", url="test") + mock_response = BingSearchResponse( + query_context={}, + webPages=MagicMock(spec=BingWebPages, value=[mock_web_pages], total_estimated_matches=5), + ) + + async_client_mock = AsyncMock() + mock_result = MagicMock() + mock_result.text = """" +{"webPages": { + "value": [{"snippet": "Snippet", "name":"Result Name", "url":"test"}], + "totalEstimatedMatches": 5}, + "queryContext": {} +}' +""" + async_client_mock.get.return_value = mock_result + + # Act + with ( + patch( + "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock + ), + patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), + ): + search_instance = BingSearch(api_key="fake_api_key") + options = TextSearchOptions(include_total_count=True) + kernel_results: KernelSearchResults[TextSearchResult] = await search_instance.get_text_search_results( + "Test query", options + ) + + # Assert + results_list = [] + async for res in kernel_results.results: + results_list.append(res) + + assert len(results_list) == 1 + assert isinstance(results_list[0], TextSearchResult) + assert results_list[0].name == "Result Name" + assert results_list[0].value == "Snippet" + assert results_list[0].link == "test" + assert kernel_results.total_count == 5 + + +async def test_get_search_results_success(): + """Test that get_search_results returns KernelSearchResults[BingWebPage].""" + # Arrange + mock_web_page = BingWebPage(name="Page Name", snippet="Page Snippet", url="test") + mock_response = BingSearchResponse( + query_context={}, + webPages=MagicMock(spec=BingWebPages, value=[mock_web_page], total_estimated_matches=3), + ) + + async_client_mock = AsyncMock() + mock_result = MagicMock() + mock_result.text = """ +{"webPages": { + "value": [{"name": "Page Name", "snippet": "Page Snippet", "url": "test"}], + "totalEstimatedMatches": 3}, + "queryContext": {} +}""" + async_client_mock.get.return_value = mock_result + + # Act + with ( + patch( + "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock + ), + patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), + ): + search_instance = BingSearch(api_key="fake_api_key") + options = TextSearchOptions(include_total_count=True) + kernel_results = await search_instance.get_search_results("Another query", options) + + # Assert + results_list = [] + async for res in kernel_results.results: + results_list.append(res) + + assert len(results_list) == 1 + assert isinstance(results_list[0], BingWebPage) + assert results_list[0].name == "Page Name" + assert results_list[0].snippet == "Page Snippet" + assert results_list[0].url == "test" + assert kernel_results.total_count == 3 + + +async def test_build_request_parameters_no_filter(): + """Test that _build_request_parameters properly sets params when no filter is provided.""" + # Arrange + search_instance = BingSearch(api_key="fake_api_key") + options = TextSearchOptions() + + # Act + params = search_instance._build_request_parameters("test query", options) + + # Assert + assert params["count"] == options.top + assert params["offset"] == options.skip + # TODO: shouldn't this output be "test query" instead of "test query+"? + assert params["q"] == "test query+" + + +async def test_build_request_parameters_equal_to_filter(): + """Test that _build_request_parameters properly sets params with an EqualTo filter.""" + + # Arrange + search_instance = BingSearch(api_key="fake_api_key") + + my_filter = TextSearchFilter.equal_to(field_name="freshness", value="Day") + options = TextSearchOptions(filter=my_filter) + + # Act + params = search_instance._build_request_parameters("test query", options) + + # Assert + assert params["count"] == options.top + assert params["offset"] == options.skip + # 'freshness' is recognized in QUERY_PARAMETERS, so 'freshness' should be set + assert "freshness" in params + assert params["freshness"] == "Day" + # 'q' should be a combination of the original query plus a plus sign + assert params["q"] == "test query+".strip() + + +async def test_build_request_parameters_not_recognized_filter(): + """Test that _build_request_parameters properly appends non-recognized filters to the q parameter.""" + + # Arrange + search_instance = BingSearch(api_key="fake_api_key") + + # 'customProperty' is presumably not in QUERY_PARAMETERS + my_filter = TextSearchFilter.equal_to(field_name="customProperty", value="customValue") + options = TextSearchOptions(filter=my_filter) + + # Act + params = search_instance._build_request_parameters("test query", options) + + # Assert + assert params["count"] == options.top + assert params["offset"] == options.skip + assert "customProperty" not in params + # We expect 'q' to contain the extra query param in a plus-joined format + assert isinstance(params["q"], str) + assert "customProperty:customValue" in params["q"] From 70a41c4b79d802929102d15d7c6f8b193086a455 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Tue, 25 Feb 2025 14:49:48 -0300 Subject: [PATCH 05/18] tests for google search --- .../search/google/test_google_search.py | 354 ++++++++++++++++++ 1 file changed, 354 insertions(+) create mode 100644 python/tests/unit/connectors/search/google/test_google_search.py diff --git a/python/tests/unit/connectors/search/google/test_google_search.py b/python/tests/unit/connectors/search/google/test_google_search.py new file mode 100644 index 000000000000..51c84c7e6cd7 --- /dev/null +++ b/python/tests/unit/connectors/search/google/test_google_search.py @@ -0,0 +1,354 @@ +# Copyright (c) Microsoft. All rights reserved. + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from httpx import HTTPStatusError, RequestError +from pydantic import ValidationError + +from semantic_kernel.connectors.search.google.google_search import GoogleSearch +from semantic_kernel.connectors.search.google.google_search_response import ( + GoogleSearchInformation, + GoogleSearchResponse, +) +from semantic_kernel.connectors.search.google.google_search_result import GoogleSearchResult +from semantic_kernel.connectors.search.google.google_search_settings import GoogleSearchSettings +from semantic_kernel.data.text_search.text_search_filter import TextSearchFilter +from semantic_kernel.data.text_search.text_search_options import TextSearchOptions +from semantic_kernel.data.text_search.text_search_result import TextSearchResult +from semantic_kernel.exceptions import ServiceInitializationError, ServiceInvalidRequestError + + +async def test_google_search_init_success() -> None: + """Test that GoogleSearch is initialized successfully when settings are valid.""" + with patch.object( + GoogleSearchSettings, "create", return_value=MagicMock(spec=GoogleSearchSettings) + ) as mock_settings_create: + # Arrange + api_key = "fake_api_key" + search_engine_id = "fake_engine_id" + + # Act + google_search = GoogleSearch(api_key=api_key, search_engine_id=search_engine_id) + + # Assert + mock_settings_create.assert_called_once_with( + api_key=api_key, + engine_id=search_engine_id, + env_file_path=None, + env_file_encoding=None, + ) + assert google_search.settings is not None + + +async def test_google_search_init_validation_error() -> None: + """Test that ServiceInitializationError is raised if settings creation fails validation.""" + with patch.object(GoogleSearchSettings, "create", side_effect=ValidationError("error", [])): + # Arrange & Act & Assert + with pytest.raises(ServiceInitializationError) as exc: + GoogleSearch(api_key="invalid", search_engine_id="invalid") + assert "Failed to create Google settings." in str(exc.value) + + +async def test_google_search_search_calls_inner_search() -> None: + """Test the search method calls _inner_search with the correct arguments.""" + # Arrange + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + mock_response = MagicMock() + with patch.object(google_search, "_inner_search", return_value=mock_response) as mock_inner: + # Act + await google_search.search(query="test query", options=None) + + # Assert + mock_inner.assert_called_once() + + +async def test_google_search_get_text_search_results_calls_inner_search() -> None: + """Test the get_text_search_results method calls _inner_search with the correct arguments.""" + # Arrange + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + mock_response = MagicMock() + with patch.object(google_search, "_inner_search", return_value=mock_response) as mock_inner: + # Act + await google_search.get_text_search_results(query="test query", options=None) + + # Assert + mock_inner.assert_called_once() + + +async def test_google_search_get_search_results_calls_inner_search() -> None: + """Test the get_search_results method calls _inner_search with the correct arguments.""" + # Arrange + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + mock_response = MagicMock() + with patch.object(google_search, "_inner_search", return_value=mock_response) as mock_inner: + # Act + await google_search.get_search_results(query="test query", options=None) + + # Assert + mock_inner.assert_called_once() + + +async def test__validate_options_raises_error_if_top_too_high() -> None: + """Test that _validate_options raises ServiceInvalidRequestError if top results > 10.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + options = TextSearchOptions(top=11, skip=0) + with pytest.raises(ServiceInvalidRequestError) as exc: + google_search._validate_options(options) + assert "count value must be less than or equal to 10." in str(exc.value) + + +async def test__validate_options_does_not_raise_if_top_ok() -> None: + """Test that _validate_options does not raise if top <= 10.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + options = TextSearchOptions(top=10, skip=0) + # Should not raise + google_search._validate_options(options) + + +async def test__get_options_returns_existing_text_search_options() -> None: + """Test _get_options returns the provided TextSearchOptions if already of correct type.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + original_options = TextSearchOptions(top=5, skip=2) + returned_options = google_search._get_options(original_options) + assert returned_options == original_options + + +async def test__get_options_creates_new_text_search_options_if_none() -> None: + """Test _get_options creates new TextSearchOptions if none is provided.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + returned_options = google_search._get_options(None) + assert isinstance(returned_options, TextSearchOptions) + + +async def test__get_options_creates_new_options_if_wrong_type_in_kwargs() -> None: + """Test _get_options handles invalid kwargs gracefully and returns a default TextSearchOptions.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + returned_options = google_search._get_options(None, top="invalid") + # should still return a default object + assert isinstance(returned_options, TextSearchOptions) + + +async def test__inner_search_success_returns_parsed_response() -> None: + """Test _inner_search returns the parsed JSON response on success.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + + mock_response = GoogleSearchResponse() + + # mock an OK response from the client + mock_ok_response = MagicMock() + mock_ok_response.raise_for_status = MagicMock() + mock_ok_response.text = "fake_json_text" + + async_client_mock = AsyncMock() + async_client_mock.get.return_value = mock_ok_response + + with ( + patch.object(google_search, "_validate_options") as mock_validate, + patch( + "semantic_kernel.connectors.search.google.google_search.AsyncClient.__aenter__", + return_value=async_client_mock, + ), + patch( + "semantic_kernel.connectors.search.google.google_search_response.GoogleSearchResponse.model_validate_json", + return_value=mock_response, + ) as mock_model_validate_json, + ): + result = await google_search._inner_search("test query", TextSearchOptions()) + + mock_validate.assert_called_once() + # ensure the mock_model_validate_json method was called + mock_model_validate_json.assert_called_once_with("fake_json_text") + assert result == mock_response + + +async def test__inner_search_http_status_error_raises_service_invalid_request_error() -> None: + """Test _inner_search raises ServiceInvalidRequestError if an HTTPStatusError occurs.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + + with patch.object(google_search, "_validate_options"), patch("httpx.AsyncClient") as mock_client: + instance = mock_client.return_value.__aenter__.return_value + # Mock a failing get call that raises HTTPStatusError + instance.get.side_effect = HTTPStatusError("Error", request=MagicMock(), response=MagicMock()) + + with pytest.raises(ServiceInvalidRequestError) as exc: + await google_search._inner_search("test query", TextSearchOptions()) + assert "Failed to get search results." in str(exc.value) + + +async def test__inner_search_request_error_raises_service_invalid_request_error() -> None: + """Test _inner_search raises ServiceInvalidRequestError if a client-side RequestError occurs.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + + async_client_mock = AsyncMock() + # Mock a failing get call that raises RequestError + async_client_mock.get.side_effect = RequestError("Client error") + + with ( + patch.object(google_search, "_validate_options"), + patch( + "semantic_kernel.connectors.search.google.google_search.AsyncClient.__aenter__", + return_value=async_client_mock, + ), + ): + with pytest.raises(ServiceInvalidRequestError) as exc: + await google_search._inner_search("test query", TextSearchOptions()) + assert "A client error occurred while getting search results." in str(exc.value) + + +async def test__inner_search_unexpected_error_raises_service_invalid_request_error() -> None: + """Test _inner_search raises ServiceInvalidRequestError if an unexpected error occurs.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + + async_client_mock = AsyncMock() + # Trigger a generic exception + async_client_mock.get.side_effect = Exception("Unexpected error") + + with ( + patch.object(google_search, "_validate_options"), + patch( + "semantic_kernel.connectors.search.google.google_search.AsyncClient.__aenter__", + return_value=async_client_mock, + ), + ): + with pytest.raises(ServiceInvalidRequestError) as exc: + await google_search._inner_search("test query", TextSearchOptions()) + assert "An unexpected error occurred while getting search results." in str(exc.value) + + +async def test__build_query_no_filter() -> None: + """Test _build_query constructs the expected Google Search URL part with no filter clauses.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + options = TextSearchOptions(top=5, skip=2) + query_str = google_search._build_query("hello world", options) + # We expect it to produce something like ?q=hello+world&key=fake_api_key&cx=fake_engine_id&num=5&start=2 + assert "hello+world" in query_str + assert "key=fake_api_key" in query_str + assert "cx=fake_engine_id" in query_str + assert "num=5" in query_str + assert "start=2" in query_str + + +async def test__build_query_equal_to_filter() -> None: + """Test _build_query includes EqualTo filter parameters if matching the known query parameters.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + options = TextSearchOptions(top=3, skip=0) + options.filter = TextSearchFilter.equal_to(field_name="lr", value="lang_en") + + query_str = google_search._build_query("test", options) + assert "lr=lang_en" in query_str + + +@pytest.mark.asyncio +async def test__build_query_any_tags_equal_to_unsupported_filter() -> None: + """Test _build_query logs debug message when AnyTagsEqualTo filter is present but doesn't break query.""" + from semantic_kernel.data.filter_clauses.any_tags_equal_to_filter_clause import AnyTagsEqualTo + + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + options = TextSearchOptions(top=3, skip=0) + options.filter.filters.append(AnyTagsEqualTo(field_name="tags", value="some_tag")) + + query_str = google_search._build_query("test", options) + assert "q=test" in query_str + assert "cx=fake_engine_id" in query_str + + +async def test__get_result_strings_no_items() -> None: + """Test _get_result_strings yields nothing if response contains no items.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + response = GoogleSearchResponse(items=None) + results = [r async for r in google_search._get_result_strings(response)] + assert results == [] + + +async def test__get_result_strings_with_items() -> None: + """Test _get_result_strings yields the snippet of each item.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + item1 = GoogleSearchResult(snippet="snippet1") + item2 = GoogleSearchResult(snippet="snippet2") + response = GoogleSearchResponse(items=[item1, item2]) + + results = [r async for r in google_search._get_result_strings(response)] + assert results == ["snippet1", "snippet2"] + + +async def test__get_text_search_results_no_items() -> None: + """Test _get_text_search_results yields nothing if response contains no items.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + response = GoogleSearchResponse(items=None) + results = [r async for r in google_search._get_text_search_results(response)] + assert results == [] + + +async def test__get_text_search_results_with_items() -> None: + """Test _get_text_search_results yields TextSearchResult objects for each item.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + item = GoogleSearchResult(title="test title", snippet="test snippet", link="test") + response = GoogleSearchResponse(items=[item]) + + results = [r async for r in google_search._get_text_search_results(response)] + assert len(results) == 1 + assert isinstance(results[0], TextSearchResult) + assert results[0].name == "test title" + assert results[0].value == "test snippet" + assert results[0].link == "test" + + +async def test__get_google_search_results_no_items() -> None: + """Test _get_google_search_results yields nothing if response contains no items.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + response = GoogleSearchResponse(items=None) + results = [r async for r in google_search._get_google_search_results(response)] + assert results == [] + + +async def test__get_google_search_results_with_items() -> None: + """Test _get_google_search_results yields each GoogleSearchResult in the response items.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + item1 = GoogleSearchResult() + item2 = GoogleSearchResult() + response = GoogleSearchResponse(items=[item1, item2]) + + results = [r async for r in google_search._get_google_search_results(response)] + assert len(results) == 2 + assert results[0] is item1 + assert results[1] is item2 + + +async def test__get_total_count_included() -> None: + """Test _get_total_count returns total if include_total_count is True.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + response = GoogleSearchResponse() + response.search_information = MagicMock(spec=GoogleSearchInformation) + response.search_information.total_results = 42 + + options = TextSearchOptions() + options.include_total_count = True + + total = google_search._get_total_count(response, options) + assert total == 42 + + +async def test__get_total_count_excluded() -> None: + """Test _get_total_count returns None if include_total_count is False.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + response = GoogleSearchResponse() + response.search_information = MagicMock(spec=GoogleSearchInformation) + response.search_information.total_results = 42 + + options = TextSearchOptions() + options.include_total_count = False + + total = google_search._get_total_count(response, options) + assert total is None + + +async def test__get_metadata_includes_search_time() -> None: + """Test _get_metadata includes search_time if present.""" + google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") + response = GoogleSearchResponse() + response.search_information = MagicMock(spec=GoogleSearchInformation) + response.search_information.search_time = 1.234 + + metadata = google_search._get_metadata(response) + assert metadata["search_time"] == 1.234 From 41ec18ad22028d30a41c6959330ca43e2c69e3f5 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Tue, 25 Feb 2025 15:48:55 -0300 Subject: [PATCH 06/18] import fix --- .../unit/agents/bedrock_agent/test_bedrock_agent_channel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py index bdc60da9cf23..d95200172e2d 100644 --- a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py +++ b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py @@ -1,6 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. -from typing import AsyncIterable +from collections.abc import AsyncIterable from unittest.mock import MagicMock, patch import pytest From a1cde8d005d8e29cc0cd587440ae4e266d869662 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 10:23:05 -0300 Subject: [PATCH 07/18] removing @pytest.mark.asyncio decorators --- .../ai/bedrock/services/test_bedrock_model_provider_utils.py | 1 - python/tests/unit/connectors/search/google/test_google_search.py | 1 - 2 files changed, 2 deletions(-) diff --git a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py index 5ca6370ea7af..b7820dec5c05 100644 --- a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py +++ b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py @@ -167,7 +167,6 @@ def test_inference_profile_with_bedrock_model() -> None: BedrockModelProvider.to_model_provider(unknown_inference_profile) -@pytest.mark.asyncio async def test_run_in_executor_calls_executor_properly() -> None: """Test that run_in_executor properly calls an executor and function.""" mock_executor = MagicMock() diff --git a/python/tests/unit/connectors/search/google/test_google_search.py b/python/tests/unit/connectors/search/google/test_google_search.py index 51c84c7e6cd7..01bf2ed49267 100644 --- a/python/tests/unit/connectors/search/google/test_google_search.py +++ b/python/tests/unit/connectors/search/google/test_google_search.py @@ -239,7 +239,6 @@ async def test__build_query_equal_to_filter() -> None: assert "lr=lang_en" in query_str -@pytest.mark.asyncio async def test__build_query_any_tags_equal_to_unsupported_filter() -> None: """Test _build_query logs debug message when AnyTagsEqualTo filter is present but doesn't break query.""" from semantic_kernel.data.filter_clauses.any_tags_equal_to_filter_clause import AnyTagsEqualTo From 4586bbe02494dc1a3d8cd68dd7b87b9843cd1920 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 10:24:02 -0300 Subject: [PATCH 08/18] addressing review comments on test bedrock agent channel --- .../test_bedrock_agent_channel.py | 227 ++++-------------- 1 file changed, 51 insertions(+), 176 deletions(-) diff --git a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py index d95200172e2d..8880f7b0df12 100644 --- a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py +++ b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py @@ -1,7 +1,7 @@ # Copyright (c) Microsoft. All rights reserved. from collections.abc import AsyncIterable -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock import pytest @@ -42,9 +42,9 @@ def chat_history_not_alternate_role() -> list[ChatMessageContent]: @pytest.fixture -def channel_and_agent(): +def mock_agent(): """ - Fixture that creates a BedrockAgentChannel instance and a mock BedrockAgent. + Fixture that creates a mock BedrockAgent. """ from semantic_kernel.agents.bedrock.bedrock_agent import BedrockAgent @@ -55,14 +55,7 @@ def channel_and_agent(): mock_agent.agent_model = MagicMock(spec=BedrockAgentModel) mock_agent.agent_model.foundation_model = "mock-foundation-model" - # Mock create_session_id to return a fixed session id - mock_agent.create_session_id.return_value = "test-session-id" - - # Initialize the BedrockAgentChannel with no messages initially - channel = BedrockAgentChannel() - - # Return both - return channel, mock_agent + return mock_agent async def test_receive_message(mock_channel, chat_history): @@ -92,14 +85,14 @@ async def test_channel_reset(mock_channel, chat_history): # Test to verify the reset functionality await mock_channel.receive(chat_history) assert len(mock_channel) == len(chat_history) + assert len(mock_channel.messages) == len(chat_history) await mock_channel.reset() assert len(mock_channel) == 0 + assert len(mock_channel.messages) == 0 -async def test_receive_appends_history_correctly(channel_and_agent): +async def test_receive_appends_history_correctly(mock_channel): """Test that the receive method appends messages while ensuring they alternate in author role.""" - channel, _ = channel_and_agent - # Provide a list of messages with identical roles to see if placeholders are inserted incoming_messages = [ ChatMessageContent(role=AuthorRole.USER, content="User message 1"), @@ -108,7 +101,7 @@ async def test_receive_appends_history_correctly(channel_and_agent): ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant message 2"), ] - await channel.receive(incoming_messages) + await mock_channel.receive(incoming_messages) # The final channel.messages should be: # user message 1, user placeholder, user message 2, assistant placeholder, assistant message 1, @@ -130,66 +123,55 @@ async def test_receive_appends_history_correctly(channel_and_agent): "Assistant message 2", ] - a = channel.messages - - print(a) - - assert len(channel.messages) == len(expected_roles) - for i, (msg, exp_role, exp_content) in enumerate(zip(channel.messages, expected_roles, expected_contents)): + assert len(mock_channel.messages) == len(expected_roles) + for i, (msg, exp_role, exp_content) in enumerate(zip(mock_channel.messages, expected_roles, expected_contents)): assert msg.role == exp_role, f"Role mismatch at index {i}" assert msg.content == exp_content, f"Content mismatch at index {i}" -async def test_invoke_raises_exception_for_non_bedrock_agent(channel_and_agent): +async def test_invoke_raises_exception_for_non_bedrock_agent(mock_channel): """Test invoke method raises AgentChatException if the agent provided is not a BedrockAgent.""" - channel, _ = channel_and_agent - # Place a message in the channel so it's not empty - channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) + mock_channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) # Create a dummy agent that is not BedrockAgent non_bedrock_agent = Agent() with pytest.raises(AgentChatException) as exc_info: - _ = [msg async for msg in channel.invoke(non_bedrock_agent)] + _ = [msg async for msg in mock_channel.invoke(non_bedrock_agent)] assert "Agent is not of the expected type" in str(exc_info.value) -async def test_invoke_raises_exception_if_no_history(channel_and_agent): +async def test_invoke_raises_exception_if_no_history(mock_channel, mock_agent): """Test invoke method raises AgentChatException if no chat history is available.""" - channel, agent = channel_and_agent - with pytest.raises(AgentChatException) as exc_info: - _ = [msg async for msg in channel.invoke(agent)] + _ = [msg async for msg in mock_channel.invoke(mock_agent)] assert "No chat history available" in str(exc_info.value) -@patch("semantic_kernel.agents.bedrock.bedrock_agent.BedrockAgent.create_session_id", return_value="session-id") -async def test_invoke_inserts_placeholders_when_history_needs_to_alternate(mock_session, channel_and_agent): +async def test_invoke_inserts_placeholders_when_history_needs_to_alternate(mock_channel, mock_agent): """Test invoke ensures _ensure_history_alternates and _ensure_last_message_is_user are called.""" - channel, agent = channel_and_agent - # Put messages in the channel such that the last message is an assistant's - channel.messages.append(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant 1")) + mock_channel.messages.append(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant 1")) # Mock agent.invoke to return an async generator async def mock_invoke(session_id: str, input_text: str, sessionState=None, **kwargs): # We just yield one message as if the agent responded yield ChatMessageContent(role=AuthorRole.ASSISTANT, content="Mock Agent Response") - agent.invoke = mock_invoke + mock_agent.invoke = mock_invoke # Because the last message is from the assistant, we expect a placeholder user message to be appended # also the history might need to alternate. # But since there's only one message, there's nothing to fix except the last message is user. # We will now add a user message so we do not get the "No chat history available" error - channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User 1")) + mock_channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User 1")) # Now we do invoke - outputs = [msg async for msg in channel.invoke(agent)] + outputs = [msg async for msg in mock_channel.invoke(mock_agent)] # We'll check if the response is appended to channel.messages assert len(outputs) == 1 @@ -198,40 +180,35 @@ async def mock_invoke(session_id: str, input_text: str, sessionState=None, **kwa assert agent_response.content == "Mock Agent Response" # The channel messages should now have 3 messages: the assistant, the user, and the new agent message - assert len(channel.messages) == 3 - assert channel.messages[-1].role == AuthorRole.ASSISTANT - assert channel.messages[-1].content == "Mock Agent Response" + assert len(mock_channel.messages) == 3 + assert mock_channel.messages[-1].role == AuthorRole.ASSISTANT + assert mock_channel.messages[-1].content == "Mock Agent Response" -async def test_invoke_stream_raises_error_for_non_bedrock_agent(channel_and_agent): +async def test_invoke_stream_raises_error_for_non_bedrock_agent(mock_channel): """Test invoke_stream raises AgentChatException if the agent provided is not a BedrockAgent.""" - channel, _ = channel_and_agent - channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) + mock_channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) non_bedrock_agent = Agent() with pytest.raises(AgentChatException) as exc_info: - _ = [msg async for msg in channel.invoke_stream(non_bedrock_agent, [])] + _ = [msg async for msg in mock_channel.invoke_stream(non_bedrock_agent, [])] assert "Agent is not of the expected type" in str(exc_info.value) -async def test_invoke_stream_raises_no_chat_history(channel_and_agent): +async def test_invoke_stream_raises_no_chat_history(mock_channel, mock_agent): """Test invoke_stream raises AgentChatException if no messages in the channel.""" - channel, agent = channel_and_agent with pytest.raises(AgentChatException) as exc_info: - _ = [msg async for msg in channel.invoke_stream(agent, [])] + _ = [msg async for msg in mock_channel.invoke_stream(mock_agent, [])] assert "No chat history available." in str(exc_info.value) -@patch("semantic_kernel.agents.bedrock.bedrock_agent.BedrockAgent.create_session_id", return_value="session-id") -async def test_invoke_stream_appends_response_message(mock_session, channel_and_agent): +async def test_invoke_stream_appends_response_message(mock_channel, mock_agent): """Test invoke_stream properly yields streaming content and appends an aggregated message at the end.""" - channel, agent = channel_and_agent - # Put a user message in the channel so it won't raise No chat history - channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="Last user message")) + mock_channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="Last user message")) async def mock_invoke_stream( session_id: str, input_text: str, sessionState=None, **kwargs @@ -247,11 +224,11 @@ async def mock_invoke_stream( content=" World", ) - agent.invoke_stream = mock_invoke_stream + mock_agent.invoke_stream = mock_invoke_stream # Check that we get the streamed messages and that the summarized message is appended afterward messages_param = [ChatMessageContent(role=AuthorRole.USER, content="Last user message")] # just to pass the param - streamed_content = [msg async for msg in channel.invoke_stream(agent, messages_param)] + streamed_content = [msg async for msg in mock_channel.invoke_stream(mock_agent, messages_param)] # We expect two streamed chunks: "Hello" and " World" assert len(streamed_content) == 2 @@ -265,98 +242,33 @@ async def mock_invoke_stream( assert appended.content == "Hello World" -async def test_get_history(channel_and_agent): +async def test_get_history(mock_channel, chat_history): """Test get_history yields messages in reverse order.""" - channel, _ = channel_and_agent - - channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User1")) - channel.messages.append(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant1")) - channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User2")) + mock_channel.messages = chat_history - reversed_history = [msg async for msg in channel.get_history()] + reversed_history = [msg async for msg in mock_channel.get_history()] # Should be reversed - assert reversed_history[0].content == "User2" - assert reversed_history[1].content == "Assistant1" - assert reversed_history[2].content == "User1" - - -async def test_reset(channel_and_agent): - """Test that reset clears all messages from the channel.""" - channel, _ = channel_and_agent - - channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User1")) - channel.messages.append(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant1")) + assert reversed_history[0].content == "I'm good, thank you!" + assert reversed_history[1].content == "How are you?" + assert reversed_history[2].content == "Hello, User!" + assert reversed_history[3].content == "Hello, Bedrock!" - assert len(channel.messages) == 2 - await channel.reset() - - assert len(channel.messages) == 0 - - -async def test_ensure_history_alternates_handles_multiple_consecutive_roles(): - """Direct test for _ensure_history_alternates by simulating various consecutive roles.""" - channel = BedrockAgentChannel() - - channel.messages = [ +async def test_invoke_alternates_history_and_ensures_last_user_message(mock_channel, mock_agent): + """Test invoke method ensures history alternates and last message is user.""" + mock_channel.messages = [ ChatMessageContent(role=AuthorRole.USER, content="User1"), ChatMessageContent(role=AuthorRole.USER, content="User2"), ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist1"), ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist2"), ChatMessageContent(role=AuthorRole.USER, content="User3"), ChatMessageContent(role=AuthorRole.USER, content="User4"), + ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist3"), ] - channel._ensure_history_alternates() - - # Expect placeholders inserted: - # User1, placeholder assistant, User2, Assistant(Assist1), placeholder user, Assistant(Assist2), - # placeholder assistant, User3, placeholder assistant, User4 - - # Let's verify the roles: - expected_roles = [ - AuthorRole.USER, - AuthorRole.ASSISTANT, # placeholder for consecutive user - AuthorRole.USER, - AuthorRole.ASSISTANT, - AuthorRole.USER, # placeholder for consecutive assistant - AuthorRole.ASSISTANT, - AuthorRole.ASSISTANT, # placeholder for consecutive user (Wait carefully: we had Assist1 -> Assist2? We'll see) - AuthorRole.USER, - AuthorRole.ASSISTANT, # placeholder after we see user -> user - AuthorRole.USER, - ] - - # Actually let's systematically figure out the inserted placeholders. - # The original had: Index 0: U1, Index 1: U2 -> consecutive user, so after index 1 we insert placeholder assistant. - # Then index 2: Assist1. Now index 3: Assist2 -> consecutive assistant, so insert placeholder user. - # Then index 4: U3, index 5: U4 -> consecutive user, insert placeholder assistant. - - # Final roles: - # [U1, placeholder(A), U2, A, placeholder(U), A, U3, placeholder(A), U4] - # Wait, let's carefully track insertion: - # messages before insertion: - # 0: U1 - # 1: U2 - # 2: A1 - # 3: A2 - # 4: U3 - # 5: U4 - # We'll walk with current_index starting at 1: - # 1: role=U2 same as role=U1 => insert placeholder A at index=1 => increment index=3 - # now 2-> new index=3 => role= A2 same as A1 => insert placeholder user at index=3 => increment index=5 - # now 4-> new index=5 => role=U4 same as U3 => insert placeholder assistant => increment index=7 => done - # So final messages: - # index 0: U1 - # index 1: placeholder(A) - # index 2: U2 - # index 3: A1 - # index 4: placeholder(U) - # index 5: A2 - # index 6: U3 - # index 7: placeholder(A) - # index 8: U4 + async for _, msg in mock_channel.invoke(mock_agent): + pass # let's define expected roles from that final structure: expected_roles = [ @@ -369,6 +281,8 @@ async def test_ensure_history_alternates_handles_multiple_consecutive_roles(): AuthorRole.USER, AuthorRole.ASSISTANT, # placeholder AuthorRole.USER, + AuthorRole.ASSISTANT, + AuthorRole.USER, # placeholder ] expected_contents = [ "User1", @@ -380,50 +294,11 @@ async def test_ensure_history_alternates_handles_multiple_consecutive_roles(): "User3", BedrockAgentChannel.MESSAGE_PLACEHOLDER, "User4", + "Assist3", + BedrockAgentChannel.MESSAGE_PLACEHOLDER, ] - assert len(channel.messages) == len(expected_roles) - for i, (msg, exp_role, exp_content) in enumerate(zip(channel.messages, expected_roles, expected_contents)): + assert len(mock_channel.messages) == len(expected_roles) + for i, (msg, exp_role, exp_content) in enumerate(zip(mock_channel.messages, expected_roles, expected_contents)): assert msg.role == exp_role, f"Role mismatch at index {i}. Got {msg.role}, expected {exp_role}" assert msg.content == exp_content, f"Content mismatch at index {i}. Got {msg.content}, expected {exp_content}" - - -async def test_ensure_last_message_is_user_appends_placeholder_if_last_assistant(): - """Test that _ensure_last_message_is_user appends a user placeholder if the last message is an assistant one.""" - channel = BedrockAgentChannel() - - channel.messages = [ - ChatMessageContent(role=AuthorRole.USER, content="User1"), - ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist1"), - ] - - channel._ensure_last_message_is_user() - - assert len(channel.messages) == 3 - last_msg = channel.messages[-1] - assert last_msg.role == AuthorRole.USER - assert last_msg.content == BedrockAgentChannel.MESSAGE_PLACEHOLDER - - -async def test_parse_chat_history_to_session_state_returns_valid_structure(): - """Test that _parse_chat_history_to_session_state() returns expected structure ignoring the last message.""" - channel = BedrockAgentChannel() - - channel.messages = [ - ChatMessageContent(role=AuthorRole.USER, content="User1"), - ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assist1"), - ChatMessageContent(role=AuthorRole.USER, content="User2"), - ] - - session_state = channel._parse_chat_history_to_session_state() - - # The last message is not taken, so we only see the first two messages - assert "conversationHistory" in session_state - assert "messages" in session_state["conversationHistory"] - - msg_list = session_state["conversationHistory"]["messages"] - assert len(msg_list) == 2 - assert msg_list[0]["role"] == "user" - assert msg_list[0]["content"] == [{"text": "User1"}] - assert msg_list[1]["role"] == "assistant" - assert msg_list[1]["content"] == [{"text": "Assist1"}] From ff76fe481e21eef69cbfc6a605e8abe91e7499ad Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 10:47:46 -0300 Subject: [PATCH 09/18] testing message formatters through MESSAGE_CONVERTERS in bedrock model provider utils --- .../test_bedrock_model_provider_utils.py | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py index b7820dec5c05..f08153a5ff79 100644 --- a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py +++ b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py @@ -9,10 +9,7 @@ BedrockModelProvider, ) from semantic_kernel.connectors.ai.bedrock.services.model_provider.utils import ( - _format_assistant_message, - _format_system_message, - _format_tool_message, - _format_user_message, + MESSAGE_CONVERTERS, finish_reason_from_bedrock_to_semantic_kernel, format_bedrock_function_name_to_kernel_function_fully_qualified_name, remove_none_recursively, @@ -216,7 +213,7 @@ def test_remove_none_recursively_max_depth() -> None: def test_format_system_message() -> None: """Test that system message is formatted correctly.""" content = ChatMessageContent(role=AuthorRole.SYSTEM, content="System message") - formatted = _format_system_message(content) + formatted = MESSAGE_CONVERTERS[AuthorRole.SYSTEM](content) assert formatted == {"text": "System message"} @@ -225,7 +222,7 @@ def test_format_user_message_text_only() -> None: text_item = TextContent(text="Hello!") user_message = ChatMessageContent(role=AuthorRole.USER, items=[text_item]) - formatted = _format_user_message(user_message) + formatted = MESSAGE_CONVERTERS[AuthorRole.USER](user_message) assert formatted["role"] == "user" assert len(formatted["content"]) == 1 assert formatted["content"][0] == {"text": "Hello!"} @@ -236,7 +233,7 @@ def test_format_user_message_image_only() -> None: img_item = ImageContent(data=b"abc", mime_type="image/png") user_message = ChatMessageContent(role=AuthorRole.USER, items=[img_item]) - formatted = _format_user_message(user_message) + formatted = MESSAGE_CONVERTERS[AuthorRole.USER](user_message) assert formatted["role"] == "user" assert len(formatted["content"]) == 1 image_section = formatted["content"][0].get("image") @@ -251,7 +248,7 @@ def test_format_user_message_unsupported_content() -> None: user_message = ChatMessageContent(role=AuthorRole.USER, items=[func_call_item]) with pytest.raises(ServiceInvalidRequestError) as exc: - _format_user_message(user_message) + MESSAGE_CONVERTERS[AuthorRole.USER](user_message) assert "Only text and image content are supported in a user message." in str(exc.value) @@ -261,7 +258,7 @@ def test_format_assistant_message_text_content() -> None: text_item = TextContent(text="Assistant response") assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[text_item]) - formatted = _format_assistant_message(assistant_message) + formatted = MESSAGE_CONVERTERS[AuthorRole.ASSISTANT](assistant_message) assert formatted["role"] == "assistant" assert formatted["content"] == [{"text": "Assistant response"}] @@ -273,7 +270,7 @@ def test_format_assistant_message_function_call_content() -> None: ) assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[func_item]) - formatted = _format_assistant_message(assistant_message) + formatted = MESSAGE_CONVERTERS[AuthorRole.ASSISTANT](assistant_message) assert len(formatted["content"]) == 1 tool_use = formatted["content"][0].get("toolUse") assert tool_use @@ -288,7 +285,7 @@ def test_format_assistant_message_image_content_raises() -> None: assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[img_item]) with pytest.raises(ServiceInvalidRequestError) as exc: - _format_assistant_message(assistant_message) + MESSAGE_CONVERTERS[AuthorRole.ASSISTANT](assistant_message) assert "Image content is not supported in an assistant message." in str(exc.value) @@ -299,7 +296,7 @@ def test_format_assistant_message_unsupported_type() -> None: assistant_message = ChatMessageContent(role=AuthorRole.ASSISTANT, items=[func_res_item]) with pytest.raises(ServiceInvalidRequestError) as exc: - _format_assistant_message(assistant_message) + MESSAGE_CONVERTERS[AuthorRole.ASSISTANT](assistant_message) assert "Unsupported content type in an assistant message:" in str(exc.value) @@ -308,7 +305,7 @@ def test_format_tool_message_text() -> None: text_item = TextContent(text="Some text") tool_message = ChatMessageContent(role=AuthorRole.TOOL, items=[text_item]) - formatted = _format_tool_message(tool_message) + formatted = MESSAGE_CONVERTERS[AuthorRole.TOOL](tool_message) assert formatted["role"] == "user" # note that for a tool message, role set to 'user' assert formatted["content"] == [{"text": "Some text"}] @@ -318,7 +315,7 @@ def test_format_tool_message_function_result() -> None: func_result_item = FunctionResultContent(id="res_id", function_name="test_function", result="some result") tool_message = ChatMessageContent(role=AuthorRole.TOOL, items=[func_result_item]) - formatted = _format_tool_message(tool_message) + formatted = MESSAGE_CONVERTERS[AuthorRole.TOOL](tool_message) assert formatted["role"] == "user" content = formatted["content"][0] assert content.get("toolResult") @@ -332,7 +329,7 @@ def test_format_tool_message_image_raises() -> None: tool_message = ChatMessageContent(role=AuthorRole.TOOL, items=[img_item]) with pytest.raises(ServiceInvalidRequestError) as exc: - _format_tool_message(tool_message) + MESSAGE_CONVERTERS[AuthorRole.TOOL](tool_message) assert "Image content is not supported in a tool message." in str(exc.value) From 243e62b64675504bac9f0a8e17a156c961fac2b3 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 11:16:18 -0300 Subject: [PATCH 10/18] updating mistralai chat completion tests to address comments --- .../test_mistralai_chat_completion.py | 157 +++--------------- 1 file changed, 23 insertions(+), 134 deletions(-) diff --git a/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py b/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py index 1a8fa18bf40a..2d255627e0fc 100644 --- a/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py +++ b/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py @@ -3,7 +3,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from mistralai import CompletionEvent, FunctionCall, Mistral +from mistralai import CompletionEvent, Mistral from mistralai.models import ( AssistantMessage, ChatCompletionChoice, @@ -11,7 +11,6 @@ CompletionChunk, CompletionResponseStreamChoice, DeltaMessage, - ToolCall, UsageInfo, ) @@ -22,11 +21,10 @@ MistralAIChatPromptExecutionSettings, ) from semantic_kernel.connectors.ai.mistral_ai.services.mistral_ai_chat_completion import MistralAIChatCompletion -from semantic_kernel.connectors.ai.mistral_ai.settings.mistral_ai_settings import MistralAISettings from semantic_kernel.connectors.ai.open_ai.prompt_execution_settings.open_ai_prompt_execution_settings import ( OpenAIChatPromptExecutionSettings, ) -from semantic_kernel.contents import FunctionCallContent, StreamingTextContent, TextContent +from semantic_kernel.contents import FunctionCallContent, TextContent from semantic_kernel.contents.chat_history import ChatHistory from semantic_kernel.contents.chat_message_content import ( ChatMessageContent, @@ -414,35 +412,23 @@ async def test_mistral_ai_chat_completion_initialization_valid(): async def test_mistral_ai_chat_completion_initialization_missing_model_id(): """Test MistralAIChatCompletion initialization should raise ServiceInitializationError when model id is missing.""" - with patch("mistralai.Mistral", return_value=MagicMock()): - with pytest.raises(ServiceInitializationError) as exc_info: - MistralAIChatCompletion( - ai_model_id=None, - api_key="some-key", - ) - assert "The MistralAI chat model ID is required." in str(exc_info.value) + with pytest.raises(ServiceInitializationError) as exc_info: + MistralAIChatCompletion( + ai_model_id=None, + api_key="some-key", + ) + assert "The MistralAI chat model ID is required." in str(exc_info.value) async def test_mistral_ai_chat_completion_initialization_validation_error(): """Test MistralAIChatCompletion initialization when MistralAISettings creation fails and raises ValidationError.""" - # About ValidationError, we can patch the MistralAISettings create method to raise it. - from pydantic import ValidationError - - with patch.object( - MistralAISettings, - "create", - side_effect=ValidationError("Validation error", []), - ): - with pytest.raises(ServiceInitializationError) as exc_info: - MistralAIChatCompletion( - ai_model_id="test", - api_key="test", - ) - assert "Failed to create MistralAI settings." in str(exc_info.value) + with pytest.raises(ServiceInitializationError) as exc_info: + MistralAIChatCompletion(env_file_path="fake_env_file_path.env") + assert "Failed to create MistralAI settings." in str(exc_info.value) -async def test_mistral_ai_chat_completion_inner_get_chat_message_contents_success(): - """Test _inner_get_chat_message_contents with a successful ChatCompletionResponse.""" +async def test_mistral_ai_chat_completion_get_chat_message_contents_success(): + """Test get_chat_message_contents with a successful ChatCompletionResponse.""" # Mock the response from the Mistral chat complete_async. mock_response = ChatCompletionResponse( @@ -474,7 +460,7 @@ async def test_mistral_ai_chat_completion_inner_get_chat_message_contents_succes chat_history = ChatHistory() settings = MistralAIChatPromptExecutionSettings() - results = await chat_completion._inner_get_chat_message_contents(chat_history, settings) + results = await chat_completion.get_chat_message_contents(chat_history, settings) # We should have exactly one ChatMessageContent. assert len(results) == 1 @@ -485,8 +471,8 @@ async def test_mistral_ai_chat_completion_inner_get_chat_message_contents_succes async_mock_client.chat.complete_async.assert_awaited_once() -async def test_mistral_ai_chat_completion_inner_get_chat_message_contents_failure(): - """Test _inner_get_chat_message_contents should raise ServiceResponseException if Mistral call fails.""" +async def test_mistral_ai_chat_completion_get_chat_message_contents_failure(): + """Test get_chat_message_contents should raise ServiceResponseException if Mistral call fails.""" async_mock_client = MagicMock(spec=Mistral) async_mock_client.chat = MagicMock() async_mock_client.chat.complete_async = AsyncMock(side_effect=Exception("API error")) @@ -501,12 +487,12 @@ async def test_mistral_ai_chat_completion_inner_get_chat_message_contents_failur settings = MistralAIChatPromptExecutionSettings() with pytest.raises(ServiceResponseException) as exc: - await chat_completion._inner_get_chat_message_contents(chat_history, settings) + await chat_completion.get_chat_message_contents(chat_history, settings) assert "service failed to complete the prompt" in str(exc.value) -async def test_mistral_ai_chat_completion_inner_get_streaming_chat_message_contents_success(): - """Test _inner_get_streaming_chat_message_contents when streaming successfully.""" +async def test_mistral_ai_chat_completion_get_streaming_chat_message_contents_success(): + """Test get_streaming_chat_message_contents when streaming successfully.""" # We'll yield multiple chunks to simulate streaming. mock_chunk1 = CompletionEvent( @@ -556,7 +542,7 @@ async def mock_stream_async(**kwargs): settings = MistralAIChatPromptExecutionSettings() collected_chunks = [] - async for chunk_list in chat_completion._inner_get_streaming_chat_message_contents(chat_history, settings): + async for chunk_list in chat_completion.get_streaming_chat_message_contents(chat_history, settings): collected_chunks.append(chunk_list) # We expect two sets of chunk_list yields. @@ -569,8 +555,8 @@ async def mock_stream_async(**kwargs): assert collected_chunks[1][0].items[0].text == "World!" -async def test_mistral_ai_chat_completion_inner_get_streaming_chat_message_contents_failure(): - """Test _inner_get_streaming_chat_message_contents raising a ServiceResponseException on failure.""" +async def test_mistral_ai_chat_completion_get_streaming_chat_message_contents_failure(): + """Test get_streaming_chat_message_contents raising a ServiceResponseException on failure.""" async_mock_client = MagicMock(spec=Mistral) async_mock_client.chat = MagicMock() async_mock_client.chat.stream_async = AsyncMock(side_effect=Exception("Streaming error")) @@ -585,108 +571,11 @@ async def test_mistral_ai_chat_completion_inner_get_streaming_chat_message_conte settings = MistralAIChatPromptExecutionSettings() with pytest.raises(ServiceResponseException) as exc: - async for _ in chat_completion._inner_get_streaming_chat_message_contents(chat_history, settings): + async for _ in chat_completion.get_streaming_chat_message_contents(chat_history, settings): pass assert "service failed to complete the prompt" in str(exc.value) -async def test_mistral_ai_chat_completion_create_chat_message_content_with_tool_calls(): - """Test _create_chat_message_content to ensure it picks up tool calls if present.""" - - # We mock a choice that has message with content and some tool calls. - mock_tool = ToolCall( - id="tool_1", - function=FunctionCall(name="function_name", arguments="function_args"), - ) - mock_message = AssistantMessage( - role="assistant", - content="Hello with tool!", - tool_calls=[mock_tool], - ) - - mock_choice = ChatCompletionChoice( - index=0, - message=mock_message, - finish_reason="stop", - ) - mock_response = ChatCompletionResponse( - id="resp_id", - object="object", - created=999, - usage=UsageInfo(prompt_tokens=10, completion_tokens=20, total_tokens=30), - model="some-model", - choices=[mock_choice], - ) - - async_mock_client = MagicMock(spec=Mistral) - chat_completion = MistralAIChatCompletion( - ai_model_id="test-model", - api_key="test_key", - async_client=async_mock_client, - ) - - # We'll directly call the underlying method. - meta = {"metadata": "test"} - result = chat_completion._create_chat_message_content(mock_response, mock_choice, meta) - - # We expect to see one text item and one function call item. - assert len(result.items) == 2 - assert any(isinstance(i, TextContent) for i in result.items) - assert any(isinstance(i, FunctionCallContent) for i in result.items) - - func_call = [i for i in result.items if isinstance(i, FunctionCallContent)][0] - assert func_call.id == "tool_1" - assert func_call.name == "function_name" - assert func_call.arguments == "function_args" - - -async def test_mistral_ai_chat_completion_create_streaming_chat_message_content_with_tool_calls(): - """Test _create_streaming_chat_message_content to ensure it picks up tool calls if present.""" - - mock_tool = ToolCall( - id="tool_2", - function=FunctionCall(name="function_name_2", arguments="function_args_2"), - ) - - mock_chunk = CompletionChunk( - id="chunk2", - created=222, - model="model-2", - choices=[], - ) - - mock_choice = CompletionResponseStreamChoice( - index=1, - delta=DeltaMessage(role="assistant", content="Hello from stream!", tool_calls=[mock_tool]), - finish_reason=None, - ) - chunk_metadata = {"chunk": "meta"} - - async_mock_client = MagicMock(spec=Mistral) - chat_completion = MistralAIChatCompletion( - ai_model_id="test-model", - api_key="test_key", - async_client=async_mock_client, - ) - - result = chat_completion._create_streaming_chat_message_content( - chunk=mock_chunk, - choice=mock_choice, - chunk_metadata=chunk_metadata, - function_invoke_attempt=0, - ) - - # We expect to see one StreamingTextContent item and one FunctionCallContent. - assert len(result.items) == 2 - stc = [i for i in result.items if isinstance(i, StreamingTextContent)][0] - assert stc.text == "Hello from stream!" - - fcc = [i for i in result.items if isinstance(i, FunctionCallContent)][0] - assert fcc.id == "tool_2" - assert fcc.name == "function_name_2" - assert fcc.arguments == "function_args_2" - - async def test_mistral_ai_chat_completion_update_settings_from_function_call_configuration_mistral(): """Test update_settings_from_function_call_configuration_mistral sets tools etc.""" From 991e971f5eca24fde993ce751f9a64d860ae76cf Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 11:28:50 -0300 Subject: [PATCH 11/18] updating bing search tests to use bing_unit_test_env fixture --- .../search/bing/test_bing_search.py | 74 ++++++++----------- 1 file changed, 32 insertions(+), 42 deletions(-) diff --git a/python/tests/unit/connectors/search/bing/test_bing_search.py b/python/tests/unit/connectors/search/bing/test_bing_search.py index afce9435be97..9368ac37d0f3 100644 --- a/python/tests/unit/connectors/search/bing/test_bing_search.py +++ b/python/tests/unit/connectors/search/bing/test_bing_search.py @@ -4,11 +4,9 @@ import httpx import pytest -from pydantic import ValidationError from semantic_kernel.connectors.search.bing.bing_search import BingSearch from semantic_kernel.connectors.search.bing.bing_search_response import BingSearchResponse, BingWebPages -from semantic_kernel.connectors.search.bing.bing_search_settings import BingSettings from semantic_kernel.connectors.search.bing.bing_web_page import BingWebPage from semantic_kernel.data.kernel_search_results import KernelSearchResults from semantic_kernel.data.text_search.text_search_filter import TextSearchFilter @@ -17,28 +15,26 @@ from semantic_kernel.exceptions import ServiceInitializationError, ServiceInvalidRequestError -async def test_bing_search_init_success(): - """Test that BingSearch initializes successfully with valid parameters.""" - # Arrange/Act - with patch.object(BingSettings, "create", return_value=MagicMock(spec=BingSettings)) as mock_settings_create: - search_instance = BingSearch(api_key="fake_api_key", custom_config="fake_config") +@pytest.fixture +def bing_search(bing_unit_test_env): + """Set up the fixture to configure the Bing Search for these tests.""" + return BingSearch() - # Assert - mock_settings_create.assert_called_once() - assert search_instance is not None + +async def test_bing_search_init_success(bing_search): + """Test that BingSearch initializes successfully with valid env.""" + assert bing_search is not None async def test_bing_search_init_validation_error(): """Test that BingSearch raises ServiceInitializationError if BingSettings creation fails.""" - # Arrange - with patch.object(BingSettings, "create", side_effect=ValidationError("error", [])): - # Act / Assert - with pytest.raises(ServiceInitializationError) as exc_info: - _ = BingSearch(api_key="invalid_api_key") - assert "Failed to create Bing settings." in str(exc_info.value) + # Act / Assert + with pytest.raises(ServiceInitializationError) as exc_info: + _ = BingSearch(env_file_path="invalid.env") + assert "Failed to create Bing settings." in str(exc_info.value) -async def test_search_success(): +async def test_search_success(bing_unit_test_env): """Test that search method returns KernelSearchResults successfully on valid response.""" # Arrange mock_web_pages = BingWebPage(snippet="Test snippet") @@ -62,7 +58,7 @@ async def test_search_success(): patch("semantic_kernel.connectors.search.bing.bing_search.AsyncClient", return_value=async_client_mock), patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), ): - search_instance = BingSearch(api_key="fake_api_key") + search_instance = BingSearch() options = TextSearchOptions(include_total_count=True) kernel_results: KernelSearchResults[str] = await search_instance.search("Test query", options) @@ -77,7 +73,7 @@ async def test_search_success(): assert kernel_results.metadata == {"altered_query": "altered something"} -async def test_search_http_status_error(): +async def test_search_http_status_error(bing_unit_test_env): """Test that search method raises ServiceInvalidRequestError on HTTPStatusError.""" # Arrange async_client_mock = AsyncMock() @@ -91,14 +87,14 @@ async def test_search_http_status_error(): with patch( "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock ): - search_instance = BingSearch(api_key="fake_api_key") + search_instance = BingSearch() # Assert with pytest.raises(ServiceInvalidRequestError) as exc_info: await search_instance.search("Test query") assert "Failed to get search results." in str(exc_info.value) -async def test_search_request_error(): +async def test_search_request_error(bing_unit_test_env): """Test that search method raises ServiceInvalidRequestError on RequestError.""" # Arrange async_client_mock = AsyncMock() @@ -108,14 +104,14 @@ async def test_search_request_error(): with patch( "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock ): - search_instance = BingSearch(api_key="fake_api_key") + search_instance = BingSearch() # Assert with pytest.raises(ServiceInvalidRequestError) as exc_info: await search_instance.search("Test query") assert "A client error occurred while getting search results." in str(exc_info.value) -async def test_search_generic_exception(): +async def test_search_generic_exception(bing_unit_test_env): """Test that search method raises ServiceInvalidRequestError on unexpected exception.""" # Arrange async_client_mock = AsyncMock() @@ -125,26 +121,25 @@ async def test_search_generic_exception(): with patch( "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock ): - search_instance = BingSearch(api_key="fake_api_key") + search_instance = BingSearch() # Assert with pytest.raises(ServiceInvalidRequestError) as exc_info: await search_instance.search("Test query") assert "An unexpected error occurred while getting search results." in str(exc_info.value) -async def test_validate_options_raises_error_for_large_top(): +async def test_validate_options_raises_error_for_large_top(bing_search): """Test that _validate_options raises ServiceInvalidRequestError when top >= 50.""" # Arrange - search_instance = BingSearch(api_key="fake_api_key") options = TextSearchOptions(top=50) # Act / Assert with pytest.raises(ServiceInvalidRequestError) as exc_info: - await search_instance._inner_search("test", options) + await bing_search._inner_search("test", options) assert "count value must be less than 50." in str(exc_info.value) -async def test_get_text_search_results_success(): +async def test_get_text_search_results_success(bing_unit_test_env): """Test that get_text_search_results returns KernelSearchResults[TextSearchResult].""" # Arrange mock_web_pages = BingWebPage(name="Result Name", snippet="Snippet", url="test") @@ -171,7 +166,7 @@ async def test_get_text_search_results_success(): ), patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), ): - search_instance = BingSearch(api_key="fake_api_key") + search_instance = BingSearch() options = TextSearchOptions(include_total_count=True) kernel_results: KernelSearchResults[TextSearchResult] = await search_instance.get_text_search_results( "Test query", options @@ -190,7 +185,7 @@ async def test_get_text_search_results_success(): assert kernel_results.total_count == 5 -async def test_get_search_results_success(): +async def test_get_search_results_success(bing_unit_test_env): """Test that get_search_results returns KernelSearchResults[BingWebPage].""" # Arrange mock_web_page = BingWebPage(name="Page Name", snippet="Page Snippet", url="test") @@ -216,7 +211,7 @@ async def test_get_search_results_success(): ), patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), ): - search_instance = BingSearch(api_key="fake_api_key") + search_instance = BingSearch() options = TextSearchOptions(include_total_count=True) kernel_results = await search_instance.get_search_results("Another query", options) @@ -233,14 +228,13 @@ async def test_get_search_results_success(): assert kernel_results.total_count == 3 -async def test_build_request_parameters_no_filter(): +async def test_build_request_parameters_no_filter(bing_search): """Test that _build_request_parameters properly sets params when no filter is provided.""" # Arrange - search_instance = BingSearch(api_key="fake_api_key") options = TextSearchOptions() # Act - params = search_instance._build_request_parameters("test query", options) + params = bing_search._build_request_parameters("test query", options) # Assert assert params["count"] == options.top @@ -249,17 +243,15 @@ async def test_build_request_parameters_no_filter(): assert params["q"] == "test query+" -async def test_build_request_parameters_equal_to_filter(): +async def test_build_request_parameters_equal_to_filter(bing_search): """Test that _build_request_parameters properly sets params with an EqualTo filter.""" # Arrange - search_instance = BingSearch(api_key="fake_api_key") - my_filter = TextSearchFilter.equal_to(field_name="freshness", value="Day") options = TextSearchOptions(filter=my_filter) # Act - params = search_instance._build_request_parameters("test query", options) + params = bing_search._build_request_parameters("test query", options) # Assert assert params["count"] == options.top @@ -271,18 +263,16 @@ async def test_build_request_parameters_equal_to_filter(): assert params["q"] == "test query+".strip() -async def test_build_request_parameters_not_recognized_filter(): +async def test_build_request_parameters_not_recognized_filter(bing_search): """Test that _build_request_parameters properly appends non-recognized filters to the q parameter.""" # Arrange - search_instance = BingSearch(api_key="fake_api_key") - # 'customProperty' is presumably not in QUERY_PARAMETERS my_filter = TextSearchFilter.equal_to(field_name="customProperty", value="customValue") options = TextSearchOptions(filter=my_filter) # Act - params = search_instance._build_request_parameters("test query", options) + params = bing_search._build_request_parameters("test query", options) # Assert assert params["count"] == options.top From 4352bd8fcd4709bcb62a8eb9a9409b9d48455409 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 11:56:13 -0300 Subject: [PATCH 12/18] bing search test updates --- .../search/bing/test_bing_search.py | 133 +++++++++--------- 1 file changed, 67 insertions(+), 66 deletions(-) diff --git a/python/tests/unit/connectors/search/bing/test_bing_search.py b/python/tests/unit/connectors/search/bing/test_bing_search.py index 9368ac37d0f3..83d6bd95f2bd 100644 --- a/python/tests/unit/connectors/search/bing/test_bing_search.py +++ b/python/tests/unit/connectors/search/bing/test_bing_search.py @@ -21,6 +21,31 @@ def bing_search(bing_unit_test_env): return BingSearch() +@pytest.fixture +def async_client_mock(): + """Set up the fixture to mock AsyncClient.""" + async_client_mock = AsyncMock() + with patch( + "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock + ): + yield async_client_mock + + +@pytest.fixture +def mock_bing_search_response(): + """Set up the fixture to mock BingSearchResponse.""" + mock_web_page = BingWebPage(name="Page Name", snippet="Page Snippet", url="test") + mock_response = BingSearchResponse( + query_context={}, + webPages=MagicMock(spec=BingWebPages, value=[mock_web_page], total_estimated_matches=3), + ) + + with ( + patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), + ): + yield mock_response + + async def test_bing_search_init_success(bing_search): """Test that BingSearch initializes successfully with valid env.""" assert bing_search is not None @@ -34,7 +59,7 @@ async def test_bing_search_init_validation_error(): assert "Failed to create Bing settings." in str(exc_info.value) -async def test_search_success(bing_unit_test_env): +async def test_search_success(bing_unit_test_env, async_client_mock): """Test that search method returns KernelSearchResults successfully on valid response.""" # Arrange mock_web_pages = BingWebPage(snippet="Test snippet") @@ -43,7 +68,6 @@ async def test_search_success(bing_unit_test_env): query_context={"alteredQuery": "altered something"}, ) - async_client_mock = AsyncMock() mock_result = MagicMock() mock_result.text = """ {"webPages": { @@ -55,7 +79,6 @@ async def test_search_success(bing_unit_test_env): # Act with ( - patch("semantic_kernel.connectors.search.bing.bing_search.AsyncClient", return_value=async_client_mock), patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), ): search_instance = BingSearch() @@ -73,10 +96,9 @@ async def test_search_success(bing_unit_test_env): assert kernel_results.metadata == {"altered_query": "altered something"} -async def test_search_http_status_error(bing_unit_test_env): +async def test_search_http_status_error(bing_unit_test_env, async_client_mock): """Test that search method raises ServiceInvalidRequestError on HTTPStatusError.""" # Arrange - async_client_mock = AsyncMock() mock_response = MagicMock() mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( "Error", request=MagicMock(), response=MagicMock() @@ -84,48 +106,38 @@ async def test_search_http_status_error(bing_unit_test_env): async_client_mock.get.return_value = mock_response # Act - with patch( - "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock - ): - search_instance = BingSearch() - # Assert - with pytest.raises(ServiceInvalidRequestError) as exc_info: - await search_instance.search("Test query") - assert "Failed to get search results." in str(exc_info.value) + search_instance = BingSearch() + # Assert + with pytest.raises(ServiceInvalidRequestError) as exc_info: + await search_instance.search("Test query") + assert "Failed to get search results." in str(exc_info.value) -async def test_search_request_error(bing_unit_test_env): + +async def test_search_request_error(bing_unit_test_env, async_client_mock): """Test that search method raises ServiceInvalidRequestError on RequestError.""" # Arrange - async_client_mock = AsyncMock() async_client_mock.get.side_effect = httpx.RequestError("Client error") # Act - with patch( - "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock - ): - search_instance = BingSearch() - # Assert - with pytest.raises(ServiceInvalidRequestError) as exc_info: - await search_instance.search("Test query") - assert "A client error occurred while getting search results." in str(exc_info.value) + search_instance = BingSearch() + + # Assert + with pytest.raises(ServiceInvalidRequestError) as exc_info: + await search_instance.search("Test query") + assert "A client error occurred while getting search results." in str(exc_info.value) -async def test_search_generic_exception(bing_unit_test_env): +async def test_search_generic_exception(bing_unit_test_env, async_client_mock): """Test that search method raises ServiceInvalidRequestError on unexpected exception.""" # Arrange - async_client_mock = AsyncMock() async_client_mock.get.side_effect = Exception("Something unexpected") - # Act - with patch( - "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock - ): - search_instance = BingSearch() - # Assert - with pytest.raises(ServiceInvalidRequestError) as exc_info: - await search_instance.search("Test query") - assert "An unexpected error occurred while getting search results." in str(exc_info.value) + search_instance = BingSearch() + # Assert + with pytest.raises(ServiceInvalidRequestError) as exc_info: + await search_instance.search("Test query") + assert "An unexpected error occurred while getting search results." in str(exc_info.value) async def test_validate_options_raises_error_for_large_top(bing_search): @@ -135,11 +147,11 @@ async def test_validate_options_raises_error_for_large_top(bing_search): # Act / Assert with pytest.raises(ServiceInvalidRequestError) as exc_info: - await bing_search._inner_search("test", options) + await bing_search.search("test", options) assert "count value must be less than 50." in str(exc_info.value) -async def test_get_text_search_results_success(bing_unit_test_env): +async def test_get_text_search_results_success(bing_unit_test_env, async_client_mock): """Test that get_text_search_results returns KernelSearchResults[TextSearchResult].""" # Arrange mock_web_pages = BingWebPage(name="Result Name", snippet="Snippet", url="test") @@ -148,7 +160,6 @@ async def test_get_text_search_results_success(bing_unit_test_env): webPages=MagicMock(spec=BingWebPages, value=[mock_web_pages], total_estimated_matches=5), ) - async_client_mock = AsyncMock() mock_result = MagicMock() mock_result.text = """" {"webPages": { @@ -161,9 +172,6 @@ async def test_get_text_search_results_success(bing_unit_test_env): # Act with ( - patch( - "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock - ), patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), ): search_instance = BingSearch() @@ -185,16 +193,9 @@ async def test_get_text_search_results_success(bing_unit_test_env): assert kernel_results.total_count == 5 -async def test_get_search_results_success(bing_unit_test_env): +async def test_get_search_results_success(bing_unit_test_env, async_client_mock, mock_bing_search_response): """Test that get_search_results returns KernelSearchResults[BingWebPage].""" # Arrange - mock_web_page = BingWebPage(name="Page Name", snippet="Page Snippet", url="test") - mock_response = BingSearchResponse( - query_context={}, - webPages=MagicMock(spec=BingWebPages, value=[mock_web_page], total_estimated_matches=3), - ) - - async_client_mock = AsyncMock() mock_result = MagicMock() mock_result.text = """ {"webPages": { @@ -205,15 +206,9 @@ async def test_get_search_results_success(bing_unit_test_env): async_client_mock.get.return_value = mock_result # Act - with ( - patch( - "semantic_kernel.connectors.search.bing.bing_search.AsyncClient.__aenter__", return_value=async_client_mock - ), - patch.object(BingSearchResponse, "model_validate_json", return_value=mock_response), - ): - search_instance = BingSearch() - options = TextSearchOptions(include_total_count=True) - kernel_results = await search_instance.get_search_results("Another query", options) + search_instance = BingSearch() + options = TextSearchOptions(include_total_count=True) + kernel_results = await search_instance.get_search_results("Another query", options) # Assert results_list = [] @@ -228,32 +223,37 @@ async def test_get_search_results_success(bing_unit_test_env): assert kernel_results.total_count == 3 -async def test_build_request_parameters_no_filter(bing_search): - """Test that _build_request_parameters properly sets params when no filter is provided.""" +async def test_search_no_filter(bing_search, async_client_mock, mock_bing_search_response): + """Test that search properly sets params when no filter is provided.""" # Arrange options = TextSearchOptions() # Act - params = bing_search._build_request_parameters("test query", options) + await bing_search.search("test query", options) # Assert + params = async_client_mock.get.call_args.kwargs["params"] + assert params["count"] == options.top assert params["offset"] == options.skip + # TODO: shouldn't this output be "test query" instead of "test query+"? assert params["q"] == "test query+" -async def test_build_request_parameters_equal_to_filter(bing_search): - """Test that _build_request_parameters properly sets params with an EqualTo filter.""" +async def test_search_equal_to_filter(bing_search, async_client_mock, mock_bing_search_response): + """Test that search properly sets params with an EqualTo filter.""" # Arrange my_filter = TextSearchFilter.equal_to(field_name="freshness", value="Day") options = TextSearchOptions(filter=my_filter) # Act - params = bing_search._build_request_parameters("test query", options) + await bing_search.search("test query", options) # Assert + params = async_client_mock.get.call_args.kwargs["params"] + assert params["count"] == options.top assert params["offset"] == options.skip # 'freshness' is recognized in QUERY_PARAMETERS, so 'freshness' should be set @@ -263,8 +263,8 @@ async def test_build_request_parameters_equal_to_filter(bing_search): assert params["q"] == "test query+".strip() -async def test_build_request_parameters_not_recognized_filter(bing_search): - """Test that _build_request_parameters properly appends non-recognized filters to the q parameter.""" +async def test_search_not_recognized_filter(bing_search, async_client_mock, mock_bing_search_response): + """Test that search properly appends non-recognized filters to the q parameter.""" # Arrange # 'customProperty' is presumably not in QUERY_PARAMETERS @@ -272,9 +272,10 @@ async def test_build_request_parameters_not_recognized_filter(bing_search): options = TextSearchOptions(filter=my_filter) # Act - params = bing_search._build_request_parameters("test query", options) + await bing_search.search("test query", options) # Assert + params = async_client_mock.get.call_args.kwargs["params"] assert params["count"] == options.top assert params["offset"] == options.skip assert "customProperty" not in params From e4ee455387bb072a95d596ff7f52b63c1800655d Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 11:56:44 -0300 Subject: [PATCH 13/18] mistralai chat completion test updates --- .../test_mistralai_chat_completion.py | 42 ------------------- 1 file changed, 42 deletions(-) diff --git a/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py b/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py index 2d255627e0fc..b8b8c25bd951 100644 --- a/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py +++ b/python/tests/unit/connectors/ai/mistral_ai/services/test_mistralai_chat_completion.py @@ -385,48 +385,6 @@ async def test_with_different_execution_settings_stream( assert mock_mistral_ai_client_completion_stream.chat.stream_async.call_args.kwargs["seed"] == 2 -async def test_mistral_ai_chat_completion_initialization_valid(): - """Test MistralAIChatCompletion initialization should succeed with valid params.""" - mistralai_settings = MagicMock() - mistralai_settings.chat_model_id = "chat_model_id" - - with ( - patch( - "semantic_kernel.connectors.ai.mistral_ai.settings.mistral_ai_settings.MistralAISettings.create", - return_value=mistralai_settings, - ) as mock_settings_create, - patch( - "semantic_kernel.connectors.ai.mistral_ai.services.mistral_ai_base.MistralAIBase.__init__" - ) as mistral_base, - ): - # We gave an api_key to ensure it doesn't raise an exception, but the rest can be None. - ai_completion = MistralAIChatCompletion( - ai_model_id="some-model-id", - api_key="some-key", - ) - assert ai_completion is not None - - mock_settings_create.assert_called_once() - mistral_base.assert_called_once() - - -async def test_mistral_ai_chat_completion_initialization_missing_model_id(): - """Test MistralAIChatCompletion initialization should raise ServiceInitializationError when model id is missing.""" - with pytest.raises(ServiceInitializationError) as exc_info: - MistralAIChatCompletion( - ai_model_id=None, - api_key="some-key", - ) - assert "The MistralAI chat model ID is required." in str(exc_info.value) - - -async def test_mistral_ai_chat_completion_initialization_validation_error(): - """Test MistralAIChatCompletion initialization when MistralAISettings creation fails and raises ValidationError.""" - with pytest.raises(ServiceInitializationError) as exc_info: - MistralAIChatCompletion(env_file_path="fake_env_file_path.env") - assert "Failed to create MistralAI settings." in str(exc_info.value) - - async def test_mistral_ai_chat_completion_get_chat_message_contents_success(): """Test get_chat_message_contents with a successful ChatCompletionResponse.""" From 6292bccf438faa83cde8a610cce5abc770bca6d4 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 12:51:12 -0300 Subject: [PATCH 14/18] bing search updates --- python/tests/unit/connectors/search/bing/test_bing_search.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/tests/unit/connectors/search/bing/test_bing_search.py b/python/tests/unit/connectors/search/bing/test_bing_search.py index 83d6bd95f2bd..348c701ab20b 100644 --- a/python/tests/unit/connectors/search/bing/test_bing_search.py +++ b/python/tests/unit/connectors/search/bing/test_bing_search.py @@ -48,7 +48,9 @@ def mock_bing_search_response(): async def test_bing_search_init_success(bing_search): """Test that BingSearch initializes successfully with valid env.""" - assert bing_search is not None + # Should not raise any exception + assert bing_search.settings.api_key.get_secret_value() == "test_api_key" + assert bing_search.settings.custom_config == "test_org_id" async def test_bing_search_init_validation_error(): From 3cdbe0d170fdec860f4f48bb66abf64c3e342d93 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 26 Feb 2025 12:52:12 -0300 Subject: [PATCH 15/18] test google search updates --- .../search/google/test_google_search.py | 449 ++++++------------ 1 file changed, 133 insertions(+), 316 deletions(-) diff --git a/python/tests/unit/connectors/search/google/test_google_search.py b/python/tests/unit/connectors/search/google/test_google_search.py index 01bf2ed49267..c54e592910a9 100644 --- a/python/tests/unit/connectors/search/google/test_google_search.py +++ b/python/tests/unit/connectors/search/google/test_google_search.py @@ -1,10 +1,9 @@ # Copyright (c) Microsoft. All rights reserved. -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest -from httpx import HTTPStatusError, RequestError -from pydantic import ValidationError +from httpx import HTTPStatusError, RequestError, Response from semantic_kernel.connectors.search.google.google_search import GoogleSearch from semantic_kernel.connectors.search.google.google_search_response import ( @@ -12,342 +11,160 @@ GoogleSearchResponse, ) from semantic_kernel.connectors.search.google.google_search_result import GoogleSearchResult -from semantic_kernel.connectors.search.google.google_search_settings import GoogleSearchSettings -from semantic_kernel.data.text_search.text_search_filter import TextSearchFilter +from semantic_kernel.data.filter_clauses.any_tags_equal_to_filter_clause import AnyTagsEqualTo +from semantic_kernel.data.filter_clauses.equal_to_filter_clause import EqualTo from semantic_kernel.data.text_search.text_search_options import TextSearchOptions -from semantic_kernel.data.text_search.text_search_result import TextSearchResult -from semantic_kernel.exceptions import ServiceInitializationError, ServiceInvalidRequestError +from semantic_kernel.exceptions import ( + ServiceInitializationError, + ServiceInvalidRequestError, +) -async def test_google_search_init_success() -> None: - """Test that GoogleSearch is initialized successfully when settings are valid.""" - with patch.object( - GoogleSearchSettings, "create", return_value=MagicMock(spec=GoogleSearchSettings) - ) as mock_settings_create: - # Arrange - api_key = "fake_api_key" - search_engine_id = "fake_engine_id" +@pytest.fixture +def google_search(google_search_unit_test_env): + """Fixture to return a GoogleSearch instance with valid settings.""" + return GoogleSearch() - # Act - google_search = GoogleSearch(api_key=api_key, search_engine_id=search_engine_id) - # Assert - mock_settings_create.assert_called_once_with( - api_key=api_key, - engine_id=search_engine_id, - env_file_path=None, - env_file_encoding=None, - ) - assert google_search.settings is not None +async def test_google_search_init_success(google_search) -> None: + """Test that GoogleSearch successfully initializes with valid parameters.""" + # Should not raise any exception + assert google_search.settings.api_key.get_secret_value() == "test_api_key" + assert google_search.settings.engine_id == "test_id" async def test_google_search_init_validation_error() -> None: - """Test that ServiceInitializationError is raised if settings creation fails validation.""" - with patch.object(GoogleSearchSettings, "create", side_effect=ValidationError("error", [])): - # Arrange & Act & Assert - with pytest.raises(ServiceInitializationError) as exc: - GoogleSearch(api_key="invalid", search_engine_id="invalid") - assert "Failed to create Google settings." in str(exc.value) - - -async def test_google_search_search_calls_inner_search() -> None: - """Test the search method calls _inner_search with the correct arguments.""" - # Arrange - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - mock_response = MagicMock() - with patch.object(google_search, "_inner_search", return_value=mock_response) as mock_inner: - # Act - await google_search.search(query="test query", options=None) - - # Assert - mock_inner.assert_called_once() - - -async def test_google_search_get_text_search_results_calls_inner_search() -> None: - """Test the get_text_search_results method calls _inner_search with the correct arguments.""" - # Arrange - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - mock_response = MagicMock() - with patch.object(google_search, "_inner_search", return_value=mock_response) as mock_inner: - # Act - await google_search.get_text_search_results(query="test query", options=None) - - # Assert - mock_inner.assert_called_once() - - -async def test_google_search_get_search_results_calls_inner_search() -> None: - """Test the get_search_results method calls _inner_search with the correct arguments.""" - # Arrange - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - mock_response = MagicMock() - with patch.object(google_search, "_inner_search", return_value=mock_response) as mock_inner: - # Act - await google_search.get_search_results(query="test query", options=None) - - # Assert - mock_inner.assert_called_once() - - -async def test__validate_options_raises_error_if_top_too_high() -> None: - """Test that _validate_options raises ServiceInvalidRequestError if top results > 10.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - options = TextSearchOptions(top=11, skip=0) - with pytest.raises(ServiceInvalidRequestError) as exc: - google_search._validate_options(options) - assert "count value must be less than or equal to 10." in str(exc.value) - - -async def test__validate_options_does_not_raise_if_top_ok() -> None: - """Test that _validate_options does not raise if top <= 10.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - options = TextSearchOptions(top=10, skip=0) - # Should not raise - google_search._validate_options(options) - - -async def test__get_options_returns_existing_text_search_options() -> None: - """Test _get_options returns the provided TextSearchOptions if already of correct type.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - original_options = TextSearchOptions(top=5, skip=2) - returned_options = google_search._get_options(original_options) - assert returned_options == original_options - + """Test that GoogleSearch raises ServiceInitializationError when GoogleSearchSettings creation fails.""" + with pytest.raises(ServiceInitializationError) as exc: + GoogleSearch(env_file_path="invalid.env") + assert "Failed to create Google settings." in str(exc.value) -async def test__get_options_creates_new_text_search_options_if_none() -> None: - """Test _get_options creates new TextSearchOptions if none is provided.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - returned_options = google_search._get_options(None) - assert isinstance(returned_options, TextSearchOptions) +async def test_google_search_top_greater_than_10_raises_error(google_search) -> None: + """Test that passing a top value greater than 10 raises ServiceInvalidRequestError.""" + options = TextSearchOptions() + options.top = 11 # Invalid + with pytest.raises(ServiceInvalidRequestError) as exc: + await google_search.search(query="test query", options=options) + assert "count value must be less than or equal to 10." in str(exc.value) -async def test__get_options_creates_new_options_if_wrong_type_in_kwargs() -> None: - """Test _get_options handles invalid kwargs gracefully and returns a default TextSearchOptions.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - returned_options = google_search._get_options(None, top="invalid") - # should still return a default object - assert isinstance(returned_options, TextSearchOptions) - - -async def test__inner_search_success_returns_parsed_response() -> None: - """Test _inner_search returns the parsed JSON response on success.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - - mock_response = GoogleSearchResponse() - - # mock an OK response from the client - mock_ok_response = MagicMock() - mock_ok_response.raise_for_status = MagicMock() - mock_ok_response.text = "fake_json_text" - - async_client_mock = AsyncMock() - async_client_mock.get.return_value = mock_ok_response - with ( - patch.object(google_search, "_validate_options") as mock_validate, - patch( - "semantic_kernel.connectors.search.google.google_search.AsyncClient.__aenter__", - return_value=async_client_mock, - ), - patch( - "semantic_kernel.connectors.search.google.google_search_response.GoogleSearchResponse.model_validate_json", - return_value=mock_response, - ) as mock_model_validate_json, +async def test_google_search_no_items_in_response(google_search) -> None: + """Test that when the response has no items, search results yield nothing.""" + mock_response = GoogleSearchResponse(items=None) + + # We'll mock _inner_search to return our mock_response + with patch.object(google_search, "_inner_search", new=AsyncMock(return_value=mock_response)): + result = await google_search.search("test") + # Extract all items from the AsyncIterable + items = [item async for item in result.results] + assert len(items) == 0 + + +async def test_google_search_partial_items_in_response(google_search) -> None: + """Test that snippets are properly returned in search results.""" + snippet_1 = "Snippet 1" + snippet_2 = "Snippet 2" + item_1 = GoogleSearchResult(snippet=snippet_1) + item_2 = GoogleSearchResult(snippet=snippet_2) + mock_response = GoogleSearchResponse(items=[item_1, item_2]) + + with patch.object(google_search, "_inner_search", new=AsyncMock(return_value=mock_response)): + result = await google_search.search("test") + items = [item async for item in result.results] + assert len(items) == 2 + assert items[0] == snippet_1 + assert items[1] == snippet_2 + + +async def test_google_search_request_http_status_error(google_search) -> None: + """Test that HTTP status errors raise ServiceInvalidRequestError.""" + # Mock the AsyncClient.get call to raise HTTPStatusError + with patch( + "httpx.AsyncClient.get", + new=AsyncMock(side_effect=HTTPStatusError("Error", request=None, response=Response(status_code=400))), ): - result = await google_search._inner_search("test query", TextSearchOptions()) - - mock_validate.assert_called_once() - # ensure the mock_model_validate_json method was called - mock_model_validate_json.assert_called_once_with("fake_json_text") - assert result == mock_response - - -async def test__inner_search_http_status_error_raises_service_invalid_request_error() -> None: - """Test _inner_search raises ServiceInvalidRequestError if an HTTPStatusError occurs.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - - with patch.object(google_search, "_validate_options"), patch("httpx.AsyncClient") as mock_client: - instance = mock_client.return_value.__aenter__.return_value - # Mock a failing get call that raises HTTPStatusError - instance.get.side_effect = HTTPStatusError("Error", request=MagicMock(), response=MagicMock()) - with pytest.raises(ServiceInvalidRequestError) as exc: - await google_search._inner_search("test query", TextSearchOptions()) + await google_search.search("query") assert "Failed to get search results." in str(exc.value) -async def test__inner_search_request_error_raises_service_invalid_request_error() -> None: - """Test _inner_search raises ServiceInvalidRequestError if a client-side RequestError occurs.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - - async_client_mock = AsyncMock() - # Mock a failing get call that raises RequestError - async_client_mock.get.side_effect = RequestError("Client error") - - with ( - patch.object(google_search, "_validate_options"), - patch( - "semantic_kernel.connectors.search.google.google_search.AsyncClient.__aenter__", - return_value=async_client_mock, - ), - ): +async def test_google_search_request_error(google_search) -> None: + """Test that request errors raise ServiceInvalidRequestError.""" + # Mock the AsyncClient.get call to raise RequestError + with patch("httpx.AsyncClient.get", new=AsyncMock(side_effect=RequestError("Client error"))): with pytest.raises(ServiceInvalidRequestError) as exc: - await google_search._inner_search("test query", TextSearchOptions()) + await google_search.search("query") assert "A client error occurred while getting search results." in str(exc.value) -async def test__inner_search_unexpected_error_raises_service_invalid_request_error() -> None: - """Test _inner_search raises ServiceInvalidRequestError if an unexpected error occurs.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - - async_client_mock = AsyncMock() - # Trigger a generic exception - async_client_mock.get.side_effect = Exception("Unexpected error") - - with ( - patch.object(google_search, "_validate_options"), - patch( - "semantic_kernel.connectors.search.google.google_search.AsyncClient.__aenter__", - return_value=async_client_mock, - ), - ): +async def test_google_search_unexpected_error(google_search) -> None: + """Test that unexpected exceptions raise ServiceInvalidRequestError.""" + # Mock the AsyncClient.get call to raise a random exception + with patch("httpx.AsyncClient.get", new=AsyncMock(side_effect=Exception("Random error"))): with pytest.raises(ServiceInvalidRequestError) as exc: - await google_search._inner_search("test query", TextSearchOptions()) + await google_search.search("query") assert "An unexpected error occurred while getting search results." in str(exc.value) -async def test__build_query_no_filter() -> None: - """Test _build_query constructs the expected Google Search URL part with no filter clauses.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - options = TextSearchOptions(top=5, skip=2) - query_str = google_search._build_query("hello world", options) - # We expect it to produce something like ?q=hello+world&key=fake_api_key&cx=fake_engine_id&num=5&start=2 - assert "hello+world" in query_str - assert "key=fake_api_key" in query_str - assert "cx=fake_engine_id" in query_str - assert "num=5" in query_str - assert "start=2" in query_str - - -async def test__build_query_equal_to_filter() -> None: - """Test _build_query includes EqualTo filter parameters if matching the known query parameters.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - options = TextSearchOptions(top=3, skip=0) - options.filter = TextSearchFilter.equal_to(field_name="lr", value="lang_en") - - query_str = google_search._build_query("test", options) - assert "lr=lang_en" in query_str - - -async def test__build_query_any_tags_equal_to_unsupported_filter() -> None: - """Test _build_query logs debug message when AnyTagsEqualTo filter is present but doesn't break query.""" - from semantic_kernel.data.filter_clauses.any_tags_equal_to_filter_clause import AnyTagsEqualTo - - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - options = TextSearchOptions(top=3, skip=0) - options.filter.filters.append(AnyTagsEqualTo(field_name="tags", value="some_tag")) - - query_str = google_search._build_query("test", options) - assert "q=test" in query_str - assert "cx=fake_engine_id" in query_str - - -async def test__get_result_strings_no_items() -> None: - """Test _get_result_strings yields nothing if response contains no items.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - response = GoogleSearchResponse(items=None) - results = [r async for r in google_search._get_result_strings(response)] - assert results == [] - - -async def test__get_result_strings_with_items() -> None: - """Test _get_result_strings yields the snippet of each item.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - item1 = GoogleSearchResult(snippet="snippet1") - item2 = GoogleSearchResult(snippet="snippet2") - response = GoogleSearchResponse(items=[item1, item2]) - - results = [r async for r in google_search._get_result_strings(response)] - assert results == ["snippet1", "snippet2"] - - -async def test__get_text_search_results_no_items() -> None: - """Test _get_text_search_results yields nothing if response contains no items.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - response = GoogleSearchResponse(items=None) - results = [r async for r in google_search._get_text_search_results(response)] - assert results == [] - - -async def test__get_text_search_results_with_items() -> None: - """Test _get_text_search_results yields TextSearchResult objects for each item.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - item = GoogleSearchResult(title="test title", snippet="test snippet", link="test") - response = GoogleSearchResponse(items=[item]) - - results = [r async for r in google_search._get_text_search_results(response)] - assert len(results) == 1 - assert isinstance(results[0], TextSearchResult) - assert results[0].name == "test title" - assert results[0].value == "test snippet" - assert results[0].link == "test" - - -async def test__get_google_search_results_no_items() -> None: - """Test _get_google_search_results yields nothing if response contains no items.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - response = GoogleSearchResponse(items=None) - results = [r async for r in google_search._get_google_search_results(response)] - assert results == [] - - -async def test__get_google_search_results_with_items() -> None: - """Test _get_google_search_results yields each GoogleSearchResult in the response items.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - item1 = GoogleSearchResult() - item2 = GoogleSearchResult() - response = GoogleSearchResponse(items=[item1, item2]) - - results = [r async for r in google_search._get_google_search_results(response)] - assert len(results) == 2 - assert results[0] is item1 - assert results[1] is item2 - - -async def test__get_total_count_included() -> None: - """Test _get_total_count returns total if include_total_count is True.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - response = GoogleSearchResponse() - response.search_information = MagicMock(spec=GoogleSearchInformation) - response.search_information.total_results = 42 - +async def test_get_text_search_results(google_search) -> None: + """Test that get_text_search_results returns TextSearchResults that contain name, value, and link.""" + item_1 = GoogleSearchResult(title="Title1", snippet="Snippet1", link="Link1") + item_2 = GoogleSearchResult(title="Title2", snippet="Snippet2", link="Link2") + mock_response = GoogleSearchResponse(items=[item_1, item_2]) + + with patch.object(google_search, "_inner_search", new=AsyncMock(return_value=mock_response)): + result = await google_search.get_text_search_results("test") + items = [item async for item in result.results] + assert len(items) == 2 + assert items[0].name == "Title1" + assert items[0].value == "Snippet1" + assert items[0].link == "Link1" + assert items[1].name == "Title2" + assert items[1].value == "Snippet2" + assert items[1].link == "Link2" + + +async def test_get_search_results(google_search) -> None: + """Test that get_search_results returns GoogleSearchResult items directly.""" + item_1 = GoogleSearchResult(title="Title1", snippet="Snippet1", link="Link1") + item_2 = GoogleSearchResult(title="Title2", snippet="Snippet2", link="Link2") + mock_response = GoogleSearchResponse(items=[item_1, item_2]) + + with patch.object(google_search, "_inner_search", new=AsyncMock(return_value=mock_response)): + result = await google_search.get_search_results("test") + items = [item async for item in result.results] + assert len(items) == 2 + assert items[0].title == "Title1" + assert items[1].link == "Link2" + + +async def test_build_query_equal_to_filter(google_search) -> None: + """Test that if an EqualTo filter is recognized, it is sent along in query params.""" + filters = [ + EqualTo(field_name="lr", value="lang_en"), + AnyTagsEqualTo(field_name="tags", value="tag1"), + ] # second one is not recognized options = TextSearchOptions() - options.include_total_count = True - - total = google_search._get_total_count(response, options) - assert total == 42 - - -async def test__get_total_count_excluded() -> None: - """Test _get_total_count returns None if include_total_count is False.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - response = GoogleSearchResponse() - response.search_information = MagicMock(spec=GoogleSearchInformation) - response.search_information.total_results = 42 - - options = TextSearchOptions() - options.include_total_count = False - - total = google_search._get_total_count(response, options) - assert total is None - - -async def test__get_metadata_includes_search_time() -> None: - """Test _get_metadata includes search_time if present.""" - google_search = GoogleSearch(api_key="fake_api_key", search_engine_id="fake_engine_id") - response = GoogleSearchResponse() - response.search_information = MagicMock(spec=GoogleSearchInformation) - response.search_information.search_time = 1.234 - - metadata = google_search._get_metadata(response) - assert metadata["search_time"] == 1.234 + options.filter.filters = filters + + with patch.object(google_search, "_inner_search", new=AsyncMock(return_value=GoogleSearchResponse())): + await google_search.search(query="hello world", options=options) + + +async def test_google_search_includes_total_count(google_search) -> None: + """Test that total_count is included if include_total_count is True.""" + search_info = GoogleSearchInformation( + searchTime=0.23, totalResults="42", formattedSearchTime="0.23s", formattedTotalResults="42" + ) + mock_response = GoogleSearchResponse(search_information=search_info, items=None) + + with patch.object(google_search, "_inner_search", new=AsyncMock(return_value=mock_response)): + options = TextSearchOptions() + options.include_total_count = True # not standard, so we'll set it dynamically + result = await google_search.search(query="test query", options=options) + assert result.total_count == 42 + # if we set it to false, total_count should be None + options.include_total_count = False + result_no_count = await google_search.search(query="test query", options=options) + assert result_no_count.total_count is None From f234a398bcec0797e6dcd31bc36ad99c38cf3b2f Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Wed, 5 Mar 2025 12:59:04 -0300 Subject: [PATCH 16/18] updating bedrock tests now that the Agent is an ABC --- .../bedrock_agent/test_bedrock_agent_channel.py | 16 ++++++++++++++-- .../test_bedrock_model_provider_utils.py | 17 +---------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py index 8880f7b0df12..a030a637393d 100644 --- a/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py +++ b/python/tests/unit/agents/bedrock_agent/test_bedrock_agent_channel.py @@ -14,6 +14,17 @@ from semantic_kernel.exceptions.agent_exceptions import AgentChatException +class ConcreteAgent(Agent): + async def get_response(self, *args, **kwargs) -> ChatMessageContent: + raise NotImplementedError + + def invoke(self, *args, **kwargs) -> AsyncIterable[ChatMessageContent]: + raise NotImplementedError + + def invoke_stream(self, *args, **kwargs) -> AsyncIterable[StreamingChatMessageContent]: + raise NotImplementedError + + @pytest.fixture def mock_channel(): from semantic_kernel.agents.channels.bedrock_agent_channel import BedrockAgentChannel @@ -135,7 +146,7 @@ async def test_invoke_raises_exception_for_non_bedrock_agent(mock_channel): mock_channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) # Create a dummy agent that is not BedrockAgent - non_bedrock_agent = Agent() + non_bedrock_agent = ConcreteAgent() with pytest.raises(AgentChatException) as exc_info: _ = [msg async for msg in mock_channel.invoke(non_bedrock_agent)] @@ -189,7 +200,8 @@ async def test_invoke_stream_raises_error_for_non_bedrock_agent(mock_channel): """Test invoke_stream raises AgentChatException if the agent provided is not a BedrockAgent.""" mock_channel.messages.append(ChatMessageContent(role=AuthorRole.USER, content="User message")) - non_bedrock_agent = Agent() + non_bedrock_agent = ConcreteAgent() + with pytest.raises(AgentChatException) as exc_info: _ = [msg async for msg in mock_channel.invoke_stream(non_bedrock_agent, [])] diff --git a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py index f08153a5ff79..c62aab95efc8 100644 --- a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py +++ b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py @@ -1,6 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import MagicMock import pytest @@ -13,7 +13,6 @@ finish_reason_from_bedrock_to_semantic_kernel, format_bedrock_function_name_to_kernel_function_fully_qualified_name, remove_none_recursively, - run_in_executor, update_settings_from_function_choice_configuration, ) from semantic_kernel.connectors.ai.function_call_choice_configuration import FunctionCallChoiceConfiguration @@ -164,20 +163,6 @@ def test_inference_profile_with_bedrock_model() -> None: BedrockModelProvider.to_model_provider(unknown_inference_profile) -async def test_run_in_executor_calls_executor_properly() -> None: - """Test that run_in_executor properly calls an executor and function.""" - mock_executor = MagicMock() - mock_func = MagicMock(return_value="test_result") - - with patch( - "asyncio.get_event_loop", return_value=MagicMock(run_in_executor=AsyncMock(return_value="test_result")) - ) as evt: - result = await run_in_executor(mock_executor, mock_func, "arg1", key="value") - - evt.assert_called_once() - assert result == "test_result" - - def test_remove_none_recursively_empty_dict() -> None: """Test that an empty dict returns an empty dict.""" assert remove_none_recursively({}) == {} From bce5c58aeec476fa37b4c01af8f555f3ca206f4a Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Fri, 7 Mar 2025 07:30:58 -0300 Subject: [PATCH 17/18] TODO updates to check tests --- python/tests/unit/connectors/search/bing/test_bing_search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/unit/connectors/search/bing/test_bing_search.py b/python/tests/unit/connectors/search/bing/test_bing_search.py index 348c701ab20b..c7a74c4a04d5 100644 --- a/python/tests/unit/connectors/search/bing/test_bing_search.py +++ b/python/tests/unit/connectors/search/bing/test_bing_search.py @@ -239,7 +239,7 @@ async def test_search_no_filter(bing_search, async_client_mock, mock_bing_search assert params["count"] == options.top assert params["offset"] == options.skip - # TODO: shouldn't this output be "test query" instead of "test query+"? + # TODO check: shouldn't this output be "test query" instead of "test query+"? assert params["q"] == "test query+" From 8f62a960b940f59fffe3d4883d875223a8aa1ce2 Mon Sep 17 00:00:00 2001 From: Rodrigo Racanicci Date: Fri, 7 Mar 2025 07:46:23 -0300 Subject: [PATCH 18/18] updating tests --- .../test_bedrock_model_provider_utils.py | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py index c62aab95efc8..ef279db2521d 100644 --- a/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py +++ b/python/tests/unit/connectors/ai/bedrock/services/test_bedrock_model_provider_utils.py @@ -11,7 +11,6 @@ from semantic_kernel.connectors.ai.bedrock.services.model_provider.utils import ( MESSAGE_CONVERTERS, finish_reason_from_bedrock_to_semantic_kernel, - format_bedrock_function_name_to_kernel_function_fully_qualified_name, remove_none_recursively, update_settings_from_function_choice_configuration, ) @@ -260,7 +259,7 @@ def test_format_assistant_message_function_call_content() -> None: tool_use = formatted["content"][0].get("toolUse") assert tool_use assert tool_use["toolUseId"] == "fc1" - assert tool_use["name"] == "plugin_function" + assert tool_use["name"] == "plugin-function" assert tool_use["input"] == {"param": "value"} @@ -318,20 +317,6 @@ def test_format_tool_message_image_raises() -> None: assert "Image content is not supported in a tool message." in str(exc.value) -def test_format_bedrock_function_name_to_kernel_function_fully_qualified_name_no_sep() -> None: - """Test function names without underscore remain unchanged.""" - name = "SinglePartName" - result = format_bedrock_function_name_to_kernel_function_fully_qualified_name(name) - assert result == name - - -def test_format_bedrock_function_name_to_kernel_function_fully_qualified_name_with_sep() -> None: - """Test function names with underscore are split into plugin/function with the default sep.""" - name = "plugin_function" - result = format_bedrock_function_name_to_kernel_function_fully_qualified_name(name) - assert result == "plugin-function" - - def test_finish_reason_from_bedrock_to_semantic_kernel_stop() -> None: """Test that 'stop_sequence' maps to FinishReason.STOP""" reason = finish_reason_from_bedrock_to_semantic_kernel("stop_sequence") @@ -377,7 +362,7 @@ def mock_function_choice_config() -> FunctionCallChoiceConfiguration: # We'll create mock kernel functions with metadata mock_func_1 = MagicMock() - mock_func_1.custom_fully_qualified_name.return_value = "plugin_function1" + mock_func_1.fully_qualified_name = "plugin-function1" mock_func_1.description = "Function 1 description" param1 = MagicMock() @@ -395,7 +380,7 @@ def mock_function_choice_config() -> FunctionCallChoiceConfiguration: param2, ] mock_func_2 = MagicMock() - mock_func_2.custom_fully_qualified_name.return_value = "plugin_function2" + mock_func_2.fully_qualified_name = "plugin-function2" mock_func_2.description = "Function 2 description" mock_func_2.parameters = [] @@ -427,7 +412,7 @@ def test_update_settings_from_function_choice_configuration_auto_two_tools( assert len(mock_bedrock_settings.tools) == 2 # Validate structure of first tool tool_spec_1 = mock_bedrock_settings.tools[0].get("toolSpec") - assert tool_spec_1["name"] == "plugin_function1" + assert tool_spec_1["name"] == "plugin-function1" assert tool_spec_1["description"] == "Function 1 description" @@ -445,7 +430,7 @@ def test_update_settings_from_function_choice_configuration_required_many( def test_update_settings_from_function_choice_configuration_required_one(mock_bedrock_settings) -> None: """Test that REQUIRED with a single function picks "tool" with that function name.""" single_func = MagicMock() - single_func.custom_fully_qualified_name.return_value = "plugin_function" + single_func.fully_qualified_name = "plugin-function" single_func.description = "Only function" single_func.parameters = [] @@ -453,6 +438,6 @@ def test_update_settings_from_function_choice_configuration_required_one(mock_be config.available_functions = [single_func] update_settings_from_function_choice_configuration(config, mock_bedrock_settings, FunctionChoiceType.REQUIRED) - assert mock_bedrock_settings.tool_choice == {"tool": {"name": "plugin_function"}} + assert mock_bedrock_settings.tool_choice == {"tool": {"name": "plugin-function"}} assert len(mock_bedrock_settings.tools) == 1 - assert mock_bedrock_settings.tools[0]["toolSpec"]["name"] == "plugin_function" + assert mock_bedrock_settings.tools[0]["toolSpec"]["name"] == "plugin-function"