cpoerschke commented on code in PR #924:
URL: https://github.com/apache/solr/pull/924#discussion_r913655672
##########
solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java:
##########
@@ -87,6 +87,7 @@
public class TestReplicationHandler extends SolrTestCaseJ4 {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final long TIMEOUT = 30000;
Review Comment:
```suggestion
private static final long TIMEOUT = 30000;
```
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -1428,8 +1423,8 @@ private long uploadBadConfigSet(
configSetName,
suffix,
username,
- overwrite,
- cleanup,
+ true,
+ true,
Review Comment:
```suggestion
true /* overwrite */,
true /* cleanup */,
```
##########
solr/core/src/test/org/apache/solr/search/TestSearchPerf.java:
##########
@@ -249,7 +243,7 @@ public void XtestFilteringPerformance() throws Exception {
createIndex2(indexSize, "foomany_s", "t10_100_ws");
// doListGen(100, q, filters, false, true);
Review Comment:
```suggestion
```
##########
solr/core/src/test/org/apache/solr/update/processor/UUIDUpdateProcessorFallbackTest.java:
##########
@@ -155,17 +155,17 @@ SolrInputDocument doc(SolrInputField... fields) {
}
/** Convenience method for building up SolrInputFields */
- SolrInputField field(String name, float boost, Object... values) {
+ SolrInputField field(String name, Object... values) {
SolrInputField f = new SolrInputField(name);
for (Object v : values) {
f.addValue(v);
}
return f;
}
- /** Convenience method for building up SolrInputFields with default boost */
+ /** Convenience method for building up SolrInputFields */
SolrInputField f(String name, Object... values) {
- return field(name, 1.0F, values);
+ return field(name, values);
}
Review Comment:
So now the `f` method has the same signature as the `field` method, and
since `f` only calls `field` the `f` method itself can be removed i.e. callers
can directly call `field` instead.
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -1333,7 +1333,8 @@ public void testUploadWithLibDirective() throws Exception
{
}
private static String getSecurityJson() throws KeeperException,
InterruptedException {
- String securityJson =
+ String securityJson;
+ securityJson =
Review Comment:
```suggestion
return
```
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -1522,8 +1517,8 @@ private long uploadSingleConfigSetFile(
username,
usePut);
assertNotNull(map);
- long statusCode =
- (long) getObjectByPath(map, false, Arrays.asList("responseHeader",
"status"));
+ long statusCode;
+ statusCode = (long) getObjectByPath(map, Arrays.asList("responseHeader",
"status"));
return statusCode;
Review Comment:
```suggestion
return (long) getObjectByPath(map, Arrays.asList("responseHeader",
"status"));
```
##########
solr/core/src/test/org/apache/solr/search/TestSearchPerf.java:
##########
@@ -153,31 +155,23 @@ int doSetGen(int iter, Query q) throws Exception {
return ret;
}
- int doListGen(int iter, Query q, List<Query> filt, boolean cacheQuery,
boolean cacheFilt)
- throws Exception {
+ int doListGen(Query q, List<Query> filt) throws Exception {
Review Comment:
Since this method is not private, need we worry about maintaining the
signature? Or if changing the signature anyhow, let's make it private at the
same time?
```suggestion
private int doListGen(Query q, List<Query> filt) throws Exception {
```
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -1688,9 +1682,6 @@ private static Object getObjectByPath(
if (obj == null) return null;
} else {
Object val = obj.get(s);
- if (onlyPrimitive && val instanceof Map) {
- return null;
- }
Review Comment:
nice simplification!
##########
solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java:
##########
@@ -431,8 +431,8 @@ private void testDoubleCollapse(String group, String hint) {
params = new ModifiableSolrParams();
params.add("q", "id:(1 2 5)");
- params.add("fq", "{!collapse cost=200 max=test_i field=term_s " + hint +
"}");
- params.add("fq", "{!collapse cost=400 max=test_i field=" + group + "" +
hint + "}");
+ params.add("fq", "{!collapse cost=200 max=test_i field=term_s " + "" +
"}");
+ params.add("fq", "{!collapse cost=400 max=test_i field=" + group + "" + ""
+ "}");
Review Comment:
```suggestion
params.add("fq", "{!collapse cost=200 max=test_i field=term_s }");
params.add("fq", "{!collapse cost=400 max=test_i field=" + group + "}");
```
##########
solr/core/src/test/org/apache/solr/search/TestCancellableCollector.java:
##########
@@ -117,20 +117,11 @@ private void executeSearchTest(
assertEquals(topDocs.totalHits.value, topScoreDocCollector.getTotalHits());
}
- private void cancelQuery(CancellableCollector cancellableCollector, final
int sleepTime) {
+ private void cancelQuery(CancellableCollector cancellableCollector) {
executor.submit(
() -> {
// Wait for some time to let the query start
Review Comment:
```suggestion
```
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -1464,8 +1459,8 @@ private long uploadGivenConfigSet(
username,
usePut);
assertNotNull(map);
- long statusCode =
- (long) getObjectByPath(map, false, Arrays.asList("responseHeader",
"status"));
+ long statusCode;
+ statusCode = (long) getObjectByPath(map, Arrays.asList("responseHeader",
"status"));
return statusCode;
Review Comment:
```suggestion
return (long) getObjectByPath(map, Arrays.asList("responseHeader",
"status"));
```
##########
solr/core/src/test/org/apache/solr/search/TestSearchPerf.java:
##########
@@ -153,31 +155,23 @@ int doSetGen(int iter, Query q) throws Exception {
return ret;
}
- int doListGen(int iter, Query q, List<Query> filt, boolean cacheQuery,
boolean cacheFilt)
- throws Exception {
+ int doListGen(Query q, List<Query> filt) throws Exception {
SolrQueryRequest req = lrf.makeRequest();
SolrIndexSearcher searcher = req.getSearcher();
final RTimer timer = new RTimer();
int ret = 0;
- for (int i = 0; i < iter; i++) {
+ for (int i = 0; i < ITERATIONS; i++) {
DocList l =
- searcher.getDocList(
- q,
- filt,
- (Sort) null,
- 0,
- 10,
- (cacheQuery ? 0 : SolrIndexSearcher.NO_CHECK_QCACHE)
- | (cacheFilt ? 0 : SolrIndexSearcher.NO_CHECK_FILTERCACHE));
+ searcher.getDocList(q, filt, (Sort) null, 0, 10,
SolrIndexSearcher.NO_CHECK_QCACHE | 0);
Review Comment:
```suggestion
searcher.getDocList(q, filt, (Sort) null, 0, 10,
SolrIndexSearcher.NO_CHECK_QCACHE);
```
##########
solr/core/src/test/org/apache/solr/request/TestStreamBody.java:
##########
@@ -127,17 +127,15 @@ public String getPath() { // don't let superclass
substitute qt for the path
SolrException se =
expectThrows(SolrException.class, () ->
queryRequest.process(getSolrClient()));
assertTrue(se.getMessage(), se.getMessage().contains("Stream Body is
disabled"));
- enableStreamBody(true);
+ enableStreamBody();
queryRequest.process(getSolrClient());
}
// Enables/disables stream.body through Config API
Review Comment:
```suggestion
// Enables stream.body through Config API
```
##########
solr/core/src/test/org/apache/solr/handler/admin/CoreAdminCreateDiscoverTest.java:
##########
@@ -60,7 +60,7 @@ public static void afterClass() throws Exception {
solrHomeDirectory = null;
}
- private static void setupCore(String coreName, boolean blivet) throws
IOException {
Review Comment:
Had to look that word up: https://en.wiktionary.org/wiki/blivet
##########
solr/core/src/test/org/apache/solr/handler/admin/TestApiFramework.java:
##########
@@ -361,7 +355,6 @@ private SolrQueryResponse invoke(
PluginBag<SolrRequestHandler> reqHandlers,
String path,
String fullPath,
- SolrRequest.METHOD method,
Review Comment:
Could we maybe keep the `method` parameter here and instead replace the
`"GET"` below with `method.toString()` or something?
##########
solr/core/src/test/org/apache/solr/schema/TestManagedSchemaThreadSafety.java:
##########
@@ -52,10 +52,11 @@
public class TestManagedSchemaThreadSafety extends SolrTestCaseJ4 {
private static final class SuspendingZkClient extends SolrZkClient {
+ public static final int ZK_CLIENT_TIMEOUT = 30000;
Review Comment:
```suggestion
private static final int ZK_CLIENT_TIMEOUT = 30000;
```
##########
solr/core/src/test/org/apache/solr/search/TestSearchPerf.java:
##########
@@ -33,6 +33,8 @@
/** */
public class TestSearchPerf extends SolrTestCaseJ4 {
+ public static final int ITERATIONS = 500;
Review Comment:
```suggestion
private static final int ITERATIONS = 500;
```
##########
solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java:
##########
@@ -421,8 +421,8 @@ private void testDoubleCollapse(String group, String hint) {
ModifiableSolrParams params = new ModifiableSolrParams();
params.add("q", "id:(1 2 5)");
- params.add("fq", "{!collapse cost=200 field=term_s " + hint + "}");
- params.add("fq", "{!collapse cost=400 field=" + group + "" + hint + "}");
+ params.add("fq", "{!collapse cost=200 field=term_s " + "" + "}");
+ params.add("fq", "{!collapse cost=400 field=" + group + "" + "" + "}");
Review Comment:
```suggestion
params.add("fq", "{!collapse cost=200 field=term_s }");
params.add("fq", "{!collapse cost=400 field=" + group + "}");
```
--
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]
