Skip to content

test: enhance resource reading tests #314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

test: enhance resource reading tests #314

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10d2f32
test: enhance resource reading tests
tzolov Jun 15, 2025
3833e53
Increase the StdioMcpAsyncClientTests initializationTimeout from 6 t…
tzolov Jun 15, 2025
15e5b01
address review comments
tzolov Jun 17, 2025
7c45960
remove unused imports
tzolov Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions ...src/test/java/io/modelcontextprotocol/client/WebClientStreamableHttpAsyncClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
63 changes: 51 additions & 12 deletions mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<McpSchema.ReadResourceResult> 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();
});
}

Expand Down Expand Up @@ -424,6 +445,20 @@ void testInitializeWithSamplingCapability() {
});
}

@Test
void testInitializeWithElicitationCapability() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to the ReadResources, but discovered a missing elicitation aync testing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you - I'll be sure to double-check this in other PRs 😓

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()
Expand All @@ -435,7 +470,11 @@ void testInitializeWithAllCapabilities() {
Function<CreateMessageRequest, Mono<CreateMessageResult>> samplingHandler = request -> Mono
.just(CreateMessageResult.builder().message("test").model("test-model").build());

withClient(createMcpTransport(), builder -> builder.capabilities(capabilities).sampling(samplingHandler),
Function<ElicitRequest, Mono<ElicitResult>> 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 -> {
Expand Down
73 changes: 67 additions & 6 deletions mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -330,18 +337,72 @@ void testReadResourceWithoutInitialization() {

@Test
void testReadResource() {
AtomicInteger readResourceCount = new AtomicInteger(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a synchronous test, can this be a regular int?

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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this and the above be replaced with assertThat(resources.resources()).isNotNull().isNotEmpty(); ?

}

// 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
Expand Down
39 changes: 28 additions & 11 deletions mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<McpSchema.ReadResourceResult> resources = client.initialize()
.then(client.listResources(null))
.flatMapMany(r -> Flux.fromIterable(r.resources()))
.flatMap(r -> client.readResource(r));
Comment on lines +352 to +356
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to not test this (and the others) with a cursor, even in the actual listResources tests? Testing a single page ensures we can read individual resource types, but we're still missing coverage on how valid pagination tokens are handled.

Possibly related to #306 (maybe we need a shared, well-tested listAllResources or similar).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid point. As you've noticed we don't have complete/proper pagination implementation. Feel free to jump on this if interested.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added support for listAllResources as part of #306 as well. That should hopefully help out for such usecases in the future.


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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of only validating one result and checking the count of the remainder, consider something like

.recordWith(ArrayList::new)
.consumeRecordedWith(items -> {
    // assertThat(items)...
})

});
}

Expand Down
Loading