[ 
https://issues.apache.org/jira/browse/LUCENE-2804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12969727#action_12969727
 ] 

Shai Erera commented on LUCENE-2804:
------------------------------------

Note that I searched for FSDirectory.open in Solr's tests too, and found 3 
tests that called it, and fixed them. But I agree this MockDirectoryFactory is 
needed too, since I assume other tests use a DirFactory outside the *test* 
code, which uses FSDirectory.open inside.

One thing I discovered is that the Directory obtained from DirFactory.open() is 
not closed by Solr code. So I guess a lot of Solr tests would fail after we 
introduce MockDirFactory. If you wish, we can revert the fixes I did to the 3 
tests, and fix them in the follow up issue. Currently, I had to hack one test 
to make sure the dir is closed in the end.

> check all tests that use FSDirectory.open
> -----------------------------------------
>
>                 Key: LUCENE-2804
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2804
>             Project: Lucene - Java
>          Issue Type: Test
>            Reporter: Robert Muir
>         Attachments: LUCENE-2804.patch
>
>
> In LUCENE-2471 we were discussing the copyBytes issue, and Shai and I had a 
> discussion about how we could prevent such bugs in the future.
> One thing that lead to the bug existing in our code for so long, was that it 
> only happened on windows (e.g. never failed in hudson!)
> This was because the bug only happened if you were copying from 
> SimpleFSDirectory, and the test used FSDirectory.open
> Today the situation is improving: most tests use newDirectory() which is 
> random by default and never use FSDir.open,
> it always uses SimpleFS or NIOFS so that the same random seed will reproduce 
> across both windows and unix.
> So I think we need to review all uses of FSDirectory.open in our tests, and 
> minimize these.
> In general tests should use newDirectory().
> If the test comes with say a zip-file and wants to explicitly open stuff from 
> disk, I think it should open the contents with say SimpleFSDir,
> and then call newDirectory(Directory) to copy into a new "random" 
> implementation for actual testing. This method already exists:
> {noformat}
>   /**
>    * Returns a new Dictionary instance, with contents copied from the
>    * provided directory. See {...@link #newDirectory()} for more
>    * information.
>    */
>   public static MockDirectoryWrapper newDirectory(Directory d) throws 
> IOException {
> {noformat}

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to