-
1. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
clebert.suconic Dec 18, 2008 5:21 PM (in response to clebert.suconic)BTW: I would fix it myself, but I was a little puzzled to why you are testing for the -1.
-
2. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
clebert.suconic Dec 18, 2008 5:59 PM (in response to clebert.suconic)deliveringCount.decrement is being called twice.
I did some debugging and deliveringCount became negative at acknowledgeInternal. (I added some System.out to debug it)protected void acknowledgeInternal(Delivery d, Transaction tx, boolean persist) throws Exception { .... if (!d.isRecovered()) { if (deliveringCount.decrement() < 0) { System.out.println("Oops... deliveringCount became negative at acknowledgeInternal"); // I added this print here } } ...... }
Why the test is asserting for -1? it seems to me the valid condition would be 0, and deliveringCount being decremented to -1 would be a bug? -
3. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
gaohoward Dec 18, 2008 10:36 PM (in response to clebert.suconic)Yes this is a bug. The 1460 fix doesn't handle the count correctly.
look at the method
public void removeAllReferences() throws Throwable
{
synchronized (lock)
{
.....//removing all non scheduled msg from memory.
deliveringCount.set(0);
}
//clear sch
clearAllScheduledDeliveries(true);
So the count will go minus. I should have put it before the
deliveringCount.set(0);
and the count should be OK.
Sorry, I'll correct it. -
4. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
timfox Dec 19, 2008 4:06 AM (in response to clebert.suconic)Howard-
Please **NEVER** hack tests to pass when there is a bug in the system (assertRemainingMessages(-1)) - this is extremely bad practice.
The test should be kept failing until the core issue is fixed.
-1 is clearly not a correct values for remaining messages.
Fudging tests is a sackable offence ;) -
5. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
timfox Dec 19, 2008 4:07 AM (in response to clebert.suconic)What JIRA is this change for?
-
6. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
gaohoward Dec 19, 2008 5:13 AM (in response to clebert.suconic)Sorry Tim, I think I was too eager to get the test passed then.
The JIRA is to fix the removeAllReferences() issue where the scheduled message not got cleared. -
7. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
gaohoward Dec 19, 2008 5:22 AM (in response to clebert.suconic)Hi Clebert, there is a test failure on my local machine:
org.jboss.test.messaging.jms.ConnectionConsumerTest.testStopWhileProcessing()
the code is here:
public void testStopWhileProcessing() throws Exception
{
if (ServerManagement.isRemote()) return;
Connection connConsumer = null;
try
{
// some test, not pasted here.
ServerManagement.stop();
connConsumer.close();
connConsumer = null;
}
finally
{
if (connConsumer != null) connConsumer.close();
}
}
The failure message is:
Server 0 has not been started!
java.lang.Exception: Server 0 has not been started!
at org.jboss.test.messaging.tools.ServerManagement.insureStarted(ServerManagement.java:1262)
at org.jboss.test.messaging.tools.ServerManagement.getAttribute(ServerManagement.java:633)
at org.jboss.test.messaging.tools.ServerManagement.getAttribute(ServerManagement.java:628)
at org.jboss.test.messaging.MessagingTestCase.checkEmpty(MessagingTestCase.java:137)
at org.jboss.test.messaging.jms.JMSTestCase.checkNotEmpty(JMSTestCase.java:101)
at org.jboss.test.messaging.jms.JMSTestCase.tearDown(JMSTestCase.java:131)
You can see in the test the
ServerManagement.stop(); //stops server 0
is called in the end. But in tearDown() it checks to ensure the server is in started state. Which will fail. Any idea? -
8. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
gaohoward Dec 19, 2008 5:24 AM (in response to clebert.suconic)Yes, I found the reason. The check is added by you to avoid message count failures today. I'll change this special test.
-
9. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
gaohoward Dec 19, 2008 5:36 AM (in response to clebert.suconic)I just move the check as follows (JMSTestCase.tearDown())
if (ServerManagement.isStarted(0))
{
if (checkNoMessageData())
{
fail("Message Data exists");
}
checkNotEmpty(); //moved from outside the block here.
}