dsmiley commented on code in PR #1218:
URL: https://github.com/apache/solr/pull/1218#discussion_r1084222675
##########
solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java:
##########
@@ -282,7 +282,12 @@ private String normalizePathToOsSeparator(String path) {
protected Path locateInstanceDir(CoreDescriptor cd) {
String configSet = cd.getConfigSet();
+
if (configSet == null) return cd.getInstanceDir();
+
+ if (configSet != null && configSet.endsWith("/conf")) {
Review Comment:
I'd rather leave this to another PR
##########
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:
still pending
##########
solr/core/src/java/org/apache/solr/update/UpdateShardHandlerConfig.java:
##########
@@ -32,6 +32,16 @@ public class UpdateShardHandlerConfig {
DEFAULT_METRICNAMESTRATEGY,
DEFAULT_MAXRECOVERYTHREADS);
+ public static final UpdateShardHandlerConfig UPDATE_SHARD_HANDLER_CONFIG =
Review Comment:
Rename to TEST_DEFAULT. TestHarness should refer to this as well.
##########
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:
(still pending; affects many tests)
##########
solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java:
##########
@@ -86,7 +85,7 @@ public void startSolr(NodeConfig nodeConfig) {
public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) {
return new NodeConfig.NodeConfigBuilder("testNode", solrHome)
- .setUpdateShardHandlerConfig(UPDATE_SHARD_HANDLER_CONFIG)
+ .setUpdateShardHandlerConfig(TEST_DEFAULT)
Review Comment:
Please have a qualified (class) reference here instead of a static field
import.
##########
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"));
Review Comment:
> Does this need the "solrhome" parameter? When I see it, it makes me think
it's important somehow
I suppose you are wondering, could we have a default of some temp directory
so that the test doesn't even need to specify it? We could do that. I leaned
to specifying it because this is such a foundational thing to Solr.
> I saw we also have the use of .createTempDir() with no name
It's cosmetic so that if we debug issues, it's more evident what purpose
this temp dir is for.
--
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
