You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Erik Hatcher <er...@ehatchersolutions.com> on 2005/11/06 18:00:58 UTC

Re: svn commit: r331111 - /lucene/java/trunk/src/test/org/apache/lucene/search/TestBoolean2.java

I haven't looked at this code in detail, but it seems this resetting  
could be done in tearDown() of the test cases rather than  
complicating the tests with try/finally?

Just a JUnit FYI... tearDown is called after _each_ testXXX method,  
so if the reset is needed in the middle of a testXXX then try/finally  
makes sense (though separating into different test methods would be  
better IMO).

     Erik

On 6 Nov 2005, at 10:32, yonik@apache.org wrote:

> Author: yonik
> Date: Sun Nov  6 07:32:28 2005
> New Revision: 331111
>
> URL: http://svn.apache.org/viewcvs?rev=331111&view=rev
> Log:
> reset useScorer14 in finally in case of test failure
>
> Modified:
>     lucene/java/trunk/src/test/org/apache/lucene/search/ 
> TestBoolean2.java
>
> Modified: lucene/java/trunk/src/test/org/apache/lucene/search/ 
> TestBoolean2.java
> URL: http://svn.apache.org/viewcvs/lucene/java/trunk/src/test/org/ 
> apache/lucene/search/TestBoolean2.java? 
> rev=331111&r1=331110&r2=331111&view=diff
> ====================================================================== 
> ========
> --- lucene/java/trunk/src/test/org/apache/lucene/search/ 
> TestBoolean2.java (original)
> +++ lucene/java/trunk/src/test/org/apache/lucene/search/ 
> TestBoolean2.java Sun Nov  6 07:32:28 2005
> @@ -68,15 +68,19 @@
>    public void queriesTest(String queryText, int[] expDocNrs)  
> throws Exception {
>  //System.out.println();
>  //System.out.println("Query: " + queryText);
> -    Query query1 = makeQuery(queryText);
> -    BooleanQuery.setUseScorer14(true);
> -    Hits hits1 = searcher.search(query1);
> -
> -    Query query2 = makeQuery(queryText); // there should be no  
> need to parse again...
> -    BooleanQuery.setUseScorer14(false);
> -    Hits hits2 = searcher.search(query2);
> +    try {
> +      Query query1 = makeQuery(queryText);
> +      BooleanQuery.setUseScorer14(true);
> +      Hits hits1 = searcher.search(query1);
> +
> +      Query query2 = makeQuery(queryText); // there should be no  
> need to parse again...
> +      BooleanQuery.setUseScorer14(false);
> +      Hits hits2 = searcher.search(query2);
>
> -    CheckHits.checkHitsQuery(query2, hits1, hits2, expDocNrs);
> +      CheckHits.checkHitsQuery(query2, hits1, hits2, expDocNrs);
> +    } finally { // even when a test fails.
> +      BooleanQuery.setUseScorer14(false);
> +    }
>    }
>
>    public void testQueries01() throws Exception {
> @@ -150,25 +154,33 @@
>      String[] vals = {"w1","w2","w3","w4","w5","xx","yy","zzz"};
>
>      int tot=0;
> -    // increase number of iterations for more complete testing
> -    for (int i=0; i<1000; i++) {
> -      int level = rnd.nextInt(3);
> -      BooleanQuery q1 = randBoolQuery(new Random(i), level, field,  
> vals, null);
> -
> -      // Can't sort by relevance since floating point numbers may  
> not quite
> -      // match up.
> -      Sort sort = Sort.INDEXORDER;
>
> -      BooleanQuery.setUseScorer14(false);
> -      Hits hits1 = searcher.search(q1,sort);
> -      if (hits1.length()>0) hits1.id(hits1.length()-1);
> +    try {
>
> -      BooleanQuery.setUseScorer14(true);
> -      Hits hits2 = searcher.search(q1,sort);
> -      if (hits2.length()>0) hits2.id(hits1.length()-1);
> -      tot+=hits2.length();
> -      CheckHits.checkEqual(q1, hits1, hits2);
> +      // increase number of iterations for more complete testing
> +      for (int i=0; i<1000; i++) {
> +        int level = rnd.nextInt(3);
> +        BooleanQuery q1 = randBoolQuery(new Random(i), level,  
> field, vals, null);
> +
> +        // Can't sort by relevance since floating point numbers  
> may not quite
> +        // match up.
> +        Sort sort = Sort.INDEXORDER;
> +
> +        BooleanQuery.setUseScorer14(false);
> +        Hits hits1 = searcher.search(q1,sort);
> +        if (hits1.length()>0) hits1.id(hits1.length()-1);
> +
> +        BooleanQuery.setUseScorer14(true);
> +        Hits hits2 = searcher.search(q1,sort);
> +        if (hits2.length()>0) hits2.id(hits1.length()-1);
> +        tot+=hits2.length();
> +        CheckHits.checkEqual(q1, hits1, hits2);
> +      }
> +
> +    } finally { // even when a test fails.
> +      BooleanQuery.setUseScorer14(false);
>      }
> +
>      // System.out.println("Total hits:"+tot);
>    }
>
>
>


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


Re: svn commit: r331111 - /lucene/java/trunk/src/test/org/apache/lucene/search/TestBoolean2.java

Posted by Yonik Seeley <ys...@gmail.com>.
I think a non-static would be cleaner in this case, but people would
have to be more careful.
Isn't it true that the old (1.4) boolean scorer can deliver docs in
non-index order?  That means you can't use it as a component of
anything that requires in-order docs (the new scorer, or Paul's
proposed FilteredQuery that uses skipTo).

Other options:
 - expose the old scorer as a different query...  class BooleanQuery14
extends BooleanQuery
  (same compatibility drawbacks as above though)
 - don't expose the old scorer at all (or make access protected or
package-protected)

-Yonik
Now hiring -- http://forms.cnet.com/slink?231706

> > The actual problem is that this flag in BooleanQuery is static, so an
> > even better solution would be to make it a normal attribute of each
> > BooleanQuery object.
>
> I agree.  Statics are often more trouble than they're worth.

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


Re: svn commit: r331111 - /lucene/java/trunk/src/test/org/apache/lucene/search/TestBoolean2.java

Posted by Erik Hatcher <er...@ehatchersolutions.com>.
On 7 Nov 2005, at 03:14, Paul Elschot wrote:

> On Sunday 06 November 2005 18:00, Erik Hatcher wrote:
>
>> I haven't looked at this code in detail, but it seems this resetting
>> could be done in tearDown() of the test cases rather than
>> complicating the tests with try/finally?
>>
>> Just a JUnit FYI... tearDown is called after _each_ testXXX method,
>> so if the reset is needed in the middle of a testXXX then try/finally
>> makes sense (though separating into different test methods would be
>> better IMO).
>>
>
> Using tearDown() is indeed cleaner, but has the disadvantage that
> changing the flag is done in two different places in the code,
> so it could be overlooked by someone using the test code as
> a starting point for their own code.

Noted.


> The actual problem is that this flag in BooleanQuery is static, so an
> even better solution would be to make it a normal attribute of each
> BooleanQuery object.

I agree.  Statics are often more trouble than they're worth.

> Since the accessing methods are not intended for normal use,
> it would also be good to deprecate them now so they can
> be easily deleted later.

If these methods were added, and I believe they were, after the 1.4.3  
release, then we could just go ahead and remove them and add instance  
methods instead.  Deprecation won't be necessary if they haven't been  
in a released version.

     Erik


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


Re: svn commit: r331111 - /lucene/java/trunk/src/test/org/apache/lucene/search/TestBoolean2.java

Posted by Paul Elschot <pa...@xs4all.nl>.
On Sunday 06 November 2005 18:00, Erik Hatcher wrote:
> I haven't looked at this code in detail, but it seems this resetting  
> could be done in tearDown() of the test cases rather than  
> complicating the tests with try/finally?
> 
> Just a JUnit FYI... tearDown is called after _each_ testXXX method,  
> so if the reset is needed in the middle of a testXXX then try/finally  
> makes sense (though separating into different test methods would be  
> better IMO).

Using tearDown() is indeed cleaner, but has the disadvantage that
changing the flag is done in two different places in the code,
so it could be overlooked by someone using the test code as 
a starting point for their own code.

The actual problem is that this flag in BooleanQuery is static, so an
even better solution would be to make it a normal attribute of each 
BooleanQuery object.

Since the accessing methods are not intended for normal use,
it would also be good to deprecate them now so they can
be easily deleted later.

...
> >
> > URL: http://svn.apache.org/viewcvs?rev=331111&view=rev
> > Log:
> > reset useScorer14 in finally in case of test failure
> >
> > Modified:
> >     lucene/java/trunk/src/test/org/apache/lucene/search/ 
> > TestBoolean2.java
> >

Regards,
Paul Elschot


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