Re: svn commit: r1357027 - /lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGe t.java

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1357027 - /lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGe t.java

Chris Hostetter-3

Yonik: would make sense to at least trigger this in the nightly?...

final int deleteByQueryPercent = TEST_NIGHTLY ? (1+random().nextInt(7)) : 0;

: tests: disable stress DBQ reorder
:
: Modified:
:     lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java
:
: Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java
: URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java?rev=1357027&r1=1357026&r2=1357027&view=diff
: ==============================================================================
: --- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java (original)
: +++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java Wed Jul  4 00:54:53 2012
: @@ -1014,7 +1014,7 @@ public class TestRealTimeGet extends Sol
:      final int commitPercent = 5 + random().nextInt(20);
:      final int softCommitPercent = 30+random().nextInt(75); // what percent of the commits are soft
:      final int deletePercent = 4+random().nextInt(25);
: -    final int deleteByQueryPercent = 1+random().nextInt(7);
: +    final int deleteByQueryPercent = 0; // 1+random().nextInt(7);
:      final int ndocs = 5 + (random().nextBoolean() ? random().nextInt(25) : random().nextInt(200));
:      int nWriteThreads = 5 + random().nextInt(25);


-Hoss

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

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1357027 - /lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGe t.java

Michael McCandless-2
On Tue, Jul 3, 2012 at 10:40 PM, Chris Hostetter
<[hidden email]> wrote:
>
> Yonik: would make sense to at least trigger this in the nightly?...
>
> final int deleteByQueryPercent = TEST_NIGHTLY ? (1+random().nextInt(7)) : 0;

If the intention is to reduce time required for the test ... I would
rather reduce iterations / stop time / etc, than reduce test coverage
like this (by disabling testing delete-by-query).

But maybe the intention was to temporarily avoid failures because
there were known bugs in distributed delete-by-query that yonik is
working on (SOLR-3559?), and he plans to re-enable once they are
fixed?

Or maybe some other unexpected reason?

In any event when reducing test coverage suddenly like this, Yonik,
please do a more thorough job explaining why.

Mike McCandless

http://blog.mikemccandless.com

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

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1357027 - /lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGe t.java

Yonik Seeley-2-2
I was working on SOLR-3559... I thought I had it nailed, so I
committed it (and I couldn't update the JIRA issue because JIRA was
down), along with
enabling DBQ in the stress reorder versions test.

That test started hitting errors on many of our jenkins boxes, so I
reverted the test change while I worked on it more.  I've since made
progress and re-enabled it.

> In any event when reducing test coverage suddenly like this, Yonik,
> please do a more thorough job explaining why.

That's a funny way of putting it.  I enabled the DBQ reordering, got
failures, then disabled it again with the commit log message of
"tests: disable stress DBQ reorder".

-Yonik
http://lucidimagination.com

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

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1357027 - /lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestRealTimeGe t.java

Michael McCandless-2
On Wed, Jul 4, 2012 at 3:00 PM, Yonik Seeley <[hidden email]> wrote:

> I was working on SOLR-3559... I thought I had it nailed, so I
> committed it (and I couldn't update the JIRA issue because JIRA was
> down), along with
> enabling DBQ in the stress reorder versions test.

That's great!

> That test started hitting errors on many of our jenkins boxes, so I
> reverted the test change while I worked on it more.

Right: it's that revert that could have used a better commit message
so that on quick glance other committers could understand the context
of the change, especially if the change is one that reduces test
coverage.

Something simple like "SOLR-3559: revert new test until this is fixed"
or something like that.

> I've since made
> progress and re-enabled it.

Excellent.  I figured something like this was the reason for the commit.

>> In any event when reducing test coverage suddenly like this, Yonik,
>> please do a more thorough job explaining why.
>
> That's a funny way of putting it.  I enabled the DBQ reordering, got
> failures, then disabled it again with the commit log message of
> "tests: disable stress DBQ reorder".

Right, but, I didn't remember/see/realize that you had just recently
added this to the test.

Out of context it suddenly looks like we are losing test coverage and
the reason isn't immediately clear.

Net/net we should all try to over-communicate in commit messages
(include the issue number, explain why a revert is happening, etc.).
We are a team here.

Anyway I think these set of tests (spawned out of TestRealTimeGet) are awesome.

Mike McCandless

http://blog.mikemccandless.com

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