You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2014/11/26 20:47:22 UTC
svn commit: r1641902 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/index/TestIndexWriter.java
test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java
test-framework/src/java/org/apache/lucene/util/TestUtil.java
Author: mikemccand
Date: Wed Nov 26 19:47:22 2014
New Revision: 1641902
URL: http://svn.apache.org/r1641902
Log:
LUCENE-6078: disable this test if get WindowsFS
Modified:
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/TestUtil.java
Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java?rev=1641902&r1=1641901&r2=1641902&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java (original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java Wed Nov 26 19:47:22 2014
@@ -2620,6 +2620,10 @@ public class TestIndexWriter extends Luc
assumeFalse("this test can't run on Windows", Constants.WINDOWS);
MockDirectoryWrapper dir = newMockDirectory();
+ if (TestUtil.isWindowsFS(dir)) {
+ dir.close();
+ assumeFalse("this test can't run on Windows", true);
+ }
// don't act like windows either, or the test won't simulate the condition
dir.setEnableVirusScanner(false);
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java?rev=1641902&r1=1641901&r2=1641902&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java Wed Nov 26 19:47:22 2014
@@ -172,4 +172,14 @@ public class FilterFileSystem extends Fi
public WatchService newWatchService() throws IOException {
return delegate.newWatchService();
}
+
+ /** Returns the {@code FileSystem} we wrap. */
+ public FileSystem getDelegate() {
+ return delegate;
+ }
+
+ /** Returns the {@code FilterFileSystemProvider} sent to this on init. */
+ public FileSystemProvider getParent() {
+ return parent;
+ }
}
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/TestUtil.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/TestUtil.java?rev=1641902&r1=1641901&r2=1641902&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/TestUtil.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/TestUtil.java Wed Nov 26 19:47:22 2014
@@ -25,6 +25,7 @@ import java.io.PrintStream;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.nio.CharBuffer;
+import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
@@ -81,12 +82,16 @@ import org.apache.lucene.index.SegmentRe
import org.apache.lucene.index.Terms;
import org.apache.lucene.index.TermsEnum;
import org.apache.lucene.index.TieredMergePolicy;
+import org.apache.lucene.mockfile.FilterFileSystem;
+import org.apache.lucene.mockfile.WindowsFS;
import org.apache.lucene.search.FieldDoc;
import org.apache.lucene.search.FilteredQuery.FilterStrategy;
import org.apache.lucene.search.FilteredQuery;
import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.TopDocs;
import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.FSDirectory;
+import org.apache.lucene.store.FilterDirectory;
import org.apache.lucene.store.NoLockFactory;
import org.junit.Assert;
import com.carrotsearch.randomizedtesting.generators.RandomInts;
@@ -1142,6 +1147,35 @@ public final class TestUtil {
return sb.toString();
}
}
+
+ /** Returns true if this is an FSDirectory backed by {@link WindowsFS}. */
+ public static boolean isWindowsFS(Directory dir) {
+ // First unwrap directory to see if there is an FSDir:
+ while (true) {
+ if (dir instanceof FSDirectory) {
+ return isWindowsFS(((FSDirectory) dir).getDirectory());
+ } else if (dir instanceof FilterDirectory) {
+ dir = ((FilterDirectory) dir).getDelegate();
+ } else {
+ return false;
+ }
+ }
+ }
+
+ /** Returns true if this Path is backed by {@link WindowsFS}. */
+ public static boolean isWindowsFS(Path path) {
+ FileSystem fs = path.getFileSystem();
+ while (true) {
+ if (fs instanceof FilterFileSystem) {
+ if (((FilterFileSystem) fs).getParent() instanceof WindowsFS) {
+ return true;
+ }
+ fs = ((FilterFileSystem) fs).getDelegate();
+ } else {
+ return false;
+ }
+ }
+ }
/** List of characters that match {@link Character#isWhitespace} */
public static final char[] WHITESPACE_CHARACTERS = new char[] {
Re: svn commit: r1641902 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/index/TestIndexWriter.java
test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java test-framework/src/java/org/apache/lucene/util/TestUtil.java
Posted by Robert Muir <rc...@gmail.com>.
On Tue, Dec 2, 2014 at 12:01 PM, Chris Hostetter
<ho...@fucit.org> wrote:
> my question: rather then assumeFalse() ~1/10 of the time (even when the
> underlying OS/FS is *NOT* windows) why not unwrap that (mock) windowsfs
> and still run the test using the real file system?
>
> Wouldn't that be trivial to do? It doesn't seem any more complicated then
> what you had to add to implement "isWindowsFS" ... can't we just change
> those "isWindowsFS" boolean methods to something that returns the actual
> WindowsFS instance and calls getDelegate() on it?
>
Personally i am against this, we shouldn't make this method public!
The test has a problem in that it does not work on windows, but the
test framework doesnt need to become flaky to cater to this kind of
stuff.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1641902 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/index/TestIndexWriter.java
test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java
test-framework/src/java/org/apache/lucene/util/TestUtil.java
Posted by Chris Hostetter <ho...@fucit.org>.
: I'll fix the message to make it clear you are not running on WINDOWS
: but rather our WindowsFS.
thanks.
: I agree we should strive to have our tests be portable across OS's ...
:
: But unfortunately this test case is for a nasty corruption bug
: (LUCENE-5574) that can't happen on Windows...
I don't really understand how that statment relates to either of my
questions?
Currently, if i a non-windows user runs all tests on their machine the
framework will select "windowsfs" as a mock file system roughly 1/10th of
the test runs (9/10 * 1/10).
With your latest commit, this test -- knowing that it is useless on
windows *OR* the mock windowsfs -- jumps through hoops to ask
"TestUtil.isWindowsFS(dir)" ? ... and if the answer is "true", it does
nothing (via: assumeFalse)
my question: rather then assumeFalse() ~1/10 of the time (even when the
underlying OS/FS is *NOT* windows) why not unwrap that (mock) windowsfs
and still run the test using the real file system?
Wouldn't that be trivial to do? It doesn't seem any more complicated then
what you had to add to implement "isWindowsFS" ... can't we just change
those "isWindowsFS" boolean methods to something that returns the actual
WindowsFS instance and calls getDelegate() on it?
-Hoss
http://www.lucidworks.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1641902 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/index/TestIndexWriter.java
test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java test-framework/src/java/org/apache/lucene/util/TestUtil.java
Posted by Michael McCandless <lu...@mikemccandless.com>.
I'll fix the message to make it clear you are not running on WINDOWS
but rather our WindowsFS.
I agree we should strive to have our tests be portable across OS's ...
But unfortunately this test case is for a nasty corruption bug
(LUCENE-5574) that can't happen on Windows...
Mike McCandless
http://blog.mikemccandless.com
On Mon, Dec 1, 2014 at 7:14 PM, Robert Muir <rc...@gmail.com> wrote:
> -1.
>
> our test framework doesnt need to support this. the problem here is
> the test, its no good and doesnt work on windows. An assume is the
> correct answer for a shitty test.
>
> On Mon, Dec 1, 2014 at 5:46 PM, Chris Hostetter
> <ho...@fucit.org> wrote:
>>
>> : assumeFalse("this test can't run on Windows", Constants.WINDOWS);
>> :
>> : MockDirectoryWrapper dir = newMockDirectory();
>> : + if (TestUtil.isWindowsFS(dir)) {
>> : + dir.close();
>> : + assumeFalse("this test can't run on Windows", true);
>> : + }
>>
>> this specific assume msg seems like a bad idea.
>>
>> ie: a new dev, who doesn't know about the FS mocking behavior of
>> the test cases, who tries to run lucene tests on a mac and sees a
>> test skipped with the message "this test can't run on Windows" i'm going
>> to be confused as hell.
>>
>> I also have to wonder: rather then just a straight assumeFalse, wouldn't
>> it be better in this case to just upwrap the mock "windowsfs" and just
>> explicitly use the "real" fs for this particular test? (in hte interest of
>> maximizing test coverage)
>>
>>
>> -Hoss
>> http://www.lucidworks.com/
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1641902 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/index/TestIndexWriter.java
test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java test-framework/src/java/org/apache/lucene/util/TestUtil.java
Posted by Robert Muir <rc...@gmail.com>.
-1.
our test framework doesnt need to support this. the problem here is
the test, its no good and doesnt work on windows. An assume is the
correct answer for a shitty test.
On Mon, Dec 1, 2014 at 5:46 PM, Chris Hostetter
<ho...@fucit.org> wrote:
>
> : assumeFalse("this test can't run on Windows", Constants.WINDOWS);
> :
> : MockDirectoryWrapper dir = newMockDirectory();
> : + if (TestUtil.isWindowsFS(dir)) {
> : + dir.close();
> : + assumeFalse("this test can't run on Windows", true);
> : + }
>
> this specific assume msg seems like a bad idea.
>
> ie: a new dev, who doesn't know about the FS mocking behavior of
> the test cases, who tries to run lucene tests on a mac and sees a
> test skipped with the message "this test can't run on Windows" i'm going
> to be confused as hell.
>
> I also have to wonder: rather then just a straight assumeFalse, wouldn't
> it be better in this case to just upwrap the mock "windowsfs" and just
> explicitly use the "real" fs for this particular test? (in hte interest of
> maximizing test coverage)
>
>
> -Hoss
> http://www.lucidworks.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1641902 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/index/TestIndexWriter.java
test-framework/src/java/org/apache/lucene/mockfile/FilterFileSystem.java
test-framework/src/java/org/apache/lucene/util/TestUtil.java
Posted by Chris Hostetter <ho...@fucit.org>.
: assumeFalse("this test can't run on Windows", Constants.WINDOWS);
:
: MockDirectoryWrapper dir = newMockDirectory();
: + if (TestUtil.isWindowsFS(dir)) {
: + dir.close();
: + assumeFalse("this test can't run on Windows", true);
: + }
this specific assume msg seems like a bad idea.
ie: a new dev, who doesn't know about the FS mocking behavior of
the test cases, who tries to run lucene tests on a mac and sees a
test skipped with the message "this test can't run on Windows" i'm going
to be confused as hell.
I also have to wonder: rather then just a straight assumeFalse, wouldn't
it be better in this case to just upwrap the mock "windowsfs" and just
explicitly use the "real" fs for this particular test? (in hte interest of
maximizing test coverage)
-Hoss
http://www.lucidworks.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org