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]

Reply via email to