jpountz commented on code in PR #12544:
URL: https://github.com/apache/lucene/pull/12544#discussion_r1319678371


##########
lucene/core/src/test/org/apache/lucene/search/TestSort.java:
##########
@@ -849,11 +849,10 @@ public void testMultiSort() throws IOException {
   }
 
   public void testRewrite() throws IOException {
-    try (Directory dir = newDirectory()) {
-      RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
-      IndexSearcher searcher = newSearcher(writer.getReader());
-      writer.close();
-
+    try (Directory dir = newDirectory();
+        RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
+        DirectoryReader reader = writer.getReader()) {
+      IndexSearcher searcher = newSearcher(reader);

Review Comment:
   Oh good catch. I think `MockDirectoryWrapper` would generally catch such 
problems, except here because the index is empty, so no files get open?



##########
lucene/monitor/src/test/org/apache/lucene/monitor/TestForceNoBulkScoringQuery.java:
##########
@@ -60,18 +60,18 @@ public void testRewrite() throws IOException {
       iw.addDocument(doc);
       iw.commit();
 
-      IndexReader reader = DirectoryReader.open(dir);
+      try (IndexReader reader = DirectoryReader.open(dir)) {

Review Comment:
   Let's use a `newDirectory()` instead of `new ByteBuffersDirectory()` to get 
additional checks enabled?



##########
lucene/queries/src/test/org/apache/lucene/queries/payloads/TestPayloadSpans.java:
##########
@@ -60,15 +60,22 @@ public class TestPayloadSpans extends LuceneTestCase {
   protected IndexReader indexReader;
   private IndexReader closeIndexReader;
   private Directory directory;
+  private PayloadHelper helper;
 
   @Override
   public void setUp() throws Exception {
     super.setUp();
-    PayloadHelper helper = new PayloadHelper();
+    helper = new PayloadHelper();
     searcher = helper.setUp(random(), similarity, 1000);
     indexReader = searcher.getIndexReader();
   }
 
+  @Override
+  public void tearDown() throws Exception {
+    super.tearDown();
+    helper.tearDown();

Review Comment:
   nit: do `helper.tearDown()` before `super.tearDown()`, ie. release resources 
that are specific to this test case before other resources?



-- 
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...@lucene.apache.org

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

Reply via email to