gemmellr commented on code in PR #4421:
URL: https://github.com/apache/activemq-artemis/pull/4421#discussion_r1166615229
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java:
##########
@@ -124,7 +124,7 @@ public void testAutoCreateOnTopic() throws Exception {
Connection connection = factory.createConnection();
SimpleString addressName =
UUIDGenerator.getInstance().generateSimpleStringUUID();
logger.debug("Address is {}", addressName);
- clientSession.createAddress(addressName, RoutingType.ANYCAST, false);
+ clientSession.createAddress(addressName, RoutingType.MULTICAST, false);
Review Comment:
Change seems reasonable on one hand, its sending to a Topic, as per the test
name, however...this class is "QueueAutoCreationTest" and this would then be
using a pre-created Topic address....with no subscribers i.e presumably no
queues at all? Seems out of place in this class at least.
EDIT: Also, this is an AMQP test class, and the 2 updated tests (and the 3rd
similar) are only using the Core client? Also seems out of place. Looks like
the only thing the test uses AMQP for is the 'testSmallString and
testHugeString'
EDIT2: I see there is a similar test being updated in
org/apache/activemq/artemis/tests/integration/client/AutoCreateJmsDestinationTest.java
that has the exact same comment above it, down to referencing
QueueAutoCreationTest. These may be effective duplicates currently since they
both use Core?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpSecurityTest.java:
##########
@@ -120,7 +120,7 @@ public void inspectOpenedResource(Sender sender) {
try {
try {
- session.createSender(getQueueName());
+ session.createSender(getPrefixedQueueName());
Review Comment:
Perhaps similarly?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java:
##########
@@ -143,7 +143,7 @@ public void testAutoCreateOnTopicManySends() throws
Exception {
Connection connection = factory.createConnection();
SimpleString addressName =
UUIDGenerator.getInstance().generateSimpleStringUUID();
logger.debug("Address is {}", addressName);
- clientSession.createAddress(addressName, RoutingType.ANYCAST, false);
+ clientSession.createAddress(addressName, RoutingType.MULTICAST, false);
Review Comment:
Similar to prev comment
There is a 3rd similar test below with the same mismatch that you didnt
update and it might make sense to. (EDIT: or not given edits to prev comment..)
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpAnonymousRelayTest.java:
##########
@@ -285,7 +285,7 @@ public void
testSendMessageOnAnonymousRelayLinkUsingMessageTo() throws Exception
AmqpSender sender = session.createAnonymousSender();
AmqpMessage message = new AmqpMessage();
- message.setAddress(getQueueName());
+ message.setAddress(getPrefixedQueueName());
Review Comment:
This change shouldnt be needed now right, if the test pre-created the queue
as I think it does?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
