dsmiley commented on code in PR #1886:
URL: https://github.com/apache/solr/pull/1886#discussion_r1313721917
##########
solr/core/src/test/org/apache/solr/search/TestThinCache.java:
##########
@@ -26,13 +26,63 @@
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.apache.solr.util.TestHarness;
import org.apache.solr.util.stats.MetricUtils;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test for {@link ThinCache}. */
public class TestThinCache extends SolrTestCaseJ4 {
+ @ClassRule public static EmbeddedSolrServerTestRule solrRule = new
EmbeddedSolrServerTestRule();
+ public static final String SOLR_NODE_LEVEL_CACHE_XML =
Review Comment:
String literal (vs a separate file) keeps the test with the config its
testing. Constant field allows it to be used in the "TestSolrXml" test too.
Looking forward to Java 17 where we could use multi-line String syntax!
(maybe worth requiring Java 17 for tests?)
##########
solr/core/src/test/org/apache/solr/search/TestThinCache.java:
##########
@@ -26,13 +26,63 @@
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.apache.solr.util.TestHarness;
import org.apache.solr.util.stats.MetricUtils;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test for {@link ThinCache}. */
public class TestThinCache extends SolrTestCaseJ4 {
+ @ClassRule public static EmbeddedSolrServerTestRule solrRule = new
EmbeddedSolrServerTestRule();
+ public static final String SOLR_NODE_LEVEL_CACHE_XML =
+ "<solr>\n"
+ + " <caches>\n"
+ + " <cache name='myNodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " <cache name='myNodeLevelCacheThin'\n"
+ + " class='solr.ThinCache$NodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " </caches>\n"
+ + "</solr>";
+
+ @BeforeClass
+ public static void setupSolrHome() throws Exception {
+ Path home = createTempDir("home");
+ Files.writeString(home.resolve("solr.xml"), SOLR_NODE_LEVEL_CACHE_XML);
+
+ solrRule.startSolr(home);
+
+ Path configSet = createTempDir("configSet");
+ copyMinConf(configSet.toFile());
+ // insert a special filterCache configuration
+ Path solrConfig = configSet.resolve("conf/solrconfig.xml");
+ Files.writeString(
+ solrConfig,
+ Files.readString(solrConfig)
+ .replace(
+ "</config>",
+ "<query>\n"
+ + "<filterCache\n"
+ + " class=\"solr.ThinCache\"\n"
+ + " parentCacheName=\"myNodeLevelCacheThin\"\n"
+ + " size=\"5\"\n"
+ + " initialSize=\"5\"/>\n"
+ + "</query></config>"));
+
+ solrRule.newCollection().withConfigSet(configSet.toString()).create();
+
+ // legacy; get rid of this someday!
+ h = new TestHarness(solrRule.getCoreContainer());
+ lrf = h.getRequestFactory("/select", 0, 20);
Review Comment:
I need to refresh the vision and discuss/debate it. Your test here, like
most Solr tests, uses `assertU`, `assertQ`, `req`, `doc`, which are implemented
today on XML strings and uses TestHarness. But the vision is to embrace
SolrClient. Nonetheless we need convenience methods to assert via XPath. I
could see a new base test class emerging that has methods named like this and
that which are called with the same idioms yet actually have different return
types (definitely not XML strings).
##########
solr/core/src/test/org/apache/solr/search/TestThinCache.java:
##########
@@ -26,13 +26,63 @@
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.apache.solr.util.TestHarness;
import org.apache.solr.util.stats.MetricUtils;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test for {@link ThinCache}. */
public class TestThinCache extends SolrTestCaseJ4 {
+ @ClassRule public static EmbeddedSolrServerTestRule solrRule = new
EmbeddedSolrServerTestRule();
+ public static final String SOLR_NODE_LEVEL_CACHE_XML =
+ "<solr>\n"
+ + " <caches>\n"
+ + " <cache name='myNodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " <cache name='myNodeLevelCacheThin'\n"
+ + " class='solr.ThinCache$NodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " </caches>\n"
+ + "</solr>";
+
+ @BeforeClass
+ public static void setupSolrHome() throws Exception {
+ Path home = createTempDir("home");
+ Files.writeString(home.resolve("solr.xml"), SOLR_NODE_LEVEL_CACHE_XML);
+
+ solrRule.startSolr(home);
+
+ Path configSet = createTempDir("configSet");
+ copyMinConf(configSet.toFile());
+ // insert a special filterCache configuration
+ Path solrConfig = configSet.resolve("conf/solrconfig.xml");
+ Files.writeString(
+ solrConfig,
+ Files.readString(solrConfig)
+ .replace(
+ "</config>",
+ "<query>\n"
+ + "<filterCache\n"
+ + " class=\"solr.ThinCache\"\n"
+ + " parentCacheName=\"myNodeLevelCacheThin\"\n"
+ + " size=\"5\"\n"
+ + " initialSize=\"5\"/>\n"
+ + "</query></config>"));
+
+ solrRule.newCollection().withConfigSet(configSet.toString()).create();
Review Comment:
the rule uses collection terminology even if it's really a core. The point
is consistent terminology. Sometimes I think we should go the other way
(standardize on "core") unless collection specific options are needed. Shrug.
##########
solr/core/src/test/org/apache/solr/search/TestThinCache.java:
##########
@@ -126,35 +176,4 @@ public void testInitCore() throws Exception {
// for the other node-level cache, simply check that metrics are accessible
assertEquals(0,
nodeMetricsSnapshot.get("CACHE.nodeLevelCache/myNodeLevelCache.size"));
}
-
- @BeforeClass
Review Comment:
Please never put lifecycle methods at the bottom!
##########
solr/core/src/test/org/apache/solr/search/TestThinCache.java:
##########
@@ -26,13 +26,63 @@
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.apache.solr.util.TestHarness;
import org.apache.solr.util.stats.MetricUtils;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test for {@link ThinCache}. */
public class TestThinCache extends SolrTestCaseJ4 {
+ @ClassRule public static EmbeddedSolrServerTestRule solrRule = new
EmbeddedSolrServerTestRule();
+ public static final String SOLR_NODE_LEVEL_CACHE_XML =
+ "<solr>\n"
+ + " <caches>\n"
+ + " <cache name='myNodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " <cache name='myNodeLevelCacheThin'\n"
+ + " class='solr.ThinCache$NodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " </caches>\n"
+ + "</solr>";
+
+ @BeforeClass
+ public static void setupSolrHome() throws Exception {
+ Path home = createTempDir("home");
+ Files.writeString(home.resolve("solr.xml"), SOLR_NODE_LEVEL_CACHE_XML);
+
+ solrRule.startSolr(home);
+
+ Path configSet = createTempDir("configSet");
+ copyMinConf(configSet.toFile());
+ // insert a special filterCache configuration
+ Path solrConfig = configSet.resolve("conf/solrconfig.xml");
+ Files.writeString(
+ solrConfig,
+ Files.readString(solrConfig)
+ .replace(
Review Comment:
When possible, the Config API can be used as is done in
`org.apache.solr.search.json.TestJsonRequestWithEdismaxDefType#test`. But
"parentCacheName" isn't supported / is too exotic. I hate that the Config API
is a custom format and only partially implements what we can specify in XML.
I'm torn between embracing it and pretending it doesn't exist.
##########
solr/core/src/test/org/apache/solr/search/TestThinCache.java:
##########
@@ -26,13 +26,63 @@
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.apache.solr.util.TestHarness;
import org.apache.solr.util.stats.MetricUtils;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test for {@link ThinCache}. */
public class TestThinCache extends SolrTestCaseJ4 {
+ @ClassRule public static EmbeddedSolrServerTestRule solrRule = new
EmbeddedSolrServerTestRule();
Review Comment:
This is fairly new; there will be more rules available in time +
randomization but not yet.
##########
solr/core/src/test/org/apache/solr/search/TestThinCache.java:
##########
@@ -26,13 +26,63 @@
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.apache.solr.util.TestHarness;
import org.apache.solr.util.stats.MetricUtils;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test for {@link ThinCache}. */
public class TestThinCache extends SolrTestCaseJ4 {
+ @ClassRule public static EmbeddedSolrServerTestRule solrRule = new
EmbeddedSolrServerTestRule();
+ public static final String SOLR_NODE_LEVEL_CACHE_XML =
+ "<solr>\n"
+ + " <caches>\n"
+ + " <cache name='myNodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " <cache name='myNodeLevelCacheThin'\n"
+ + " class='solr.ThinCache$NodeLevelCache'\n"
+ + " size='10'\n"
+ + " initialSize='10'\n"
+ + " />\n"
+ + " </caches>\n"
+ + "</solr>";
+
+ @BeforeClass
+ public static void setupSolrHome() throws Exception {
+ Path home = createTempDir("home");
+ Files.writeString(home.resolve("solr.xml"), SOLR_NODE_LEVEL_CACHE_XML);
+
+ solrRule.startSolr(home);
+
+ Path configSet = createTempDir("configSet");
+ copyMinConf(configSet.toFile());
+ // insert a special filterCache configuration
+ Path solrConfig = configSet.resolve("conf/solrconfig.xml");
+ Files.writeString(
+ solrConfig,
+ Files.readString(solrConfig)
+ .replace(
Review Comment:
This is a bit hacky I admit. I would like to add a utility method that
might look something like
`updateXml(Path, String xpath, String newXml)` where the Xpath would be
like "/config/query/filterCache". ChatGPT wrote one for me :-)
--
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]
