Skip to content

Commit 15b86ba

Browse files
adityamparikhclaude
adityamparikh
and
claude
committed
test(collection): index data and run queries before asserting metrics
The integration tests previously created an empty collection and only checked metrics were >= 0. Now @BeforeAll indexes 50 documents and runs 15 warm-up queries (including filter queries) against real Solr via Testcontainers. Tests assert on actual non-zero values: - numDocs == 50 (matches indexed count) - queryResultCache.lookups > 0 (from warm-up queries) - documentCache.lookups > 0 (from reading documents) - filterCache.lookups > 0 (from filter queries) - selectHandler.requests > 0 (from queries) - updateHandler.requests > 0 (from indexing) - health check totalDocuments == 50 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: adityamparikh <aditya.m.parikh@gmail.com>
1 parent 8b94f1d commit 15b86ba

1 file changed

Lines changed: 126 additions & 181 deletions

File tree

‎src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java‎

Lines changed: 126 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import java.util.List;
2222
import org.apache.solr.client.solrj.SolrClient;
2323
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
24+
import org.apache.solr.client.solrj.request.SolrQuery;
25+
import org.apache.solr.common.SolrInputDocument;
2426
import org.apache.solr.mcp.server.TestcontainersConfiguration;
2527
import org.junit.jupiter.api.BeforeAll;
2628
import org.junit.jupiter.api.Test;
@@ -41,250 +43,193 @@ class CollectionServiceIntegrationTest {
4143
private static final Logger log = LoggerFactory.getLogger(CollectionServiceIntegrationTest.class);
4244

4345
private static final String TEST_COLLECTION = "test_collection";
46+
47+
private static final int DOC_COUNT = 50;
48+
4449
@Autowired
4550
private CollectionService collectionService;
51+
4652
@Autowired
4753
private SolrClient solrClient;
4854

4955
@BeforeAll
50-
void setupCollection() throws Exception {
51-
CollectionAdminRequest.Create createRequest = CollectionAdminRequest.createCollection(TEST_COLLECTION,
52-
"_default", 1, 1);
53-
createRequest.process(solrClient);
56+
void setupCollectionWithData() throws Exception {
57+
// 1. Create collection
58+
CollectionAdminRequest.createCollection(TEST_COLLECTION, "_default", 1, 1).process(solrClient);
59+
log.debug("Test collection created: {}", TEST_COLLECTION);
5460

55-
// Verify collection was created successfully
56-
CollectionAdminRequest.List listRequest = new CollectionAdminRequest.List();
57-
listRequest.process(solrClient);
61+
// 2. Index documents so metrics have real data
62+
for (int i = 0; i < DOC_COUNT; i++) {
63+
SolrInputDocument doc = new SolrInputDocument();
64+
doc.addField("id", "doc-" + i);
65+
doc.addField("title_s", "Document " + i);
66+
doc.addField("category_s", (i % 2 == 0) ? "even" : "odd");
67+
doc.addField("count_i", i);
68+
solrClient.add(TEST_COLLECTION, doc);
69+
}
70+
solrClient.commit(TEST_COLLECTION);
5871

59-
log.debug("Test collection created: {}", TEST_COLLECTION);
72+
// 3. Run several queries to populate query result cache and handler stats
73+
for (int i = 0; i < 5; i++) {
74+
solrClient.query(TEST_COLLECTION, new SolrQuery("*:*").setRows(10));
75+
solrClient.query(TEST_COLLECTION, new SolrQuery("title_s:Document").setRows(5));
76+
solrClient.query(TEST_COLLECTION, new SolrQuery("*:*").addFilterQuery("category_s:even").setRows(10));
77+
}
78+
log.debug("Indexed {} documents and ran warm-up queries", DOC_COUNT);
6079
}
6180

6281
@Test
6382
void testListCollections() {
64-
// Test listing collections
6583
List<String> collections = collectionService.listCollections();
6684

6785
log.debug("Collections: {}", collections);
6886

69-
// Enhanced assertions for collections list
70-
assertNotNull(collections, "Collections list should not be null");
71-
assertFalse(collections.isEmpty(), "Collections list should not be empty");
87+
assertNotNull(collections);
88+
assertFalse(collections.isEmpty());
7289

73-
// Check if the test collection exists (either as exact name or as shard)
7490
boolean testCollectionExists = collections.contains(TEST_COLLECTION)
7591
|| collections.stream().anyMatch(col -> col.startsWith(TEST_COLLECTION + "_shard"));
7692
assertTrue(testCollectionExists,
77-
"Collections should contain the test collection: " + TEST_COLLECTION + " (found: " + collections + ")");
78-
79-
// Verify collection names are not null or empty
80-
for (String collection : collections) {
81-
assertNotNull(collection, "Collection name should not be null");
82-
assertFalse(collection.trim().isEmpty(), "Collection name should not be empty");
83-
}
93+
"Collections should contain " + TEST_COLLECTION + " (found: " + collections + ")");
8494

85-
// Verify expected collection characteristics
8695
assertEquals(collections.size(), collections.stream().distinct().count(), "Collection names should be unique");
87-
88-
// Verify that collections follow expected naming patterns
89-
for (String collection : collections) {
90-
// Collection names should either be simple names or shard names
91-
assertTrue(collection.matches("^[a-zA-Z0-9_]+(_shard\\d+_replica_n\\d+)?$"),
92-
"Collection name should follow expected pattern: " + collection);
93-
}
9496
}
9597

9698
@Test
97-
void testGetCollectionStats() throws Exception {
98-
// Test getting collection stats
99+
void testGetCollectionStats_reflectsIndexedData() throws Exception {
99100
SolrMetrics metrics = collectionService.getCollectionStats(TEST_COLLECTION);
100101

101-
// Enhanced assertions for metrics
102-
assertNotNull(metrics, "Collection stats should not be null");
103-
assertNotNull(metrics.timestamp(), "Timestamp should not be null");
102+
assertNotNull(metrics);
103+
assertNotNull(metrics.timestamp());
104104

105-
// Verify index stats
106-
assertNotNull(metrics.indexStats(), "Index stats should not be null");
105+
// Index stats should reflect the documents we indexed
107106
IndexStats indexStats = metrics.indexStats();
108-
assertNotNull(indexStats.numDocs(), "Number of documents should not be null");
109-
assertTrue(indexStats.numDocs() >= 0, "Number of documents should be non-negative");
107+
assertNotNull(indexStats);
108+
assertEquals(DOC_COUNT, indexStats.numDocs(), "numDocs should match indexed document count");
109+
assertNotNull(indexStats.segmentCount());
110+
assertTrue(indexStats.segmentCount() >= 1, "Should have at least one segment after indexing");
110111

111-
// Verify query stats
112-
assertNotNull(metrics.queryStats(), "Query stats should not be null");
112+
// Query stats come from the *:* probe query inside getCollectionStats
113113
QueryStats queryStats = metrics.queryStats();
114-
assertNotNull(queryStats.queryTime(), "Query time should not be null");
115-
assertTrue(queryStats.queryTime() >= 0, "Query time should be non-negative");
116-
assertNotNull(queryStats.totalResults(), "Total results should not be null");
117-
assertTrue(queryStats.totalResults() >= 0, "Total results should be non-negative");
118-
assertNotNull(queryStats.start(), "Start should not be null");
119-
assertTrue(queryStats.start() >= 0, "Start should be non-negative");
120-
121-
// Verify timestamp is recent (within last 10 seconds)
122-
long currentTime = System.currentTimeMillis();
123-
long timestampTime = metrics.timestamp().getTime();
124-
assertTrue(currentTime - timestampTime < 10000, "Timestamp should be recent (within 10 seconds)");
125-
126-
// Verify optional stats (cache and handler stats may be null, which is
127-
// acceptable)
128-
if (metrics.cacheStats() != null) {
129-
CacheStats cacheStats = metrics.cacheStats();
130-
// Verify at least one cache type exists if cache stats are present
131-
assertTrue(
132-
cacheStats.queryResultCache() != null || cacheStats.documentCache() != null
133-
|| cacheStats.filterCache() != null,
134-
"At least one cache type should be present if cache stats exist");
135-
}
136-
137-
if (metrics.handlerStats() != null) {
138-
HandlerStats handlerStats = metrics.handlerStats();
139-
// Verify at least one handler type exists if handler stats are present
140-
assertTrue(handlerStats.selectHandler() != null || handlerStats.updateHandler() != null,
141-
"At least one handler type should be present if handler stats exist");
142-
}
114+
assertNotNull(queryStats);
115+
assertNotNull(queryStats.queryTime());
116+
assertEquals((long) DOC_COUNT, queryStats.totalResults(), "totalResults should match indexed document count");
117+
assertEquals(0L, queryStats.start());
118+
119+
// Cache stats should be present after warm-up queries
120+
assertNotNull(metrics.cacheStats(), "Cache stats should not be null after queries ran");
121+
CacheStats cacheStats = metrics.cacheStats();
122+
assertNotNull(cacheStats.queryResultCache());
123+
assertTrue(cacheStats.queryResultCache().lookups() > 0, "Query result cache should have lookups after queries");
124+
assertNotNull(cacheStats.documentCache());
125+
assertTrue(cacheStats.documentCache().lookups() > 0, "Document cache should have lookups after queries");
126+
assertNotNull(cacheStats.filterCache());
127+
assertTrue(cacheStats.filterCache().lookups() > 0, "Filter cache should have lookups after filter queries");
128+
129+
// Handler stats should reflect actual request counts
130+
assertNotNull(metrics.handlerStats(), "Handler stats should not be null after queries ran");
131+
HandlerStats handlerStats = metrics.handlerStats();
132+
assertNotNull(handlerStats.selectHandler());
133+
assertTrue(handlerStats.selectHandler().requests() > 0, "Select handler should have processed requests");
134+
assertNotNull(handlerStats.updateHandler());
135+
assertTrue(handlerStats.updateHandler().requests() > 0,
136+
"Update handler should have processed requests from indexing");
143137
}
144138

145139
@Test
146-
void testCheckHealthHealthy() {
147-
// Test checking health of a valid collection
140+
void testCheckHealth_healthy() {
148141
SolrHealthStatus status = collectionService.checkHealth(TEST_COLLECTION);
149142

150-
log.debug("Health status for valid collection: {}", status);
151-
152-
// Enhanced assertions for healthy collection
153-
assertNotNull(status, "Health status should not be null");
154-
assertTrue(status.isHealthy(), "Collection should be healthy");
155-
156-
// Verify response time
157-
assertNotNull(status.responseTime(), "Response time should not be null");
158-
assertTrue(status.responseTime() >= 0, "Response time should be non-negative");
159-
assertTrue(status.responseTime() < 30000, "Response time should be reasonable (< 30 seconds)");
160-
161-
// Verify document count
162-
assertNotNull(status.totalDocuments(), "Total documents should not be null");
163-
assertTrue(status.totalDocuments() >= 0, "Total documents should be non-negative");
143+
log.debug("Health status: {}", status);
164144

165-
// Verify timestamp
166-
assertNotNull(status.lastChecked(), "Last checked timestamp should not be null");
167-
long currentTime = System.currentTimeMillis();
168-
long lastCheckedTime = status.lastChecked().getTime();
169-
assertTrue(currentTime - lastCheckedTime < 5000,
170-
"Last checked timestamp should be very recent (within 5 seconds)");
145+
assertNotNull(status);
146+
assertTrue(status.isHealthy());
147+
assertNull(status.errorMessage());
148+
assertEquals(TEST_COLLECTION, status.collection());
171149

172-
// Verify no error message for healthy collection
173-
assertNull(status.errorMessage(), "Error message should be null for healthy collection");
150+
assertNotNull(status.responseTime());
151+
assertTrue(status.responseTime() >= 0);
174152

175-
// Verify collection name is populated
176-
assertEquals(TEST_COLLECTION, status.collection(), "Collection name should be populated in health status");
153+
assertEquals((long) DOC_COUNT, status.totalDocuments(), "Health check should report indexed document count");
177154

178-
// Verify string representation contains meaningful information
179-
String statusString = status.toString();
180-
assertTrue(statusString.contains("healthy") || statusString.contains("true"),
181-
"Status string should indicate healthy state");
155+
assertNotNull(status.lastChecked());
156+
assertTrue(System.currentTimeMillis() - status.lastChecked().getTime() < 5000);
182157
}
183158

184159
@Test
185-
void testCheckHealthUnhealthy() {
186-
// Test checking health of an invalid collection
187-
String nonExistentCollection = "non_existent_collection";
188-
SolrHealthStatus status = collectionService.checkHealth(nonExistentCollection);
189-
190-
log.debug("Health status for invalid collection: {}", status);
191-
192-
// Enhanced assertions for unhealthy collection
193-
assertNotNull(status, "Health status should not be null");
194-
assertFalse(status.isHealthy(), "Collection should not be healthy");
195-
196-
// Verify timestamp
197-
assertNotNull(status.lastChecked(), "Last checked timestamp should not be null");
198-
long currentTime = System.currentTimeMillis();
199-
long lastCheckedTime = status.lastChecked().getTime();
200-
assertTrue(currentTime - lastCheckedTime < 5000,
201-
"Last checked timestamp should be very recent (within 5 seconds)");
202-
203-
// Verify error message
204-
assertNotNull(status.errorMessage(), "Error message should not be null for unhealthy collection");
205-
assertFalse(status.errorMessage().trim().isEmpty(),
206-
"Error message should not be empty for unhealthy collection");
207-
208-
// Verify that performance metrics are null for unhealthy collection
209-
assertNull(status.responseTime(), "Response time should be null for unhealthy collection");
210-
assertNull(status.totalDocuments(), "Total documents should be null for unhealthy collection");
211-
212-
// Verify collection name is populated even for unhealthy collection
213-
assertEquals(nonExistentCollection, status.collection(),
214-
"Collection name should be populated for unhealthy collection");
215-
216-
// Verify error message contains meaningful information
217-
String errorMessage = status.errorMessage().toLowerCase();
218-
assertTrue(
219-
errorMessage.contains("collection") || errorMessage.contains("not found")
220-
|| errorMessage.contains("error") || errorMessage.contains("fail"),
221-
"Error message should contain meaningful error information");
222-
223-
// Verify string representation indicates unhealthy state
224-
String statusString = status.toString();
225-
if (statusString != null) {
226-
assertTrue(statusString.contains("false") || statusString.contains("unhealthy")
227-
|| statusString.contains("error"), "Status string should indicate unhealthy state");
228-
}
160+
void testCheckHealth_nonExistentCollection() {
161+
String missing = "non_existent_collection";
162+
SolrHealthStatus status = collectionService.checkHealth(missing);
163+
164+
assertNotNull(status);
165+
assertFalse(status.isHealthy());
166+
assertNotNull(status.errorMessage());
167+
assertFalse(status.errorMessage().isBlank());
168+
assertEquals(missing, status.collection());
169+
assertNull(status.responseTime());
170+
assertNull(status.totalDocuments());
229171
}
230172

231173
@Test
232174
void testCollectionNameExtraction() {
233-
// Test collection name extraction functionality
234-
assertEquals(TEST_COLLECTION, collectionService.extractCollectionName(TEST_COLLECTION),
235-
"Regular collection name should be returned as-is");
236-
237-
assertEquals("films", collectionService.extractCollectionName("films_shard1_replica_n1"),
238-
"Shard name should be extracted to base collection name");
239-
240-
assertEquals("products", collectionService.extractCollectionName("products_shard2_replica_n3"),
241-
"Complex shard name should be extracted correctly");
242-
243-
assertNull(collectionService.extractCollectionName(null), "Null input should return null");
244-
245-
assertEquals("", collectionService.extractCollectionName(""), "Empty string should return empty string");
175+
assertEquals(TEST_COLLECTION, collectionService.extractCollectionName(TEST_COLLECTION));
176+
assertEquals("films", collectionService.extractCollectionName("films_shard1_replica_n1"));
177+
assertEquals("products", collectionService.extractCollectionName("products_shard2_replica_n3"));
178+
assertNull(collectionService.extractCollectionName(null));
179+
assertEquals("", collectionService.extractCollectionName(""));
246180
}
247181

248182
@Test
249-
void testGetCacheMetrics() {
183+
void testGetCacheMetrics_afterQueries() {
250184
CacheStats cacheStats = collectionService.getCacheMetrics(TEST_COLLECTION);
251185

252-
assertNotNull(cacheStats, "Cache stats should not be null for an existing collection");
186+
assertNotNull(cacheStats, "Cache stats should not be null after warm-up queries");
253187

254-
// Solr registers all three caches at collection creation
255-
assertNotNull(cacheStats.queryResultCache(), "Query result cache should not be null");
256-
assertTrue(cacheStats.queryResultCache().lookups() >= 0, "Query result cache lookups should be non-negative");
188+
// Query result cache: warm-up queries should have generated lookups
189+
CacheInfo qrc = cacheStats.queryResultCache();
190+
assertNotNull(qrc);
191+
assertTrue(qrc.lookups() > 0, "queryResultCache lookups should be positive after queries");
192+
assertNotNull(qrc.hitratio());
193+
assertNotNull(qrc.size());
257194

258-
assertNotNull(cacheStats.documentCache(), "Document cache should not be null");
259-
assertTrue(cacheStats.documentCache().lookups() >= 0, "Document cache lookups should be non-negative");
195+
// Document cache: reading documents populates this cache
196+
CacheInfo dc = cacheStats.documentCache();
197+
assertNotNull(dc);
198+
assertTrue(dc.lookups() > 0, "documentCache lookups should be positive after queries");
260199

261-
assertNotNull(cacheStats.filterCache(), "Filter cache should not be null");
262-
assertTrue(cacheStats.filterCache().lookups() >= 0, "Filter cache lookups should be non-negative");
200+
// Filter cache: the filter queries we ran should generate lookups
201+
CacheInfo fc = cacheStats.filterCache();
202+
assertNotNull(fc);
203+
assertTrue(fc.lookups() > 0, "filterCache lookups should be positive after filter queries");
263204
}
264205

265206
@Test
266-
void testGetHandlerMetrics() {
207+
void testGetHandlerMetrics_afterQueriesAndIndexing() {
267208
HandlerStats handlerStats = collectionService.getHandlerMetrics(TEST_COLLECTION);
268209

269-
assertNotNull(handlerStats, "Handler stats should not be null for an existing collection");
210+
assertNotNull(handlerStats, "Handler stats should not be null after activity");
270211

271-
assertNotNull(handlerStats.selectHandler(), "Select handler should not be null");
272-
assertTrue(handlerStats.selectHandler().requests() >= 0, "Select handler requests should be non-negative");
212+
// Select handler: warm-up queries should have driven request counts > 0
213+
HandlerInfo select = handlerStats.selectHandler();
214+
assertNotNull(select);
215+
assertTrue(select.requests() > 0, "Select handler requests should be positive after queries");
216+
assertNotNull(select.errors());
217+
assertNotNull(select.timeouts());
273218

274-
assertNotNull(handlerStats.updateHandler(), "Update handler should not be null");
275-
assertTrue(handlerStats.updateHandler().requests() >= 0, "Update handler requests should be non-negative");
219+
// Update handler: indexing 50 docs should have driven request counts > 0
220+
HandlerInfo update = handlerStats.updateHandler();
221+
assertNotNull(update);
222+
assertTrue(update.requests() > 0, "Update handler requests should be positive after indexing");
276223
}
277224

278225
@Test
279-
void testGetCacheMetrics_NonExistentCollection() {
280-
CacheStats result = collectionService.getCacheMetrics("non_existent_collection");
281-
assertNull(result, "Cache metrics for non-existent collection should be null");
226+
void testGetCacheMetrics_nonExistentCollection() {
227+
assertNull(collectionService.getCacheMetrics("non_existent_collection"));
282228
}
283229

284230
@Test
285-
void testGetHandlerMetrics_NonExistentCollection() {
286-
HandlerStats result = collectionService.getHandlerMetrics("non_existent_collection");
287-
assertNull(result, "Handler metrics for non-existent collection should be null");
231+
void testGetHandlerMetrics_nonExistentCollection() {
232+
assertNull(collectionService.getHandlerMetrics("non_existent_collection"));
288233
}
289234

290235
@Test
@@ -293,13 +238,13 @@ void createCollection_createsAndListable() throws Exception {
293238

294239
CollectionCreationResult result = collectionService.createCollection(name, null, null, null);
295240

296-
assertTrue(result.success(), "Collection creation should succeed");
297-
assertEquals(name, result.name(), "Result should contain the collection name");
298-
assertNotNull(result.createdAt(), "Creation timestamp should be set");
241+
assertTrue(result.success());
242+
assertEquals(name, result.name());
243+
assertNotNull(result.createdAt());
299244

300245
List<String> collections = collectionService.listCollections();
301-
boolean collectionExists = collections.contains(name)
246+
boolean exists = collections.contains(name)
302247
|| collections.stream().anyMatch(col -> col.startsWith(name + "_shard"));
303-
assertTrue(collectionExists, "Newly created collection should appear in list (found: " + collections + ")");
248+
assertTrue(exists, "Newly created collection should appear in list (found: " + collections + ")");
304249
}
305250
}

0 commit comments

Comments
 (0)