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
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.
> 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.
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
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.