dsmiley commented on code in PR #1218:
URL: https://github.com/apache/solr/pull/1218#discussion_r1071731157


##########
solr/core/src/test/org/apache/solr/update/RootFieldTest.java:
##########
@@ -46,7 +49,15 @@ public static void beforeTest() throws Exception {
     useRootSchema = random().nextBoolean();
     // schema15.xml declares _root_ field, while schema-rest.xml does not.
     String schema = useRootSchema ? "schema15.xml" : "schema-rest.xml";
-    initCore("solrconfig.xml", schema);
+    SolrTestCaseJ4.newRandomConfig();
+
+    solrClientTestRule.startSolr(Path.of(SolrTestCaseJ4.TEST_HOME()));
+
+    solrClientTestRule
+        .newCollection(DEFAULT_TEST_COLLECTION_NAME)

Review Comment:
   Let's make "collection1" the default if no arg is provided.  Most tests 
don't care what its name is.



##########
solr/solrj/src/test/org/apache/solr/client/solrj/embedded/TestSolrProperties.java:
##########
@@ -26,18 +33,59 @@
 import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.client.solrj.response.CoreAdminResponse;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
 import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.junit.rules.TestRule;
 
-/**
- * @since solr 1.3
- */
-public class TestSolrProperties extends AbstractEmbeddedSolrServerTestCase {
-  private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+/** Test properties in configuration files. */
+public class TestSolrProperties extends SolrTestCase {

Review Comment:
   Lets bring back AbstractEmbeddedSolrServerTestCase to keep code in common 
between its two subclasses, but mark it deprecated.



##########
solr/solrj/src/test/org/apache/solr/client/solrj/embedded/TestSolrProperties.java:
##########
@@ -57,38 +105,58 @@ public void testProperties() throws Exception {
 
     // Add to core0
     up.add(doc);
-    up.process(getSolrCore0());
+    up.process(solrClientTestRule.getSolrClient("core0"));
 
     SolrTestCaseJ4.ignoreException("unknown field");
 
     // You can't add it to core1
-    expectThrows(Exception.class, () -> up.process(getSolrCore1()));
+    expectThrows(Exception.class, () -> 
up.process(solrClientTestRule.getSolrClient("core1")));
 
     // Add to core1
     doc.setField("id", "BBB");
     doc.setField("core1", "yup stopfra stopfrb stopena stopenb");
     doc.removeField("core0");
     up.add(doc);
-    up.process(getSolrCore1());
+    up.process(solrClientTestRule.getSolrClient("core1"));
 
     // You can't add it to core1
     SolrTestCaseJ4.ignoreException("core0");
-    expectThrows(Exception.class, () -> up.process(getSolrCore0()));
+    expectThrows(Exception.class, () -> 
up.process(solrClientTestRule.getSolrClient("core0")));
     SolrTestCaseJ4.resetExceptionIgnores();
 
     // now Make sure AAA is in 0 and BBB in 1
     SolrQuery q = new SolrQuery();
     QueryRequest r = new QueryRequest(q);
     q.setQuery("id:AAA");
-    assertEquals(1, r.process(getSolrCore0()).getResults().size());
-    assertEquals(0, r.process(getSolrCore1()).getResults().size());
+    assertEquals(1, 
r.process(solrClientTestRule.getSolrClient("core0")).getResults().size());
+    assertEquals(0, 
r.process(solrClientTestRule.getSolrClient("core1")).getResults().size());
 
     // Now test Changing the default core
-    assertEquals(1, getSolrCore0().query(new 
SolrQuery("id:AAA")).getResults().size());
-    assertEquals(0, getSolrCore0().query(new 
SolrQuery("id:BBB")).getResults().size());
-
-    assertEquals(0, getSolrCore1().query(new 
SolrQuery("id:AAA")).getResults().size());
-    assertEquals(1, getSolrCore1().query(new 
SolrQuery("id:BBB")).getResults().size());
+    assertEquals(
+        1,
+        (solrClientTestRule.getSolrClient("core0"))

Review Comment:
   This is too verbose.  getSolrCore0 & 1 can return; ehhh? 



##########
solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.lucene.tests.util.LuceneTestCase;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.update.UpdateShardHandlerConfig;
+
+/** TODO NOCOMMIT document */
+public class EmbeddedSolrServerTestRule extends SolrClientTestRule {
+
+  private static final String CORE_DIR_PROP = "coreRootDirectory";
+  private EmbeddedSolrServer adminClient = null;
+  private EmbeddedSolrServer client = null;
+  private CoreContainer container = null;
+
+  private boolean clearCoreDirSysProp = false;
+
+  public EmbeddedSolrServer getAdminClient() {
+    return adminClient;
+  }
+
+  public void startSolr(Path solrHome) {
+    NodeConfig nodeConfig;
+    if (Files.exists(solrHome.resolve(SolrXmlConfig.SOLR_XML_FILE))) {
+      // existing solr.xml; perhaps not recommended for new/most tests
+
+      // solr.xml coreRootDirectory is best set to a temp directory in a test 
so that
+      //  (a) we don't load existing cores
+      //      Because it's better for tests to explicitly create cores.
+      //  (b) we don't write data in the test to a likely template directory
+      //  But a test can insist on something if it sets the property.
+      if (System.getProperty(CORE_DIR_PROP) == null) {
+        clearCoreDirSysProp = true;
+        System.setProperty(CORE_DIR_PROP, 
LuceneTestCase.createTempDir("cores").toString());
+      }
+
+      nodeConfig = SolrXmlConfig.fromSolrHome(solrHome, null);
+    } else {
+      // test oriented config (preferred)
+      nodeConfig = newNodeConfigBuilder(solrHome).build();
+    }
+
+    startSolr(nodeConfig);
+  }
+
+  public void startSolr(NodeConfig nodeConfig) {
+    container = new CoreContainer(nodeConfig);
+    container.load();
+    adminClient = new EmbeddedSolrServer(container, null);
+  }
+
+  public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) {
+    // TODO nocommit dedupe this with TestHarness
+    var updateShardHandlerConfig =
+        new UpdateShardHandlerConfig(
+            HttpClientUtil.DEFAULT_MAXCONNECTIONS,
+            HttpClientUtil.DEFAULT_MAXCONNECTIONSPERHOST,
+            30000,
+            30000,
+            UpdateShardHandlerConfig.DEFAULT_METRICNAMESTRATEGY,
+            UpdateShardHandlerConfig.DEFAULT_MAXRECOVERYTHREADS);
+
+    return new NodeConfig.NodeConfigBuilder("testNode", solrHome)
+        .setUpdateShardHandlerConfig(updateShardHandlerConfig)
+        
.setCoreRootDirectory(LuceneTestCase.createTempDir("cores").toString());
+  }
+
+  public NewCollectionBuilder newCollection(String name) {
+    return new NewCollectionBuilder(name);
+  }
+
+  public class NewCollectionBuilder {
+    private String name;
+    private String configSet;
+    private String configFile;
+    private String schemaFile;
+
+    public NewCollectionBuilder(String name) {
+      this.name = name;
+    }
+
+    public NewCollectionBuilder withConfigSet(String configSet) {
+      // Chop off "/conf" if found -- configSet can be a path.
+      // This is a hack so that we can continue to use 
ExternalPaths.DEFAULT_CONFIGSET etc. as-is.
+      // TODO FileSystemConfigSetService.locateInstanceDir should have this 
logic.
+      // Without this, managed resources might be written to
+      // conf/conf/_schema_analysis_stopwords_english.json because 
SolrResourceLoader points to the
+      // wrong dir.
+      if (configSet != null && configSet.endsWith("/conf")) {
+        configSet = configSet.substring(0, configSet.length() - 
"/conf".length());
+      }

Review Comment:
   I'll tend to this improvement in another issue/PR.



##########
solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java:
##########
@@ -0,0 +1,273 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.tests.util.LuceneTestCase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
+import 
org.apache.solr.client.solrj.embedded.EmbeddedSolrServer.RequestWriterSupplier;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.core.CloudConfig;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.CoreDescriptor;
+import org.apache.solr.core.MetricsConfig;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrConfig;
+import org.apache.solr.metrics.reporters.SolrJmxReporter;
+import org.apache.solr.schema.IndexSchemaFactory;
+import org.apache.solr.update.UpdateShardHandlerConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class EmbeddedSolrServerTestRule extends SolrClientTestRule {

Review Comment:
   @joshgog still waiting for docs in general.  Just give it a shot; I can 
clean up after.



##########
solr/solrj/src/test/org/apache/solr/client/solrj/response/TermsResponseTest.java:
##########
@@ -31,16 +35,19 @@ public class TermsResponseTest extends 
EmbeddedSolrServerTestBase {
 
   @BeforeClass
   public static void beforeClass() throws Exception {
-    initCore();
+    solrClientTestRule.startSolr(LuceneTestCase.createTempDir("solrhome"));
+
+    solrClientTestRule
+        .newCollection(DEFAULT_TEST_COLLECTION_NAME)
+        .withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET)
+        .create();

Review Comment:
   Exemplifies we're not after shaving lines of code.  Starting Solr and 
creating a collection with a known configSet is explicit and should be -- isn't 
verbose either.



##########
solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.lucene.tests.util.LuceneTestCase;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.update.UpdateShardHandlerConfig;
+
+/** TODO NOCOMMIT document */
+public class EmbeddedSolrServerTestRule extends SolrClientTestRule {
+
+  private static final String CORE_DIR_PROP = "coreRootDirectory";
+  private EmbeddedSolrServer adminClient = null;
+  private EmbeddedSolrServer client = null;
+  private CoreContainer container = null;
+
+  private boolean clearCoreDirSysProp = false;
+
+  public EmbeddedSolrServer getAdminClient() {
+    return adminClient;
+  }
+
+  public void startSolr(Path solrHome) {
+    NodeConfig nodeConfig;
+    if (Files.exists(solrHome.resolve(SolrXmlConfig.SOLR_XML_FILE))) {
+      // existing solr.xml; perhaps not recommended for new/most tests
+
+      // solr.xml coreRootDirectory is best set to a temp directory in a test 
so that
+      //  (a) we don't load existing cores
+      //      Because it's better for tests to explicitly create cores.
+      //  (b) we don't write data in the test to a likely template directory
+      //  But a test can insist on something if it sets the property.
+      if (System.getProperty(CORE_DIR_PROP) == null) {
+        clearCoreDirSysProp = true;
+        System.setProperty(CORE_DIR_PROP, 
LuceneTestCase.createTempDir("cores").toString());
+      }
+
+      nodeConfig = SolrXmlConfig.fromSolrHome(solrHome, null);
+    } else {
+      // test oriented config (preferred)
+      nodeConfig = newNodeConfigBuilder(solrHome).build();
+    }
+
+    startSolr(nodeConfig);
+  }
+
+  public void startSolr(NodeConfig nodeConfig) {
+    container = new CoreContainer(nodeConfig);
+    container.load();
+    adminClient = new EmbeddedSolrServer(container, null);
+  }
+
+  public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) {
+    // TODO nocommit dedupe this with TestHarness
+    var updateShardHandlerConfig =
+        new UpdateShardHandlerConfig(
+            HttpClientUtil.DEFAULT_MAXCONNECTIONS,
+            HttpClientUtil.DEFAULT_MAXCONNECTIONSPERHOST,
+            30000,
+            30000,
+            UpdateShardHandlerConfig.DEFAULT_METRICNAMESTRATEGY,
+            UpdateShardHandlerConfig.DEFAULT_MAXRECOVERYTHREADS);

Review Comment:
   Let's create a constant declared at package scope so that TestHarness can 
refer to it, and we'll be done.  Actually, defining it on 
UpdateShardHandlerConfig would be nicer (debatable).



##########
solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.lucene.tests.util.LuceneTestCase;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.update.UpdateShardHandlerConfig;
+
+/** TODO NOCOMMIT document */
+public class EmbeddedSolrServerTestRule extends SolrClientTestRule {
+
+  private static final String CORE_DIR_PROP = "coreRootDirectory";
+  private EmbeddedSolrServer adminClient = null;
+  private EmbeddedSolrServer client = null;
+  private CoreContainer container = null;
+
+  private boolean clearCoreDirSysProp = false;
+
+  public EmbeddedSolrServer getAdminClient() {
+    return adminClient;
+  }
+
+  public void startSolr(Path solrHome) {
+    NodeConfig nodeConfig;
+    if (Files.exists(solrHome.resolve(SolrXmlConfig.SOLR_XML_FILE))) {
+      // existing solr.xml; perhaps not recommended for new/most tests
+
+      // solr.xml coreRootDirectory is best set to a temp directory in a test 
so that
+      //  (a) we don't load existing cores
+      //      Because it's better for tests to explicitly create cores.
+      //  (b) we don't write data in the test to a likely template directory
+      //  But a test can insist on something if it sets the property.
+      if (System.getProperty(CORE_DIR_PROP) == null) {
+        clearCoreDirSysProp = true;
+        System.setProperty(CORE_DIR_PROP, 
LuceneTestCase.createTempDir("cores").toString());
+      }
+
+      nodeConfig = SolrXmlConfig.fromSolrHome(solrHome, null);
+    } else {
+      // test oriented config (preferred)
+      nodeConfig = newNodeConfigBuilder(solrHome).build();
+    }
+
+    startSolr(nodeConfig);
+  }
+
+  public void startSolr(NodeConfig nodeConfig) {
+    container = new CoreContainer(nodeConfig);
+    container.load();
+    adminClient = new EmbeddedSolrServer(container, null);
+  }
+
+  public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) {
+    // TODO nocommit dedupe this with TestHarness
+    var updateShardHandlerConfig =
+        new UpdateShardHandlerConfig(
+            HttpClientUtil.DEFAULT_MAXCONNECTIONS,
+            HttpClientUtil.DEFAULT_MAXCONNECTIONSPERHOST,
+            30000,
+            30000,
+            UpdateShardHandlerConfig.DEFAULT_METRICNAMESTRATEGY,
+            UpdateShardHandlerConfig.DEFAULT_MAXRECOVERYTHREADS);
+
+    return new NodeConfig.NodeConfigBuilder("testNode", solrHome)
+        .setUpdateShardHandlerConfig(updateShardHandlerConfig)
+        
.setCoreRootDirectory(LuceneTestCase.createTempDir("cores").toString());
+  }
+
+  public NewCollectionBuilder newCollection(String name) {

Review Comment:
   Define on SolrClientTestRule as it's going to be common to all rules.  
Obviously, the builder would tag along with it.



##########
solr/solrj/src/test/org/apache/solr/client/solrj/request/json/DirectJsonQueryRequestFacetingEmbeddedTest.java:
##########
@@ -53,32 +50,15 @@ public class DirectJsonQueryRequestFacetingEmbeddedTest 
extends EmbeddedSolrServ
 
   @BeforeClass
   public static void beforeClass() throws Exception {
-    final String sourceHome = ExternalPaths.SOURCE_HOME;
-
-    final File tempSolrHome = LuceneTestCase.createTempDir().toFile();
-    FileUtils.copyFileToDirectory(new File(sourceHome, 
"server/solr/solr.xml"), tempSolrHome);
-    final File collectionDir = new File(tempSolrHome, COLLECTION_NAME);
-    FileUtils.forceMkdir(collectionDir);
-    final File configSetDir =
-        new File(sourceHome, 
"server/solr/configsets/sample_techproducts_configs/conf");
-    FileUtils.copyDirectoryToDirectory(configSetDir, collectionDir);
-
-    final Properties props = new Properties();
-    props.setProperty("name", COLLECTION_NAME);
-
-    try (Writer writer =
-        new OutputStreamWriter(
-            FileUtils.openOutputStream(new File(collectionDir, 
"core.properties")), "UTF-8"); ) {
-      props.store(writer, null);
-    }
 
-    final String config =
-        tempSolrHome.getAbsolutePath() + "/" + COLLECTION_NAME + 
"/conf/solrconfig.xml";
-    final String schema =
-        tempSolrHome.getAbsolutePath() + "/" + COLLECTION_NAME + 
"/conf/managed-schema";
-    initCore(config, schema, tempSolrHome.getAbsolutePath(), COLLECTION_NAME);
+    solrClientTestRule.startSolr(LuceneTestCase.createTempDir());
+
+    solrClientTestRule
+        .newCollection(COLLECTION_NAME)
+        .withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET)
+        .create();

Review Comment:
   Huge improvement as I said before; maybe best example.  Interestingly this 
shows how we can use some empty temp dir as Solr home, and all we need to do is 
reference a path to the configSet we want when creating a collection.  
@gerlowskija I believe you wrote this originally.



##########
solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java:
##########
@@ -46,35 +50,58 @@
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.metrics.SolrCoreMetricManager;
 import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
 import org.hamcrest.MatcherAssert;
-import org.junit.After;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
-public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase {
+public class TestCoreAdmin extends SolrTestCase {

Review Comment:
   Again, let's keep AbstractEmbeddedSolrServerTestCase to avoid the 
duplication between here & the properties test.  I don't think its worth or 
time now to eliminate some questionable base classes.



##########
solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.lucene.tests.util.LuceneTestCase;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.update.UpdateShardHandlerConfig;
+
+/** TODO NOCOMMIT document */
+public class EmbeddedSolrServerTestRule extends SolrClientTestRule {
+
+  private static final String CORE_DIR_PROP = "coreRootDirectory";
+  private EmbeddedSolrServer adminClient = null;
+  private EmbeddedSolrServer client = null;
+  private CoreContainer container = null;
+
+  private boolean clearCoreDirSysProp = false;
+
+  public EmbeddedSolrServer getAdminClient() {
+    return adminClient;
+  }
+
+  public void startSolr(Path solrHome) {
+    NodeConfig nodeConfig;
+    if (Files.exists(solrHome.resolve(SolrXmlConfig.SOLR_XML_FILE))) {
+      // existing solr.xml; perhaps not recommended for new/most tests
+
+      // solr.xml coreRootDirectory is best set to a temp directory in a test 
so that
+      //  (a) we don't load existing cores
+      //      Because it's better for tests to explicitly create cores.
+      //  (b) we don't write data in the test to a likely template directory
+      //  But a test can insist on something if it sets the property.
+      if (System.getProperty(CORE_DIR_PROP) == null) {
+        clearCoreDirSysProp = true;
+        System.setProperty(CORE_DIR_PROP, 
LuceneTestCase.createTempDir("cores").toString());
+      }
+
+      nodeConfig = SolrXmlConfig.fromSolrHome(solrHome, null);
+    } else {
+      // test oriented config (preferred)
+      nodeConfig = newNodeConfigBuilder(solrHome).build();
+    }
+
+    startSolr(nodeConfig);
+  }
+
+  public void startSolr(NodeConfig nodeConfig) {
+    container = new CoreContainer(nodeConfig);
+    container.load();
+    adminClient = new EmbeddedSolrServer(container, null);
+  }
+
+  public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) {
+    // TODO nocommit dedupe this with TestHarness
+    var updateShardHandlerConfig =
+        new UpdateShardHandlerConfig(
+            HttpClientUtil.DEFAULT_MAXCONNECTIONS,
+            HttpClientUtil.DEFAULT_MAXCONNECTIONSPERHOST,
+            30000,
+            30000,
+            UpdateShardHandlerConfig.DEFAULT_METRICNAMESTRATEGY,
+            UpdateShardHandlerConfig.DEFAULT_MAXRECOVERYTHREADS);
+
+    return new NodeConfig.NodeConfigBuilder("testNode", solrHome)
+        .setUpdateShardHandlerConfig(updateShardHandlerConfig)
+        
.setCoreRootDirectory(LuceneTestCase.createTempDir("cores").toString());
+  }
+
+  public NewCollectionBuilder newCollection(String name) {
+    return new NewCollectionBuilder(name);
+  }
+
+  public class NewCollectionBuilder {
+    private String name;
+    private String configSet;
+    private String configFile;
+    private String schemaFile;
+
+    public NewCollectionBuilder(String name) {
+      this.name = name;
+    }
+
+    public NewCollectionBuilder withConfigSet(String configSet) {
+      // Chop off "/conf" if found -- configSet can be a path.
+      // This is a hack so that we can continue to use 
ExternalPaths.DEFAULT_CONFIGSET etc. as-is.
+      // TODO FileSystemConfigSetService.locateInstanceDir should have this 
logic.
+      // Without this, managed resources might be written to
+      // conf/conf/_schema_analysis_stopwords_english.json because 
SolrResourceLoader points to the
+      // wrong dir.
+      if (configSet != null && configSet.endsWith("/conf")) {
+        configSet = configSet.substring(0, configSet.length() - 
"/conf".length());
+      }
+
+      this.configSet = configSet;
+      return this;
+    }
+
+    public NewCollectionBuilder withConfigFile(String configFile) {
+      this.configFile = configFile;
+      return this;
+    }
+
+    public NewCollectionBuilder withSchemaFile(String schemaFile) {
+      this.schemaFile = schemaFile;
+      return this;
+    }
+
+    public String getName() {
+      return name;
+    }
+
+    public String getConfigSet() {
+      return configSet;
+    }
+
+    public String getConfigFile() {
+      return configFile;
+    }
+
+    public String getSchemaFile() {
+      return schemaFile;
+    }
+
+    public void create() throws SolrServerException, IOException {
+      EmbeddedSolrServerTestRule.this.create(this);
+    }
+  }
+
+  private void create(NewCollectionBuilder b) throws SolrServerException, 
IOException {
+
+    client = new EmbeddedSolrServer(container, b.getName());
+
+    CoreAdminRequest.Create req = new CoreAdminRequest.Create();
+    req.setCoreName(b.getName());
+    req.setInstanceDir(b.getName());
+
+    if (b.getConfigSet() != null) {
+      req.setConfigSet(b.getConfigSet());
+    }
+    if (b.getConfigFile() != null) {
+      req.setConfigName(b.getConfigFile());
+    }
+    if (b.getSchemaFile() != null) {
+      req.setSchemaName(b.getSchemaFile());
+    }
+
+    req.process(client);
+  }
+
+  @Override
+  protected void after() {
+    if (container != null) container.shutdown();
+
+    if (clearCoreDirSysProp) {
+      System.clearProperty(CORE_DIR_PROP);
+    }
+  }
+
+  @Override
+  public EmbeddedSolrServer getSolrClient() {
+    if (client == null) {
+      return adminClient;
+    } else {
+      return client;
+    }
+  }
+
+  @Override
+  public EmbeddedSolrServer getSolrClient(String name) {
+    if (client == null) {
+      return new EmbeddedSolrServer(adminClient.getCoreContainer(), name);
+    } else {
+      return new EmbeddedSolrServer(client.getCoreContainer(), name);
+    }

Review Comment:
   always use adminClient; no point in flip-flopping.



##########
solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.lucene.tests.util.LuceneTestCase;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.update.UpdateShardHandlerConfig;
+
+/** TODO NOCOMMIT document */
+public class EmbeddedSolrServerTestRule extends SolrClientTestRule {
+
+  private static final String CORE_DIR_PROP = "coreRootDirectory";
+  private EmbeddedSolrServer adminClient = null;
+  private EmbeddedSolrServer client = null;
+  private CoreContainer container = null;
+
+  private boolean clearCoreDirSysProp = false;
+
+  public EmbeddedSolrServer getAdminClient() {
+    return adminClient;
+  }
+
+  public void startSolr(Path solrHome) {
+    NodeConfig nodeConfig;
+    if (Files.exists(solrHome.resolve(SolrXmlConfig.SOLR_XML_FILE))) {
+      // existing solr.xml; perhaps not recommended for new/most tests
+
+      // solr.xml coreRootDirectory is best set to a temp directory in a test 
so that
+      //  (a) we don't load existing cores
+      //      Because it's better for tests to explicitly create cores.
+      //  (b) we don't write data in the test to a likely template directory
+      //  But a test can insist on something if it sets the property.
+      if (System.getProperty(CORE_DIR_PROP) == null) {
+        clearCoreDirSysProp = true;
+        System.setProperty(CORE_DIR_PROP, 
LuceneTestCase.createTempDir("cores").toString());
+      }
+
+      nodeConfig = SolrXmlConfig.fromSolrHome(solrHome, null);
+    } else {
+      // test oriented config (preferred)
+      nodeConfig = newNodeConfigBuilder(solrHome).build();
+    }
+
+    startSolr(nodeConfig);
+  }
+
+  public void startSolr(NodeConfig nodeConfig) {
+    container = new CoreContainer(nodeConfig);
+    container.load();
+    adminClient = new EmbeddedSolrServer(container, null);
+  }
+
+  public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) {
+    // TODO nocommit dedupe this with TestHarness
+    var updateShardHandlerConfig =
+        new UpdateShardHandlerConfig(
+            HttpClientUtil.DEFAULT_MAXCONNECTIONS,
+            HttpClientUtil.DEFAULT_MAXCONNECTIONSPERHOST,
+            30000,
+            30000,
+            UpdateShardHandlerConfig.DEFAULT_METRICNAMESTRATEGY,
+            UpdateShardHandlerConfig.DEFAULT_MAXRECOVERYTHREADS);
+
+    return new NodeConfig.NodeConfigBuilder("testNode", solrHome)
+        .setUpdateShardHandlerConfig(updateShardHandlerConfig)
+        
.setCoreRootDirectory(LuceneTestCase.createTempDir("cores").toString());
+  }
+
+  public NewCollectionBuilder newCollection(String name) {
+    return new NewCollectionBuilder(name);
+  }
+
+  public class NewCollectionBuilder {
+    private String name;
+    private String configSet;
+    private String configFile;
+    private String schemaFile;
+
+    public NewCollectionBuilder(String name) {
+      this.name = name;
+    }
+
+    public NewCollectionBuilder withConfigSet(String configSet) {
+      // Chop off "/conf" if found -- configSet can be a path.
+      // This is a hack so that we can continue to use 
ExternalPaths.DEFAULT_CONFIGSET etc. as-is.
+      // TODO FileSystemConfigSetService.locateInstanceDir should have this 
logic.
+      // Without this, managed resources might be written to
+      // conf/conf/_schema_analysis_stopwords_english.json because 
SolrResourceLoader points to the
+      // wrong dir.
+      if (configSet != null && configSet.endsWith("/conf")) {
+        configSet = configSet.substring(0, configSet.length() - 
"/conf".length());
+      }
+
+      this.configSet = configSet;
+      return this;
+    }
+
+    public NewCollectionBuilder withConfigFile(String configFile) {
+      this.configFile = configFile;
+      return this;
+    }
+
+    public NewCollectionBuilder withSchemaFile(String schemaFile) {
+      this.schemaFile = schemaFile;
+      return this;
+    }
+
+    public String getName() {
+      return name;
+    }
+
+    public String getConfigSet() {
+      return configSet;
+    }
+
+    public String getConfigFile() {
+      return configFile;
+    }
+
+    public String getSchemaFile() {
+      return schemaFile;
+    }
+
+    public void create() throws SolrServerException, IOException {
+      EmbeddedSolrServerTestRule.this.create(this);
+    }
+  }
+
+  private void create(NewCollectionBuilder b) throws SolrServerException, 
IOException {
+
+    client = new EmbeddedSolrServer(container, b.getName());
+
+    CoreAdminRequest.Create req = new CoreAdminRequest.Create();
+    req.setCoreName(b.getName());
+    req.setInstanceDir(b.getName());
+
+    if (b.getConfigSet() != null) {
+      req.setConfigSet(b.getConfigSet());
+    }
+    if (b.getConfigFile() != null) {
+      req.setConfigName(b.getConfigFile());
+    }
+    if (b.getSchemaFile() != null) {
+      req.setSchemaName(b.getSchemaFile());
+    }
+
+    req.process(client);
+  }
+
+  @Override
+  protected void after() {
+    if (container != null) container.shutdown();
+
+    if (clearCoreDirSysProp) {
+      System.clearProperty(CORE_DIR_PROP);
+    }
+  }
+
+  @Override
+  public EmbeddedSolrServer getSolrClient() {
+    if (client == null) {
+      return adminClient;
+    } else {
+      return client;
+    }

Review Comment:
   Let's not assume the last call to create a collection is the default client 
(the `client` field).  Instead, let's establish that `getSolrClient` will 
always return a client to "collection1".  It should work even if there is such 
a core loaded by Solr in startSolr (i.e. no call to explicitly create a 
collection).  Don't return adminClient here either.



##########
solr/solrj/src/test/org/apache/solr/client/solrj/request/SolrPingTest.java:
##########
@@ -31,18 +32,21 @@ public class SolrPingTest extends 
EmbeddedSolrServerTestBase {
 
   @BeforeClass
   public static void beforeClass() throws Exception {
-    File testHome = createTempDir().toFile();
-    FileUtils.copyDirectory(getFile("solrj/solr"), testHome);
-    initCore("solrconfig.xml", "schema.xml", testHome.getAbsolutePath(), 
"collection1");
+    
solrClientTestRule.startSolr(SolrTestCaseJ4.getFile("solrj/solr").toPath());

Review Comment:
   I think this is cool as it shows that in more cases than previously, we can 
use existing read-only Solr homes that act as templates directly without having 
to copy them, doing wasteful IO.



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org


Reply via email to