epugh commented on code in PR #4147:
URL: https://github.com/apache/solr/pull/4147#discussion_r2830537006
##########
solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java:
##########
@@ -71,14 +80,17 @@ public void test404Locally() {
// bypass TestHarness since it will throw any exception found in the
Review Comment:
maybe.. I was trying not to get too frisky with changing the tests to be
better, I was focused on just the migration. I DO think we could tee up a
whole set of analysis to run... What tests add value? what tests are
duplicative? what tests can be simplified... let's get 5000 bucks of claude
credits and have him go to town.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -95,11 +99,22 @@
public abstract class SolrExampleTests extends SolrExampleTestsBase {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ EnvUtils.setProperty(
+ ALLOW_PATHS_SYSPROP,
ExternalPaths.SERVER_HOME.toAbsolutePath().toString());
+ solrTestRule.startSolr(createTempDir());
+ solrTestRule
+ .newCollection(DEFAULT_TEST_COLLECTION_NAME)
+ .withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET)
+ .create();
+ }
+
@Before
public void emptyCollection() throws Exception {
SolrClient client = getSolrClient();
// delete everything!
- client.deleteByQuery("*:*");
+ client.deleteByQuery(DEFAULT_TEST_COLLECTION_NAME, "*:*");
client.commit();
Review Comment:
It's funny that everyting is passing with the issue on the client commit....
I kind of think maybe we just need to bite bullet and make core speicfic api
calls require a core. collection specific api calls require a collection.
And node ones not require either!
##########
solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleStreamingHttp2Test.java:
##########
@@ -30,19 +30,13 @@
import org.apache.solr.client.solrj.request.XMLRequestWriter;
import org.apache.solr.client.solrj.response.XMLResponseParser;
import org.apache.solr.common.SolrInputDocument;
-import org.junit.BeforeClass;
/**
* A subclass of SolrExampleTests that explicitly uses the HTTP2 client and
the streaming update
* client for communication.
*/
public class SolrExampleStreamingHttp2Test extends SolrExampleTests {
- @BeforeClass
- public static void beforeTest() throws Exception {
Review Comment:
Yeah, the parent is a SolrJettyTestRule. The only unique thing is the
`createNewSolrClient` method.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java:
##########
@@ -17,17 +17,10 @@
package org.apache.solr.client.solrj.embedded;
import org.apache.solr.client.solrj.SolrExampleTests;
-import org.junit.BeforeClass;
/**
* This runs SolrServer test using
*
* @since solr 1.3
*/
-public class SolrExampleEmbeddedTest extends SolrExampleTests {
-
- @BeforeClass
Review Comment:
Yeah, I don't know, because the line
`solrTestRule.startSolr(legacyExampleCollection1SolrHome());` doesn't start up
solr... I looked a bit, and I think this class has been suing a solrTestRule
that was inherited from SolrExampleTestsBase, and there it was defined as a
`SolrJettyTestRule`. I will look about overriding the `@ClassRule` and see
if we can have a `EmbeddedSolrServerTestRule` here...
##########
solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleJettyTest.java:
##########
@@ -46,11 +45,6 @@
@SuppressSSL(bugUrl = "https://issues.apache.org/jira/browse/SOLR-5776";)
public class SolrExampleJettyTest extends SolrExampleTests {
- @BeforeClass
- public static void beforeTest() throws Exception {
Review Comment:
That is the default test rule in `SolrExampleTestsBase`, it is a
`SolrJettyTestRule` so we are inheritiing that and using it.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -95,11 +99,22 @@
public abstract class SolrExampleTests extends SolrExampleTestsBase {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ EnvUtils.setProperty(
+ ALLOW_PATHS_SYSPROP,
ExternalPaths.SERVER_HOME.toAbsolutePath().toString());
+ solrTestRule.startSolr(createTempDir());
+ solrTestRule
+ .newCollection(DEFAULT_TEST_COLLECTION_NAME)
+ .withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET)
+ .create();
+ }
+
@Before
public void emptyCollection() throws Exception {
SolrClient client = getSolrClient();
// delete everything!
- client.deleteByQuery("*:*");
+ client.deleteByQuery(DEFAULT_TEST_COLLECTION_NAME, "*:*");
client.commit();
Review Comment:
I asked claude and it turns out because the client was created with a
default collection, if i pass it in great, and if I don't that's fine too.
So, do we have a preference? I could move the tests to always taking a
collection name, or maybe remove them everywhere, just to pick one of the other.
--
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]
