dsmiley commented on code in PR #3947:
URL: https://github.com/apache/solr/pull/3947#discussion_r2643241292
##########
solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java:
##########
@@ -17,67 +17,72 @@
package org.apache.solr.handler.component;
import java.io.IOException;
+import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Properties;
import java.util.Set;
-import org.apache.commons.io.file.PathUtils;
-import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.client.solrj.request.CoreAdminRequest;
import org.apache.solr.client.solrj.request.SolrQuery;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.ShardParams;
import org.apache.solr.common.util.NamedList;
+import org.apache.solr.embedded.JettyConfig;
import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.SolrJettyTestRule;
import org.junit.AfterClass;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
-public class DistributedDebugComponentTest extends SolrJettyTestBase {
+public class DistributedDebugComponentTest extends SolrTestCaseJ4 {
+
+ @ClassRule public static SolrJettyTestRule solrJettyTestRule = new
SolrJettyTestRule();
private static SolrClient collection1;
private static SolrClient collection2;
private static String shard1;
private static String shard2;
private static Path createSolrHome() throws Exception {
- Path workDir = createTempDir();
- setupJettyTestHome(workDir, "collection1");
- PathUtils.copyDirectory(workDir.resolve("collection1"),
workDir.resolve("collection2"));
+ Path workDir = createTempDir().toRealPath();
+
+ Files.copy(
+ SolrTestCaseJ4.TEST_PATH().resolve("solr.xml"),
+ workDir.resolve("solr.xml"),
+ StandardCopyOption.REPLACE_EXISTING);
+
+ Path collection1Dir = workDir.resolve("collection1");
+ Path collection2Dir = workDir.resolve("collection2");
+
+ copyMinConf(collection1Dir, "name=collection1\n");
+ copyMinConf(collection2Dir, "name=collection2\n");
+
return workDir;
}
@BeforeClass
public static void createThings() throws Exception {
systemSetPropertyEnableUrlAllowList(false);
Path solrHome = createSolrHome();
- createAndStartJetty(solrHome);
- String url = getBaseUrl();
-
- collection1 = getHttpSolrClient(url, "collection1");
- collection2 = getHttpSolrClient(url, "collection2");
-
- String urlCollection1 = getBaseUrl() + "/" + "collection1";
- String urlCollection2 = getBaseUrl() + "/" + "collection2";
+ solrJettyTestRule.startSolr(solrHome, new Properties(),
JettyConfig.builder().build());
+ String urlCollection1 = solrJettyTestRule.getBaseUrl() + "/" +
"collection1";
+ String urlCollection2 = solrJettyTestRule.getBaseUrl() + "/" +
"collection2";
shard1 = urlCollection1.replaceAll("https?://", "");
shard2 = urlCollection2.replaceAll("https?://", "");
-
- // create second core
- try (SolrClient nodeClient = getHttpSolrClient(url)) {
- CoreAdminRequest.Create req = new CoreAdminRequest.Create();
- req.setCoreName("collection2");
- req.setConfigSet("collection1");
- nodeClient.request(req);
- }
Review Comment:
Is this test _not_ amenable to `SolrJettyTestRule.create` or maybe
Claude/you didn't know it exists because of too few callers? Some tests may
not be but I haven't examined this one closely to know. When I created the
Solr rule APIs, I wanted to ensure it has a generalized API for core/collection
creation so that we could have some tests minimally change between an
(eventual) SolrCloud vs Jetty vs simply embedded. I also wanted tests to look
more "normal" with respect to SolrClient API. Pre-creation is kind of low
level and also questionable with SolrCloud. If we were to lean in the other
direction (pre-creation), I'd want to see more/better solr-home dir setup
methods. That wouldn't be a bad thing anyway.
@hossman maybe you might have an opinion as an old-timer on test infra here,
if I can be so lucky to get your attention on the GitHub platform ;-).
So the approach you are doing is not wrong. But I was trying to get away
from, not attract to. It's a "preference" matter, definitely not a strong
deprecate/going-away.
--
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]