dsmiley commented on code in PR #3988:
URL: https://github.com/apache/solr/pull/3988#discussion_r2659297826
##########
solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java:
##########
@@ -2259,12 +2258,6 @@ public static void copyMinConf(Path dstRoot, String
propertiesContent, String so
subHome.resolve("solrconfig.snippet.randomindexconfig.xml"));
}
- // Just copies the file indicated to the tmp home directory naming it
"solr.xml"
Review Comment:
I hate this method; thanks for removing it.
##########
solr/core/src/test/org/apache/solr/schema/TestBinaryField.java:
##########
@@ -33,44 +30,34 @@
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.util.SolrJettyTestRule;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
@SuppressSSL(bugUrl = "https://issues.apache.org/jira/browse/SOLR-5776";)
-public class TestBinaryField extends SolrJettyTestBase {
+public class TestBinaryField extends SolrTestCaseJ4 {
+
+ @ClassRule public static SolrJettyTestRule solrTestRule = new
SolrJettyTestRule();
@BeforeClass
public static void beforeTest() throws Exception {
Path homeDir = createTempDir();
-
Path collDir = homeDir.resolve("collection1");
- Path dataDir = collDir.resolve("data");
- Path confDir = collDir.resolve("conf");
- Files.createDirectories(homeDir);
- Files.createDirectories(collDir);
- Files.createDirectories(dataDir);
- Files.createDirectories(confDir);
+ copyMinConf(collDir, "name=collection1\n", "solrconfig-basic.xml");
- String src_dir = TEST_HOME() + "/collection1/conf";
- Files.copy(Path.of(src_dir, "schema-binaryfield.xml"),
confDir.resolve("schema.xml"));
- Files.copy(Path.of(src_dir, "solrconfig-basic.xml"),
confDir.resolve("solrconfig.xml"));
+ // Copy the custom schema for binary field tests
+ String sourceConfDir = TEST_HOME() + "/collection1/conf";
Review Comment:
_(okay to defer this; problem pre-existed)_
If you look carefully at what's happening here (was before), this is sad.
TEST_HOME() returns a Path, which we toString via string concatenation to a
String representing a path, and then a line below we create a Path via
`Path.of`. Obviously we should instead be using the `resolve` method on Path.
The tell-tale sign of the problem is the usage of `Path.of` which we should
ideally use sparingly (when we truly have a String input that can't be a Path).
CC @mlbiscoc
--
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]
