9 Replies Latest reply on Dec 19, 2008 5:36 AM by gaohoward

    JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4

    clebert.suconic

      Howard,

      testRemoveAllReferences was added on JBM 1.4 Branch.


      //https://jira.jboss.org/jira/browse/JBMESSAGING-1460
       public void testRemoveAllReferences() throws Exception
       {
      
      ....
       assertRemainingMessages(-1); // (Why?)
      
      ....
       }
      



      Why assertRemainingMessages for -1 and not 0?

      The following test after this is failing, as setup aways check for remainingMessages = 0.


      I'm adding the same check on setUp, so it is more clear the test is failing.

      I'm reopening the JIRA as the test is failing.

        • 1. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
          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

            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

              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

                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

                  What JIRA is this change for?

                  • 6. Re: JBMESSAGING-1460 - ScheduledDeliveryTest on JBM 1.4
                    gaohoward

                    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

                      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

                        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

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