You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Chris Hostetter <ho...@fucit.org> on 2014/12/01 23:46:17 UTC
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
: 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
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