Cleanup comments and asserts in TestIndexWriter (#13006)

lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java

@@ -189,8 +189,7 @@ public class TestIndexWriter extends LuceneTestCase {
     assertEquals(60, reader.numDocs());
     reader.close();
 
-    // make sure opening a new index for create over
+    // make sure opening a new index for create over this existing one works correctly:
-    // this existing one works correctly:
     writer =
         new IndexWriter(
             dir, newIndexWriterConfig(new MockAnalyzer(random())).setOpenMode(OpenMode.CREATE));
@@ -215,8 +214,7 @@ public class TestIndexWriter extends LuceneTestCase {
   }
 
   // TODO: we have the logic in MDW to do this check, and it's better, because it knows about files
-  // it tried
+  //   it tried to delete but couldn't: we should replace this!!!!
-  // to delete but couldn't: we should replace this!!!!
   public static void assertNoUnreferencedFiles(Directory dir, String message) throws IOException {
     String[] startFiles = dir.listAll();
     new IndexWriter(dir, new IndexWriterConfig(new MockAnalyzer(random()))).rollback();
@@ -229,26 +227,14 @@ public class TestIndexWriter extends LuceneTestCase {
       fail(
           message
               + ": before delete:\n    "
-              + arrayToString(startFiles)
+              + String.join("\n    ", startFiles)
               + "\n  after delete:\n    "
-              + arrayToString(endFiles));
+              + String.join("\n    ", endFiles));
     }
   }
 
-  static String arrayToString(String[] l) {
-    String s = "";
-    for (int i = 0; i < l.length; i++) {
-      if (i > 0) {
-        s += "\n    ";
-      }
-      s += l[i];
-    }
-    return s;
-  }
-
   // Make sure we can open an index for create even when a
-  // reader holds it open (this fails pre lock-less
+  // reader holds it open (this fails pre lock-less commits on Windows):
-  // commits on windows):
   public void testCreateWithReader() throws IOException {
     Directory dir = newDirectory();
 
@@ -328,8 +314,7 @@ public class TestIndexWriter extends LuceneTestCase {
       Document doc = new Document();
       doc.add(newField("field", "aaa" + j, storedTextType));
       writer.addDocument(doc);
-      // Verify that with a tiny RAM buffer we see new
+      // Verify that with a tiny RAM buffer we see new segment after every doc
-      // segment after every doc
       int numSegments = getSegmentCount(dir);
       assertTrue(numSegments > lastNumSegments);
       lastNumSegments = numSegments;
@@ -348,8 +333,7 @@ public class TestIndexWriter extends LuceneTestCase {
     return segments.size();
   }
 
-  // Make sure it's OK to change RAM buffer size and
+  // Make sure it's OK to change RAM buffer size and maxBufferedDocs in a write session
-  // maxBufferedDocs in a write session
   public void testChangingRAMBuffer() throws IOException {
     Directory dir = newDirectory();
     IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())));
@@ -514,15 +498,13 @@ public class TestIndexWriter extends LuceneTestCase {
     doc.add(newField("field", "aaa", customType));
     for (int i = 0; i < 19; i++) writer.addDocument(doc);
     writer.flush(false, true);
-    // Since we flushed w/o allowing merging we should now
+    // Since we flushed w/o allowing merging we should now have 10 segments
-    // have 10 segments
     assertEquals(10, writer.getSegmentCount());
     writer.close();
     dir.close();
   }
 
-  // Make sure we can flush segment w/ norms, then add
+  // Make sure we can flush segment w/ norms, then add empty doc (no norms) and flush
-  // empty doc (no norms) and flush
   public void testEmptyDocAfterFlushingRealDoc() throws IOException {
     Directory dir = newDirectory();
     IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())));
@@ -643,10 +625,7 @@ public class TestIndexWriter extends LuceneTestCase {
     IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())));
 
     Document doc = new Document();
-    StringBuilder b = new StringBuilder();
+    doc.add(newTextField("field", " a".repeat(10_000) + " x", Field.Store.NO));
-    for (int i = 0; i < 10000; i++) b.append(" a");
-    b.append(" x");
-    doc.add(newTextField("field", b.toString(), Field.Store.NO));
     writer.addDocument(doc);
     writer.close();
 
@@ -936,11 +915,9 @@ public class TestIndexWriter extends LuceneTestCase {
 
           while (!finish) {
             if (w != null) {
-              // If interrupt arrives inside here, it's
+              // If interrupt arrives inside here, it's fine: we will cycle back
-              // fine: we will cycle back and the first
+              // and the first thing we do is try to close again,
-              // thing we do is try to close again,
+              // i.e. we'll never try to open a new writer until this one successfully closes:
-              // i.e. we'll never try to open a new writer
-              // until this one successfully closes:
               // w.rollback();
               try {
                 w.close();
@@ -1020,22 +997,18 @@ public class TestIndexWriter extends LuceneTestCase {
             w = null;
             DirectoryReader.open(dir).close();
 
-            // Strangely, if we interrupt a thread before
+            // Strangely, if we interrupt a thread before all classes are loaded,
-            // all classes are loaded, the class loader
+            // the class loader seems to do scary things with the interrupt status.
-            // seems to do scary things with the interrupt
+            // In java 1.5, it'll throw an incorrect ClassNotFoundException.
-            // status.  In java 1.5, it'll throw an
+            // In java 1.6, it'll silently clear the interrupt.
-            // incorrect ClassNotFoundException.  In java
+            // So, on first iteration through here we don't open ourselves up for interrupts
-            // 1.6, it'll silently clear the interrupt.
-            // So, on first iteration through here we
-            // don't open ourselves up for interrupts
             // until we've done the above loop.
             allowInterrupt = true;
           }
         } catch (ThreadInterruptedException re) {
-          // NOTE: important to leave this verbosity/noise
+          // NOTE: important to leave this verbosity/noise on!!
-          // on!!  This test doesn't repro easily so when
+          // This test doesn't repro easily so when Jenkins hits a fail
-          // Jenkins hits a fail we need to study where the
+          // we need to study where the interrupts struck!
-          // interrupts struck!
           log.println("TEST thread " + id + ": got interrupt");
           re.printStackTrace(log);
           Throwable e = re.getCause();
@@ -1118,9 +1091,8 @@ public class TestIndexWriter extends LuceneTestCase {
     t.setDaemon(true);
     t.start();
 
-    // Force class loader to load ThreadInterruptedException
+    // Force class loader to load ThreadInterruptedException up front...
-    // up front... else we can see a false failure if 2nd
+    // else we can see a false failure if 2nd interrupt arrives while class loader is trying to
-    // interrupt arrives while class loader is trying to
     // init this class (in servicing a first interrupt):
     assertTrue(
         new ThreadInterruptedException(new InterruptedException()).getCause()
@@ -1130,8 +1102,7 @@ public class TestIndexWriter extends LuceneTestCase {
     final int numInterrupts = atLeast(100);
     int i = 0;
     while (i < numInterrupts) {
-      // TODO: would be nice to also sometimes interrupt the
+      // TODO: would be nice to also sometimes interrupt the CMS merge threads too ...
-      // CMS merge threads too ...
       Thread.sleep(10);
       if (t.allowInterrupt) {
         i++;
@@ -1208,13 +1179,13 @@ public class TestIndexWriter extends LuceneTestCase {
     Document doc2 = storedFields.document(0);
     IndexableField f3 = doc2.getField("binary");
     b = f3.binaryValue().bytes;
-    assertTrue(b != null);
+    assertNotNull(b);
-    assertEquals(17, b.length, 17);
+    assertEquals(17, b.length);
     assertEquals(87, b[0]);
 
-    assertTrue(storedFields.document(0).getField("binary").binaryValue() != null);
+    assertNotNull(storedFields.document(0).getField("binary").binaryValue());
-    assertTrue(storedFields.document(1).getField("binary").binaryValue() != null);
+    assertNotNull(storedFields.document(1).getField("binary").binaryValue());
-    assertTrue(storedFields.document(2).getField("binary").binaryValue() != null);
+    assertNotNull(storedFields.document(2).getField("binary").binaryValue());
 
     assertEquals("value", storedFields.document(0).get("string"));
     assertEquals("value", storedFields.document(1).get("string"));
@@ -1264,7 +1235,7 @@ public class TestIndexWriter extends LuceneTestCase {
     assumeWorkingMMapOnWindows();
 
     for (int iter = 0; iter < 2; iter++) {
-      // relies on windows semantics
+      // relies on Windows semantics
       Path path = createTempDir();
       WindowsFS provider = new WindowsFS(path.getFileSystem());
       Path indexPath = provider.wrapPath(path);
@@ -1321,12 +1292,11 @@ public class TestIndexWriter extends LuceneTestCase {
       }
       IndexReader r2 = DirectoryReader.openIfChanged(r);
       assertNotNull(r2);
-      assertTrue(r != r2);
+      assertNotSame(r, r2);
 
-      // NOTE: here we rely on "Windows" behavior, ie, even
+      // NOTE: here we rely on "Windows" behavior, i.e. even though IW wanted to delete _0.cfs
-      // though IW wanted to delete _0.cfs since it was
+      // since it was merged away, because we have a reader open against this file,
-      // merged away, because we have a reader open
+      // it should still be here:
-      // against this file, it should still be here:
       assertTrue(Files.exists(indexPath.resolve("_0.cfs")));
       // forceMerge created this
       // assertTrue(files.contains("_2.cfs"));
@@ -1426,8 +1396,7 @@ public class TestIndexWriter extends LuceneTestCase {
                 .setUseCompoundFile(false));
     String[] files = dir.listAll();
 
-    // Creating over empty dir should not create any files,
+    // Creating over empty dir should not create any files, or, at most the write.lock file
-    // or, at most the write.lock file
     final int extraFileCount = files.length - origFiles.length;
     if (extraFileCount == 1) {
       assertTrue(Arrays.asList(files).contains(IndexWriter.WRITE_LOCK_NAME));
@@ -1612,16 +1581,15 @@ public class TestIndexWriter extends LuceneTestCase {
     assertEquals(1, reader.docFreq(new Term("content", "bbb")));
     assertEquals(0, reader.docFreq(new Term("content", "term")));
 
-    // Make sure the doc that has the massive term is NOT in
+    // Make sure the doc that has the massive term is NOT in the index:
-    // the index:
     assertEquals("document with wicked long term is in the index!", 1, reader.numDocs());
 
     reader.close();
     dir.close();
     dir = newDirectory();
 
-    // Make sure we can add a document with exactly the
+    // Make sure we can add a document with exactly the maximum length term,
-    // maximum length term, and search on that term:
+    // and search on that term:
     doc = new Document();
     FieldType customType = new FieldType(TextField.TYPE_NOT_STORED);
     customType.setTokenized(false);
@@ -1667,8 +1635,7 @@ public class TestIndexWriter extends LuceneTestCase {
 
       w.deleteAll();
       w.commit();
-      // Make sure we accumulate no files except for empty
+      // Make sure we accumulate no files except for empty segments_N and segments.gen:
-      // segments_N and segments.gen:
       assertTrue(d.listAll().length <= 2);
     }
 
@@ -1702,8 +1669,8 @@ public class TestIndexWriter extends LuceneTestCase {
   }
 
   public void testWhetherDeleteAllDeletesWriteLock() throws Exception {
-    // Must use SimpleFSLockFactory... NativeFSLockFactory
+    // Must use SimpleFSLockFactory...
-    // somehow "knows" a lock is held against write.lock
+    // NativeFSLockFactory somehow "knows" a lock is held against write.lock
     // even if you remove that file:
     Directory d =
         newFSDirectory(
@@ -2071,8 +2038,8 @@ public class TestIndexWriter extends LuceneTestCase {
     assertEquals("value2", getLiveCommitData(writer).get("key"));
     writer.close();
 
-    // validate that when opening writer from older snapshotted index commit, the old commit data is
+    // validate that when opening writer from older snapshotted index commit,
-    // visible
+    // the old commit data is visible
     writer =
         new IndexWriter(
             dir,
@@ -2364,8 +2331,7 @@ public class TestIndexWriter extends LuceneTestCase {
           new IndexWriter(dir, iwc).rollback();
         }
       } catch (IOException ioe) {
-        // OpenMode.APPEND should throw an exception since no
+        // OpenMode.APPEND should throw an exception since no index exists:
-        // index exists:
         if (mode == 0) {
           // Unexpected
           throw ioe;
@@ -2440,8 +2406,7 @@ public class TestIndexWriter extends LuceneTestCase {
     iwc.setMergePolicy(
         new FilterMergePolicy(iwc.getMergePolicy()) {
           @Override
-          public boolean keepFullyDeletedSegment(IOSupplier<CodecReader> readerIOSupplier)
+          public boolean keepFullyDeletedSegment(IOSupplier<CodecReader> readerIOSupplier) {
-              throws IOException {
             return keepFullyDeletedSegments.get();
           }
         });
@@ -2480,9 +2445,8 @@ public class TestIndexWriter extends LuceneTestCase {
     doc.add(new TextField("a", "foo", Field.Store.NO));
     w.addDocument(doc);
 
-    // Should not delete the document; with LUCENE-5239 the
+    // Should not delete the document;
-    // "foo" from the 2nd delete term would incorrectly
+    // with LUCENE-5239 the "foo" from the 2nd delete term would incorrectly match field a's "foo":
-    // match field a's "foo":
     w.deleteDocuments(new Term("a", "xxx"));
     w.deleteDocuments(new Term("b", "foo"));
     IndexReader r = DirectoryReader.open(w);
@@ -2615,7 +2579,7 @@ public class TestIndexWriter extends LuceneTestCase {
 
     Directory dir = newDirectory();
     IndexWriterConfig iwc = new IndexWriterConfig(null);
-    // use an infostream that "takes a long time" to commit
+    // use an InfoStream that "takes a long time" to commit
     final IndexWriter iw =
         RandomIndexWriter.mockIndexWriter(
             random(),
@@ -2867,8 +2831,8 @@ public class TestIndexWriter extends LuceneTestCase {
     DirectoryReader r2 = DirectoryReader.openIfChanged(r);
     assertNotNull(r2);
     // segments_N should have changed:
-    assertFalse(
+    assertNotEquals(
-        r2.getIndexCommit().getSegmentsFileName().equals(r.getIndexCommit().getSegmentsFileName()));
+        r2.getIndexCommit().getSegmentsFileName(), r.getIndexCommit().getSegmentsFileName());
     IOUtils.close(r, r2, w, dir);
   }
 
@@ -2941,8 +2905,8 @@ public class TestIndexWriter extends LuceneTestCase {
   }
 
   public void testPendingDeletionsRollbackWithReader() throws IOException {
-    // irony: currently we don't emulate windows well enough to work on windows!
+    // irony: currently we don't emulate Windows well enough to work on Windows!
-    assumeFalse("windows is not supported", Constants.WINDOWS);
+    assumeFalse("Windows is not supported", Constants.WINDOWS);
 
     Path path = createTempDir();
 
@@ -2980,8 +2944,8 @@ public class TestIndexWriter extends LuceneTestCase {
   }
 
   public void testWithPendingDeletions() throws Exception {
-    // irony: currently we don't emulate windows well enough to work on windows!
+    // irony: currently we don't emulate Windows well enough to work on Windows!
-    assumeFalse("windows is not supported", Constants.WINDOWS);
+    assumeFalse("Windows is not supported", Constants.WINDOWS);
 
     Path path = createTempDir();
 
@@ -3032,8 +2996,8 @@ public class TestIndexWriter extends LuceneTestCase {
 
   public void testPendingDeletesAlreadyWrittenFiles() throws IOException {
     Path path = createTempDir();
-    // irony: currently we don't emulate windows well enough to work on windows!
+    // irony: currently we don't emulate Windows well enough to work on Windows!
-    assumeFalse("windows is not supported", Constants.WINDOWS);
+    assumeFalse("Windows is not supported", Constants.WINDOWS);
 
     // Use WindowsFS to prevent open files from being deleted:
     WindowsFS provider = new WindowsFS(path.getFileSystem());
@@ -3417,8 +3381,7 @@ public class TestIndexWriter extends LuceneTestCase {
   }
 
   private static void waitForDocsInBuffers(IndexWriter w, int buffersWithDocs) {
-    // wait until at least N DWPTs have a doc in order to observe
+    // wait until at least N DWPTs have a doc in order to observe who flushes the segments.
-    // who flushes the segments.
     while (true) {
       int numStatesWithDocs = 0;
       DocumentsWriterPerThreadPool perThreadPool = w.docWriter.perThreadPool;