Skip to content

Commit 6b82523

Browse files
epughepugh
authored
Migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule (#3947)
Embrace the use of @ClassRule setup to manage the Solr lifecycle using SolrClientTestRule in our tests. This PR is to finish the migration to using SolrJettyTestRule, which extends SolrClientTestRule across our tests. There is also plenty of small cleanups of the tests throughout. Lint cleanups and places where we had extra setup steps that did not need to happen for the goal of a individual test to be accomplished.
1 parent a2c0944 commit 6b82523

File tree

82 files changed

+807
-847
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+807
-847
lines changed

‎solr/core/src/test/org/apache/solr/TestCustomCoreProperties.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
*/
3737
public class TestCustomCoreProperties extends SolrTestCaseJ4 {
3838

39-
@ClassRule public static SolrJettyTestRule solrClientTestRule = new SolrJettyTestRule();
39+
@ClassRule public static SolrJettyTestRule solrTestRule = new SolrJettyTestRule();
4040

4141
// TODO these properties files don't work with configsets
4242

@@ -49,7 +49,6 @@ public static void beforeClass() throws Exception {
4949

5050
Files.createDirectories(confDir);
5151

52-
Files.copy(SolrTestCaseJ4.TEST_HOME().resolve("solr.xml"), homeDir.resolve("solr.xml"));
5352
String src_dir = TEST_HOME() + "/collection1/conf";
5453
Files.copy(Path.of(src_dir, "schema-tiny.xml"), confDir.resolve("schema.xml"));
5554
Files.copy(
@@ -79,7 +78,7 @@ public static void beforeClass() throws Exception {
7978
nodeProperties.setProperty("solr.data.dir", createTempDir().toRealPath().toString());
8079
}
8180

82-
solrClientTestRule.startSolr(homeDir, nodeProperties, JettyConfig.builder().build());
81+
solrTestRule.startSolr(homeDir, nodeProperties, JettyConfig.builder().build());
8382
}
8483

8584
@Test
@@ -88,7 +87,7 @@ public void testSimple() throws Exception {
8887
params(
8988
"q", "*:*",
9089
"echoParams", "all");
91-
QueryResponse res = solrClientTestRule.getSolrClient("collection1").query(params);
90+
QueryResponse res = solrTestRule.getSolrClient("collection1").query(params);
9291
assertEquals(0, res.getResults().getNumFound());
9392

9493
NamedList<?> echoedParams = (NamedList<?>) res.getHeader().get("params");

‎solr/core/src/test/org/apache/solr/TestTolerantSearch.java‎

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.nio.file.Files;
2222
import java.nio.file.Path;
2323
import java.nio.file.StandardCopyOption;
24-
import org.apache.commons.io.FileUtils;
2524
import org.apache.solr.client.solrj.SolrClient;
2625
import org.apache.solr.client.solrj.SolrServerException;
2726
import org.apache.solr.client.solrj.request.CoreAdminRequest;
@@ -34,49 +33,62 @@
3433
import org.apache.solr.request.SolrQueryRequest;
3534
import org.apache.solr.response.JavaBinResponseWriter;
3635
import org.apache.solr.response.SolrQueryResponse;
36+
import org.apache.solr.util.SolrJettyTestRule;
3737
import org.junit.AfterClass;
3838
import org.junit.BeforeClass;
39+
import org.junit.ClassRule;
3940

40-
public class TestTolerantSearch extends SolrJettyTestBase {
41+
public class TestTolerantSearch extends SolrTestCaseJ4 {
42+
43+
@ClassRule public static SolrJettyTestRule solrTestRule = new SolrJettyTestRule();
4144

4245
private static SolrClient collection1;
4346
private static SolrClient collection2;
4447
private static String shard1;
4548
private static String shard2;
4649

4750
private static Path createSolrHome() throws Exception {
48-
Path workDir = createTempDir();
49-
setupJettyTestHome(workDir, "collection1");
51+
Path workDir = createTempDir().toRealPath();
52+
53+
// Copy solr.xml
5054
Files.copy(
51-
Path.of(SolrTestCaseJ4.TEST_HOME() + "/collection1/conf/solrconfig-tolerant-search.xml"),
52-
workDir.resolve("collection1").resolve("conf").resolve("solrconfig.xml"),
55+
SolrTestCaseJ4.TEST_PATH().resolve("solr.xml"),
56+
workDir.resolve("solr.xml"),
5357
StandardCopyOption.REPLACE_EXISTING);
54-
FileUtils.copyDirectory(
55-
workDir.resolve("collection1").toFile(), workDir.resolve("collection2").toFile());
58+
59+
// Set up collection1 with minimal config + tolerant search solrconfig
60+
Path collection1Dir = workDir.resolve("collection1");
61+
copyMinConf(collection1Dir, "name=collection1\n", "solrconfig-tolerant-search.xml");
62+
63+
// Set up configset for CoreAdminRequest.Create (reuse the same config)
64+
Path configSetDir = workDir.resolve("configsets").resolve("collection1");
65+
copyMinConf(configSetDir, null, "solrconfig-tolerant-search.xml");
66+
5667
return workDir;
5768
}
5869

5970
@BeforeClass
6071
public static void createThings() throws Exception {
6172
systemSetPropertyEnableUrlAllowList(false);
6273
Path solrHome = createSolrHome();
63-
createAndStartJetty(solrHome);
64-
String url = getBaseUrl();
65-
collection1 = getHttpSolrClient(url, "collection1");
66-
collection2 = getHttpSolrClient(url, "collection2");
74+
solrTestRule.startSolr(solrHome);
6775

68-
String urlCollection1 = getBaseUrl() + "/" + "collection1";
69-
String urlCollection2 = getBaseUrl() + "/" + "collection2";
76+
collection1 = solrTestRule.getSolrClient("collection1");
77+
78+
String urlCollection1 = solrTestRule.getBaseUrl() + "/" + "collection1";
79+
String urlCollection2 = solrTestRule.getBaseUrl() + "/" + "collection2";
7080
shard1 = urlCollection1.replaceAll("https?://", "");
7181
shard2 = urlCollection2.replaceAll("https?://", "");
7282

7383
// create second core
74-
try (SolrClient nodeClient = getHttpSolrClient(url)) {
75-
CoreAdminRequest.Create req = new CoreAdminRequest.Create();
76-
req.setCoreName("collection2");
77-
req.setConfigSet("collection1");
78-
nodeClient.request(req);
79-
}
84+
SolrClient nodeClient = solrTestRule.getSolrClient();
85+
CoreAdminRequest.Create req = new CoreAdminRequest.Create();
86+
req.setCoreName("collection2");
87+
req.setConfigSet("collection1");
88+
nodeClient.request(req);
89+
90+
// Now get the client for collection2 after it's been created
91+
collection2 = solrTestRule.getSolrClient("collection2");
8092

8193
SolrInputDocument doc = new SolrInputDocument();
8294
doc.setField("id", "1");
@@ -100,14 +112,8 @@ public static void createThings() throws Exception {
100112

101113
@AfterClass
102114
public static void destroyThings() throws Exception {
103-
if (null != collection1) {
104-
collection1.close();
105-
collection1 = null;
106-
}
107-
if (null != collection2) {
108-
collection2.close();
109-
collection2 = null;
110-
}
115+
collection1 = null;
116+
collection2 = null;
111117
resetExceptionIgnores();
112118
systemClearPropertySolrEnableUrlAllowList();
113119
}

‎solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public void before() throws Exception {
6161

6262
@After
6363
public void after() throws Exception {
64-
solrClientTestRule.reset();
64+
solrTestRule.reset();
6565

6666
if (restTestHarness != null) {
6767
restTestHarness.close();

‎solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void before() throws Exception {
140140

141141
@After
142142
public void after() throws Exception {
143-
solrClientTestRule.reset();
143+
solrTestRule.reset();
144144

145145
if (restTestHarness != null) {
146146
restTestHarness.close();

‎solr/core/src/test/org/apache/solr/handler/TestHttpRequestId.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import java.util.concurrent.TimeUnit;
2727
import java.util.concurrent.TimeoutException;
2828
import org.apache.logging.log4j.core.LogEvent;
29-
import org.apache.solr.SolrJettyTestBase;
29+
import org.apache.solr.SolrTestCaseJ4;
3030
import org.apache.solr.client.solrj.jetty.HttpJettySolrClient;
3131
import org.apache.solr.client.solrj.request.SolrPing;
3232
import org.apache.solr.common.util.ExecutorUtil;
@@ -35,16 +35,20 @@
3535
import org.apache.solr.common.util.SuppressForbidden;
3636
import org.apache.solr.util.LogLevel;
3737
import org.apache.solr.util.LogListener;
38+
import org.apache.solr.util.SolrJettyTestRule;
3839
import org.junit.BeforeClass;
40+
import org.junit.ClassRule;
3941
import org.junit.Test;
4042
import org.slf4j.MDC;
4143

4244
@LogLevel("org.apache.solr.client.solrj.jetty.HttpJettySolrClient=DEBUG")
43-
public class TestHttpRequestId extends SolrJettyTestBase {
45+
public class TestHttpRequestId extends SolrTestCaseJ4 {
46+
47+
@ClassRule public static SolrJettyTestRule solrTestRule = new SolrJettyTestRule();
4448

4549
@BeforeClass
4650
public static void beforeTest() throws Exception {
47-
createAndStartJetty(legacyExampleCollection1SolrHome());
51+
solrTestRule.startSolr(createTempDir());
4852
}
4953

5054
@Test
@@ -95,7 +99,7 @@ private void setupClientAndRun(
9599
false);
96100
CompletableFuture<NamedList<Object>> cf;
97101
try (var client =
98-
new HttpJettySolrClient.Builder(getBaseUrl())
102+
new HttpJettySolrClient.Builder(solrTestRule.getBaseUrl())
99103
.withDefaultCollection(collection)
100104
.withExecutor(commExecutor)
101105
.build()) {

‎solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import org.apache.lucene.store.NIOFSDirectory;
3939
import org.apache.lucene.tests.util.LuceneTestCase;
4040
import org.apache.lucene.tests.util.TestUtil;
41-
import org.apache.solr.SolrJettyTestBase;
4241
import org.apache.solr.SolrTestCaseJ4;
4342
import org.apache.solr.client.solrj.SolrClient;
4443
import org.apache.solr.client.solrj.apache.HttpSolrClient;
@@ -53,7 +52,7 @@
5352
// Backups do checksum validation against a footer value not present in 'SimpleText'
5453
@LuceneTestCase.SuppressCodecs({"SimpleText"})
5554
@SolrTestCaseJ4.SuppressSSL // Currently, unknown why SSL does not work with this test
56-
public class TestReplicationHandlerBackup extends SolrJettyTestBase {
55+
public class TestReplicationHandlerBackup extends SolrTestCaseJ4 {
5756

5857
JettySolrRunner leaderJetty;
5958
ReplicationTestHelper.SolrInstance leader = null;
@@ -78,9 +77,8 @@ private static JettySolrRunner createAndStartJetty(ReplicationTestHelper.SolrIns
7877
return jetty;
7978
}
8079

81-
private static SolrClient createNewSolrClient(int port) {
82-
final String baseUrl = buildUrl(port);
83-
return new HttpSolrClient.Builder(baseUrl)
80+
private static SolrClient createNewSolrClient(JettySolrRunner jetty) {
81+
return new HttpSolrClient.Builder(jetty.getBaseUrl().toString())
8482
.withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
8583
.withSocketTimeout(60000, TimeUnit.MILLISECONDS)
8684
.build();
@@ -102,7 +100,7 @@ public void setUp() throws Exception {
102100
leader.copyConfigFile(CONF_DIR.resolve(configFile).toString(), "solrconfig.xml");
103101

104102
leaderJetty = createAndStartJetty(leader);
105-
leaderClient = createNewSolrClient(leaderJetty.getLocalPort());
103+
leaderClient = createNewSolrClient(leaderJetty);
106104
docsSeed = random().nextLong();
107105
}
108106

@@ -247,7 +245,7 @@ private void testDeleteNamedBackup(String[] backupNames) throws Exception {
247245
public static void runBackupCommand(JettySolrRunner leaderJetty, String cmd, String params)
248246
throws IOException {
249247
String leaderUrl =
250-
buildUrl(leaderJetty.getLocalPort())
248+
leaderJetty.getBaseUrl().toString()
251249
+ "/"
252250
+ DEFAULT_TEST_CORENAME
253251
+ ReplicationHandler.PATH

‎solr/core/src/test/org/apache/solr/handler/TestRestoreCore.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.apache.lucene.index.IndexFileNames;
2828
import org.apache.lucene.tests.util.LuceneTestCase;
2929
import org.apache.lucene.tests.util.TestUtil;
30-
import org.apache.solr.SolrJettyTestBase;
3130
import org.apache.solr.SolrTestCaseJ4;
3231
import org.apache.solr.client.solrj.SolrClient;
3332
import org.apache.solr.client.solrj.apache.HttpSolrClient;
@@ -41,7 +40,7 @@
4140
@SolrTestCaseJ4.SuppressSSL // Currently, unknown why SSL does not work with this test
4241
// Backups do checksum validation against a footer value not present in 'SimpleText'
4342
@LuceneTestCase.SuppressCodecs("SimpleText")
44-
public class TestRestoreCore extends SolrJettyTestBase {
43+
public class TestRestoreCore extends SolrTestCaseJ4 {
4544

4645
JettySolrRunner leaderJetty;
4746
ReplicationTestHelper.SolrInstance leader = null;

‎solr/core/src/test/org/apache/solr/handler/TestStressIncrementalBackup.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ public void beforeTest() throws Exception {
5252
backupPath = createTempDir(getTestClass().getSimpleName() + "_backups");
5353
System.setProperty("solr.security.allow.paths", backupPath.toString());
5454

55-
// NOTE: we don't actually care about using SolrCloud, but we want to use SolrClient and I can't
56-
// bring myself to deal with the nonsense that is SolrJettyTestBase.
55+
// NOTE: we don't actually care about using SolrCloud, but we want to use SolrClient.
5756

5857
// We do however explicitly want a fresh "cluster" every time a test is run
5958
configureCluster(1).addConfig("conf1", configset("cloud-minimal")).configure();

‎solr/core/src/test/org/apache/solr/handler/TestStressThreadBackup.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ public static void afterClass() {
8989
public void beforeTest() throws Exception {
9090
backupDir = createTempDir(getTestClass().getSimpleName() + "_backups");
9191

92-
// NOTE: we don't actually care about using SolrCloud, but we want to use SolrClient and I can't
93-
// bring myself to deal with the nonsense that is SolrJettyTestBase.
92+
// NOTE: we don't actually care about using SolrCloud, but we want to use SolrClient
9493

9594
// We do however explicitly want a fresh "cluster" every time a test is run
9695
configureCluster(1)

‎solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import java.io.InputStream;
2121
import java.util.Set;
2222
import java.util.concurrent.atomic.AtomicBoolean;
23-
import org.apache.solr.SolrJettyTestBase;
23+
import org.apache.solr.SolrTestCaseJ4;
2424
import org.apache.solr.client.solrj.SolrClient;
2525
import org.apache.solr.client.solrj.SolrRequest;
2626
import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
@@ -38,18 +38,18 @@
3838
import org.apache.solr.request.SolrQueryRequest;
3939
import org.apache.solr.request.SolrRequestHandler;
4040
import org.apache.solr.response.SolrQueryResponse;
41+
import org.apache.solr.util.SolrJettyTestRule;
4142
import org.junit.BeforeClass;
43+
import org.junit.ClassRule;
4244

43-
/**
44-
* Extend SolrJettyTestBase because the SOLR-2535 bug only manifested itself when the {@link
45-
* org.apache.solr.servlet.SolrDispatchFilter} is used, which isn't for embedded Solr use.
46-
*/
47-
public class ShowFileRequestHandlerTest extends SolrJettyTestBase {
45+
public class ShowFileRequestHandlerTest extends SolrTestCaseJ4 {
46+
47+
@ClassRule public static SolrJettyTestRule solrTestRule = new SolrJettyTestRule();
4848

4949
@BeforeClass
5050
public static void beforeTest() throws Exception {
5151
initCore("solrconfig.xml", "schema.xml");
52-
createAndStartJetty(legacyExampleCollection1SolrHome());
52+
solrTestRule.startSolr(legacyExampleCollection1SolrHome());
5353
}
5454

5555
private GenericSolrRequest createShowFileRequest(SolrParams params) {
@@ -59,7 +59,7 @@ private GenericSolrRequest createShowFileRequest(SolrParams params) {
5959
}
6060

6161
public void test404ViaHttp() {
62-
SolrClient client = getSolrClient();
62+
SolrClient client = solrTestRule.getSolrClient();
6363
var request = createShowFileRequest(params("file", "does-not-exist-404.txt"));
6464
SolrException e = expectThrows(SolrException.class, () -> request.process(client));
6565
assertEquals(404, e.code());
@@ -82,18 +82,14 @@ public void test404Locally() {
8282
}
8383

8484
public void testDirList() throws SolrServerException, IOException {
85-
SolrClient client = getSolrClient();
86-
// assertQ(req("qt", "/admin/file")); TODO file bug that SolrJettyTestBase extends
87-
// SolrTestCaseJ4
85+
SolrClient client = solrTestRule.getSolrClient();
8886
var request = createShowFileRequest(new ModifiableSolrParams());
8987
var resp = request.process(client);
9088
assertTrue(((NamedList) resp.getResponse().get("files")).size() > 0); // some files
9189
}
9290

9391
public void testGetRawFile() throws SolrServerException, IOException {
94-
SolrClient client = getSolrClient();
95-
// assertQ(req("qt", "/admin/file"));
96-
// TODO file bug that SolrJettyTestBase extends SolrTestCaseJ4
92+
SolrClient client = solrTestRule.getSolrClient();
9793
var request = createShowFileRequest(params("file", "managed-schema.xml"));
9894
final AtomicBoolean readFile = new AtomicBoolean();
9995
request.setResponseParser(
@@ -146,7 +142,7 @@ public void testContentTypeHtmlDefault() {
146142
}
147143

148144
public void testIllegalContentType() throws SolrServerException, IOException {
149-
SolrClient client = getSolrClient();
145+
SolrClient client = solrTestRule.getSolrClient();
150146
var request =
151147
createShowFileRequest(params("file", "managed-schema", "contentType", "not/known"));
152148
request.setResponseParser(new InputStreamResponseParser("xml"));
@@ -155,7 +151,7 @@ public void testIllegalContentType() throws SolrServerException, IOException {
155151
}
156152

157153
public void testAbsoluteFilename() throws SolrServerException, IOException {
158-
SolrClient client = getSolrClient();
154+
SolrClient client = solrTestRule.getSolrClient();
159155
final var request =
160156
createShowFileRequest(
161157
params("file", "/etc/passwd", "contentType", "text/plain; charset=utf-8"));
@@ -165,7 +161,7 @@ public void testAbsoluteFilename() throws SolrServerException, IOException {
165161
}
166162

167163
public void testEscapeConfDir() throws SolrServerException, IOException {
168-
SolrClient client = getSolrClient();
164+
SolrClient client = solrTestRule.getSolrClient();
169165
final var request =
170166
createShowFileRequest(
171167
params("file", "../../solr.xml", "contentType", "application/xml; charset=utf-8"));
@@ -175,7 +171,7 @@ public void testEscapeConfDir() throws SolrServerException, IOException {
175171
}
176172

177173
public void testPathTraversalFilename() throws SolrServerException, IOException {
178-
SolrClient client = getSolrClient();
174+
SolrClient client = solrTestRule.getSolrClient();
179175
final var request =
180176
createShowFileRequest(
181177
params(

0 commit comments

Comments
 (0)