You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/08/18 12:57:25 UTC
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3697: ARTEMIS-3423 - create correct queue when durable subs recreated via AMQP
gemmellr commented on a change in pull request #3697:
URL: https://github.com/apache/activemq-artemis/pull/3697#discussion_r691184948
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSSharedDurableConsumerTest.java
##########
@@ -138,4 +140,46 @@ public void testSharedDurableConsumerWithArtemisClientAndAMQPClient() throws Exc
testSharedDurableConsumer(connection, connection2);
}
+
+ @Test(timeout = 30000)
+ public void testSharedDurableConsumerWithSelector() throws JMSException {
Review comment:
```suggestion
public void testSharedDurableConsumerWithSelectorChange() throws JMSException {
```
Clarify it doesnt test the selector behaviour itself, only the effect of changing it.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSSharedDurableConsumerTest.java
##########
@@ -138,4 +140,46 @@ public void testSharedDurableConsumerWithArtemisClientAndAMQPClient() throws Exc
testSharedDurableConsumer(connection, connection2);
}
+
+ @Test(timeout = 30000)
+ public void testSharedDurableConsumerWithSelector() throws JMSException {
+ SimpleString qName = amqpUseCoreSubscriptionNaming ? new SimpleString("SharedConsumer") : new SimpleString("SharedConsumer:global");
+ Connection connection = createConnection(true);
+ try {
+ Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+ Topic topic = session.createTopic(getTopicName());
+
+ MessageConsumer consumer = session.createSharedDurableConsumer(topic, "SharedConsumer", "a=b");
+ QueueImpl queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(-1, queue.getMaxConsumers());
+ consumer.close();
+ MessageConsumer consumer2 = session.createSharedDurableConsumer(topic, "SharedConsumer", "a=b and b=c");
+ queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(-1, queue.getMaxConsumers());
+ } finally {
+ connection.close();
+ }
+ }
+
+ @Test(timeout = 30000)
+ public void testUnSharedDurableConsumerWithSelector() throws JMSException {
Review comment:
```suggestion
public void testDurableConsumerWithSelectorChange() throws JMSException {
```
Shouldn't be in the JMSSharedDurableConsumerTest class, but rather the JMSDurableConsumerTest class. Can then go with just testDurableConsumerWithSelectorChange.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSSharedDurableConsumerTest.java
##########
@@ -138,4 +140,46 @@ public void testSharedDurableConsumerWithArtemisClientAndAMQPClient() throws Exc
testSharedDurableConsumer(connection, connection2);
}
+
+ @Test(timeout = 30000)
+ public void testSharedDurableConsumerWithSelector() throws JMSException {
+ SimpleString qName = amqpUseCoreSubscriptionNaming ? new SimpleString("SharedConsumer") : new SimpleString("SharedConsumer:global");
+ Connection connection = createConnection(true);
+ try {
+ Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+ Topic topic = session.createTopic(getTopicName());
+
+ MessageConsumer consumer = session.createSharedDurableConsumer(topic, "SharedConsumer", "a=b");
+ QueueImpl queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(-1, queue.getMaxConsumers());
+ consumer.close();
+ MessageConsumer consumer2 = session.createSharedDurableConsumer(topic, "SharedConsumer", "a=b and b=c");
+ queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(-1, queue.getMaxConsumers());
Review comment:
This only asserts the getMaxConsumers return didnt change, it would be better to verify the wider expected behaviour when the selector changed as well, i.e that the queue was deleted or cleared. As is this test would presumably still pass if we instead just removed the entire selector change handling it aims to test.
E.g assert the queue objects differ, or send a message before creating the new consumer and validate the message count changes.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSSharedDurableConsumerTest.java
##########
@@ -138,4 +140,46 @@ public void testSharedDurableConsumerWithArtemisClientAndAMQPClient() throws Exc
testSharedDurableConsumer(connection, connection2);
}
+
+ @Test(timeout = 30000)
+ public void testSharedDurableConsumerWithSelector() throws JMSException {
+ SimpleString qName = amqpUseCoreSubscriptionNaming ? new SimpleString("SharedConsumer") : new SimpleString("SharedConsumer:global");
+ Connection connection = createConnection(true);
+ try {
+ Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+ Topic topic = session.createTopic(getTopicName());
+
+ MessageConsumer consumer = session.createSharedDurableConsumer(topic, "SharedConsumer", "a=b");
+ QueueImpl queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(-1, queue.getMaxConsumers());
+ consumer.close();
+ MessageConsumer consumer2 = session.createSharedDurableConsumer(topic, "SharedConsumer", "a=b and b=c");
+ queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(-1, queue.getMaxConsumers());
+ } finally {
+ connection.close();
+ }
+ }
+
+ @Test(timeout = 30000)
+ public void testUnSharedDurableConsumerWithSelector() throws JMSException {
+ SimpleString qName = new SimpleString("foo.SharedConsumer");
+ Connection connection = createConnection("foo", true);
+ try {
+ Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+ Topic topic = session.createTopic(getTopicName());
+
+ MessageConsumer consumer = session.createDurableConsumer(topic, "SharedConsumer", "a=b", false);
+ QueueImpl queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(1, queue.getMaxConsumers());
+ consumer.close();
+ MessageConsumer consumer2 = session.createDurableConsumer(topic, "SharedConsumer", "a=b and b=c", false);
+ queue = (QueueImpl) server.getPostOffice().getBinding(qName).getBindable();
+ assertEquals(1, queue.getMaxConsumers());
Review comment:
Same comment around better verifying behaviour as in the other test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org