Martin Ritchie wrote:
2009/7/8 Rafael Schloming <[email protected]>:
Martin Ritchie wrote:
2009/7/8 Rafael Schloming <[email protected]>:
Martin Ritchie wrote:
2009/7/8 Rafael Schloming <[email protected]>:
Aidan Skinner wrote:
On Tue, Jul 7, 2009 at 8:45 PM, <[email protected]> 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:[email protected]
