dsmiley commented on code in PR #3947:
URL: https://github.com/apache/solr/pull/3947#discussion_r2642026013
##########
solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java:
##########
Review Comment:
This entire package is going away soon.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestsBase.java:
##########
@@ -32,15 +35,20 @@
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.embedded.JettyConfig;
+import org.apache.solr.util.SolrJettyTestRule;
import org.apache.solr.util.TimeOut;
-import org.junit.After;
+import org.junit.ClassRule;
import org.junit.Test;
-public abstract class SolrExampleTestsBase extends SolrJettyTestBase {
+public abstract class SolrExampleTestsBase extends SolrTestCaseJ4 {
+
+ @ClassRule public static SolrJettyTestRule solrJettyTestRule = new
SolrJettyTestRule();
Review Comment:
perhaps we should standardize on a field name for the rule. Like "solrRule"
is fairly short & sweet. We needn't put it's "Jetty"-ness in the name.
##########
solr/core/src/test/org/apache/solr/servlet/ResponseHeaderTest.java:
##########
@@ -21,55 +21,61 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.util.Properties;
import org.apache.http.Header;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
-import org.apache.solr.SolrJettyTestBase;
+import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.apache.HttpClientUtil;
+import org.apache.solr.embedded.JettyConfig;
import org.apache.solr.handler.component.ResponseBuilder;
import org.apache.solr.handler.component.SearchComponent;
-import org.junit.AfterClass;
+import org.apache.solr.util.SolrJettyTestRule;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
-public class ResponseHeaderTest extends SolrJettyTestBase {
+public class ResponseHeaderTest extends SolrTestCaseJ4 {
+
+ @ClassRule public static SolrJettyTestRule solrJettyTestRule = new
SolrJettyTestRule();
private static Path solrHomeDirectory;
@BeforeClass
public static void beforeTest() throws Exception {
solrHomeDirectory = createTempDir();
- setupJettyTestHome(solrHomeDirectory, "collection1");
- String top = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
+ Path collectionDirectory = solrHomeDirectory.resolve("collection1");
+
Files.copy(
- Path.of(top, "solrconfig-headers.xml"),
- Path.of(solrHomeDirectory + "/collection1/conf", "solrconfig.xml"),
+ SolrTestCaseJ4.TEST_PATH().resolve("solr.xml"),
+ solrHomeDirectory.resolve("solr.xml"),
StandardCopyOption.REPLACE_EXISTING);
- createAndStartJetty(solrHomeDirectory);
- }
- @AfterClass
- public static void afterTest() throws Exception {
- if (null != solrHomeDirectory) {
- cleanUpJettyHome(solrHomeDirectory);
- }
+ // Create minimal config with custom solrconfig for headers testing
+ SolrTestCaseJ4.copyMinConf(collectionDirectory, "name=collection1\n",
"solrconfig-headers.xml");
+
+ solrJettyTestRule.startSolr(solrHomeDirectory, new Properties(),
JettyConfig.builder().build());
}
@Test
public void testHttpResponse() throws IOException {
- URI uri = URI.create(getBaseUrl() + "/collection1/withHeaders?q=*:*");
+ URI uri = URI.create(solrJettyTestRule.getBaseUrl() +
"/collection1/withHeaders?q=*:*");
HttpGet httpGet = new HttpGet(uri);
- HttpResponse response = getHttpClient().execute(httpGet);
- Header[] headers = response.getAllHeaders();
- boolean containsWarningHeader = false;
- for (Header header : headers) {
- if ("Warning".equals(header.getName())) {
- containsWarningHeader = true;
- assertEquals("This is a test warning", header.getValue());
- break;
+ try (CloseableHttpClient httpClient = HttpClientUtil.createClient(null)) {
+ HttpResponse response = httpClient.execute(httpGet);
+ Header[] headers = response.getAllHeaders();
+ boolean containsWarningHeader = false;
+ for (Header header : headers) {
+ if ("Warning".equals(header.getName())) {
+ containsWarningHeader = true;
+ assertEquals("This is a test warning", header.getValue());
+ break;
+ }
}
+ assertTrue("Expected header not found", containsWarningHeader);
+ HttpClientUtil.close(httpClient);
Review Comment:
the try-with-resources means you don't need this explicit close
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestsBase.java:
##########
@@ -49,24 +57,41 @@ public void after() {
}
}
client = null;
+ super.tearDown();
}
- @Override
- public SolrClient getSolrClient() {
+ protected SolrClient getSolrClient() {
if (client == null) {
client = createNewSolrClient();
}
return client;
}
/**
- * Create a new solr client. If createJetty was called, a http
implementation will be created,
+ * Create a new solr client. If createJetty was called, an http
implementation will be created,
* otherwise an embedded implementation will be created. Subclasses should
override for other
* options.
*/
- @Override
public SolrClient createNewSolrClient() {
- return getHttpSolrClient(getBaseUrl(), DEFAULT_TEST_CORENAME);
+ return getHttpSolrClient(solrJettyTestRule.getBaseUrl(),
DEFAULT_TEST_CORENAME);
+ }
+
+ public static HttpSolrClient getHttpSolrClient(String baseUrl, String
coreName) {
Review Comment:
protected
##########
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:
what happened to the 2nd core?
--
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]
