Copilot commented on code in PR #3938:
URL: https://github.com/apache/solr/pull/3938#discussion_r2607761646
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java:
##########
@@ -302,45 +302,60 @@ public void testHttpCspPerf() throws Exception {
System.setProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS, "" + Integer.MAX_VALUE);
Review Comment:
The system property `SYS_PROP_CACHE_TIMEOUT_SECONDS` is set but never
cleared. This could affect subsequent tests if they run in the same JVM.
Consider saving the original value before setting it and restoring it in a
`finally` block at the end of the test, or use a try-finally pattern to ensure
cleanup.
Example:
```java
String originalValue = System.getProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS);
try {
System.setProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS, "" + Integer.MAX_VALUE);
// ... test code ...
} finally {
if (originalValue != null) {
System.setProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS, originalValue);
} else {
System.clearProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS);
}
}
```
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java:
##########
@@ -302,45 +302,60 @@ public void testHttpCspPerf() throws Exception {
System.setProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS, "" + Integer.MAX_VALUE);
String collectionName = "HTTPCSPTEST";
- CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1)
- .process(cluster.getSolrClient());
- cluster.waitForActiveCollection(collectionName, 2, 2);
- try (LogListener adminLogs =
LogListener.info(HttpSolrCall.class).substring("[admin]");
- CloudSolrClient solrClient = createHttpCSPBasedCloudSolrClient(); ) {
- solrClient.getClusterStateProvider().getLiveNodes(); // talks to Solr
+ // Create collection using a dedicated client to avoid polluting the test
client's cache.
+ // We avoid using cluster.getSolrClient() because it may have a background
refresh job
+ // already running with the default cache timeout from before the system
property was set.
+ CloudSolrClient setupClient = createHttpCSPBasedCloudSolrClient();
+ try {
+ CollectionAdminRequest.createCollection(collectionName, "conf", 2,
1).process(setupClient);
+ cluster.waitForActiveCollection(collectionName, 2, 2);
+ } finally {
+ setupClient.close();
Review Comment:
Consider using try-with-resources instead of try-finally for `setupClient`
to follow Java best practices for resource management. This is more idiomatic
and ensures proper exception handling.
Example:
```java
try (CloudSolrClient setupClient = createHttpCSPBasedCloudSolrClient()) {
CollectionAdminRequest.createCollection(collectionName, "conf", 2,
1).process(setupClient);
cluster.waitForActiveCollection(collectionName, 2, 2);
}
```
```suggestion
try (CloudSolrClient setupClient = createHttpCSPBasedCloudSolrClient()) {
CollectionAdminRequest.createCollection(collectionName, "conf", 2,
1).process(setupClient);
cluster.waitForActiveCollection(collectionName, 2, 2);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
