uschindler commented on a change in pull request #2052:
URL: https://github.com/apache/lucene-solr/pull/2052#discussion_r524954118



##########
File path: lucene/misc/src/java/overview.html
##########
@@ -35,7 +35,7 @@ <h2>NativeUnixDirectory</h2>
 have to compile on your platform.
 
 <p>
-{@link org.apache.lucene.misc.store.NativeUnixDirectory} is a Directory 
implementation that bypasses the
+{@link org.apache.lucene.misc.store.DirectIODirectory} is a Directory 
implementation that bypasses the

Review comment:
       More of the javadocs below need to go away, too.

##########
File path: 
lucene/misc/src/test/org/apache/lucene/misc/store/DirectIODirectoryTest.java
##########
@@ -18,29 +18,37 @@
 
 import com.carrotsearch.randomizedtesting.LifecycleScope;
 import com.carrotsearch.randomizedtesting.RandomizedTest;
-import org.apache.lucene.store.ByteBuffersDirectory;
-import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.IOContext;
-import org.apache.lucene.store.MergeInfo;
+import org.apache.lucene.store.*;
 import org.apache.lucene.util.LuceneTestCase;
-import org.junit.Rule;
-import org.junit.rules.TestRule;
 
 import java.io.IOException;
-import java.util.EnumSet;
+import java.nio.file.Files;
 
-public class NativeUnixDirectoryTest extends LuceneTestCase {
-  @Rule
-  public static TestRule requiresNative = new NativeLibEnableRule(
-      EnumSet.of(NativeLibEnableRule.OperatingSystem.MAC,
-          NativeLibEnableRule.OperatingSystem.FREE_BSD,
-          NativeLibEnableRule.OperatingSystem.LINUX));
+import static 
org.apache.lucene.misc.store.DirectIODirectory.DEFAULT_MIN_BYTES_DIRECT;
 
-  public void testLibraryLoaded() throws IOException {
+public class DirectIODirectoryTest extends LuceneTestCase {

Review comment:
       In my opinion, this test should also extend the BaseDirectoryTestCase, 
because it then does a full test of all I/O functionality, including 
multi-threaded and corner cases - this is important to make sure that the 
directory works as expected, also if somebody uses it when not merging (e.g. 
for searching). The original test did not do this as this relied on a native 
library.
   
   Nevertheless: I am not fully sure if Windows supports this 
ExtendedOpenOption. I had no time to test this yet.




----------------------------------------------------------------
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

Reply via email to