diff --git a/src/adcp/client.py b/src/adcp/client.py index d60f2a5..8aee77d 100644 --- a/src/adcp/client.py +++ b/src/adcp/client.py @@ -159,19 +159,19 @@ async def list_creative_formats( type=ActivityType.PROTOCOL_REQUEST, operation_id=operation_id, agent_id=self.agent_config.id, - task_type="update_media_buy", + task_type="list_creative_formats", timestamp=datetime.now(timezone.utc).isoformat(), ) ) - result = await self.adapter.call_tool("update_media_buy", params) + result = await self.adapter.call_tool("list_creative_formats", params) self._emit_activity( Activity( type=ActivityType.PROTOCOL_RESPONSE, operation_id=operation_id, agent_id=self.agent_config.id, - task_type="update_media_buy", + task_type="list_creative_formats", status=result.status, timestamp=datetime.now(timezone.utc).isoformat(), ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 3744778..d4869bc 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -252,34 +252,6 @@ def test_invalid_protocol(self, tmp_path, monkeypatch): class TestCLIIntegration: """Integration tests for CLI (with mocked network calls).""" - @pytest.mark.asyncio - async def test_tool_execution_flow(self, tmp_path, monkeypatch): - """Test complete tool execution flow (mocked).""" - # Setup config - config_file = tmp_path / "config.json" - config_data = { - "agents": { - "test": { - "id": "test", - "agent_uri": "https://test.com", - "protocol": "mcp", - } - } - } - config_file.write_text(json.dumps(config_data)) - - import adcp.config - monkeypatch.setattr(adcp.config, "CONFIG_FILE", config_file) - - # This is an integration test concept - would need actual mocking - # of ADCPClient to fully test. Showing the pattern here. - # In practice, you'd mock the client's call_tool method. - - def test_json_output_format(self): - """Test that --json flag produces valid JSON output.""" - # Would require mocking the actual tool call - # Conceptual test showing what we'd verify - pass class TestSpecialCharactersInPayload: diff --git a/tests/test_client.py b/tests/test_client.py index c2764cf..2acbfe9 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -97,7 +97,8 @@ async def test_get_products(): request = GetProductsRequest(brief="test campaign") result = await client.get_products(request) - mock_call.assert_called_once() + # Verify correct tool name is called + mock_call.assert_called_once_with("get_products", {"brief": "test campaign"}) assert result.success is True assert result.status == TaskStatus.COMPLETED assert "products" in result.data @@ -126,11 +127,76 @@ async def test_all_client_methods(): assert hasattr(client, "provide_performance_feedback") +@pytest.mark.parametrize( + "method_name,request_class,request_data", + [ + ("get_products", "GetProductsRequest", {}), + ("list_creative_formats", "ListCreativeFormatsRequest", {}), + ("sync_creatives", "SyncCreativesRequest", {"creatives": []}), + ("list_creatives", "ListCreativesRequest", {}), + ("get_media_buy_delivery", "GetMediaBuyDeliveryRequest", {}), + ("list_authorized_properties", "ListAuthorizedPropertiesRequest", {}), + ("get_signals", "GetSignalsRequest", {"signal_spec": "test", "deliver_to": {}}), + ( + "activate_signal", + "ActivateSignalRequest", + {"signal_agent_segment_id": "test", "platform": "test"}, + ), + ( + "provide_performance_feedback", + "ProvidePerformanceFeedbackRequest", + {"media_buy_id": "test", "measurement_period": {}, "performance_index": 0.5}, + ), + ], +) +@pytest.mark.asyncio +async def test_method_calls_correct_tool_name(method_name, request_class, request_data): + """Test that each method calls adapter.call_tool with the correct tool name. + + This test prevents copy-paste bugs where method bodies are copied but + tool names aren't updated to match the method name. + """ + from unittest.mock import patch + from adcp.types.core import TaskResult, TaskStatus + import adcp.types.generated as gen + + config = AgentConfig( + id="test_agent", + agent_uri="https://test.example.com", + protocol=Protocol.A2A, + ) + + client = ADCPClient(config) + + # Create request instance with required fields + request_cls = getattr(gen, request_class) + request = request_cls(**request_data) + + mock_result = TaskResult( + status=TaskStatus.COMPLETED, + data={}, + success=True, + ) + + with patch.object(client.adapter, "call_tool", return_value=mock_result) as mock_call: + method = getattr(client, method_name) + await method(request) + + # CRITICAL: Verify the tool name matches the method name + mock_call.assert_called_once() + actual_tool_name = mock_call.call_args[0][0] + assert actual_tool_name == method_name, ( + f"Method {method_name} called tool '{actual_tool_name}' instead of '{method_name}'. " + f"This is likely a copy-paste bug." + ) + + @pytest.mark.asyncio async def test_multi_agent_parallel_execution(): """Test parallel execution across multiple agents.""" from unittest.mock import patch from adcp.types.core import TaskResult, TaskStatus + from adcp.types.generated import GetProductsRequest agents = [ AgentConfig( @@ -153,11 +219,19 @@ async def test_multi_agent_parallel_execution(): success=True, ) - # Mock both agents' adapters - for agent_client in client.agents.values(): - with patch.object(agent_client.adapter, "call_tool", return_value=mock_result): - pass - - # Test that get_products can be called on multi-agent client - # (actual execution would require proper mocking of asyncio.gather) - assert callable(client.get_products) + # Mock both agents' adapters - keep context active during execution + with patch.object( + client.agents["agent1"].adapter, "call_tool", return_value=mock_result + ) as mock1, patch.object( + client.agents["agent2"].adapter, "call_tool", return_value=mock_result + ) as mock2: + request = GetProductsRequest(brief="test") + results = await client.get_products(request) + + # Verify both agents were called with correct tool name + mock1.assert_called_once_with("get_products", {"brief": "test"}) + mock2.assert_called_once_with("get_products", {"brief": "test"}) + + # Verify results from both agents + assert len(results) == 2 + assert all(r.success for r in results) diff --git a/tests/test_protocols.py b/tests/test_protocols.py index a57daf4..9e8738b 100644 --- a/tests/test_protocols.py +++ b/tests/test_protocols.py @@ -55,6 +55,26 @@ async def test_call_tool_success(self, a2a_config): with patch.object(adapter, "_get_client", return_value=mock_client): result = await adapter.call_tool("get_products", {"brief": "test"}) + # Verify the adapter logic - check HTTP request details + mock_client.post.assert_called_once() + call_args = mock_client.post.call_args + + # Verify URL includes /message/send endpoint + assert call_args[0][0] == "https://a2a.example.com/message/send" + + # Verify headers include auth token (default auth_type is "token", not "bearer") + headers = call_args[1]["headers"] + assert "x-adcp-auth" in headers + assert headers["x-adcp-auth"] == "test_token" + + # Verify request body structure matches A2A spec + json_body = call_args[1]["json"] + assert "message" in json_body + assert json_body["message"]["role"] == "user" + assert "parts" in json_body["message"] + assert "context_id" in json_body + + # Verify result parsing assert result.success is True assert result.status == TaskStatus.COMPLETED assert result.data == {"result": "success"} @@ -78,6 +98,14 @@ async def test_call_tool_failure(self, a2a_config): with patch.object(adapter, "_get_client", return_value=mock_client): result = await adapter.call_tool("get_products", {"brief": "test"}) + # Verify HTTP request was made with correct parameters + mock_client.post.assert_called_once() + call_args = mock_client.post.call_args + assert call_args[0][0] == "https://a2a.example.com/message/send" + assert call_args[1]["headers"]["x-adcp-auth"] == "test_token" + assert "message" in call_args[1]["json"] + + # Verify failure handling assert result.success is False assert result.status == TaskStatus.FAILED @@ -103,9 +131,22 @@ async def test_list_tools(self, a2a_config): with patch.object(adapter, "_get_client", return_value=mock_client): tools = await adapter.list_tools() + # Verify agent card URL construction (A2A spec uses agent.json) + mock_client.get.assert_called_once() + call_args = mock_client.get.call_args + expected_url = "https://a2a.example.com/.well-known/agent.json" + assert call_args[0][0] == expected_url + + # Verify auth headers are included (default auth_type is "token") + headers = call_args[1]["headers"] + assert "x-adcp-auth" in headers + assert headers["x-adcp-auth"] == "test_token" + + # Verify tool list parsing assert len(tools) == 3 assert "get_products" in tools assert "create_media_buy" in tools + assert "list_creative_formats" in tools class TestMCPAdapter: @@ -125,6 +166,15 @@ async def test_call_tool_success(self, mcp_config): with patch.object(adapter, "_get_session", return_value=mock_session): result = await adapter.call_tool("get_products", {"brief": "test"}) + # Verify MCP protocol details - tool name and arguments + mock_session.call_tool.assert_called_once() + call_args = mock_session.call_tool.call_args + + # Verify tool name and params are passed as positional args + assert call_args[0][0] == "get_products" + assert call_args[0][1] == {"brief": "test"} + + # Verify result parsing assert result.success is True assert result.status == TaskStatus.COMPLETED assert result.data == [{"type": "text", "text": "Success"}] @@ -140,6 +190,13 @@ async def test_call_tool_error(self, mcp_config): with patch.object(adapter, "_get_session", return_value=mock_session): result = await adapter.call_tool("get_products", {"brief": "test"}) + # Verify call_tool was attempted with correct parameters (positional args) + mock_session.call_tool.assert_called_once() + call_args = mock_session.call_tool.call_args + assert call_args[0][0] == "get_products" + assert call_args[0][1] == {"brief": "test"} + + # Verify error handling assert result.success is False assert result.status == TaskStatus.FAILED assert "Connection failed" in result.error @@ -161,6 +218,10 @@ async def test_list_tools(self, mcp_config): with patch.object(adapter, "_get_session", return_value=mock_session): tools = await adapter.list_tools() + # Verify list_tools was called on the session + mock_session.list_tools.assert_called_once() + + # Verify adapter correctly extracts tool names from MCP response assert len(tools) == 2 assert "get_products" in tools assert "create_media_buy" in tools