[GitHub] [lucene-solr] mikemccand commented on a change in pull request #2088: LUCENE-9617: Reset lowestUnassignedFieldNumber in FieldNumbers.clear()

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org