mikemccand commented on a change in pull request #2088:
URL: https://github.com/apache/lucene-solr/pull/2088#discussion_r527730165
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestFieldInfos.java
##########
@@ -187,4 +187,23 @@ public void testMergedFieldInfos_singleLeaf() throws
IOException {
writer.close();
dir.close();
}
+
+ public void testFieldNumbersAutoIncrement() {
+ FieldInfos.FieldNumbers fieldNumbers = new
FieldInfos.FieldNumbers("softDeletes");
+ for (int i = 0; i < 10; i++) {
+ fieldNumbers.addOrGet("field" + i, -1, IndexOptions.NONE,
DocValuesType.NONE,
+ 0, 0, 0, 0,
+ VectorValues.SearchStrategy.NONE, false);
+ }
+ int idx = fieldNumbers.addOrGet("EleventhField", -1, IndexOptions.NONE,
DocValuesType.NONE,
+ 0, 0, 0, 0,
+ VectorValues.SearchStrategy.NONE, false);
+ assertEquals("Field numbers 0 through 9 were allocated", 10, idx);
+
+ fieldNumbers.clear();
Review comment:
Uhm, but this is the purpose of a unit-test?
It should test the lowest "unit" of the software, in this case `FieldInfos`
and its `clear` method.
The fact that `IW.deleteAll()` is the "user space manifestation" of this
particular low level problem is actually unimportant, from the standpoint of
this unit test. The unit test should only test the smallest / shrink-wrapped
problem, which is indeed that `FieldInfos.clear()` failed to do its job
correctly.
We could consider adding a unit test that "IW.deleteAll does not leak
memory", but I don't think that is required for committing this fix to an
obvious bug ...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
