From ce611dac8a2fb9dbaf619c26dac22c174ea936da Mon Sep 17 00:00:00 2001 From: Stenal P Jolly Date: Wed, 12 Aug 2026 13:57:34 +0530 Subject: [PATCH 1/4] test: expand E2E integration test suite for cross-SDK parity - Add missing toolset and tool loading negative error tests - Add argument validation tests for missing and wrong parameter types - Add bound parameter schema pruning assertion in live integration tests - Add auth failure and missing token negative test cases - Add complex data types suite covering optional search-rows and process-data - Add protocol version selection and client headers E2E tests - Add TOOLBOX_SERVER_URL environment support to ToolboxE2ESetup --- ...Test.java => McpToolboxClientE2ETest.java} | 121 ++++++++++++- .../e2e/McpToolboxComplexTypesE2ETest.java | 169 ++++++++++++++++++ .../mcp/e2e/McpToolboxProtocolE2ETest.java | 85 +++++++++ .../google/cloud/mcp/e2e/ToolboxE2ESetup.java | 24 ++- 4 files changed, 390 insertions(+), 9 deletions(-) rename src/test/java/com/google/cloud/mcp/e2e/{McpToolboxClientTest.java => McpToolboxClientE2ETest.java} (61%) create mode 100644 src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java create mode 100644 src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientTest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java similarity index 61% rename from src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientTest.java rename to src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java index 883557b..9f4f074 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientTest.java +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java @@ -18,6 +18,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.cloud.mcp.McpToolboxClient; @@ -26,10 +28,14 @@ import com.google.cloud.mcp.tool.ToolResult; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.RegisterExtension; +@Timeout(value = 60, unit = TimeUnit.SECONDS) class McpToolboxClientE2ETest { @RegisterExtension static ToolboxE2ESetup server = new ToolboxE2ESetup(); @@ -41,7 +47,7 @@ void setUp() { client = McpToolboxClient.builder().baseUrl(server.getBaseUrl()).build(); } - // --- TestBasicE2E --- + // --- Toolset Loading & Error Tests --- @Test void testLoadToolsetSpecific() { @@ -68,15 +74,31 @@ void testLoadToolsetDefault() { assertTrue(tools.containsKey("process-data")); } + @Test + void testLoadNonExistentToolset() { + assertThrows( + Exception.class, + () -> { + client.loadToolset("non-existent-toolset").join(); + }); + } + + @Test + void testLoadNonExistentTool() { + assertThrows( + Exception.class, + () -> { + client.loadTool("non-existent-tool").join(); + }); + } + + // --- Tool Invocation & Argument Validations --- + @Test void testRunTool() { Tool tool = client.loadTool("get-n-rows").join(); ToolResult result = tool.execute(Map.of("num_rows", "2")).join(); - if (result.isError()) { - System.out.println("ERROR OUTPUT: " + getTextContent(result)); - } - assertFalse( result.isError(), "Expected successful result, but got error: " + getTextContent(result)); String output = getTextContent(result); @@ -85,7 +107,43 @@ void testRunTool() { assertFalse(output.contains("row3")); } - // --- TestBindParams --- + @Test + void testRunToolMissingRequiredParams() { + Tool tool = client.loadTool("get-n-rows").join(); + CompletionException ex = + assertThrows( + CompletionException.class, + () -> { + tool.execute(Map.of()).join(); + }); + assertNotNull(ex.getCause()); + assertTrue( + ex.getCause() instanceof IllegalArgumentException, + "Expected IllegalArgumentException but got: " + ex.getCause().getClass().getName()); + assertTrue( + ex.getCause().getMessage().contains("Missing required parameter 'num_rows'"), + "Unexpected message: " + ex.getCause().getMessage()); + } + + @Test + void testRunToolWrongParamType() { + Tool tool = client.loadTool("get-n-rows").join(); + CompletionException ex = + assertThrows( + CompletionException.class, + () -> { + tool.execute(Map.of("num_rows", 2)).join(); + }); + assertNotNull(ex.getCause()); + assertTrue( + ex.getCause() instanceof IllegalArgumentException, + "Expected IllegalArgumentException but got: " + ex.getCause().getClass().getName()); + assertTrue( + ex.getCause().getMessage().contains("expected type 'string'"), + "Unexpected message: " + ex.getCause().getMessage()); + } + + // --- Parameter Binding & Schema Pruning --- @Test void testBindParams() { @@ -115,7 +173,23 @@ void testBindParamsCallable() { assertFalse(output.contains("row4")); } - // --- TestAuth --- + @Test + void testBoundParamPruningSchema() { + Tool tool = client.loadTool("get-n-rows").join(); + boolean hadParam = + tool.definition().parameters() != null + && tool.definition().parameters().stream().anyMatch(p -> "num_rows".equals(p.name())); + assertTrue(hadParam, "Original tool definition should have 'num_rows' parameter"); + + Tool boundTool = tool.bindParam("num_rows", "3"); + boolean hasParamAfter = + boundTool.definition().parameters() != null + && boundTool.definition().parameters().stream() + .anyMatch(p -> "num_rows".equals(p.name())); + assertFalse(hasParamAfter, "Bound parameter 'num_rows' must be pruned from definition schema"); + } + + // --- Authentication & Claim Injections --- @Test void testRunToolAuth() { @@ -150,6 +224,22 @@ void testRunToolWrongAuth() { "Actual output: " + getTextContent(result)); } + @Test + void testRunToolAuthWithoutProvidingAuth() { + Tool tool = client.loadTool("get-row-by-id-auth").join(); + // Running authenticated tool without adding auth token getter + try { + ToolResult result = tool.execute(Map.of("id", "2")).join(); + assertTrue( + result.isError(), + "Expected error when invoking tool without auth token. Output: " + + getTextContent(result)); + } catch (CompletionException e) { + // An exception on unauthenticated execution is also valid + assertNotNull(e.getCause()); + } + } + @Test void testRunToolParamAuth() { Tool tool = @@ -181,6 +271,23 @@ void testRunToolParamAuthNoField() { assertTrue(getTextContent(result).contains("no field named row_data")); } + @Test + void testRunToolWithFailingTokenSupplier() { + Tool tool = + client + .loadTool("get-row-by-id-auth") + .join() + .addAuthTokenGetter( + "my-test-auth", + () -> CompletableFuture.failedFuture(new RuntimeException("Token unavailable"))); + + assertThrows( + Exception.class, + () -> { + tool.execute(Map.of("id", "2")).join(); + }); + } + private String getTextContent(ToolResult result) { if (result.content() == null) return ""; return result.content().stream() diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java new file mode 100644 index 0000000..59781d9 --- /dev/null +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java @@ -0,0 +1,169 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.mcp.e2e; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.google.cloud.mcp.McpToolboxClient; +import com.google.cloud.mcp.tool.Tool; +import com.google.cloud.mcp.tool.ToolDefinition; +import com.google.cloud.mcp.tool.ToolResult; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CompletionException; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.extension.RegisterExtension; + +@Timeout(value = 60, unit = TimeUnit.SECONDS) +class McpToolboxComplexTypesE2ETest { + + @RegisterExtension static ToolboxE2ESetup server = new ToolboxE2ESetup(); + + private McpToolboxClient client; + + @BeforeEach + void setUp() { + client = McpToolboxClient.builder().baseUrl(server.getBaseUrl()).build(); + } + + // --- Optional Parameters Suite (search-rows) --- + + @Test + void testSearchRowsDefinitionSchema() { + Tool tool = client.loadTool("search-rows").join(); + assertEquals("search-rows", tool.name()); + assertNotNull(tool.definition()); + + boolean hasEmail = false; + boolean hasData = false; + boolean hasId = false; + + if (tool.definition().parameters() != null) { + for (ToolDefinition.Parameter p : tool.definition().parameters()) { + if ("email".equals(p.name())) { + hasEmail = true; + assertTrue(p.required(), "Parameter 'email' should be required"); + } else if ("data".equals(p.name())) { + hasData = true; + assertFalse(p.required(), "Parameter 'data' should be optional"); + } else if ("id".equals(p.name())) { + hasId = true; + assertFalse(p.required(), "Parameter 'id' should be optional"); + } + } + } + assertTrue(hasEmail, "Missing required parameter 'email' in definition"); + assertTrue(hasData, "Missing optional parameter 'data' in definition"); + assertTrue(hasId, "Missing optional parameter 'id' in definition"); + } + + @Test + void testSearchRowsOmittingOptionals() { + Tool tool = client.loadTool("search-rows").join(); + ToolResult result = tool.execute(Map.of("email", "twishabansal@google.com")).join(); + + assertFalse(result.isError(), "Expected success: " + getTextContent(result)); + String output = getTextContent(result); + assertTrue(output.contains("twishabansal@google.com"), "Output: " + output); + assertTrue(output.contains("row2"), "Output: " + output); + assertFalse(output.contains("row1"), "Output should not contain row1: " + output); + assertFalse(output.contains("row3"), "Output should not contain row3: " + output); + } + + @Test + void testSearchRowsWithAllParamsProvided() { + Tool tool = client.loadTool("search-rows").join(); + Map args = new HashMap<>(); + args.put("email", "twishabansal@google.com"); + args.put("data", "row3"); + args.put("id", 2); + + ToolResult result = tool.execute(args).join(); + assertFalse(result.isError(), "Expected success: " + getTextContent(result)); + String output = getTextContent(result); + assertTrue(output.contains("twishabansal@google.com")); + assertTrue(output.contains("row2")); + } + + @Test + void testSearchRowsMissingRequiredParam() { + Tool tool = client.loadTool("search-rows").join(); + CompletionException ex = + assertThrows( + CompletionException.class, + () -> { + tool.execute(Map.of("data", "row3")).join(); + }); + assertNotNull(ex.getCause()); + assertTrue( + ex.getCause().getMessage().contains("Missing required parameter 'email'"), + "Unexpected message: " + ex.getCause().getMessage()); + } + + @Test + void testSearchRowsNonMatchingData() { + Tool tool = client.loadTool("search-rows").join(); + ToolResult result = + tool.execute( + Map.of("email", "twishabansal@google.com", "data", "non-existent-row-pattern-xyz")) + .join(); + + assertFalse(result.isError()); + String output = getTextContent(result); + assertFalse(output.contains("row1")); + assertFalse(output.contains("row3")); + } + + // --- Map / Structured Payloads Suite (process-data) --- + + @Test + void testProcessDataDefinitionSchema() { + Tool tool = client.loadTool("process-data").join(); + assertEquals("process-data", tool.name()); + assertNotNull(tool.definition()); + } + + @Test + void testProcessDataWithMapParams() { + Tool tool = client.loadTool("process-data").join(); + Map inputData = + Map.of("key1", "val1", "count", 5, "nested", Map.of("inner", "value")); + + try { + ToolResult result = tool.execute(Map.of("input_data", inputData)).join(); + assertNotNull(result); + } catch (Exception e) { + // In case server expects specific parameters for process-data + assertNotNull(e); + } + } + + private String getTextContent(ToolResult result) { + if (result.content() == null) return ""; + return result.content().stream() + .filter(c -> "text".equals(c.type()) && c.text() != null) + .map(c -> c.text()) + .collect(java.util.stream.Collectors.joining("\n")); + } +} diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java new file mode 100644 index 0000000..dee49b5 --- /dev/null +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java @@ -0,0 +1,85 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.mcp.e2e; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.google.cloud.mcp.McpToolboxClient; +import com.google.cloud.mcp.ProtocolVersion; +import com.google.cloud.mcp.tool.Tool; +import com.google.cloud.mcp.tool.ToolResult; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.extension.RegisterExtension; + +@Timeout(value = 60, unit = TimeUnit.SECONDS) +class McpToolboxProtocolE2ETest { + + @RegisterExtension static ToolboxE2ESetup server = new ToolboxE2ESetup(); + + @Test + void testClientWithCustomHeaders() { + McpToolboxClient client = + McpToolboxClient.builder() + .baseUrl(server.getBaseUrl()) + .headers(Map.of("X-Integration-Test-Suite", "Java-Parity-E2E")) + .build(); + + Tool tool = client.loadTool("get-n-rows").join(); + assertNotNull(tool); + ToolResult result = tool.execute(Map.of("num_rows", "2")).join(); + assertFalse(result.isError()); + String output = getTextContent(result); + assertTrue(output.contains("row1")); + assertTrue(output.contains("row2")); + } + + @Test + void testClientWithExplicitProtocolVersions() { + ProtocolVersion[] versions = + new ProtocolVersion[] { + ProtocolVersion.VERSION_2024_11_05, + ProtocolVersion.VERSION_2025_03_26, + ProtocolVersion.VERSION_2025_06_18, + ProtocolVersion.VERSION_2025_11_25 + }; + + for (ProtocolVersion version : versions) { + McpToolboxClient client = + McpToolboxClient.builder().baseUrl(server.getBaseUrl()).protocolVersion(version).build(); + + Tool tool = client.loadTool("get-n-rows").join(); + assertNotNull(tool, "Failed to load tool with protocol " + version); + ToolResult result = tool.execute(Map.of("num_rows", "1")).join(); + assertFalse(result.isError(), "Execution failed for protocol " + version); + String output = getTextContent(result); + assertTrue(output.contains("row1"), "Expected row1 for protocol " + version); + } + } + + private String getTextContent(ToolResult result) { + if (result.content() == null) return ""; + return result.content().stream() + .filter(c -> "text".equals(c.type()) && c.text() != null) + .map(c -> c.text()) + .collect(java.util.stream.Collectors.joining("\n")); + } +} diff --git a/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java b/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java index 28a3757..b495b7a 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java +++ b/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java @@ -39,6 +39,7 @@ public class ToolboxE2ESetup implements BeforeAllCallback, AfterAllCallback { private static final String PROJECT_ID_ENV = "GOOGLE_CLOUD_PROJECT"; private static final String TOOLBOX_VERSION_ENV = "TOOLBOX_VERSION"; private static final String TOOLBOX_MANIFEST_VERSION_ENV = "TOOLBOX_MANIFEST_VERSION"; + private static final String TOOLBOX_SERVER_URL_ENV = "TOOLBOX_SERVER_URL"; private static final String BINARY_NAME = "toolbox"; private Process serverProcess; @@ -48,10 +49,25 @@ public class ToolboxE2ESetup implements BeforeAllCallback, AfterAllCallback { @Override public void beforeAll(ExtensionContext context) throws Exception { + String serverUrl = System.getenv(TOOLBOX_SERVER_URL_ENV); String projectId = System.getenv(PROJECT_ID_ENV); + org.junit.jupiter.api.Assumptions.assumeTrue( - projectId != null && !projectId.trim().isEmpty(), - "Skipping E2E tests because " + PROJECT_ID_ENV + " is not set."); + (projectId != null && !projectId.trim().isEmpty()) + || (serverUrl != null && !serverUrl.trim().isEmpty()), + "Skipping E2E tests because neither " + + PROJECT_ID_ENV + + " nor " + + TOOLBOX_SERVER_URL_ENV + + " is set."); + + // If an external server URL is provided without GCP project, use it directly + if (serverUrl != null + && !serverUrl.trim().isEmpty() + && (projectId == null || projectId.trim().isEmpty())) { + logger.info("Using pre-configured TOOLBOX_SERVER_URL: " + serverUrl); + return; + } String toolboxVersion = getEnvVar(TOOLBOX_VERSION_ENV); String manifestVersion = getEnvVar(TOOLBOX_MANIFEST_VERSION_ENV); @@ -124,6 +140,10 @@ public void afterAll(ExtensionContext context) { } public String getBaseUrl() { + String envUrl = System.getenv(TOOLBOX_SERVER_URL_ENV); + if (envUrl != null && !envUrl.trim().isEmpty()) { + return envUrl; + } return "http://localhost:5000/mcp"; } From e827baaccfe743b574b8927cf7a216d78ffb7d06 Mon Sep 17 00:00:00 2001 From: Stenal P Jolly Date: Wed, 12 Aug 2026 14:01:31 +0530 Subject: [PATCH 2/4] test: align search-rows test arguments with expected dataset --- .../google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java index 59781d9..e788375 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java @@ -97,13 +97,14 @@ void testSearchRowsWithAllParamsProvided() { Map args = new HashMap<>(); args.put("email", "twishabansal@google.com"); args.put("data", "row3"); - args.put("id", 2); + args.put("id", 3L); ToolResult result = tool.execute(args).join(); assertFalse(result.isError(), "Expected success: " + getTextContent(result)); String output = getTextContent(result); assertTrue(output.contains("twishabansal@google.com")); - assertTrue(output.contains("row2")); + assertTrue(output.contains("row3")); + assertFalse(output.contains("row2")); } @Test From e6826b99a6d7388596f51e79a8c39173b24e0a49 Mon Sep 17 00:00:00 2001 From: Stenal P Jolly Date: Thu, 3 Sep 2026 15:00:11 +0530 Subject: [PATCH 3/4] test: resolve PR review feedback and align integration test parity - Correct process-data parameter payload to use execution_context, user_scores, and feature_flags instead of fictional input_data - Remove exception-swallowing try-catch blocks in testProcessDataWithMapParams and testRunToolAuthWithoutProvidingAuth - Implement missing testProcessDataOmittingOptionalMap test method - Add comprehensive parameter schema assertions in testProcessDataDefinitionSchema - Parse JSON structurally in process-data tests to eliminate non-deterministic map ordering flakes - Narrow exception expectations from Exception.class to CompletionException with specific cause message checks - Consolidate duplicated getTextContent helper into ToolboxE2ESetup for DRY compliance - Align testSearchRowsNonMatchingData with expected empty or 'null' return on argument mismatch - Support TOOLBOX_AUTH_TOKEN_1 and TOOLBOX_AUTH_TOKEN_2 fallback environment variables in ToolboxE2ESetup TAG=agy CONV=f9ec8a85-66e3-479e-a944-3b621f8bcd18 --- .../mcp/e2e/McpToolboxClientE2ETest.java | 73 ++++++----- .../e2e/McpToolboxComplexTypesE2ETest.java | 113 ++++++++++++++---- .../mcp/e2e/McpToolboxProtocolE2ETest.java | 9 +- .../google/cloud/mcp/e2e/ToolboxE2ESetup.java | 14 +++ 4 files changed, 145 insertions(+), 64 deletions(-) diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java index 9f4f074..ccf8a8d 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java @@ -16,6 +16,7 @@ package com.google.cloud.mcp.e2e; +import static com.google.cloud.mcp.e2e.ToolboxE2ESetup.getTextContent; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -76,20 +77,31 @@ void testLoadToolsetDefault() { @Test void testLoadNonExistentToolset() { - assertThrows( - Exception.class, - () -> { - client.loadToolset("non-existent-toolset").join(); - }); + CompletionException ex = + assertThrows( + CompletionException.class, + () -> { + client.loadToolset("non-existent-toolset").join(); + }); + assertNotNull(ex.getCause()); + assertTrue( + ex.getCause().getMessage().contains("non-existent-toolset") + || ex.getCause().getMessage().contains("Toolset not found"), + "Unexpected cause: " + ex.getCause().getMessage()); } @Test void testLoadNonExistentTool() { - assertThrows( - Exception.class, - () -> { - client.loadTool("non-existent-tool").join(); - }); + CompletionException ex = + assertThrows( + CompletionException.class, + () -> { + client.loadTool("non-existent-tool").join(); + }); + assertNotNull(ex.getCause()); + assertTrue( + ex.getCause().getMessage().contains("Tool not found: non-existent-tool"), + "Unexpected cause: " + ex.getCause().getMessage()); } // --- Tool Invocation & Argument Validations --- @@ -228,16 +240,14 @@ void testRunToolWrongAuth() { void testRunToolAuthWithoutProvidingAuth() { Tool tool = client.loadTool("get-row-by-id-auth").join(); // Running authenticated tool without adding auth token getter - try { - ToolResult result = tool.execute(Map.of("id", "2")).join(); - assertTrue( - result.isError(), - "Expected error when invoking tool without auth token. Output: " - + getTextContent(result)); - } catch (CompletionException e) { - // An exception on unauthenticated execution is also valid - assertNotNull(e.getCause()); - } + ToolResult result = tool.execute(Map.of("id", "2")).join(); + assertTrue( + result.isError(), + "Expected error when invoking tool without auth token. Output: " + getTextContent(result)); + assertTrue( + getTextContent(result).toLowerCase().contains("unauthorized") + || getTextContent(result).contains("401"), + "Expected unauthorized/401 error message. Actual output: " + getTextContent(result)); } @Test @@ -281,18 +291,15 @@ void testRunToolWithFailingTokenSupplier() { "my-test-auth", () -> CompletableFuture.failedFuture(new RuntimeException("Token unavailable"))); - assertThrows( - Exception.class, - () -> { - tool.execute(Map.of("id", "2")).join(); - }); - } - - private String getTextContent(ToolResult result) { - if (result.content() == null) return ""; - return result.content().stream() - .filter(c -> "text".equals(c.type()) && c.text() != null) - .map(c -> c.text()) - .collect(java.util.stream.Collectors.joining("\n")); + CompletionException ex = + assertThrows( + CompletionException.class, + () -> { + tool.execute(Map.of("id", "2")).join(); + }); + assertNotNull(ex.getCause()); + assertTrue( + ex.getCause().getMessage().contains("Token unavailable"), + "Unexpected cause: " + ex.getCause().getMessage()); } } diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java index e788375..d463cf4 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java @@ -16,12 +16,16 @@ package com.google.cloud.mcp.e2e; +import static com.google.cloud.mcp.e2e.ToolboxE2ESetup.getTextContent; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.cloud.mcp.McpToolboxClient; import com.google.cloud.mcp.tool.Tool; import com.google.cloud.mcp.tool.ToolDefinition; @@ -117,6 +121,9 @@ void testSearchRowsMissingRequiredParam() { tool.execute(Map.of("data", "row3")).join(); }); assertNotNull(ex.getCause()); + assertTrue( + ex.getCause() instanceof IllegalArgumentException, + "Expected IllegalArgumentException but got: " + ex.getCause().getClass().getName()); assertTrue( ex.getCause().getMessage().contains("Missing required parameter 'email'"), "Unexpected message: " + ex.getCause().getMessage()); @@ -125,14 +132,19 @@ void testSearchRowsMissingRequiredParam() { @Test void testSearchRowsNonMatchingData() { Tool tool = client.loadTool("search-rows").join(); - ToolResult result = - tool.execute( - Map.of("email", "twishabansal@google.com", "data", "non-existent-row-pattern-xyz")) - .join(); + Map args = new HashMap<>(); + args.put("email", "twishabansal@google.com"); + args.put("id", 3L); + args.put("data", "row4"); - assertFalse(result.isError()); - String output = getTextContent(result); + ToolResult result = tool.execute(args).join(); + assertFalse(result.isError(), "Expected success: " + getTextContent(result)); + String output = getTextContent(result).trim(); + assertTrue( + output.isEmpty() || "null".equals(output), + "Expected empty or 'null' response for non-matching data, got: " + output); assertFalse(output.contains("row1")); + assertFalse(output.contains("row2")); assertFalse(output.contains("row3")); } @@ -143,28 +155,83 @@ void testProcessDataDefinitionSchema() { Tool tool = client.loadTool("process-data").join(); assertEquals("process-data", tool.name()); assertNotNull(tool.definition()); + + boolean hasExecutionContext = false; + boolean hasUserScores = false; + boolean hasFeatureFlags = false; + + if (tool.definition().parameters() != null) { + for (ToolDefinition.Parameter p : tool.definition().parameters()) { + if ("execution_context".equals(p.name())) { + hasExecutionContext = true; + assertTrue(p.required(), "Parameter 'execution_context' should be required"); + assertNotNull(p.type()); + assertTrue( + "object".equalsIgnoreCase(p.type()), + "Parameter 'execution_context' type should be 'object', got: " + p.type()); + } else if ("user_scores".equals(p.name())) { + hasUserScores = true; + assertTrue(p.required(), "Parameter 'user_scores' should be required"); + assertNotNull(p.type()); + assertTrue( + "object".equalsIgnoreCase(p.type()), + "Parameter 'user_scores' type should be 'object', got: " + p.type()); + } else if ("feature_flags".equals(p.name())) { + hasFeatureFlags = true; + assertFalse(p.required(), "Parameter 'feature_flags' should be optional"); + assertNotNull(p.type()); + assertTrue( + "object".equalsIgnoreCase(p.type()), + "Parameter 'feature_flags' type should be 'object', got: " + p.type()); + } + } + } + assertTrue(hasExecutionContext, "Missing required parameter 'execution_context' in definition"); + assertTrue(hasUserScores, "Missing required parameter 'user_scores' in definition"); + assertTrue(hasFeatureFlags, "Missing optional parameter 'feature_flags' in definition"); } @Test - void testProcessDataWithMapParams() { + void testProcessDataWithMapParams() throws JsonProcessingException { Tool tool = client.loadTool("process-data").join(); - Map inputData = - Map.of("key1", "val1", "count", 5, "nested", Map.of("inner", "value")); - - try { - ToolResult result = tool.execute(Map.of("input_data", inputData)).join(); - assertNotNull(result); - } catch (Exception e) { - // In case server expects specific parameters for process-data - assertNotNull(e); - } + ToolResult result = + tool.execute( + Map.of( + "execution_context", + Map.of("env", "prod", "id", 1234, "user", 1234.5), + "user_scores", + Map.of("user1", 100, "user2", 200), + "feature_flags", + Map.of("new_feature", true))) + .join(); + + assertFalse(result.isError(), "Expected success: " + getTextContent(result)); + String output = getTextContent(result); + JsonNode root = new ObjectMapper().readTree(output); + assertEquals("prod", root.path("execution_context").path("env").asText()); + assertEquals(1234, root.path("execution_context").path("id").asInt()); + assertEquals(1234.5, root.path("execution_context").path("user").asDouble(), 0.001); + assertEquals(100, root.path("user_scores").path("user1").asInt()); + assertEquals(200, root.path("user_scores").path("user2").asInt()); + assertTrue(root.path("feature_flags").path("new_feature").asBoolean()); } - private String getTextContent(ToolResult result) { - if (result.content() == null) return ""; - return result.content().stream() - .filter(c -> "text".equals(c.type()) && c.text() != null) - .map(c -> c.text()) - .collect(java.util.stream.Collectors.joining("\n")); + @Test + void testProcessDataOmittingOptionalMap() throws JsonProcessingException { + Tool tool = client.loadTool("process-data").join(); + ToolResult result = + tool.execute( + Map.of( + "execution_context", Map.of("env", "dev"), "user_scores", Map.of("user3", 300))) + .join(); + + assertFalse(result.isError(), "Expected success: " + getTextContent(result)); + String output = getTextContent(result); + JsonNode root = new ObjectMapper().readTree(output); + assertEquals("dev", root.path("execution_context").path("env").asText()); + assertEquals(300, root.path("user_scores").path("user3").asInt()); + assertTrue( + root.path("feature_flags").isNull() || root.path("feature_flags").isMissingNode(), + "Expected null feature_flags: " + output); } } diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java index dee49b5..13d09c1 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxProtocolE2ETest.java @@ -16,6 +16,7 @@ package com.google.cloud.mcp.e2e; +import static com.google.cloud.mcp.e2e.ToolboxE2ESetup.getTextContent; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -74,12 +75,4 @@ void testClientWithExplicitProtocolVersions() { assertTrue(output.contains("row1"), "Expected row1 for protocol " + version); } } - - private String getTextContent(ToolResult result) { - if (result.content() == null) return ""; - return result.content().stream() - .filter(c -> "text".equals(c.type()) && c.text() != null) - .map(c -> c.text()) - .collect(java.util.stream.Collectors.joining("\n")); - } } diff --git a/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java b/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java index b495b7a..e5d5aaa 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java +++ b/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java @@ -40,6 +40,8 @@ public class ToolboxE2ESetup implements BeforeAllCallback, AfterAllCallback { private static final String TOOLBOX_VERSION_ENV = "TOOLBOX_VERSION"; private static final String TOOLBOX_MANIFEST_VERSION_ENV = "TOOLBOX_MANIFEST_VERSION"; private static final String TOOLBOX_SERVER_URL_ENV = "TOOLBOX_SERVER_URL"; + private static final String TOOLBOX_AUTH_TOKEN_1_ENV = "TOOLBOX_AUTH_TOKEN_1"; + private static final String TOOLBOX_AUTH_TOKEN_2_ENV = "TOOLBOX_AUTH_TOKEN_2"; private static final String BINARY_NAME = "toolbox"; private Process serverProcess; @@ -66,6 +68,8 @@ public void beforeAll(ExtensionContext context) throws Exception { && !serverUrl.trim().isEmpty() && (projectId == null || projectId.trim().isEmpty())) { logger.info("Using pre-configured TOOLBOX_SERVER_URL: " + serverUrl); + authToken1 = System.getenv(TOOLBOX_AUTH_TOKEN_1_ENV); + authToken2 = System.getenv(TOOLBOX_AUTH_TOKEN_2_ENV); return; } @@ -147,6 +151,16 @@ public String getBaseUrl() { return "http://localhost:5000/mcp"; } + public static String getTextContent(com.google.cloud.mcp.tool.ToolResult result) { + if (result == null || result.content() == null) { + return ""; + } + return result.content().stream() + .filter(c -> "text".equals(c.type()) && c.text() != null) + .map(com.google.cloud.mcp.tool.ToolResult.Content::text) + .collect(java.util.stream.Collectors.joining("\n")); + } + private void startServer() throws IOException, InterruptedException { logger.info("Starting Toolbox Server..."); ProcessBuilder pb = From 11e90efebec9738c29d9320b3ac4c37dce279656 Mon Sep 17 00:00:00 2001 From: Stenal P Jolly Date: Thu, 3 Sep 2026 17:59:38 +0530 Subject: [PATCH 4/4] test: address review feedback on schema types, immutability, and server precedence - Align testLoadNonExistentToolset cause check with server error message - Assert parameter immutability on original tool definition after bindParam - Check parameter types ('string', 'integer') in search-rows definition schema - Use LinkedHashMap for map parameters to ensure deterministic key ordering - Ensure TOOLBOX_SERVER_URL takes precedence when configured in test setup TAG=agy CONV=f9ec8a85-66e3-479e-a944-3b621f8bcd18 --- .../mcp/e2e/McpToolboxClientE2ETest.java | 10 ++- .../e2e/McpToolboxComplexTypesE2ETest.java | 84 ++++++++++++------- .../google/cloud/mcp/e2e/ToolboxE2ESetup.java | 14 +++- 3 files changed, 71 insertions(+), 37 deletions(-) diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java index ccf8a8d..ab244e3 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxClientE2ETest.java @@ -85,7 +85,8 @@ void testLoadNonExistentToolset() { }); assertNotNull(ex.getCause()); assertTrue( - ex.getCause().getMessage().contains("non-existent-toolset") + ex.getCause().getMessage().contains("toolset does not exist") + || ex.getCause().getMessage().contains("non-existent-toolset") || ex.getCause().getMessage().contains("Toolset not found"), "Unexpected cause: " + ex.getCause().getMessage()); } @@ -199,6 +200,13 @@ void testBoundParamPruningSchema() { && boundTool.definition().parameters().stream() .anyMatch(p -> "num_rows".equals(p.name())); assertFalse(hasParamAfter, "Bound parameter 'num_rows' must be pruned from definition schema"); + + boolean originalStillHasParam = + tool.definition().parameters() != null + && tool.definition().parameters().stream().anyMatch(p -> "num_rows".equals(p.name())); + assertTrue( + originalStillHasParam, + "Original tool definition must still contain 'num_rows' to ensure immutability"); } // --- Authentication & Claim Injections --- diff --git a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java index d463cf4..f654596 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java +++ b/src/test/java/com/google/cloud/mcp/e2e/McpToolboxComplexTypesE2ETest.java @@ -23,14 +23,12 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.cloud.mcp.McpToolboxClient; import com.google.cloud.mcp.tool.Tool; import com.google.cloud.mcp.tool.ToolDefinition; import com.google.cloud.mcp.tool.ToolResult; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.concurrent.CompletionException; import java.util.concurrent.TimeUnit; @@ -68,12 +66,15 @@ void testSearchRowsDefinitionSchema() { if ("email".equals(p.name())) { hasEmail = true; assertTrue(p.required(), "Parameter 'email' should be required"); + assertEquals("string", p.type()); } else if ("data".equals(p.name())) { hasData = true; assertFalse(p.required(), "Parameter 'data' should be optional"); + assertEquals("string", p.type()); } else if ("id".equals(p.name())) { hasId = true; assertFalse(p.required(), "Parameter 'id' should be optional"); + assertEquals("integer", p.type()); } } } @@ -192,46 +193,65 @@ void testProcessDataDefinitionSchema() { } @Test - void testProcessDataWithMapParams() throws JsonProcessingException { + void testProcessDataWithMapParams() { Tool tool = client.loadTool("process-data").join(); - ToolResult result = - tool.execute( - Map.of( - "execution_context", - Map.of("env", "prod", "id", 1234, "user", 1234.5), - "user_scores", - Map.of("user1", 100, "user2", 200), - "feature_flags", - Map.of("new_feature", true))) - .join(); + Map execCtx = new LinkedHashMap<>(); + execCtx.put("env", "prod"); + execCtx.put("id", 1234); + execCtx.put("user", 1234.5); + + Map userScores = new LinkedHashMap<>(); + userScores.put("user1", 100); + userScores.put("user2", 200); + + Map featureFlags = new LinkedHashMap<>(); + featureFlags.put("new_feature", true); + + Map args = new LinkedHashMap<>(); + args.put("execution_context", execCtx); + args.put("user_scores", userScores); + args.put("feature_flags", featureFlags); + + ToolResult result = tool.execute(args).join(); assertFalse(result.isError(), "Expected success: " + getTextContent(result)); String output = getTextContent(result); - JsonNode root = new ObjectMapper().readTree(output); - assertEquals("prod", root.path("execution_context").path("env").asText()); - assertEquals(1234, root.path("execution_context").path("id").asInt()); - assertEquals(1234.5, root.path("execution_context").path("user").asDouble(), 0.001); - assertEquals(100, root.path("user_scores").path("user1").asInt()); - assertEquals(200, root.path("user_scores").path("user2").asInt()); - assertTrue(root.path("feature_flags").path("new_feature").asBoolean()); + assertTrue( + output.contains("\"execution_context\":{\"env\":\"prod\",\"id\":1234,\"user\":1234.5}"), + "Output did not contain expected execution_context: " + output); + assertTrue( + output.contains("\"user_scores\":{\"user1\":100,\"user2\":200}"), + "Output did not contain expected user_scores: " + output); + assertTrue( + output.contains("\"feature_flags\":{\"new_feature\":true}"), + "Output did not contain expected feature_flags: " + output); } @Test - void testProcessDataOmittingOptionalMap() throws JsonProcessingException { + void testProcessDataOmittingOptionalMap() { Tool tool = client.loadTool("process-data").join(); - ToolResult result = - tool.execute( - Map.of( - "execution_context", Map.of("env", "dev"), "user_scores", Map.of("user3", 300))) - .join(); + Map execCtx = new LinkedHashMap<>(); + execCtx.put("env", "dev"); + + Map userScores = new LinkedHashMap<>(); + userScores.put("user3", 300); + + Map args = new LinkedHashMap<>(); + args.put("execution_context", execCtx); + args.put("user_scores", userScores); + + ToolResult result = tool.execute(args).join(); assertFalse(result.isError(), "Expected success: " + getTextContent(result)); String output = getTextContent(result); - JsonNode root = new ObjectMapper().readTree(output); - assertEquals("dev", root.path("execution_context").path("env").asText()); - assertEquals(300, root.path("user_scores").path("user3").asInt()); assertTrue( - root.path("feature_flags").isNull() || root.path("feature_flags").isMissingNode(), - "Expected null feature_flags: " + output); + output.contains("\"execution_context\":{\"env\":\"dev\"}"), + "Output did not contain expected execution_context: " + output); + assertTrue( + output.contains("\"user_scores\":{\"user3\":300}"), + "Output did not contain expected user_scores: " + output); + assertTrue( + output.contains("\"feature_flags\":null"), + "Output did not contain expected null feature_flags: " + output); } } diff --git a/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java b/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java index e5d5aaa..b2bea2a 100644 --- a/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java +++ b/src/test/java/com/google/cloud/mcp/e2e/ToolboxE2ESetup.java @@ -63,13 +63,19 @@ public void beforeAll(ExtensionContext context) throws Exception { + TOOLBOX_SERVER_URL_ENV + " is set."); - // If an external server URL is provided without GCP project, use it directly - if (serverUrl != null - && !serverUrl.trim().isEmpty() - && (projectId == null || projectId.trim().isEmpty())) { + // If an external server URL is provided, use it directly + if (serverUrl != null && !serverUrl.trim().isEmpty()) { logger.info("Using pre-configured TOOLBOX_SERVER_URL: " + serverUrl); authToken1 = System.getenv(TOOLBOX_AUTH_TOKEN_1_ENV); authToken2 = System.getenv(TOOLBOX_AUTH_TOKEN_2_ENV); + if (authToken1 == null && projectId != null && !projectId.trim().isEmpty()) { + String client1Id = accessSecretVersion(projectId, "sdk_testing_client1", "latest"); + authToken1 = getAuthToken(client1Id); + } + if (authToken2 == null && projectId != null && !projectId.trim().isEmpty()) { + String client2Id = accessSecretVersion(projectId, "sdk_testing_client2", "latest"); + authToken2 = getAuthToken(client2Id); + } return; }