You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by Clebert Suconic <cl...@gmail.com> on 2019/09/11 13:40:39 UTC

[DISCUSS/REQUEST] AssertNull after receive messages (valid for both ActiveMQ and Artemis)

I would like to request everybody to not do the following pattern on
tests. That means also look for this pattern on other people's pull
request (including mine.. I'm writing to myself also here).


Assert.assertNull(consumer.receive(ANY-TIMEOUT));


Instead, please use consumer.receiveNoWait();


some tests taking 2 seconds on a loop for each time the method is
called, being executed several time can amount for a few extra minutes
on the testsuite.


Even though I have seen this pattern in Artemis, this same request is
valid for ActiveMQ.


Thank you!


-- 
Clebert Suconic

Re: [DISCUSS/REQUEST] AssertNull after receive messages (valid for both ActiveMQ and Artemis)

Posted by Clebert Suconic <cl...@gmail.com>.
Right: I don't rule out using receive(timeout) and assertNull for a real test...

But so far in my 10 years working in messaging, I still haven't seen a
case where assertNull(receiveNoWait) wasn't just fine.

At least, please avoid it as much as you can

On Wed, Sep 11, 2019 at 10:33 AM Robbie Gemmell
<ro...@gmail.com> wrote:
>
> The Qpid JMS client will round trip to the server on any receive or
> receiveNoWait calls where it doesnt have a message available locally
> in time (unless it has been configured not to do so).
>
> On Wed, 11 Sep 2019 at 15:21, Clebert Suconic <cl...@gmail.com> wrote:
> >
> > Even if that's the case.. It would eventually fail.
> >
> > As far as I know, AMQP client is sending a disposition to the server.
> >
> > At one point in Artemis we would just catch from the current local cache.
> >
> > It is the user's expectation to perform the roundtrip, even though
> > there's nothing specified on the JMS API.
> >
> > Users would be complaining to qpid-jms if that was the case.
> >
> >
> > There's no reason really to do Assert.assertNull(receive(5000));
> >
> > On Wed, Sep 11, 2019 at 9:45 AM Jiri Daněk <jd...@redhat.com> wrote:
> > >
> > > On Wed, Sep 11, 2019 at 3:40 PM Clebert Suconic <cl...@gmail.com>
> > > wrote:
> > >
> > > > Instead, please use consumer.receiveNoWait();
> > > >
> > >
> > > Depending on which JMS client is used (artemis-core-jms,
> > > activemq-jms-client, qpid-jms-client), receiveNoWait either does a
> > > roundtrip to server to check for new messages, or it does not. This
> > > difference can be important for the test.
> > >
> > > c.f. https://github.com/eclipse-ee4j/jms-api/issues/85
> > > --
> > > Mit freundlichen Grüßen / Kind regards
> > > Jiri Daněk
> >
> >
> >
> > --
> > Clebert Suconic



-- 
Clebert Suconic

Re: [DISCUSS/REQUEST] AssertNull after receive messages (valid for both ActiveMQ and Artemis)

Posted by Robbie Gemmell <ro...@gmail.com>.
The Qpid JMS client will round trip to the server on any receive or
receiveNoWait calls where it doesnt have a message available locally
in time (unless it has been configured not to do so).

On Wed, 11 Sep 2019 at 15:21, Clebert Suconic <cl...@gmail.com> wrote:
>
> Even if that's the case.. It would eventually fail.
>
> As far as I know, AMQP client is sending a disposition to the server.
>
> At one point in Artemis we would just catch from the current local cache.
>
> It is the user's expectation to perform the roundtrip, even though
> there's nothing specified on the JMS API.
>
> Users would be complaining to qpid-jms if that was the case.
>
>
> There's no reason really to do Assert.assertNull(receive(5000));
>
> On Wed, Sep 11, 2019 at 9:45 AM Jiri Daněk <jd...@redhat.com> wrote:
> >
> > On Wed, Sep 11, 2019 at 3:40 PM Clebert Suconic <cl...@gmail.com>
> > wrote:
> >
> > > Instead, please use consumer.receiveNoWait();
> > >
> >
> > Depending on which JMS client is used (artemis-core-jms,
> > activemq-jms-client, qpid-jms-client), receiveNoWait either does a
> > roundtrip to server to check for new messages, or it does not. This
> > difference can be important for the test.
> >
> > c.f. https://github.com/eclipse-ee4j/jms-api/issues/85
> > --
> > Mit freundlichen Grüßen / Kind regards
> > Jiri Daněk
>
>
>
> --
> Clebert Suconic

Re: [DISCUSS/REQUEST] AssertNull after receive messages (valid for both ActiveMQ and Artemis)

Posted by Clebert Suconic <cl...@gmail.com>.
Even if that's the case.. It would eventually fail.

As far as I know, AMQP client is sending a disposition to the server.

At one point in Artemis we would just catch from the current local cache.

It is the user's expectation to perform the roundtrip, even though
there's nothing specified on the JMS API.

Users would be complaining to qpid-jms if that was the case.


There's no reason really to do Assert.assertNull(receive(5000));

On Wed, Sep 11, 2019 at 9:45 AM Jiri Daněk <jd...@redhat.com> wrote:
>
> On Wed, Sep 11, 2019 at 3:40 PM Clebert Suconic <cl...@gmail.com>
> wrote:
>
> > Instead, please use consumer.receiveNoWait();
> >
>
> Depending on which JMS client is used (artemis-core-jms,
> activemq-jms-client, qpid-jms-client), receiveNoWait either does a
> roundtrip to server to check for new messages, or it does not. This
> difference can be important for the test.
>
> c.f. https://github.com/eclipse-ee4j/jms-api/issues/85
> --
> Mit freundlichen Grüßen / Kind regards
> Jiri Daněk



-- 
Clebert Suconic

Re: [DISCUSS/REQUEST] AssertNull after receive messages (valid for both ActiveMQ and Artemis)

Posted by Jiri Daněk <jd...@redhat.com>.
On Wed, Sep 11, 2019 at 3:40 PM Clebert Suconic <cl...@gmail.com>
wrote:

> Instead, please use consumer.receiveNoWait();
>

Depending on which JMS client is used (artemis-core-jms,
activemq-jms-client, qpid-jms-client), receiveNoWait either does a
roundtrip to server to check for new messages, or it does not. This
difference can be important for the test.

c.f. https://github.com/eclipse-ee4j/jms-api/issues/85
-- 
Mit freundlichen Grüßen / Kind regards
Jiri Daněk

Re: [DISCUSS/REQUEST] AssertNull after receive messages (valid for both ActiveMQ and Artemis)

Posted by Robbie Gemmell <ro...@gmail.com>.
When testing for non-reciept, I think depending on the scenario its
reasonable to use receiveNoWait, or receive(tiny timeout), or some
entirely out-of-band check such as management calls to e.g verify no
messages remain on the queue etc.

I'd agree that nothing should really ever be doing anything like
receive(2000) type calls to verify non-arrival though, except when
theres basically a bug in the test logic eg like your note of a loop
using the same check to verify delivery and then non-delivery of
messages.

Robbie

On Wed, 11 Sep 2019 at 14:40, Clebert Suconic <cl...@gmail.com> wrote:
>
> I would like to request everybody to not do the following pattern on
> tests. That means also look for this pattern on other people's pull
> request (including mine.. I'm writing to myself also here).
>
>
> Assert.assertNull(consumer.receive(ANY-TIMEOUT));
>
>
> Instead, please use consumer.receiveNoWait();
>
>
> some tests taking 2 seconds on a loop for each time the method is
> called, being executed several time can amount for a few extra minutes
> on the testsuite.
>
>
> Even though I have seen this pattern in Artemis, this same request is
> valid for ActiveMQ.
>
>
> Thank you!
>
>
> --
> Clebert Suconic