This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-mcp.git
The following commit(s) were added to refs/heads/main by this push:
new be84c90 test: remove brittle assertions and a null image default
(#177)
be84c90 is described below
commit be84c902fb14e840f0c8d3fa46cdc6845c649105
Author: Aditya Parikh <[email protected]>
AuthorDate: Tue Aug 18 17:18:58 2026 -0400
test: remove brittle assertions and a null image default (#177)
Verified findings from a CodeRabbit review.
- DockerImageHttpIntegrationTest read solr.test.image with no default, so
DockerImageName.parse received null whenever the property was unset. Use
the same default as DockerImageMcpClientStdioIntegrationTest.
- SampleClient asserted an exact tool count of 8; the server exposes 11, and
the list went stale as soon as create-collection, add-fields and
add-field-types were added. Assert a lower bound plus per-name presence.
- CollectionServiceIntegrationTest required select.errors() and
select.timeouts() to be null. Solr may omit these counters or report an
explicit 0, and both mean "nothing went wrong"; the test depended on
which.
- McpClientIntegrationTestBase.assertNotError cast the first content item to
TextContent, so a non-text error payload raised ClassCastException and hid
the actual failure message.
- Renamed SolrConfigTest to SolrConfigIntegrationTest. It is
@Tag("integration") and starts Testcontainers, so the old name
contradicted
the project's *Test = unit / *IntegrationTest = integration convention.
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Co-authored-by: Eric Pugh <[email protected]>
---
.../solr/mcp/server/McpClientIntegrationTestBase.java | 9 ++++++---
src/test/java/org/apache/solr/mcp/server/SampleClient.java | 13 ++++++++-----
.../server/collection/CollectionServiceIntegrationTest.java | 6 ++++--
.../{SolrConfigTest.java => SolrConfigIntegrationTest.java} | 2 +-
.../containerization/DockerImageHttpIntegrationTest.java | 4 +++-
5 files changed, 22 insertions(+), 12 deletions(-)
diff --git
a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
index 2f617ec..d354082 100644
--- a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
+++ b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
@@ -821,9 +821,12 @@ public abstract class McpClientIntegrationTestBase {
protected static void assertNotError(CallToolResult result) {
if (Boolean.TRUE.equals(result.isError())) {
- String errorText = result.content().isEmpty()
- ? "unknown error"
- : ((TextContent)
result.content().getFirst()).text();
+ // Do not cast blindly: a non-text error payload would
raise
+ // ClassCastException here and hide the actual failure
message.
+ String errorText = result.content().isEmpty() ?
"unknown error" : switch (result.content().getFirst()) {
+ case TextContent text -> text.text();
+ case Object other -> "non-text error content: "
+ other;
+ };
fail("MCP tool call returned error: " + errorText);
}
}
diff --git a/src/test/java/org/apache/solr/mcp/server/SampleClient.java
b/src/test/java/org/apache/solr/mcp/server/SampleClient.java
index 7b5a3df..f9b5f0f 100644
--- a/src/test/java/org/apache/solr/mcp/server/SampleClient.java
+++ b/src/test/java/org/apache/solr/mcp/server/SampleClient.java
@@ -152,13 +152,16 @@ public class SampleClient {
assertNotNull(toolsList, "Tools list should not be
null");
assertNotNull(toolsList.tools(), "Tools collection
should not be null");
- // Validate expected tool count based on MCP server
implementation
- assertEquals(8, toolsList.tools().size(), "Expected 8
tools to be available");
-
- // Define expected tools based on the log output
+ // Every tool the server is expected to expose.
Asserted as a lower
+ // bound plus per-name checks so adding a tool does not
break this
+ // client - an exact count went stale as soon as
create-collection,
+ // add-fields and add-field-types were added.
Set<String> expectedToolNames =
Set.of("index-json-documents", "index-csv-documents",
"get-collection-stats", "search",
"list-collections", "check-health", "index-xml-documents",
- "get-schema");
+ "get-schema", "create-collection",
"add-fields", "add-field-types");
+
+ assertTrue(toolsList.tools().size() >=
expectedToolNames.size(),
+ "Expected at least " +
expectedToolNames.size() + " tools, got " + toolsList.tools().size());
// Validate each expected tool is present
List<String> actualToolNames =
toolsList.tools().stream().map(Tool::name).toList();
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
index 4cad9b0..6560af9 100644
---
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
@@ -233,8 +233,10 @@ class CollectionServiceIntegrationTest {
HandlerInfo select = handlerStats.selectHandler();
assertNotNull(select);
assertTrue(select.requests() > 0, "Select handler requests
should be positive after queries");
- assertNull(select.errors());
- assertNull(select.timeouts());
+ // Solr may omit these counters entirely or report an explicit
0 - both mean
+ // "nothing went wrong". Requiring null made the test depend on
which.
+ assertTrue(select.errors() == null || select.errors() == 0L,
"Select handler should report no errors");
+ assertTrue(select.timeouts() == null || select.timeouts() ==
0L, "Select handler should report no timeouts");
// Update handler: indexing 50 docs should have driven request
counts > 0
HandlerInfo update = handlerStats.updateHandler();
diff --git
a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java
b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigIntegrationTest.java
similarity index 98%
rename from src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java
rename to
src/test/java/org/apache/solr/mcp/server/config/SolrConfigIntegrationTest.java
index 31e88af..07ebd57 100644
--- a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigIntegrationTest.java
@@ -33,7 +33,7 @@ import org.testcontainers.junit.jupiter.Testcontainers;
@Import(TestcontainersConfiguration.class)
@Tag("integration")
@Testcontainers(disabledWithoutDocker = true)
-class SolrConfigTest {
+class SolrConfigIntegrationTest {
@Autowired
private SolrClient solrClient;
diff --git
a/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
index cbf0f88..1aebc0d 100644
---
a/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
@@ -95,7 +95,9 @@ class DockerImageHttpIntegrationTest {
// Docker image name and tag from build-info.properties
private static final String DOCKER_IMAGE =
BuildInfoReader.getDockerImageName();
- private static final String SOLR_IMAGE =
System.getProperty("solr.test.image");
+ // Same default as DockerImageMcpClientStdioIntegrationTest - without
it,
+ // DockerImageName.parse receives null whenever the property is unset.
+ private static final String SOLR_IMAGE =
System.getProperty("solr.test.image", "solr:9.9-slim");
private static final int HTTP_PORT = 8080;
// Network for container communication
