qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rafael Schloming <rafa...@redhat.com>
Subject Re: svn commit: r791954 - /qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/FailoverBaseCase.java
Date Wed, 08 Jul 2009 11:57:38 GMT
Martin Ritchie wrote:
> 2009/7/8 Rafael Schloming <rafaels@redhat.com>:
>> Martin Ritchie wrote:
>>> 2009/7/8 Rafael Schloming <rafaels@redhat.com>:
>>>> Martin Ritchie wrote:
>>>>> 2009/7/8 Rafael Schloming <rafaels@redhat.com>:
>>>>>> Aidan Skinner wrote:
>>>>>>> On Tue, Jul 7, 2009 at 8:45 PM, <rhs@apache.org> wrote:
>>>>>>>
>>>>>>>> Author: rhs
>>>>>>>> Date: Tue Jul  7 19:45:04 2009
>>>>>>>> New Revision: 791954
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=791954&view=rev
>>>>>>>> Log:
>>>>>>>> Reverted 787626 as it was causing the regular profiles to
fail.
>>>>>>> Uh, it is? They passed fine for me. What problem were you seeing?
>>>>>> The cleanBroker() ends up deleting the data directory out from
>>>>>> underneath
>>>>>> the running broker causing it to die.
>>>>>>
>>>>>> I suspect the problem you're seeing with QPID-1935 is actually a
real
>>>>>> broker
>>>>>> bug where it's failing to update the persistent store when messages
are
>>>>>> dequeued causing one test to interfere with the next.
>>>>>>
>>>>>> --Rafael
>>>>> If the cleanBoker/delete was done before the super.setUp() call then
>>>>> the broker would not be running so it should't be a problem.
>>>> Possibly, but cleanBroker() isn't really meant to be used that way, and
>>>> it
>>>> invalidates the test anyways. You really shouldn't have to clean for the
>>>> test to pass. As I said, I'm pretty sure if the test is failing when
>>>> persistence is enabled it's pointing to an actual bug.
>>> I don't recall how the cpp broker tests run but the transient java
>>> tests have a fresh broker for each run. So if I put messages on the
>>> queue and test they are they as part of the test then stop the broker
>>> the messages go away. If use persistence then they won't. So the fact
>>> that most of our tests use the queue 'queue' will result in previous
>>> tests affecting the current test.
>> The tests clean up after themselves, so if they run correctly, you shouldn't
>> get messages spilling over from one test to the next.
> 
> How do the test clean themselves up? on tearDown all that is done is
> the closing of connections. I do recall adding a check in QpidTestCase
> last year that checked the queue depth of the queue at the end of the
> test. We had to remove it as there were to many tests that did not
> ensure the queue was empty at the end of the run.

They don't clean up in tearDown, but most tests end up consuming any 
messages they publish when they run successfully. I think some tests 
that don't do this are excluded from the persistent profiles.

>>  Running clean between each test case
>>> should not cause a failure in the broker as the broker is stopped and
>>> started between each test. As long as the order is stop, clean ,start
>>> then there should be no problem.
>> Running clean properly between each test shouldn't cause a failure, however
>> it will cause bugs like QPID-1917 to go undetected.
> 
> I don't think that we should rely on side-effects such as this for
> detecting bugs. If we had a series of persistent messaging tests, such
> as one that verified consumed messages are not recovered. Then we
> would have found 1917. We haven't previously tested this because when
> we started Qpid Java broker didn't have an ASL compatible store. Now
> we do we should start increasing our test coverage to include that.

I agree having a series of tests targeted at persistence is a good idea, 
however I don't see this as a side-effect. Any series of tests that run 
against a non persistent broker and consume what they produce should 
also succeed against a persistent broker.

In fact, I think those tests that rely on the test harness to do their 
cleanup for them by bouncing the broker are really the ones relying on a 
"side effect" of the test harness. It's really not that hard for a test 
to clean up after itself, and it has some significant benefits since it 
becomes feasible to run the test against an external broker.

IMHO the tests should whenever possible assume that they are running 
against a broker that doesn't get shutdown between individual tests or 
overall test runs, and we should really only use the broker start/stop 
stuff for tests that actually need it, e.g. tests specifically geared 
towards failover, transactions, and persistence.

>> Also, cleanBroker() doesn't really distinguish which broker it's running
>> for, it simply wipes out all the data directories, and so it should never be
>> called when there is a running broker.
> 
> Agreed, Though I think cleanBroker() needs to be improved as it would
> be good to have the ablity to delete a data set from one of the
> non-running brokers (Meaning we have other brokers running in the
> test). I was thinking of a cluster test scenario where you stop broker
> B and need to ensure it has no current data before you start it up
> again and veify that it regains the cluster state. Not that we have
> clustering in the Java code yet, but given that we can stop and start
> any number of brokers via our tests we should think about how we can
> clear a broker's data directories.

I do agree it would be nice to have a base test case that provides tests 
with explicit control over starting, stoping, and cleaning brokers. The 
current cleanBroker() really wasn't intended to be used outside of the 
way it currently is inside of QpidTestCase. (In fact it should probably 
be private.)

--Rafael


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Mime
View raw message