I'll be attaching a new patch to JIRA-1844 after all the tests run, a bit
later this
evening. The two files I changed move/remove calls to
CheckUtils.check(query);

I arbitrarily limited the random query test in TestBooleanShouldMatch
to only checking the first 100, on the theory that we were at diminishing
returns after 100, whoever commits this feel free to disagree.

But a couple of process questions.

1> I almost reflexively reformat files I'm working on. Which makes diffing
     them really a pain. But the "how to contribute" page can be read as
this
     is OK as long as one is working on the file. What's preferred? Separate
     checkins for reformatting-only? And how is that accomplished
2> I cleaned up the warnings from IntelliJs inspections, I assume that's
reasonable.
3> Did some upgrading-to-java-5 modifications....

Let me know what I should do differently next time, or what needs to be
changed.

Thanks
Erick

On Thu, Oct 29, 2009 at 8:05 PM, Mark Miller <markrmil...@gmail.com> wrote:

> Erick Erickson wrote:
> > I *finally* looked at the Jira and lo! you specifically identified the
> > check method.... Someday I'll learn to look first, *then* explore.
> > Siiigghhh.
>
> No worries ;) So many half baked JIRA issues out there, it happens to
> everyone - no way around it.
> This way worked just as well I think - and sometimes coming at it fresh
> gives a different insight anyway.
> >
> > Anyway, looking at things a bit more, I'm more confident that
> > TestCustomScoreQuery wouldn't lose anything by moving the
> > query check outside of the loop. The queries don't change and
> > we're not re-querying....
> >
> > TestBooleanShouldMatch is trickier. It produces 1,000 queries
> > that have widely varying forms. I don't see a clean way to test
> > one of each "form". We could decide that, say, the first 50
> > were enough. Or we shouldn't be running check on randomly-
> > generated queries. Or if we find a query that's actually a bug
> > we should test for it explicitly. Or... But I'm not comfortable
> > changing the check in the random test without some
> > consensus.
> >
> > I'll give folks a chance to chime in between now and Monday
> > and generate a patch for one or both depending on the response.
> >
> > Erick
> >
> > On Thu, Oct 29, 2009 at 3:12 PM, Mark Miller <markrmil...@gmail.com
> > <mailto:markrmil...@gmail.com>> wrote:
> >
> >     +1 - I made a similar observation a while back and started an issue
> to
> >     address Junit test speeds:
> >
> >     https://issues.apache.org/jira/browse/LUCENE-1844
> >
> >     Erick Erickson wrote:
> >     > OK, I'm actually getting of my duff and trying to do something. And
> >     > I'm off today feeling ill, so I have a bit of time. So the rational
> >     > place to start is to get all the code and run the unit tests,
> >     and I've
> >     > even got them running in IntelliJ as well as the ant task. Will
> >     > wonders never cease.
> >     >
> >     > The unit tests in TestCustomScoreQuery take on the order of 80
> >     seconds
> >     > on my machine. Before I go off the deep end I wanted to run what
> >     I've
> >     > found past y'all to see if it makes sense.
> >     >
> >     > Virtually all the time is taken up in the method logResult on
> >     the call
> >     > to QueryUtils.check(q, s). But the logResult method is called from
> >     > verifyResults method in a loop like:
> >     >
> >     > for (Iterator it = h1.keySet().iterator(); it.hasNext();) {
> >     >       call logResult for 5 different queries.
> >     > }
> >     > But the queries don't change that I can see...
> >     >
> >     > Why couldn't we just call QueryUtils.check once for each of the 5
> >     > queries outside the for loop? Doing so reduces the time it takes
> for
> >     > the test from ~80 seconds to 9 seconds.
> >     >
> >     > If there're no objections, I'll make a patch. Which folks will
> >     > probably have to be patient with me since it'll be the first one
> >     I've
> >     > produced for this project.....
> >     >
> >     > While I'm at it, what are we thinking about using JUnit4? Looking
> >     > *briefly* at the code, this actually seems like it'd be difficult.
> >     > We'd have to deal with the LuceneTestCase superclass...
> >     >
> >     > Best
> >     > Erick
> >
> >
> >     --
> >     - Mark
> >
> >     http://www.lucidimagination.com
> >
> >
> >
> >
> >     ---------------------------------------------------------------------
> >     To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org
> >     <mailto:java-dev-unsubscr...@lucene.apache.org>
> >     For additional commands, e-mail: java-dev-h...@lucene.apache.org
> >     <mailto:java-dev-h...@lucene.apache.org>
> >
> >
>
>
> --
> - Mark
>
> http://www.lucidimagination.com
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: java-dev-h...@lucene.apache.org
>
>

Reply via email to