cpoerschke commented on code in PR #927:
URL: https://github.com/apache/solr/pull/927#discussion_r916884938
##########
solr/core/src/test/org/apache/solr/search/TestStressVersions.java:
##########
@@ -116,7 +112,6 @@ public void run() {
}
int id = rand.nextInt(ndocs);
Review Comment:
```suggestion
int id = rand.nextInt(ndocs);
// Object sync = syncArr[id];
```
because there's some commented out `synchronized (sync)` code below
##########
solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKG.java:
##########
@@ -726,7 +726,7 @@ public static String randomPrefixParam(final Random r,
final String facetField)
* @return a sort string (w/direction), or null to specify nothing
(trigger default behavior)
* @see #randomSortParam
Review Comment:
```suggestion
```
##########
solr/core/src/test/org/apache/solr/update/processor/UniqFieldsUpdateProcessorFactoryTest.java:
##########
@@ -54,8 +54,6 @@ public void setUp() throws Exception {
public void testUniqFields() throws Exception {
SolrCore core = h.getCore();
UpdateRequestProcessorChain chained =
core.getUpdateProcessingChain("uniq-fields");
- UniqFieldsUpdateProcessorFactory factory =
- ((UniqFieldsUpdateProcessorFactory) chained.getProcessors().get(0));
assertNotNull(chained);
Review Comment:
```suggestion
assertNotNull(chained);
UniqFieldsUpdateProcessorFactory factory =
((UniqFieldsUpdateProcessorFactory) chained.getProcessors().get(0));
assertNotNull(factory);
```
##########
solr/core/src/test/org/apache/solr/handler/TestSolrConfigHandlerConcurrent.java:
##########
@@ -97,12 +96,7 @@ public void test() throws Exception {
assertTrue(collectErrors.toString(), success);
}
- private void invokeBulkCall(
- String cacheName,
- List<String> errs,
- // TODO this is unused - is that a bug?
Review Comment:
on the basis of this comment, how about excluding this file from the cleanup?
##########
solr/core/src/test/org/apache/solr/cloud/MockSimpleZkController.java:
##########
@@ -26,14 +26,22 @@
public class MockSimpleZkController extends ZkController {
+ /**
+ * @param cc Core container associated with this controller. cannot be null.
+ * @param zkServerAddress where to connect to the zk server
+ * @param zkClientConnectTimeout timeout in ms
+ * @param cloudConfig configuration for this controller. TODO: possibly
redundant with
+ * CoreContainer
+ * @param descriptorsSupplier a supplier of the current core descriptors.
used to know which cores
+ */
Review Comment:
minor/subjective: seems to just repeat javadocs from super class, not sure
if that's useful
##########
solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKGEquiv.java:
##########
@@ -1184,7 +1183,7 @@ public static String randomSortParam(final Random r) {
* @return a sort string (w/direction), or null to specify nothing
(trigger default behavior)
* @see #randomSortParam
Review Comment:
```suggestion
```
--
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]
