From 10d2f32c39d07b9d4defe6cc315600b076907454 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Sun, 15 Jun 2025 10:13:07 +0200 Subject: [PATCH 1/4] test: enhance resource reading tests - Implement comprehensive resource content validation for text/plain and application/octet-stream MIME types - Validate resource count expectations (10 resources) - Add elicitation capability testing with ElicitRequest/ElicitResult support - Clean up unused imports (ObjectMapper, ImageFromDockerfile) Signed-off-by: Christian Tzolov --- ...bClientStreamableHttpAsyncClientTests.java | 5 +- .../client/AbstractMcpAsyncClientTests.java | 63 ++++++++++++--- .../client/AbstractMcpSyncClientTests.java | 73 +++++++++++++++-- .../client/AbstractMcpAsyncClientTests.java | 39 ++++++--- .../client/AbstractMcpSyncClientTests.java | 79 ++++++++++++++++--- 5 files changed, 218 insertions(+), 41 deletions(-) diff --git a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebClientStreamableHttpAsyncClientTests.java b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebClientStreamableHttpAsyncClientTests.java index 4c8032659..f824193fd 100644 --- a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebClientStreamableHttpAsyncClientTests.java +++ b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebClientStreamableHttpAsyncClientTests.java @@ -1,13 +1,12 @@ package io.modelcontextprotocol.client; -import com.fasterxml.jackson.databind.ObjectMapper; import io.modelcontextprotocol.client.transport.WebClientStreamableHttpTransport; import io.modelcontextprotocol.spec.McpClientTransport; import org.junit.jupiter.api.Timeout; -import org.springframework.web.reactive.function.client.WebClient; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; -import org.testcontainers.images.builder.ImageFromDockerfile; + +import org.springframework.web.reactive.function.client.WebClient; @Timeout(15) public class WebClientStreamableHttpAsyncClientTests extends AbstractMcpAsyncClientTests { diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index 049bea008..3384e4f9f 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -19,6 +19,8 @@ import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities; import io.modelcontextprotocol.spec.McpSchema.CreateMessageRequest; import io.modelcontextprotocol.spec.McpSchema.CreateMessageResult; +import io.modelcontextprotocol.spec.McpSchema.ElicitRequest; +import io.modelcontextprotocol.spec.McpSchema.ElicitResult; import io.modelcontextprotocol.spec.McpSchema.GetPromptRequest; import io.modelcontextprotocol.spec.McpSchema.Prompt; import io.modelcontextprotocol.spec.McpSchema.Resource; @@ -38,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; /** * Test suite for the {@link McpAsyncClient} that can be used with different @@ -339,18 +342,36 @@ void testRemoveNonExistentRoot() { } @Test - @Disabled void testReadResource() { - withClient(createMcpTransport(), mcpAsyncClient -> { - StepVerifier.create(mcpAsyncClient.listResources()).consumeNextWith(resources -> { - if (!resources.resources().isEmpty()) { - Resource firstResource = resources.resources().get(0); - StepVerifier.create(mcpAsyncClient.readResource(firstResource)).consumeNextWith(result -> { - assertThat(result).isNotNull(); - assertThat(result.contents()).isNotNull(); - }).verifyComplete(); - } - }).verifyComplete(); + withClient(createMcpTransport(), client -> { + Flux resources = client.initialize() + .then(client.listResources(null)) + .flatMapMany(r -> Flux.fromIterable(r.resources())) + .flatMap(r -> client.readResource(r)); + + StepVerifier.create(resources).consumeNextWith(resourceResult -> { + assertThat(resourceResult.contents()).allSatisfy(content -> { + if (content.mimeType().equals("text/plain")) { + McpSchema.TextResourceContents text = assertInstanceOf(McpSchema.TextResourceContents.class, + content); + assertThat(text.mimeType()).isEqualTo("text/plain"); + assertThat(text.uri()).isNotEmpty(); + assertThat(text.text()).isNotEmpty(); + } + else if (content.mimeType().equals("application/octet-stream")) { + McpSchema.BlobResourceContents blob = assertInstanceOf(McpSchema.BlobResourceContents.class, + content); + assertThat(blob.mimeType()).isEqualTo("application/octet-stream"); + assertThat(blob.uri()).isNotEmpty(); + assertThat(blob.blob()).isNotEmpty(); + } + else { + throw new IllegalArgumentException("Unexpected content type: " + content.mimeType()); + } + }); + }) + .expectNextCount(9) // Expect 9 more elements + .verifyComplete(); }); } @@ -424,6 +445,20 @@ void testInitializeWithSamplingCapability() { }); } + @Test + void testInitializeWithElicitationCapability() { + ClientCapabilities capabilities = ClientCapabilities.builder().elicitation().build(); + ElicitResult elicitResult = ElicitResult.builder() + .message(ElicitResult.Action.ACCEPT) + .content(Map.of("foo", "bar")) + .build(); + withClient(createMcpTransport(), + builder -> builder.capabilities(capabilities).elicitation(request -> Mono.just(elicitResult)), + client -> { + StepVerifier.create(client.initialize()).expectNextMatches(Objects::nonNull).verifyComplete(); + }); + } + @Test void testInitializeWithAllCapabilities() { var capabilities = ClientCapabilities.builder() @@ -435,7 +470,11 @@ void testInitializeWithAllCapabilities() { Function> samplingHandler = request -> Mono .just(CreateMessageResult.builder().message("test").model("test-model").build()); - withClient(createMcpTransport(), builder -> builder.capabilities(capabilities).sampling(samplingHandler), + Function> elicitationHandler = request -> Mono + .just(ElicitResult.builder().message(ElicitResult.Action.ACCEPT).content(Map.of("foo", "bar")).build()); + + withClient(createMcpTransport(), + builder -> builder.capabilities(capabilities).sampling(samplingHandler).elicitation(elicitationHandler), client -> StepVerifier.create(client.initialize()).assertNext(result -> { diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java index 3785fd645..b4102251d 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java @@ -8,12 +8,14 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities; @@ -22,22 +24,25 @@ import io.modelcontextprotocol.spec.McpSchema.ListToolsResult; import io.modelcontextprotocol.spec.McpSchema.ReadResourceResult; import io.modelcontextprotocol.spec.McpSchema.Resource; +import io.modelcontextprotocol.spec.McpSchema.ResourceContents; import io.modelcontextprotocol.spec.McpSchema.Root; import io.modelcontextprotocol.spec.McpSchema.SubscribeRequest; import io.modelcontextprotocol.spec.McpSchema.TextContent; +import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpSchema.UnsubscribeRequest; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import reactor.core.publisher.Mono; -import reactor.core.scheduler.Scheduler; -import reactor.core.scheduler.Schedulers; import reactor.test.StepVerifier; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; /** * Unit tests for MCP Client Session functionality. @@ -47,6 +52,8 @@ */ public abstract class AbstractMcpSyncClientTests { + private static final Logger logger = LoggerFactory.getLogger(AbstractMcpSyncClientTests.class); + private static final String TEST_MESSAGE = "Hello MCP Spring AI!"; abstract protected McpClientTransport createMcpTransport(); @@ -330,18 +337,72 @@ void testReadResourceWithoutInitialization() { @Test void testReadResource() { + AtomicInteger readResourceCount = new AtomicInteger(0); withClient(createMcpTransport(), mcpSyncClient -> { mcpSyncClient.initialize(); ListResourcesResult resources = mcpSyncClient.listResources(null); - if (!resources.resources().isEmpty()) { - Resource firstResource = resources.resources().get(0); - ReadResourceResult result = mcpSyncClient.readResource(firstResource); + assertThat(resources).isNotNull(); + assertThat(resources.resources()).isNotNull(); + + if (resources.resources().isEmpty()) { + throw new IllegalStateException("No resources available for testing readResource functionality"); + } + + // Test reading each resource individually for better error isolation + for (Resource resource : resources.resources()) { + ReadResourceResult result = mcpSyncClient.readResource(resource); assertThat(result).isNotNull(); - assertThat(result.contents()).isNotNull(); + assertThat(result.contents()).isNotNull().isNotEmpty(); + + readResourceCount.incrementAndGet(); + + // Validate each content item + for (ResourceContents content : result.contents()) { + assertThat(content).isNotNull(); + assertThat(content.uri()).isNotNull().isNotEmpty(); + assertThat(content.mimeType()).isNotNull().isNotEmpty(); + + // Validate content based on its type with more comprehensive + // checks + switch (content.mimeType()) { + case "text/plain" -> { + TextResourceContents textContent = assertInstanceOf(TextResourceContents.class, content); + assertThat(textContent.text()).isNotNull().isNotEmpty(); + // Verify URI consistency + assertThat(textContent.uri()).isEqualTo(resource.uri()); + } + case "application/octet-stream" -> { + BlobResourceContents blobContent = assertInstanceOf(BlobResourceContents.class, content); + assertThat(blobContent.blob()).isNotNull().isNotEmpty(); + // Verify URI consistency + assertThat(blobContent.uri()).isEqualTo(resource.uri()); + // Validate base64 encoding format + assertThat(blobContent.blob()).matches("^[A-Za-z0-9+/]*={0,2}$"); + } + default -> { + // More flexible handling of additional MIME types + // Log the unexpected type for debugging but don't fail + // the test + logger.warn("Warning: Encountered unexpected MIME type: {} for resource: {}", + content.mimeType(), resource.uri()); + + // Still validate basic properties + if (content instanceof TextResourceContents textContent) { + assertThat(textContent.text()).isNotNull(); + } + else if (content instanceof BlobResourceContents blobContent) { + assertThat(blobContent.blob()).isNotNull(); + } + } + } + } } }); + + // Assert that we read exactly 10 resources + assertThat(readResourceCount.get()).isEqualTo(10); } @Test diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index 37f9e71a7..cf8090564 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -40,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; /** * Test suite for the {@link McpAsyncClient} that can be used with different @@ -342,18 +343,34 @@ void testRemoveNonExistentRoot() { } @Test - @Disabled void testReadResource() { - withClient(createMcpTransport(), mcpAsyncClient -> { - StepVerifier.create(mcpAsyncClient.listResources()).consumeNextWith(resources -> { - if (!resources.resources().isEmpty()) { - Resource firstResource = resources.resources().get(0); - StepVerifier.create(mcpAsyncClient.readResource(firstResource)).consumeNextWith(result -> { - assertThat(result).isNotNull(); - assertThat(result.contents()).isNotNull(); - }).verifyComplete(); - } - }).verifyComplete(); + withClient(createMcpTransport(), client -> { + Flux resources = client.initialize() + .then(client.listResources(null)) + .flatMapMany(r -> Flux.fromIterable(r.resources())) + .flatMap(r -> client.readResource(r)); + + StepVerifier.create(resources).consumeNextWith(resourceResult -> { + assertThat(resourceResult.contents()).allSatisfy(content -> { + if (content.mimeType().equals("text/plain")) { + McpSchema.TextResourceContents text = assertInstanceOf(McpSchema.TextResourceContents.class, + content); + assertThat(text.mimeType()).isEqualTo("text/plain"); + assertThat(text.uri()).isNotEmpty(); + assertThat(text.text()).isNotEmpty(); + } + else if (content.mimeType().equals("application/octet-stream")) { + McpSchema.BlobResourceContents blob = assertInstanceOf(McpSchema.BlobResourceContents.class, + content); + assertThat(blob.mimeType()).isEqualTo("application/octet-stream"); + assertThat(blob.uri()).isNotEmpty(); + assertThat(blob.blob()).isNotEmpty(); + } + else { + throw new IllegalArgumentException("Unexpected content type: " + content.mimeType()); + } + }); + }).expectNextCount(9).verifyComplete(); }); } diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java index 77989577a..e41f1f245 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java @@ -8,12 +8,14 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities; @@ -22,22 +24,25 @@ import io.modelcontextprotocol.spec.McpSchema.ListToolsResult; import io.modelcontextprotocol.spec.McpSchema.ReadResourceResult; import io.modelcontextprotocol.spec.McpSchema.Resource; +import io.modelcontextprotocol.spec.McpSchema.ResourceContents; import io.modelcontextprotocol.spec.McpSchema.Root; import io.modelcontextprotocol.spec.McpSchema.SubscribeRequest; import io.modelcontextprotocol.spec.McpSchema.TextContent; +import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpSchema.UnsubscribeRequest; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import reactor.core.publisher.Mono; -import reactor.core.scheduler.Scheduler; -import reactor.core.scheduler.Schedulers; import reactor.test.StepVerifier; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; /** * Unit tests for MCP Client Session functionality. @@ -48,6 +53,8 @@ // KEEP IN SYNC with the class in mcp-test module public abstract class AbstractMcpSyncClientTests { + private static final Logger logger = LoggerFactory.getLogger(AbstractMcpSyncClientTests.class); + private static final String TEST_MESSAGE = "Hello MCP Spring AI!"; abstract protected McpClientTransport createMcpTransport(); @@ -122,9 +129,9 @@ void verifyNotificationSucceedsWithImplicitInitialization(Consumer void verifyCallSucceedsWithImplicitInitialization(Function blockingOperation, String action) { withClient(createMcpTransport(), mcpSyncClient -> { - StepVerifier.create(Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient)) - // Offload the blocking call to the real scheduler - .subscribeOn(Schedulers.boundedElastic())).expectNextCount(1).verifyComplete(); + StepVerifier.create(Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient))) + .expectNextCount(1) + .verifyComplete(); }); } @@ -331,18 +338,72 @@ void testReadResourceWithoutInitialization() { @Test void testReadResource() { + AtomicInteger readResourceCount = new AtomicInteger(0); withClient(createMcpTransport(), mcpSyncClient -> { mcpSyncClient.initialize(); ListResourcesResult resources = mcpSyncClient.listResources(null); - if (!resources.resources().isEmpty()) { - Resource firstResource = resources.resources().get(0); - ReadResourceResult result = mcpSyncClient.readResource(firstResource); + assertThat(resources).isNotNull(); + assertThat(resources.resources()).isNotNull(); + + if (resources.resources().isEmpty()) { + throw new IllegalStateException("No resources available for testing readResource functionality"); + } + + // Test reading each resource individually for better error isolation + for (Resource resource : resources.resources()) { + ReadResourceResult result = mcpSyncClient.readResource(resource); assertThat(result).isNotNull(); - assertThat(result.contents()).isNotNull(); + assertThat(result.contents()).isNotNull().isNotEmpty(); + + readResourceCount.incrementAndGet(); + + // Validate each content item + for (ResourceContents content : result.contents()) { + assertThat(content).isNotNull(); + assertThat(content.uri()).isNotNull().isNotEmpty(); + assertThat(content.mimeType()).isNotNull().isNotEmpty(); + + // Validate content based on its type with more comprehensive + // checks + switch (content.mimeType()) { + case "text/plain" -> { + TextResourceContents textContent = assertInstanceOf(TextResourceContents.class, content); + assertThat(textContent.text()).isNotNull().isNotEmpty(); + // Verify URI consistency + assertThat(textContent.uri()).isEqualTo(resource.uri()); + } + case "application/octet-stream" -> { + BlobResourceContents blobContent = assertInstanceOf(BlobResourceContents.class, content); + assertThat(blobContent.blob()).isNotNull().isNotEmpty(); + // Verify URI consistency + assertThat(blobContent.uri()).isEqualTo(resource.uri()); + // Validate base64 encoding format + assertThat(blobContent.blob()).matches("^[A-Za-z0-9+/]*={0,2}$"); + } + default -> { + // More flexible handling of additional MIME types + // Log the unexpected type for debugging but don't fail + // the test + logger.warn("Warning: Encountered unexpected MIME type: {} for resource: {}", + content.mimeType(), resource.uri()); + + // Still validate basic properties + if (content instanceof TextResourceContents textContent) { + assertThat(textContent.text()).isNotNull(); + } + else if (content instanceof BlobResourceContents blobContent) { + assertThat(blobContent.blob()).isNotNull(); + } + } + } + } } }); + + // Assert that we read exactly 10 resources + assertThat(readResourceCount.get()).isEqualTo(10); } @Test From 3833e536312b55a7a1f1718f8dbf3c24d2692f15 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Sun, 15 Jun 2025 11:07:59 +0200 Subject: [PATCH 2/4] Increase the StdioMcpAsyncClientTests initializationTimeout from 6 to 10 sec Signed-off-by: Christian Tzolov --- .../modelcontextprotocol/client/StdioMcpAsyncClientTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java index 8c0069d6d..a101f0177 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java @@ -37,7 +37,7 @@ protected McpClientTransport createMcpTransport() { } protected Duration getInitializationTimeout() { - return Duration.ofSeconds(6); + return Duration.ofSeconds(10); } } From 15e5b01e5fd83d94f7a73e4a76452dd59bb5ad22 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Tue, 17 Jun 2025 17:06:32 +0200 Subject: [PATCH 3/4] address review comments Signed-off-by: Christian Tzolov --- .../client/WebFluxSseMcpAsyncClientTests.java | 14 +++ .../client/AbstractMcpAsyncClientTests.java | 86 ++++++++++++------ .../client/AbstractMcpSyncClientTests.java | 45 +++++----- .../transport/StdioClientTransport.java | 4 +- .../client/AbstractMcpAsyncClientTests.java | 88 +++++++++++++------ .../client/AbstractMcpSyncClientTests.java | 45 +++++----- 6 files changed, 178 insertions(+), 104 deletions(-) diff --git a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java index f0533cb49..599bac3d7 100644 --- a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java +++ b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java @@ -4,10 +4,24 @@ package io.modelcontextprotocol.client; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + import java.time.Duration; +import java.util.ArrayList; import io.modelcontextprotocol.client.transport.WebFluxSseClientTransport; import io.modelcontextprotocol.spec.McpClientTransport; +import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; +import io.modelcontextprotocol.spec.McpSchema.ReadResourceResult; +import io.modelcontextprotocol.spec.McpSchema.Resource; +import io.modelcontextprotocol.spec.McpSchema.ResourceContents; +import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; +import reactor.core.publisher.Flux; +import reactor.test.StepVerifier; + +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index 3384e4f9f..8b06dcbad 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -4,7 +4,13 @@ package io.modelcontextprotocol.client; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + import java.time.Duration; +import java.util.ArrayList; import java.util.Map; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; @@ -12,9 +18,14 @@ import java.util.function.Consumer; import java.util.function.Function; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities; import io.modelcontextprotocol.spec.McpSchema.CreateMessageRequest; @@ -23,25 +34,19 @@ import io.modelcontextprotocol.spec.McpSchema.ElicitResult; import io.modelcontextprotocol.spec.McpSchema.GetPromptRequest; import io.modelcontextprotocol.spec.McpSchema.Prompt; +import io.modelcontextprotocol.spec.McpSchema.ReadResourceResult; import io.modelcontextprotocol.spec.McpSchema.Resource; +import io.modelcontextprotocol.spec.McpSchema.ResourceContents; import io.modelcontextprotocol.spec.McpSchema.Root; import io.modelcontextprotocol.spec.McpSchema.SubscribeRequest; +import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpSchema.UnsubscribeRequest; import io.modelcontextprotocol.spec.McpTransport; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; - /** * Test suite for the {@link McpAsyncClient} that can be used with different * {@link McpTransport} implementations. @@ -349,28 +354,51 @@ void testReadResource() { .flatMapMany(r -> Flux.fromIterable(r.resources())) .flatMap(r -> client.readResource(r)); - StepVerifier.create(resources).consumeNextWith(resourceResult -> { - assertThat(resourceResult.contents()).allSatisfy(content -> { - if (content.mimeType().equals("text/plain")) { - McpSchema.TextResourceContents text = assertInstanceOf(McpSchema.TextResourceContents.class, - content); - assertThat(text.mimeType()).isEqualTo("text/plain"); - assertThat(text.uri()).isNotEmpty(); - assertThat(text.text()).isNotEmpty(); - } - else if (content.mimeType().equals("application/octet-stream")) { - McpSchema.BlobResourceContents blob = assertInstanceOf(McpSchema.BlobResourceContents.class, - content); - assertThat(blob.mimeType()).isEqualTo("application/octet-stream"); - assertThat(blob.uri()).isNotEmpty(); - assertThat(blob.blob()).isNotEmpty(); - } - else { - throw new IllegalArgumentException("Unexpected content type: " + content.mimeType()); + StepVerifier.create(resources).recordWith(ArrayList::new).consumeRecordedWith(readResourceResults -> { + + for (ReadResourceResult result : readResourceResults) { + + assertThat(result).isNotNull(); + assertThat(result.contents()).isNotNull().isNotEmpty(); + + // Validate each content item + for (ResourceContents content : result.contents()) { + assertThat(content).isNotNull(); + assertThat(content.uri()).isNotNull().isNotEmpty(); + assertThat(content.mimeType()).isNotNull().isNotEmpty(); + + // Validate content based on its type with more comprehensive + // checks + switch (content.mimeType()) { + case "text/plain" -> { + TextResourceContents textContent = assertInstanceOf(TextResourceContents.class, + content); + assertThat(textContent.text()).isNotNull().isNotEmpty(); + assertThat(textContent.uri()).isNotEmpty(); + } + case "application/octet-stream" -> { + BlobResourceContents blobContent = assertInstanceOf(BlobResourceContents.class, + content); + assertThat(blobContent.blob()).isNotNull().isNotEmpty(); + assertThat(blobContent.uri()).isNotNull().isNotEmpty(); + // Validate base64 encoding format + assertThat(blobContent.blob()).matches("^[A-Za-z0-9+/]*={0,2}$"); + } + default -> { + + // Still validate basic properties + if (content instanceof TextResourceContents textContent) { + assertThat(textContent.text()).isNotNull(); + } + else if (content instanceof BlobResourceContents blobContent) { + assertThat(blobContent.blob()).isNotNull(); + } + } + } } - }); + } }) - .expectNextCount(9) // Expect 9 more elements + .expectNextCount(10) // Expect 10 elements .verifyComplete(); }); } diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java index b4102251d..9462fc135 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java @@ -4,15 +4,25 @@ package io.modelcontextprotocol.client; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + import java.time.Duration; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; @@ -31,19 +41,10 @@ import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpSchema.UnsubscribeRequest; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import reactor.test.StepVerifier; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; - /** * Unit tests for MCP Client Session functionality. * @@ -128,9 +129,9 @@ void verifyNotificationSucceedsWithImplicitInitialization(Consumer void verifyCallSucceedsWithImplicitInitialization(Function blockingOperation, String action) { withClient(createMcpTransport(), mcpSyncClient -> { - StepVerifier.create(Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient))) - .expectNextCount(1) - .verifyComplete(); + StepVerifier.create(Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient)) + // Offload the blocking call to the real scheduler + .subscribeOn(Schedulers.boundedElastic())).expectNextCount(1).verifyComplete(); }); } @@ -337,17 +338,17 @@ void testReadResourceWithoutInitialization() { @Test void testReadResource() { - AtomicInteger readResourceCount = new AtomicInteger(0); withClient(createMcpTransport(), mcpSyncClient -> { + + int readResourceCount = 0; + mcpSyncClient.initialize(); ListResourcesResult resources = mcpSyncClient.listResources(null); assertThat(resources).isNotNull(); assertThat(resources.resources()).isNotNull(); - if (resources.resources().isEmpty()) { - throw new IllegalStateException("No resources available for testing readResource functionality"); - } + assertThat(resources.resources()).isNotNull().isNotEmpty(); // Test reading each resource individually for better error isolation for (Resource resource : resources.resources()) { @@ -356,7 +357,7 @@ void testReadResource() { assertThat(result).isNotNull(); assertThat(result.contents()).isNotNull().isNotEmpty(); - readResourceCount.incrementAndGet(); + readResourceCount++; // Validate each content item for (ResourceContents content : result.contents()) { @@ -399,10 +400,10 @@ else if (content instanceof BlobResourceContents blobContent) { } } } - }); - // Assert that we read exactly 10 resources - assertThat(readResourceCount.get()).isEqualTo(10); + // Assert that we read exactly 10 resources + assertThat(readResourceCount).isEqualTo(10); + }); } @Test diff --git a/mcp/src/main/java/io/modelcontextprotocol/client/transport/StdioClientTransport.java b/mcp/src/main/java/io/modelcontextprotocol/client/transport/StdioClientTransport.java index 246ff11c1..8545348ed 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/client/transport/StdioClientTransport.java +++ b/mcp/src/main/java/io/modelcontextprotocol/client/transport/StdioClientTransport.java @@ -346,14 +346,14 @@ public Mono closeGracefully() { return Mono.fromRunnable(() -> { isClosing = true; logger.debug("Initiating graceful shutdown"); - }).then(Mono.defer(() -> { + }).then(Mono.defer(() -> { // First complete all sinks to stop accepting new messages inboundSink.tryEmitComplete(); outboundSink.tryEmitComplete(); errorSink.tryEmitComplete(); // Give a short time for any pending messages to be processed - return Mono.delay(Duration.ofMillis(100)); + return Mono.delay(Duration.ofMillis(100)).then(); })).then(Mono.defer(() -> { logger.debug("Sending TERM to process"); if (this.process != null) { diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index cf8090564..3fb34fe85 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -4,7 +4,13 @@ package io.modelcontextprotocol.client; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + import java.time.Duration; +import java.util.ArrayList; import java.util.Map; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; @@ -12,9 +18,14 @@ import java.util.function.Consumer; import java.util.function.Function; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities; import io.modelcontextprotocol.spec.McpSchema.CreateMessageRequest; @@ -23,25 +34,19 @@ import io.modelcontextprotocol.spec.McpSchema.ElicitResult; import io.modelcontextprotocol.spec.McpSchema.GetPromptRequest; import io.modelcontextprotocol.spec.McpSchema.Prompt; +import io.modelcontextprotocol.spec.McpSchema.ReadResourceResult; import io.modelcontextprotocol.spec.McpSchema.Resource; +import io.modelcontextprotocol.spec.McpSchema.ResourceContents; import io.modelcontextprotocol.spec.McpSchema.Root; import io.modelcontextprotocol.spec.McpSchema.SubscribeRequest; +import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpSchema.UnsubscribeRequest; import io.modelcontextprotocol.spec.McpTransport; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; - /** * Test suite for the {@link McpAsyncClient} that can be used with different * {@link McpTransport} implementations. @@ -350,27 +355,52 @@ void testReadResource() { .flatMapMany(r -> Flux.fromIterable(r.resources())) .flatMap(r -> client.readResource(r)); - StepVerifier.create(resources).consumeNextWith(resourceResult -> { - assertThat(resourceResult.contents()).allSatisfy(content -> { - if (content.mimeType().equals("text/plain")) { - McpSchema.TextResourceContents text = assertInstanceOf(McpSchema.TextResourceContents.class, - content); - assertThat(text.mimeType()).isEqualTo("text/plain"); - assertThat(text.uri()).isNotEmpty(); - assertThat(text.text()).isNotEmpty(); - } - else if (content.mimeType().equals("application/octet-stream")) { - McpSchema.BlobResourceContents blob = assertInstanceOf(McpSchema.BlobResourceContents.class, - content); - assertThat(blob.mimeType()).isEqualTo("application/octet-stream"); - assertThat(blob.uri()).isNotEmpty(); - assertThat(blob.blob()).isNotEmpty(); - } - else { - throw new IllegalArgumentException("Unexpected content type: " + content.mimeType()); + StepVerifier.create(resources).recordWith(ArrayList::new).consumeRecordedWith(readResourceResults -> { + + for (ReadResourceResult result : readResourceResults) { + + assertThat(result).isNotNull(); + assertThat(result.contents()).isNotNull().isNotEmpty(); + + // Validate each content item + for (ResourceContents content : result.contents()) { + assertThat(content).isNotNull(); + assertThat(content.uri()).isNotNull().isNotEmpty(); + assertThat(content.mimeType()).isNotNull().isNotEmpty(); + + // Validate content based on its type with more comprehensive + // checks + switch (content.mimeType()) { + case "text/plain" -> { + TextResourceContents textContent = assertInstanceOf(TextResourceContents.class, + content); + assertThat(textContent.text()).isNotNull().isNotEmpty(); + assertThat(textContent.uri()).isNotEmpty(); + } + case "application/octet-stream" -> { + BlobResourceContents blobContent = assertInstanceOf(BlobResourceContents.class, + content); + assertThat(blobContent.blob()).isNotNull().isNotEmpty(); + assertThat(blobContent.uri()).isNotNull().isNotEmpty(); + // Validate base64 encoding format + assertThat(blobContent.blob()).matches("^[A-Za-z0-9+/]*={0,2}$"); + } + default -> { + + // Still validate basic properties + if (content instanceof TextResourceContents textContent) { + assertThat(textContent.text()).isNotNull(); + } + else if (content instanceof BlobResourceContents blobContent) { + assertThat(blobContent.blob()).isNotNull(); + } + } + } } - }); - }).expectNextCount(9).verifyComplete(); + } + }) + .expectNextCount(10) // Expect 10 elements + .verifyComplete(); }); } diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java index e41f1f245..7f7c58b32 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java @@ -4,15 +4,25 @@ package io.modelcontextprotocol.client; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + import java.time.Duration; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; @@ -31,19 +41,10 @@ import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpSchema.UnsubscribeRequest; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import reactor.test.StepVerifier; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; - /** * Unit tests for MCP Client Session functionality. * @@ -129,9 +130,9 @@ void verifyNotificationSucceedsWithImplicitInitialization(Consumer void verifyCallSucceedsWithImplicitInitialization(Function blockingOperation, String action) { withClient(createMcpTransport(), mcpSyncClient -> { - StepVerifier.create(Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient))) - .expectNextCount(1) - .verifyComplete(); + StepVerifier.create(Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient)) + // Offload the blocking call to the real scheduler + .subscribeOn(Schedulers.boundedElastic())).expectNextCount(1).verifyComplete(); }); } @@ -338,17 +339,17 @@ void testReadResourceWithoutInitialization() { @Test void testReadResource() { - AtomicInteger readResourceCount = new AtomicInteger(0); withClient(createMcpTransport(), mcpSyncClient -> { + + int readResourceCount = 0; + mcpSyncClient.initialize(); ListResourcesResult resources = mcpSyncClient.listResources(null); assertThat(resources).isNotNull(); assertThat(resources.resources()).isNotNull(); - if (resources.resources().isEmpty()) { - throw new IllegalStateException("No resources available for testing readResource functionality"); - } + assertThat(resources.resources()).isNotNull().isNotEmpty(); // Test reading each resource individually for better error isolation for (Resource resource : resources.resources()) { @@ -357,7 +358,7 @@ void testReadResource() { assertThat(result).isNotNull(); assertThat(result.contents()).isNotNull().isNotEmpty(); - readResourceCount.incrementAndGet(); + readResourceCount++; // Validate each content item for (ResourceContents content : result.contents()) { @@ -400,10 +401,10 @@ else if (content instanceof BlobResourceContents blobContent) { } } } - }); - // Assert that we read exactly 10 resources - assertThat(readResourceCount.get()).isEqualTo(10); + // Assert that we read exactly 10 resources + assertThat(readResourceCount).isEqualTo(10); + }); } @Test From 7c45960cb1d368e1c2fceff3e85017a9d559bf98 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Tue, 17 Jun 2025 17:15:19 +0200 Subject: [PATCH 4/4] remove unused imports Signed-off-by: Christian Tzolov --- .../client/WebFluxSseMcpAsyncClientTests.java | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java index 599bac3d7..0edf4cd54 100644 --- a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java +++ b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/client/WebFluxSseMcpAsyncClientTests.java @@ -4,29 +4,15 @@ package io.modelcontextprotocol.client; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; - import java.time.Duration; -import java.util.ArrayList; - -import io.modelcontextprotocol.client.transport.WebFluxSseClientTransport; -import io.modelcontextprotocol.spec.McpClientTransport; -import io.modelcontextprotocol.spec.McpSchema; -import io.modelcontextprotocol.spec.McpSchema.BlobResourceContents; -import io.modelcontextprotocol.spec.McpSchema.ReadResourceResult; -import io.modelcontextprotocol.spec.McpSchema.Resource; -import io.modelcontextprotocol.spec.McpSchema.ResourceContents; -import io.modelcontextprotocol.spec.McpSchema.TextResourceContents; -import reactor.core.publisher.Flux; -import reactor.test.StepVerifier; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import org.springframework.web.reactive.function.client.WebClient; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; -import org.springframework.web.reactive.function.client.WebClient; +import io.modelcontextprotocol.client.transport.WebFluxSseClientTransport; +import io.modelcontextprotocol.spec.McpClientTransport; /** * Tests for the {@link McpAsyncClient} with {@link WebFluxSseClientTransport}.