cpoerschke commented on code in PR #900:
URL: https://github.com/apache/solr/pull/900#discussion_r896668330
##########
solr/core/src/test/org/apache/solr/search/TestSolr4Spatial.java:
##########
@@ -444,7 +444,7 @@ public void testSort() throws Exception {
req(
"q", radiusQuery(3, 4, 9, "distance", null),
"fl", "id,score",
- "sort", "score asc"), // want ascending due to increasing distance
+ "sort", "score asc"), // want ascending due to increasing distances
Review Comment:
```suggestion
"sort", "score asc"), // want ascending due to increasing
'distance' values
```
##########
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java:
##########
@@ -67,13 +66,9 @@ public class TestSchemaDesignerAPI extends SolrCloudTestCase
implements SchemaDe
@BeforeClass
public static void createCluster() throws Exception {
- System.setProperty("managed.schema.mutable", "true");
Review Comment:
Not a typos or grammar change, could we out-scope from this pull request
i.e. defer to a separate later one?
##########
solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java:
##########
@@ -477,7 +477,7 @@ public void testManyClauses_Lucene() throws Exception {
sb.append(")");
// this should trip the lucene level global
BooleanQuery.getMaxClauseCount() limit,
- // causing a parsing error, before Solr even get's a chance to enforce
it's lower level limit
+ // causing a parsing error, before Solr even gets a chance to enforce it's
lower level limit
Review Comment:
```suggestion
// causing a parsing error, before Solr even gets a chance to enforce
its lower level limit
```
##########
solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java:
##########
@@ -150,11 +150,11 @@ public void testDocsWithValuesInField() throws Exception {
41,
HAS_VAL_FIELDS.size());
for (String f : HAS_VAL_FIELDS) {
- // for all of these fields, these 2 syntaxes should be functionally
equivilent
+ // for all of these fields, these 2 syntax's should be functionally
equivalent
Review Comment:
```suggestion
// for all of these fields, these 2 query forms should be functionally
equivalent
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKG.java:
##########
@@ -199,7 +199,7 @@ private static void createMiniSolrCloudCluster() throws
Exception {
/**
* Given a (random) number, and a (static) array of possible suffixes
returns a consistent field
- * name that uses that number and one of hte specified suffixes in it's name.
+ * name that uses that number and one of hte specified suffixes in its name.
Review Comment:
```suggestion
* name that uses that number and one of the specified suffixes in its
name.
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacetsStatsParsing.java:
##########
@@ -30,7 +30,7 @@
import org.junit.BeforeClass;
import org.noggit.ObjectBuilder;
-/** Whitebox test of the various syntaxes for specifying stats in JSON Facets
*/
+/** Whitebox test of the various syntax's for specifying stats in JSON Facets
*/
Review Comment:
```suggestion
/** Whitebox test of the various syntaxes for specifying stats in JSON
Facets */
```
##########
solr/core/src/test/org/apache/solr/uninverting/TestDocTermOrdsUninvertLimit.java:
##########
@@ -98,7 +98,7 @@ public void testTriggerUnInvertLimit() throws IOException {
+ " with message "
+ e.getMessage());
}
- // This is (hopefully) "Too many values for UnInvertedField faceting on
field field", so all
+ // This is (hopefully) "Too many values for UnInvertedField faceting on
field "field", so all
Review Comment:
```suggestion
// This is (hopefully) "Too many values for UnInvertedField faceting
on field", so all
```
##########
solr/core/src/test/org/apache/solr/update/processor/DistributedUpdateProcessorTest.java:
##########
@@ -65,7 +65,7 @@ public static void beforeClass() throws Exception {
@AfterClass
public static void AfterClass() {
- if (null != executor) { // may not have inited due to lack of mockito
+ if (null != executor) { // may not have been inited due to lack of mockito
Review Comment:
```suggestion
if (null != executor) { // may not have been initialized due to lack of
mockito
```
##########
solr/core/src/test/org/apache/solr/util/hll/HLLSerializationTest.java:
##########
@@ -171,8 +171,8 @@ public void manyValuesMonsterHLLSerializationTest() throws
Exception {
}
/**
- * Iterates over all possible constructor args, with the exception of log2m,
which is only
- * iterated up to the specified max so the test runs in a "reasonable"
amount of time and ram.
+ * Iterates over all possible constructor args, except for log2m, which is
only iterated up to the
+ * specified max so the test runs in a "reasonable" amount of time and ram.
Review Comment:
```suggestion
* specified max so the test runs in a "reasonable" amount of time and RAM.
```
##########
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java:
##########
@@ -55,13 +54,9 @@ public class TestSchemaDesignerConfigSetHelper extends
SolrCloudTestCase
@BeforeClass
public static void createCluster() throws Exception {
- System.setProperty("managed.schema.mutable", "true");
configureCluster(1)
.addConfig(DEFAULT_CONFIGSET_NAME, new
File(ExternalPaths.DEFAULT_CONFIGSET).toPath())
.configure();
- // SchemaDesignerConfigSetHelper depends on the blob store
- CollectionAdminRequest.createCollection(BLOB_STORE_ID, 1,
1).process(cluster.getSolrClient());
- cluster.waitForActiveCollection(BLOB_STORE_ID, 1, 1);
Review Comment:
Not a typos or grammar change, could we out-scope from this pull request
i.e. defer to a separate later one?
##########
solr/core/src/test/org/apache/solr/search/SpatialFilterTest.java:
##########
@@ -66,8 +66,8 @@ public void testLatLonType() {
// Try some edge cases
checkHits(fieldName, "1,1", 175, 3, 5, 6, 7);
checkHits(fieldName, "0,179.8", 200, 2, 8, 9);
- checkHits(fieldName, "89.8, 50", 200, 2, 10, 11); // this goes over the
north pole
- checkHits(fieldName, "-89.8, 50", 200, 2, 12, 13); // this goes over the
south pole
+ checkHits(fieldName, "89.8, 50", 200, 2, 10, 11); // this goes over the
North Pole
Review Comment:
```suggestion
checkHits(fieldName, "89.8, 50", 200, 2, 10, 11); // this goes over the
North Pole
```
##########
solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java:
##########
@@ -150,11 +150,11 @@ public void testDocsWithValuesInField() throws Exception {
41,
HAS_VAL_FIELDS.size());
for (String f : HAS_VAL_FIELDS) {
- // for all of these fields, these 2 syntaxes should be functionally
equivilent
+ // for all of these fields, these 2 syntax's should be functionally
equivalent
// in matching the one doc that contains these fields
for (String q : Arrays.asList(f + ":*", f + ":[* TO *]")) {
assertJQ(req("q", q), "/response/numFound==1",
"/response/docs/[0]/id=='999'");
- // the same syntaxes should be valid even if no doc has the field...
+ // the same syntax's should be valid even if no doc has the field...
Review Comment:
```suggestion
// the same syntax should be valid even if no doc has the field...
```
##########
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerSettingsDAO.java:
##########
@@ -40,7 +40,6 @@ public class TestSchemaDesignerSettingsDAO extends
SolrCloudTestCase
@BeforeClass
public static void createCluster() throws Exception {
- System.setProperty("managed.schema.mutable", "true");
Review Comment:
Not a typos or grammar change, could we out-scope from this pull request
i.e. defer to a separate later one?
##########
solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetJoinDomain.java:
##########
@@ -153,7 +153,7 @@ private static void createMiniSolrCloudCluster() throws
Exception {
/**
* Given a (random) number, and a (static) array of possible suffixes
returns a consistent field
- * name that uses that number and one of hte specified suffixes in it's name.
+ * name that uses that number and one of hte specified suffixes in its name.
Review Comment:
```suggestion
* name that uses that number and one of the specified suffixes in its
name.
```
##########
solr/core/src/test/org/apache/solr/search/TestSolrQueryParser.java:
##########
@@ -171,7 +171,7 @@ public void testDocsWithNaNInField() throws Exception {
assertJQ(req("q", f + ":[* TO *]"), "/response/numFound==0");
assertJQ(req("q", f + ":[-Infinity TO Infinity]"),
"/response/numFound==0");
for (String q : Arrays.asList(f + ":*", f + ":[* TO *]", f +
":[-Infinity TO Infinity]")) {
- // the same syntaxes should be valid even if no doc has the field...
+ // the same syntax's should be valid even if no doc has the field...
Review Comment:
```suggestion
// the same syntax should be valid even if no doc has the field...
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java:
##########
@@ -3743,7 +3743,7 @@ public void testPrelimSortingDistribExtraStatAndFacet()
throws Exception {
*/
public void doTestPrelimSortingDistrib(final boolean extraAgg, final boolean
extraSubFacet)
throws Exception {
- // we only use 2 shards, but we also want to to sanity check code paths if
one (additional)
+ // we only use 2 shards, but we also want to sanity check code paths if
one (additional)
Review Comment:
```suggestion
// we only use 2 shards, but we also want to check code paths if one
(additional)
```
##########
solr/core/src/test/org/apache/solr/search/join/TestScoreJoinQPScore.java:
##########
@@ -346,8 +346,8 @@ public void testCacheHit() throws Exception {
"true"),
SolrException.ErrorCode.BAD_REQUEST);
}
- // this queries are not overlap, with other in this test case.
- // however it might be better to extract this method into the separate
suite
+ // these queries are not overlapping with other in this test case.
+ // however, it might be better to extract this method into the separate
suite
Review Comment:
```suggestion
// these queries are not overlapping with others in this test case.
// however, it might be better to extract this method into a separate
suite
```
##########
solr/core/src/test/org/apache/solr/search/facet/DistributedFacetSimpleRefinementLongTailTest.java:
##########
@@ -26,7 +26,7 @@
import org.junit.Test;
/**
- * A test the demonstrates some of the expected behavior fo "long tail" terms
when using <code>
+ * A test the demonstrates some expected behavior for "long tail" terms when
using <code>
Review Comment:
```suggestion
* A test that demonstrates some expected behavior for "long tail" terms
when using <code>
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKGEquiv.java:
##########
@@ -188,7 +188,7 @@ private static void createMiniSolrCloudCluster() throws
Exception {
/**
* Given a (random) number, and a (static) array of possible suffixes
returns a consistent field
- * name that uses that number and one of hte specified suffixes in it's name.
+ * name that uses that number and one of hte specified suffixes in its name.
Review Comment:
```suggestion
* name that uses that number and one of the specified suffixes in its
name.
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java:
##########
@@ -421,7 +421,7 @@ public void testBehaviorEquivilenceOfUninvertibleFalse()
throws Exception {
// faceting on an "uninvertible=false docValues=false" field should be
possible
// when using method:enum w/sort:index
//
- // it should behave equivilent to it's copyField source...
+ // it should behave equivalent to it's copyField source...
Review Comment:
```suggestion
// it should behave equivalent to its copyField source...
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java:
##########
@@ -401,7 +401,7 @@ public void testBehaviorEquivilenceOfUninvertibleFalse()
throws Exception {
// regardless of the facet method (parameterized via default at test class
level)
// faceting on an "uninvertible=false docValues=true" field should work,
//
- // it should behave equivilently to it's copyField source...
+ // it should behave equivalently to it's copyField source...
Review Comment:
```suggestion
// it should behave equivalently to its copyField source...
```
##########
solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java:
##########
@@ -601,17 +601,19 @@ public void close() throws Exception {
public List<AuditEvent> waitForAuditEvents(final int expected) throws
InterruptedException {
final LinkedList<AuditEvent> results = new LinkedList<>();
- for (int i = 1; i <= expected; i++) { // NOTE: counting from 1 for error
message readabiity...
+ for (int i = 1;
+ i <= expected;
+ i++) { // NOTE: counting from 1 for error message readability...
Review Comment:
```suggestion
for (int i = 1; i <= expected; i++) { // NOTE: counting from 1 for
error message readability
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java:
##########
@@ -4247,19 +4247,19 @@ public void doTestPrelimSorting(
+ " },"
+ "}");
// the prelim_sort should prevent 'sum(bar_i)' from being computed for
every doc
- // only the choosen buckets should be collected (as a set) once per node...
+ // only the chosen buckets should be collected (as a set) once per node...
assertEqualsAndReset(0, DebugAgg.Acc.collectDocs);
// 5 bucket, on each shard
assertEqualsAndReset(numShardsWithData * 5, DebugAgg.Acc.collectDocSets);
- { // sanity check how defered stats are handled
+ { // sanity check how deferred stats are handled
Review Comment:
```suggestion
{ // check how deferred stats are handled
```
##########
solr/core/src/test/org/apache/solr/search/join/TestCloudNestedDocsSort.java:
##########
@@ -111,7 +111,7 @@ public static void setupCluster() throws Exception {
final List<String> chVals = addValsField(child, "childFilter_s");
parent.addChildDocument(child);
- // let's pickup at least matching child
+ // lets pickup at least matching child
Review Comment:
```suggestion
// let's pickup at least matching child
```
##########
solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java:
##########
@@ -295,7 +295,7 @@ public void authValid() throws Exception {
req.setBasicAuthCredentials("solr", SOLR_PASS);
client.request(req);
- // collection createion leads to AuditEvent's for the core as well...
+ // collection creation leads to AuditEvent's for the core as well...
Review Comment:
```suggestion
// collection creation leads to audit events for the core as well...
```
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -313,10 +313,10 @@ public void testRandomNestPathQueryFiltering() {
// but there will be more because we'll try lots of sub-paths that
have no docs
docs.numDocsWithPathWithKids.keySet().size()
<
docs.recursiveCheckChildQueryOfAllParents(Collections.<String>emptyList()));
- // sanity check: path that is garunteed not to exist...
+ // sanity check: path that is guaranteed not to exist...
Review Comment:
```suggestion
// check: path that is guaranteed not to exist...
```
##########
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java:
##########
@@ -2046,9 +2046,9 @@ public static void doStatsTemplated(Client client,
ModifiableSolrParams p) throw
+ ", f2:{ 'buckets':[{ val:'B', count:3, n1:-3.0}, { val:'A',
count:2, n1:6.0 }]} }");
// test trivial re-sorting by stats
- // (there are other more indepth tests of this in doTestPrelimSorting, but
this let's us sanity
+ // (there are other more in depth tests of this in doTestPrelimSorting,
but this lets us sanity
// check
Review Comment:
```suggestion
// (there are other more in depth tests of this in doTestPrelimSorting,
but this lets us check
```
##########
solr/core/src/test/org/apache/solr/uninverting/TestFieldCacheVsDocValues.java:
##########
@@ -122,7 +122,7 @@ public void testSortedSetVariableLengthVsUninvertedField()
throws Exception {
// LUCENE-4853
public void testHugeBinaryValues() throws Exception {
Analyzer analyzer = new MockAnalyzer(random());
- // FSDirectory because SimpleText will consume gobbs of
+ // FSDirectory because SimpleText will consume gobs of
Review Comment:
```suggestion
// FSDirectory because SimpleText will consume lots of
```
assuming natural language "lots" was meant here rather than technical terms
"blobs" or similar?
##########
solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java:
##########
@@ -162,7 +162,7 @@ public static void createMiniSolrCloudCluster() throws
Exception {
fail("unexpected shard: " + shardName);
}
}
- assertEquals("Should be exactly one server left (nost hosting either
shard)", 1, urlMap.size());
+ assertEquals("Should be exactly one server left (hosting either shard)",
1, urlMap.size());
Review Comment:
```suggestion
assertEquals("Should be exactly one server left (not hosting either
shard)", 1, urlMap.size());
```
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -296,15 +296,15 @@ public void testRandomNestPathQueryFiltering() {
// now do some systematic parent/child queries.
// we're checking both for "parser errors" (ie: children matching "parent
filter")
- // as well as that the test_path_s of all matching docs meets our
expectations
+ // and that the test_path_s of all matching docs meets our expectations
// *:* w/ parent parser...
// starts at "root" parent_path and recurses until we get no (expected)
results
assertTrue( // we expected at least one query for every "real" path,
// but there will be more because we'll try lots of sub-paths that
have no docs
docs.numDocsDescendentFromPath.keySet().size()
<
docs.recursiveCheckParentQueryOfAllChildren(Collections.<String>emptyList()));
- // sanity check: path that is garunteed not to exist...
+ // sanity check: path that is guaranteed not to exist...
Review Comment:
```suggestion
// check: path that is guaranteed not to exist...
```
##########
solr/core/src/test/org/apache/solr/cloud/TestCloudDeleteByQuery.java:
##########
@@ -173,7 +173,7 @@ private static void createMiniSolrCloudCluster() throws
Exception {
fail("unexpected shard: " + shardName);
}
}
- assertEquals("Should be exactly one server left (nost hosting either
shard)", 1, urlMap.size());
+ assertEquals("Should be exactly one server left (hosting either shard)",
1, urlMap.size());
Review Comment:
```suggestion
assertEquals("Should be exactly one server left (not hosting either
shard)", 1, urlMap.size());
```
##########
solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorRandomCloud.java:
##########
@@ -220,12 +220,12 @@ public void testRandomUpdates() throws Exception {
final String q;
if (causeError) {
// even though our DBQ is gibberish that's going to fail, record
a docId as affected
- // so that we don't generate the same random DBQ and get
redundent errors
- // (problematic because of how DUP forwarded DBQs have to have
their errors deduped by
+ // so that we don't generate the same random DBQ and get
redundant errors
+ // (problematic because of how DUP forwarded DBQs have to have
their errors deduced by
Review Comment:
could "deduplicated" rather than "deduced" have been the intended meaning
here?
```suggestion
// (problematic because of how DUP forwarded DBQs have to have
their errors deduped by
```
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
