You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Mike Hogan <me...@mikehogan.net> on 2003/10/05 11:09:00 UTC

Please make org.apache.lucene.index.IndexWriter non-final

Hi,

Please can you make org.apache.lucene.index.IndexWriter non-final.  It's
being final makes it impossible for me to test using mock objects.  Also, if
there are other public final classes that are significant to the Lucene API,
please make them non-final also, for the same reason.

One more thing, I see that there are cvs commits on the Lucene project, but
no releases in a long while.  What is the status with repect to new
releases?  (In other words, if you do go ahead and make these classes
non-final, when can I expect to be able to make use of the changes :)).

Thanks,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik mate, its not closed :-)  You still have not told me what benefit you see
in making a class final :-)

Seriously though, you reckon I should go ahead and use 1.3 RC1 then?  Its safe?

Cheers,
Mike.

Quoting Erik Hatcher <er...@ehatchersolutions.com>:

> Thanks for a fun discussion and for showing me clearly what you want to 
> be able to do.  I certainly am sympathetic to your cause, and love to 
> see folks taking TDD to heart.  We can probably consider this case 
> closed since the latest Lucene codebase is how you wanted.
> 
> I do concur that what you want to test (if IndexWriter were still 
> final, that is) is not cleanly possible.
> 
> 	Erik
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
Thanks for a fun discussion and for showing me clearly what you want to 
be able to do.  I certainly am sympathetic to your cause, and love to 
see folks taking TDD to heart.  We can probably consider this case 
closed since the latest Lucene codebase is how you wanted.

I do concur that what you want to test (if IndexWriter were still 
final, that is) is not cleanly possible.

	Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik,

Erik,

> > public class LuceneSearchService implements SearchService {
> >     private static final String INDEX_FILE_PATH = "index";
> >     private Context context = new DefaultContext();
> >
> >     public void setContext(Context c) {
> >         this.context = c;
> >     }
> >
> > public void index(String componentId, String componentDescription)
> > throws
> > SearchService.Exception {
> >         IndexWriter writer = null;
> >         try {
> >             writer = context.createIndexWriter(INDEX_FILE_PATH, new
> > StandardAnalyzer(), !indexExists());
>
> Take this a step further, and have a createDirectory instead, that can
> return a RAMDirectory.  Forget about a MockIndexWriter.  Trust me :)

I understand what you are saying here, it is a good point and I will try to
work it into what I am doing.  But installing a createDirectory() method and
using a RAMDirectory will not help me verify() that IndexWriter.close() is
called when an IOException is thrown.  MockIndexWriter gives me more
control.

> You just aren't seeing the forest from the trees here.  You need to
> refactor and just let IndexWriter be internal and not mocked, but how
> you get at a Directory be flexible for testing.  Even if you always use
> a file system directory even for testing, I highly recommend you bind
> to abstractions not concreteness.  Bind to Directory, not IndexWriter
> as your "interface".

Yes, I will do this.  Its is more flexible for sure.  But I still need
MockIndexWriter for the control I talk about above.

Cheers,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On Sunday, October 5, 2003, at 04:56  PM, Mike Hogan wrote:
> Setting up an interface as you suggest is what I did, and I create mock
> impls of that as I need in other test cases.  But I also want to unit 
> test
> the  _implementation_ itself?  Here is the full code of the 
> implementation:

Ok, so let's say you can subclass IndexWriter (which you can with the 
current codebase).  How would this change what you have below?

> public class LuceneSearchService implements SearchService {
>     private static final String INDEX_FILE_PATH = "index";

This is something you'd need to change, I'd assume.  Perhaps put in 
some kind of DirectoryFactory - and return a RAMDirectory or 
FSDirectory??

I don't see what a MockIndexWriter would do.

>             writer = new IndexWriter(INDEX_FILE_PATH, new
> StandardAnalyzer(), !indexExists());

Because you'd need to change this somehow.

>     //This is impossible to unit test as Hits is final, and the 
> constructor
> is package protected
>     public int search(final String query, int beginIndex, final int
> endIndex, final List collector) throws SearchService.Exception {


I don't understand your comment here.  Show me how you'd like to phrase 
the test if package permissions would let you.

You aren't passing Hits back out from here, what good would it do to 
mock it?

> You are right that this code as it currently stands cannot be unit 
> tested
> using mock-objects.  Thats because I tried to do so, ran into the final
> problem, then rolled back.  I will show you roughly how the code 
> looked when
> I refactored it to be mock-ready:

Ah, ok, so forget what I said above....

> public class LuceneSearchService implements SearchService {
>     private static final String INDEX_FILE_PATH = "index";
>     private Context context = new DefaultContext();
>
>     public void setContext(Context c) {
>         this.context = c;
>     }
>
> public void index(String componentId, String componentDescription) 
> throws
> SearchService.Exception {
>         IndexWriter writer = null;
>         try {
>             writer = context.createIndexWriter(INDEX_FILE_PATH, new
> StandardAnalyzer(), !indexExists());

Take this a step further, and have a createDirectory instead, that can 
return a RAMDirectory.  Forget about a MockIndexWriter.  Trust me :)

> public static interface Context {
>     IndexWriter createIndexWriter(String path, Analyzer a, boolean 
> create);
> }

Don't lock yourself into a file system index.  Please.  You might want 
it in RAM one day :)  (like in your unit tests)

> I don't know if you see any value in that, but I do.  I cannot do it 
> because
> IndexWriter is final and I cannot create a MockIndexWriter.  I can do a
> RAMDirectory and use the real Lucene API, but I find that less 
> convenient
> than the above.  Do you not agree?

You just aren't seeing the forest from the trees here.  You need to 
refactor and just let IndexWriter be internal and not mocked, but how 
you get at a Directory be flexible for testing.  Even if you always use 
a file system directory even for testing, I highly recommend you bind 
to abstractions not concreteness.  Bind to Directory, not IndexWriter 
as your "interface".

	Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik,

> It appears you are wrapping Lucene.  So, if you truly want to mock
> things for testing purposes, come up with a generic interface
> (SearchManager?!) that has the search method signature that you have
> above and all the other pieces you're wrapping.  Then create a mock
> implementation of that particular interface.

Setting up an interface as you suggest is what I did, and I create mock
impls of that as I need in other test cases.  But I also want to unit test
the  _implementation_ itself?  Here is the full code of the implementation:

package org.componenthaus.search;

import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.queryParser.QueryParser;
import org.apache.lucene.search.Hits;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.Searcher;

import java.io.File;
import java.io.IOException;
import java.util.List;

public class LuceneSearchService implements SearchService {
    private static final String INDEX_FILE_PATH = "index";

    //This is impossible to unit test as IndexWriter is final
    public void index(String componentId, String componentDescription)
throws SearchService.Exception {
        IndexWriter writer = null;
        try {
            writer = new IndexWriter(INDEX_FILE_PATH, new
StandardAnalyzer(), !indexExists());
            final Document document = new Document();
            document.add(Field.Text("id", componentId));
            document.add(Field.Text("contents", componentDescription));
            writer.addDocument(document);
            writer.optimize();
            writer.close();
        } catch (IOException e) {
            throw new SearchService.Exception("Exception updating Lucene
index", e);
        }
    }

    private boolean indexExists() {
        return new File(INDEX_FILE_PATH).exists();
    }

    //This is impossible to unit test as Hits is final, and the constructor
is package protected
    public int search(final String query, int beginIndex, final int
endIndex, final List collector) throws SearchService.Exception {
        int numHits = 0;
        try {
            final Searcher searcher = new IndexSearcher(INDEX_FILE_PATH);
            final Analyzer analyzer = new StandardAnalyzer();
            final Query q = QueryParser.parse(query, "contents", analyzer);
            final Hits hits = searcher.search(q);
            numHits = hits.length();
            while (beginIndex <= endIndex && beginIndex < numHits) {
                final Document doc = hits.doc(beginIndex++);
                collector.add(doc.get("id"));
            }
        } catch (java.lang.Exception e) {
            throw new SearchService.Exception("Exception performing Lucene
search",e);
        }
        return numHits;
    }

}

You are right that this code as it currently stands cannot be unit tested
using mock-objects.  Thats because I tried to do so, ran into the final
problem, then rolled back.  I will show you roughly how the code looked when
I refactored it to be mock-ready:

public class LuceneSearchService implements SearchService {
    private static final String INDEX_FILE_PATH = "index";
    private Context context = new DefaultContext();

    public void setContext(Context c) {
        this.context = c;
    }

public void index(String componentId, String componentDescription) throws
SearchService.Exception {
        IndexWriter writer = null;
        try {
            writer = context.createIndexWriter(INDEX_FILE_PATH, new
StandardAnalyzer(), !indexExists());
            final Document document = new Document();
            document.add(Field.Text("id", componentId));
            document.add(Field.Text("contents", componentDescription));
            writer.addDocument(document);
            writer.optimize();
            writer.close();
        } catch (IOException e) {
            throw new SearchService.Exception("Exception updating Lucene
index", e);
        }
    }

public static interface Context {
    IndexWriter createIndexWriter(String path, Analyzer a, boolean create);
}

private static final class DefaultContext {
    IndexWriter createIndexWriter(String path, Analyzer a, boolean create) {
        return new IndexWriter(path, a, create);
    }
}

Now my test case becomes:

public void testCase() {
    MockContext context = new MockContext();
    MockIndexWriter indexWriter = new IndexWriter();

    context.setupExpectedIndexWriter(indexWriter);
    indexWriter.setupExpectedCloseCalls(1);
    indexWriter.setupIOException(true);

    LuceneSearchService service = new LuceneSearchService();
    service.setContext(context);
    //Should throw IOException as instructed to do above
    service.index(componentId, componentDescription);

    //But we should still have a closed indexWriter.
    context.verify();
}

public class MockContext implements LuceneSearchService.Context {
    IndexWriter createIndexWriter(String path, Analyzer a, boolean create) {
        return new MockIndexWriter(path, a, create);
    }
}

I don't know if you see any value in that, but I do.  I cannot do it because
IndexWriter is final and I cannot create a MockIndexWriter.  I can do a
RAMDirectory and use the real Lucene API, but I find that less convenient
than the above.  Do you not agree?

Cheers,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On Sunday, October 5, 2003, at 10:02  AM, Mike Hogan wrote:
>     public int search(final String query, int beginIndex, final int
> endIndex, final List collector) throws SearchService.Exception {
>         int numHits = 0;
>         try {
>             final Searcher searcher = new 
> IndexSearcher(INDEX_FILE_PATH);
>             final Analyzer analyzer = new StandardAnalyzer();
>             final Query q = QueryParser.parse(query, "contents", 
> analyzer);
>             final Hits hits = searcher.search(q);
>             numHits = hits.length();
>             while (beginIndex <= endIndex && beginIndex < numHits) {
>                 final Document doc = hits.doc(beginIndex++);
>                 collector.add(doc.get("id"));
>             }
>         } catch (java.lang.Exception e) {
>             throw new SearchService.Exception("Exception performing 
> Lucene
> search",e);
>         }
>         return numHits;
>     }
>
> So sometimes I want QueryParser.parse() to throw an exception, 
> sometimes not
> etc etc - the usual mock stuff.

I give a hearty "second" to Hani's too much mock comments.  A mock is a 
_probe_ to send in somewhere to find out how some black box operates on 
it.  In this case, your search method is not a black box at all, and 
does not take any Lucene API's as a parameter, so there is no sense 
mocking anything in this case.  As for your QueryParser comments - you 
can instantiate a QueryParser and call the *instance* parse method on 
it if that helps.

I would highly recommend you decouple what you're doing here a bit so 
that you are not reliant on the file system deep down in your code.

It appears you are wrapping Lucene.  So, if you truly want to mock 
things for testing purposes, come up with a generic interface 
(SearchManager?!) that has the search method signature that you have 
above and all the other pieces you're wrapping.  Then create a mock 
implementation of that particular interface.

Whenever you feel the desire to change an API to accommodate testing 
you really have to reevaluate the situation.  And trust me, I'm a firm 
believer (and a learning practitioner) of Test Driven Development, so 
you don't have to convince me of the benefits.  But it does take 
reversing some thinking from time to time to resist the urge to make 
everything public and open just to test it, when that compromises the 
integrity of the underlying architecture.

	Erik



---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
> >             final Hits hits = searcher.search(q);
> >             numHits = hits.length();
> >             while (beginIndex <= endIndex && beginIndex < numHits) {
> >                 final Document doc = hits.doc(beginIndex++);
> >                 collector.add(doc.get("id"));
> >             }
> 
> Another suggestion - use the HitCollector search method rather than 
> getting Hits back and then walking them.  Hits is adding a level of 
> indirection and caching that you obviously don't need here - and 
> HitCollector would streamline your code a bit here.

Cheers Erik, much appreciated.

---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On Sunday, October 5, 2003, at 10:02  AM, Mike Hogan wrote:
> I am trying to unit test this:
>
>     public int search(final String query, int beginIndex, final int
> endIndex, final List collector) throws SearchService.Exception {
>         int numHits = 0;
>         try {
>             final Searcher searcher = new 
> IndexSearcher(INDEX_FILE_PATH);
>             final Analyzer analyzer = new StandardAnalyzer();
>             final Query q = QueryParser.parse(query, "contents", 
> analyzer);
>             final Hits hits = searcher.search(q);
>             numHits = hits.length();
>             while (beginIndex <= endIndex && beginIndex < numHits) {
>                 final Document doc = hits.doc(beginIndex++);
>                 collector.add(doc.get("id"));
>             }

Another suggestion - use the HitCollector search method rather than 
getting Hits back and then walking them.  Hits is adding a level of 
indirection and caching that you obviously don't need here - and 
HitCollector would streamline your code a bit here.

	Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik,

> The static parse method is simply for convenience.... it does nothing
> that you couldn't do by instantiating an instance of QueryParser and
> calling the instance 'parse' method.  In fact, its more flexible to use
> QueryParser as an instance than through the static method.
>
> So, you already have what you're requesting with QueryParser.
>
> What exactly are you unit testing and how?  I have a feeling if you
> give us more details of what you're trying to accomplish we could help.

I am trying to unit test this:

    public int search(final String query, int beginIndex, final int
endIndex, final List collector) throws SearchService.Exception {
        int numHits = 0;
        try {
            final Searcher searcher = new IndexSearcher(INDEX_FILE_PATH);
            final Analyzer analyzer = new StandardAnalyzer();
            final Query q = QueryParser.parse(query, "contents", analyzer);
            final Hits hits = searcher.search(q);
            numHits = hits.length();
            while (beginIndex <= endIndex && beginIndex < numHits) {
                final Document doc = hits.doc(beginIndex++);
                collector.add(doc.get("id"));
            }
        } catch (java.lang.Exception e) {
            throw new SearchService.Exception("Exception performing Lucene
search",e);
        }
        return numHits;
    }

So sometimes I want QueryParser.parse() to throw an exception, sometimes not
etc etc - the usual mock stuff.

Cheers,
Mike


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
The static parse method is simply for convenience.... it does nothing 
that you couldn't do by instantiating an instance of QueryParser and 
calling the instance 'parse' method.  In fact, its more flexible to use 
QueryParser as an instance than through the static method.

So, you already have what you're requesting with QueryParser.

What exactly are you unit testing and how?  I have a feeling if you 
give us more details of what you're trying to accomplish we could help.

	Erik


On Sunday, October 5, 2003, at 05:13  AM, Mike Hogan wrote:

> QueryParser.parse also makes it hard for me to unit test.  Would it be
> possible to make this a non-static method?
>
> Thanks again,
> Mike.
>
> ----- Original Message -----
> From: "Mike Hogan" <me...@mikehogan.net>
> To: <lu...@jakarta.apache.org>
> Sent: Sunday, October 05, 2003 11:09 AM
> Subject: [subscriptions] Please make 
> org.apache.lucene.index.IndexWriter
> non-final
>
>
>> Hi,
>>
>> Please can you make org.apache.lucene.index.IndexWriter non-final.  
>> It's
>> being final makes it impossible for me to test using mock objects.  
>> Also,
> if
>> there are other public final classes that are significant to the 
>> Lucene
> API,
>> please make them non-final also, for the same reason.
>>
>> One more thing, I see that there are cvs commits on the Lucene 
>> project,
> but
>> no releases in a long while.  What is the status with repect to new
>> releases?  (In other words, if you do go ahead and make these classes
>> non-final, when can I expect to be able to make use of the changes 
>> :)).
>>
>> Thanks,
>> Mike.
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
QueryParser.parse also makes it hard for me to unit test.  Would it be
possible to make this a non-static method?

Thanks again,
Mike.

----- Original Message -----
From: "Mike Hogan" <me...@mikehogan.net>
To: <lu...@jakarta.apache.org>
Sent: Sunday, October 05, 2003 11:09 AM
Subject: [subscriptions] Please make org.apache.lucene.index.IndexWriter
non-final


> Hi,
>
> Please can you make org.apache.lucene.index.IndexWriter non-final.  It's
> being final makes it impossible for me to test using mock objects.  Also,
if
> there are other public final classes that are significant to the Lucene
API,
> please make them non-final also, for the same reason.
>
> One more thing, I see that there are cvs commits on the Lucene project,
but
> no releases in a long while.  What is the status with repect to new
> releases?  (In other words, if you do go ahead and make these classes
> non-final, when can I expect to be able to make use of the changes :)).
>
> Thanks,
> Mike.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On Sunday, October 5, 2003, at 11:01  AM, Mike Hogan wrote:
> This is true, but I enter into the relationship with mocks with my eyes
> open.  If I was trying to mock-out some really complex API, with a lot 
> of
> states etc, then your criticism would be justified.  But here I am 
> trying to
> mock-out a few simple API calls, which may grow in the future, yes, 
> but not
> hugely.  Personally I am happen with the risk in this case.  If only 
> the
> classes I am trying to mock-out where non-final.....:)

I think your design is the one we need to call into question here.  If 
you have a few simple API calls (your own, that is, the search one you 
showed before), then your design should accommodate the mock situation 
you're grasping for here.  Again, how about a SearchManager interface, 
and then a LuceneSearchManager implementation, then a MockSearchManager 
interface that you can send into your higher level methods to see what 
happens without actually calling Lucene at all?  Or perhaps a 
DirectoryFactory that is used to construct either a RAMDirectory (for 
testing) or FSDirectory (for production)?

>> away with the bare minimum of functionality. I've yet to come across a
>> mock API that actually works as well as the real thing.
>
> Thats the point - they are not supposed to work at all, never mind as 
> well
> as the real thing.  But I know you know that.

I disagree here.  A mock *is* supposed to work.  Its the Real Deal, but 
with some reporting capabilities added on so you can see what happens 
to it as it goes through a black box.  We need to know precisely what 
we are testing here to really say whether a particular use of mocks is 
bad or good, but I'm sitting between Mike and Hani here.... mocks are 
great, but must be focused on one layer to test, not more.  Mocks can 
be abused, and this I feel is Mike's situation.  Mike.... refactor and 
redesign - don't try to change Lucene to make your testing easier - 
what you're after can be done without touching Lucene.  Besides, we 
won't let you touch Lucene that way anyway :))

	Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Hani,

> No matter how good your mock is, it won't be as good as a lucene
> provided and maintained implementation (RAMDirectory). Mocks in fact
> are dangerous for that very reason, they seek to emulate behaviour, and
> give you a false sense of security in the correctness of the mock.

This is true, but I enter into the relationship with mocks with my eyes
open.  If I was trying to mock-out some really complex API, with a lot of
states etc, then your criticism would be justified.  But here I am trying to
mock-out a few simple API calls, which may grow in the future, yes, but not
hugely.  Personally I am happen with the risk in this case.  If only the
classes I am trying to mock-out where non-final.....:)

> The same argument is extended to JDBC usage. I much prefer to have my
> unit tests call mckoi or some other java db that can be run inline with
> an in-memory store. it's a real world db, rather than an attempt to get
> away with the bare minimum of functionality. I've yet to come across a
> mock API that actually works as well as the real thing.

Thats the point - they are not supposed to work at all, never mind as well
as the real thing.  But I know you know that.  I agree that there is a lot
more room for error and a false sense of security when mock-testing JDBC
code.  I have been burned by that myself.  Like so much in this great
science of ours, it comes down to tradeoffs.  In the case of JDBC, its
complex enough to require Real Deal testing and mock-testing.  In this
simple case, I find mock-testing is sufficient.

Cheers,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Hani Suleiman <ha...@formicary.net>.
On Sunday, October 5, 2003, at 02:28 PM, Erik Hatcher wrote:

> I'm curious what your take of the two mocks I've recently added to 
> Lucene's codebase is - MockFilter and MockInputStream.  Both of these 
> are for sending in to somewhere else and probing what happens to them. 
>  How else could a Real Deal (tm) work in these scenarios?  And, I 
> would argue that these two are the real deals since they merely are 
> implementations (although certainly minimal and exploratory) of the 
> real interfaces of Lucene.
>
I looked at both of these and I think that in this case neither of them 
seems an 'abuse' of mocking. If anything, this is the exact usage of 
what a mock should be, whiteboxing a blackbox scenario, where you can 
interact with it, then poke about and verify the results of that 
interaction.

I don't know if you're used dynamic mocks or the big fat library of 
mocks in the mockobjects-java project, but THAT stuff to me is a pretty 
blatant abuse. Full (useless)  mocks of all of j2ee, where you're 
endlessly forced to define expectations and results for every call. 
Yuck!

Hani


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On Sunday, October 5, 2003, at 10:50  AM, Hani Suleiman wrote:
> To be honest (and yes, this is becoming a bit OT so hit delete if 
> you're so inclined)...

We'll indulge ourselves a bit more here before we let this thread go :)

> The same argument is extended to JDBC usage. I much prefer to have my 
> unit tests call mckoi or some other java db that can be run inline 
> with an in-memory store. it's a real world db, rather than an attempt 
> to get away with the bare minimum of functionality. I've yet to come 
> across a mock API that actually works as well as the real thing. To do 
> that you often have to fill in all sorts of bits by hand, thus 
> introducing the headache of yet more code to keep up to date and 
> maintain, when you could have just used the Real Deal (tm) in the 
> first place.

I think it really boils down to what you're trying to test.  We all 
need to know precisely what we're trying to test because we want to 
focus a test precisely on a small "unit".  With JDBC - if you're 
testing how your code interacts with the JDBC layer itself, it makes 
sense to use a mock to avoid testing the actual implementation of the 
driver as well and blurring what is being tested and what the results 
may mean.

I'm curious what your take of the two mocks I've recently added to 
Lucene's codebase is - MockFilter and MockInputStream.  Both of these 
are for sending in to somewhere else and probing what happens to them.  
How else could a Real Deal (tm) work in these scenarios?  And, I would 
argue that these two are the real deals since they merely are 
implementations (although certainly minimal and exploratory) of the 
real interfaces of Lucene.

> Hmmm, time to blog ;)

I agree!

	Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Hani Suleiman <ha...@formicary.net>.
To be honest (and yes, this is becoming a bit OT so hit delete if  
you're so inclined)...

I think that this shows an obsession with mocks for the sake of mocks,  
rather than any real benefit.

No matter how good your mock is, it won't be as good as a lucene  
provided and maintained implementation (RAMDirectory). Mocks in fact  
are dangerous for that very reason, they seek to emulate behaviour, and  
give you a false sense of security in the correctness of the mock.

The same argument is extended to JDBC usage. I much prefer to have my  
unit tests call mckoi or some other java db that can be run inline with  
an in-memory store. it's a real world db, rather than an attempt to get  
away with the bare minimum of functionality. I've yet to come across a  
mock API that actually works as well as the real thing. To do that you  
often have to fill in all sorts of bits by hand, thus introducing the  
headache of yet more code to keep up to date and maintain, when you  
could have just used the Real Deal (tm) in the first place.

Hmmm, time to blog ;)

Hani

On Sunday, October 5, 2003, at 10:39 AM, Mike Hogan wrote:

> Erik,
>
>> Looks pretty much like you're testing Lucene here, not your  
>> application
>> around it.... nothing inside the try block is your own stuff its just
>> Lucene API calls.
>
> No, I am not testing Lucene.  I have written code that uses Lucene,  
> yes, but
> I want to _divorce_ myself from Lucene when it comes to unit testing  
> that
> code.  I just want to make sure I call the Lucene API correctly.  Not  
> by
> actually calling it in actuality, but by calling mock equivalents and
> verify()'ing that I got the correct sequence of calls.  I do not want  
> to
> have to worry about Lucene or the file system or a RAMDirectory.  This  
> is
> what mock object testing is all about, as I understand it.  Its the  
> same
> when unit testing code that does a bunch of JDBC calls.  I do not  
> actually
> want to call the real JDBC driver, I want to call a mock version and
> verify() that the parameters to my PreparedStatement and that my  
> connections
> were closed etc etc.  Involving the JDBC driver for real is an  
> integration
> test, not a unit test.
>
>>> What ya reckon?
>>
>> I reckon ya oughta have a look at Lucene's test cases and source code.
>> Ever hear of RAMDirectory?!  :))
>
> No, I have not until now :-)  I will take a look at the Lucene test  
> cases,
> but if you are suggesting that I should slot in a file system  
> replacement to
> test my application, you're asking me to take the harder of two roads.
> Doing this makes sense for those testing Lucene itself, but not for me  
> (in
> the same way as using a database replacement makes sense for those  
> testing
> JDBC drivers).  As I said, I am concerned only with verifying that I  
> called
> the Lucene API correctly, not that Lucene does what its supposed to do  
> - I
> will test that separately.
>
> One final thing: what is the rationale behind using final at all on a  
> class?
> I come from this line of thinking:
> http://lists.codehaus.org/pipermail/picocontainer-dev/2003-July/ 
> 000743.html,
> so am interested to see what value you get from final.
>
> Cheers,
> Mike.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik,

> Again, a RAMDirectory would allow you to verify that you called the
> Lucene API correctly.  The code you showed, though, was locked into
> using a file system index making it inflexible and tougher to test.
> Your test case could construct a RAMDirectory with some know documents
> indexed, then call your search method and assert it got the right ones
> back.  Yes, this impacts your design - no question.  But this is a
> benefit of unit test driven development - you'll end up with a better
> design *because* you've refactored during testing.

A few posts back I pasted the refactored design I had when I tried to
mock-test my code.  You are right to say my code needs refactoring to unit
test.  I just rolled it back to the untestable state once I ran into the
public final IndexWriter problem.

> I would even go so far as to argue that RAMDirectory is a mock object
> (albeit a very sophisticated one) of sorts.

Yes it is.  But its too sophisticated for what I need.  I can do the same
with a small test case and two small mocks.  My preference only.  Hence this
email thread.

> I think anyone arguing that final is bad because it makes unit testing
> more difficult is not quite seeing the big picture.  'final' is a good
> thing when used appropriately.  And it does not make testing any harder
> when using 'final' the right decision for your design.  In the case of
> Lucene, protection access was very well thought out and it is by
> design.  It is always up for debate, but making something easier to
> test is not going to be a sufficient enough reason to change it.  Is
> there a real issue with Lucene being inflexible for extension in spots?
>   History has proven it and things have been opened up in the past
> couple of releases of Lucene, but there real use cases that demanded
> this, not just testing.

Well, this is a side issue and I probably should not have started it, but
I've started so I might as well continue :-).  What is the benefit of final
on a class?  Are you concerned that somebody will subclass an IndexWriter
and shag things up?  If so, shag _them_!

Cheers,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On Sunday, October 5, 2003, at 10:39  AM, Mike Hogan wrote:
> JDBC drivers).  As I said, I am concerned only with verifying that I  
> called
> the Lucene API correctly, not that Lucene does what its supposed to do  
> - I
> will test that separately.

Again, a RAMDirectory would allow you to verify that you called the  
Lucene API correctly.  The code you showed, though, was locked into  
using a file system index making it inflexible and tougher to test.   
Your test case could construct a RAMDirectory with some know documents  
indexed, then call your search method and assert it got the right ones  
back.  Yes, this impacts your design - no question.  But this is a  
benefit of unit test driven development - you'll end up with a better  
design *because* you've refactored during testing.

I would even go so far as to argue that RAMDirectory is a mock object  
(albeit a very sophisticated one) of sorts.

> One final thing: what is the rationale behind using final at all on a  
> class?
> I come from this line of thinking:
> http://lists.codehaus.org/pipermail/picocontainer-dev/2003-July/ 
> 000743.html,
> so am interested to see what value you get from final.

I think anyone arguing that final is bad because it makes unit testing  
more difficult is not quite seeing the big picture.  'final' is a good  
thing when used appropriately.  And it does not make testing any harder  
when using 'final' the right decision for your design.  In the case of  
Lucene, protection access was very well thought out and it is by  
design.  It is always up for debate, but making something easier to  
test is not going to be a sufficient enough reason to change it.  Is  
there a real issue with Lucene being inflexible for extension in spots?  
  History has proven it and things have been opened up in the past  
couple of releases of Lucene, but there real use cases that demanded  
this, not just testing.

	Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik,

> Ummm.... but.... you must be using Lucene 1.2 as IndexWriter was made
> non-final over a year ago.

Yes, I am using 1.2.   1.3 is only at RC1 as far as I could see, certainly
this indicates so: http://jakarta.apache.org/site/binindex.cgi.  What
version would you advise me to use?

> So, now that you ultimately have what you asked for, could you do us a
> favor and post back how you plan on using this, and how it will impact
> the design of your production code?  I really am curious how a
> MockIndexWriter will be written to really benefit what you're after.

I posted refactored code a few posts back.  But basically I delegated the
creation of Lucene classes to an interface, of which there are two impls:
the production one and the mock one.

Cheers,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
Ummm.... but.... you must be using Lucene 1.2 as IndexWriter was made 
non-final over a year ago.

So, now that you ultimately have what you asked for, could you do us a 
favor and post back how you plan on using this, and how it will impact 
the design of your production code?  I really am curious how a 
MockIndexWriter will be written to really benefit what you're after.

	Erik


---- from cvs log....
revision 1.7
date: 2002/07/17 17:38:04;  author: cutting;  state: Exp;  lines: +7 -7
Made many methods and classes non-final, per requests.  This includes
IndexWriter and IndexSearcher, among others.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik,

> Looks pretty much like you're testing Lucene here, not your application
> around it.... nothing inside the try block is your own stuff its just
> Lucene API calls.

No, I am not testing Lucene.  I have written code that uses Lucene, yes, but
I want to _divorce_ myself from Lucene when it comes to unit testing that
code.  I just want to make sure I call the Lucene API correctly.  Not by
actually calling it in actuality, but by calling mock equivalents and
verify()'ing that I got the correct sequence of calls.  I do not want to
have to worry about Lucene or the file system or a RAMDirectory.  This is
what mock object testing is all about, as I understand it.  Its the same
when unit testing code that does a bunch of JDBC calls.  I do not actually
want to call the real JDBC driver, I want to call a mock version and
verify() that the parameters to my PreparedStatement and that my connections
were closed etc etc.  Involving the JDBC driver for real is an integration
test, not a unit test.

> > What ya reckon?
>
> I reckon ya oughta have a look at Lucene's test cases and source code.
> Ever hear of RAMDirectory?!  :))

No, I have not until now :-)  I will take a look at the Lucene test cases,
but if you are suggesting that I should slot in a file system replacement to
test my application, you're asking me to take the harder of two roads.
Doing this makes sense for those testing Lucene itself, but not for me (in
the same way as using a database replacement makes sense for those testing
JDBC drivers).  As I said, I am concerned only with verifying that I called
the Lucene API correctly, not that Lucene does what its supposed to do - I
will test that separately.

One final thing: what is the rationale behind using final at all on a class?
I come from this line of thinking:
http://lists.codehaus.org/pipermail/picocontainer-dev/2003-July/000743.html,
so am interested to see what value you get from final.

Cheers,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On Sunday, October 5, 2003, at 09:56  AM, Mike Hogan wrote:
> I am trying to test this:
>
> public void index(String componentId, String componentDescription) 
> throws
> SearchService.Exception {
>         IndexWriter writer = null;
>         try {
>             writer = new IndexWriter(INDEX_FILE_PATH, new
> StandardAnalyzer(), !indexExists());
>             final Document document = new Document();
>             document.add(Field.Text("id", componentId));
>             document.add(Field.Text("contents", componentDescription));
>             writer.addDocument(document);
>             writer.optimize();
>             writer.close();
>         } catch (IOException e) {
>             throw new SearchService.Exception("Exception updating 
> Lucene
> index", e);
>         }
>     }

Looks pretty much like you're testing Lucene here, not your application 
around it.... nothing inside the try block is your own stuff its just 
Lucene API calls.

>
> What ya reckon?

I reckon ya oughta have a look at Lucene's test cases and source code.  
Ever hear of RAMDirectory?!  :))

	Erik



---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: [subscriptions] Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Mike Hogan <me...@mikehogan.net>.
Erik,

> Mike,
>
> I disagree with your request to change things just for testing
> purposes.  There are ways to test most (if not all) of Lucene without
> having to affect method/class access levels.  As big as I am into
> testing, I'm also big into using the right design for the right job.
> In the case of Lucene, 'final' is used liberally as well as other
> non-public access levels.
>
> Could you give us an example of what you're trying to test and how
> you're wishing you could go about it so that we could perhaps offer
> alternatives?

I am trying to test this:

public void index(String componentId, String componentDescription) throws
SearchService.Exception {
        IndexWriter writer = null;
        try {
            writer = new IndexWriter(INDEX_FILE_PATH, new
StandardAnalyzer(), !indexExists());
            final Document document = new Document();
            document.add(Field.Text("id", componentId));
            document.add(Field.Text("contents", componentDescription));
            writer.addDocument(document);
            writer.optimize();
            writer.close();
        } catch (IOException e) {
            throw new SearchService.Exception("Exception updating Lucene
index", e);
        }
    }

I want to do it without a dependency on the file system.  So I want a mock
IndexWriter that does what I configure it to, sometimes throwing an
IOException, some times not, and always storing the Document passed to it,
so I can verify() the document is as it should be.

> Look at Lucene's current codebase.  I've added a couple of mock objects
> recently to test various things - maybe that could give you some ideas?

I am not trying to unit test Lucene.  I am trying to unit test an
application of Lucene.

What ya reckon?

Thanks,
Mike.


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Re: Please make org.apache.lucene.index.IndexWriter non-final

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
Mike,

I disagree with your request to change things just for testing 
purposes.  There are ways to test most (if not all) of Lucene without 
having to affect method/class access levels.  As big as I am into 
testing, I'm also big into using the right design for the right job.  
In the case of Lucene, 'final' is used liberally as well as other 
non-public access levels.

Could you give us an example of what you're trying to test and how 
you're wishing you could go about it so that we could perhaps offer 
alternatives?

Look at Lucene's current codebase.  I've added a couple of mock objects 
recently to test various things - maybe that could give you some ideas?

	Erik


On Sunday, October 5, 2003, at 05:09  AM, Mike Hogan wrote:

> Hi,
>
> Please can you make org.apache.lucene.index.IndexWriter non-final.  
> It's
> being final makes it impossible for me to test using mock objects.  
> Also, if
> there are other public final classes that are significant to the 
> Lucene API,
> please make them non-final also, for the same reason.
>
> One more thing, I see that there are cvs commits on the Lucene 
> project, but
> no releases in a long while.  What is the status with repect to new
> releases?  (In other words, if you do go ahead and make these classes
> non-final, when can I expect to be able to make use of the changes :)).
>
> Thanks,
> Mike.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org