dsmiley commented on code in PR #3920:
URL: https://github.com/apache/solr/pull/3920#discussion_r2589256176
##########
solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java:
##########
@@ -109,10 +103,6 @@ public String getConfigSet() {
return configSet;
}
- public String getConfigFile() {
Review Comment:
basically, don't remove code from this whole test rule
##########
solr/test-framework/src/java/org/apache/solr/util/SolrMetricTestUtils.java:
##########
@@ -207,17 +196,6 @@ public static CounterSnapshot.CounterDataPointSnapshot
newCloudSelectRequestsDat
.build());
}
- public static CounterSnapshot.CounterDataPointSnapshot
newStandaloneUpdateRequestsDatapoint(
Review Comment:
keep
##########
solr/test-framework/src/java/org/apache/solr/cloud/AbstractZkTestCase.java:
##########
@@ -65,7 +65,7 @@ public static void azt_beforeClass() throws Exception {
}
@AfterClass
- public static void azt_afterClass() throws Exception {
Review Comment:
minor: probably has your attention because of the underscore but you could
simply change to `aztAfterClass`. The author likely wanted to disambiguate on
a common base class -- reasonable.
##########
solr/test-framework/src/java/org/apache/solr/search/facet/DebugAgg.java:
##########
@@ -38,14 +38,12 @@ public ValueSource parse(FunctionQParser fp) throws
SyntaxError {
parses.incrementAndGet();
final String what = fp.hasMoreArguments() ? fp.parseId() : "wrap";
- switch (what) {
- case "wrap":
- return new DebugAgg(fp);
- case "numShards":
- return new DebugAggNumShards();
- default: /* No-Op */
- }
- throw new RuntimeException("No idea what to do with " + what);
+ /* No-Op */
+ return switch (what) {
Review Comment:
I see you took a moment of pleasure to do Java 21 stuff :-)
##########
solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java:
##########
@@ -61,11 +61,6 @@ public class SSLTestConfig {
private final boolean useSsl;
private final boolean clientAuth;
- /** Creates an SSLTestConfig that does not use SSL or client authentication
*/
- public SSLTestConfig() {
Review Comment:
keep
##########
solr/test-framework/src/java/org/apache/solr/cluster/placement/Builders.java:
##########
@@ -501,10 +493,6 @@ public String getShardName() {
return shardName;
}
- public List<ReplicaBuilder> getReplicaBuilders() {
Review Comment:
I suggest keeping the methods you removed in this file
##########
solr/test-framework/src/java/org/apache/solr/common/cloud/ClusterStateUtil.java:
##########
@@ -31,18 +31,6 @@ public class ClusterStateUtil {
private static final int TIMEOUT_POLL_MS = 1000;
- /**
- * Wait to see *all* cores live and active.
- *
- * @param zkStateReader to use for ClusterState
- * @param timeoutInMs how long to wait before giving up
- * @return false if timed out
- */
- public static boolean waitForAllActiveAndLiveReplicas(
Review Comment:
keep
--
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]
