gemmellr commented on code in PR #4710:
URL: https://github.com/apache/activemq-artemis/pull/4710#discussion_r1464750566
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java:
##########
@@ -653,6 +654,10 @@ public void messageArrived(String topic, MqttMessage
message) throws Exception {
consumer.close();
}
+ private Queue getRetainedMessageQueue(String TOPIC) {
+ return
server.locateQueue(MQTTUtil.convertMqttTopicFilterToCore(MQTTUtil.MQTT_RETAIN_ADDRESS_PREFIX,
TOPIC, server.getConfiguration().getWildcardConfiguration()));
+ }
Review Comment:
The MQTTSubscriptionManager.getQueueNameForTopic method (which delegates to
this util method now) seems to have been the source of multiple breaks, or
changes in behaviour, over the last several releases. Obviously bugs happen,
but it feels like some of these would have been caught by comprehensive unit
testing of the method. The broker calls this method to name its queues, but the
system tests also call the method to get the queue name the broker used. If for
example the method returns "wrong-value" as it effectively has been doing, then
broker names things in that way, and the system tests also just use that as the
expected value, and so in those cases theyll always match and wont notice if
anything is wrong, e.g I go in there later and mess something up with the
naming.
If on the other hand there is a direct unit testing of the method
comprehensively checking the output, ensuring that the exact value expected is
returned, then the behaviour of that method is then more directly tested (and a
few ms should tell if it is later broken...vs running minutes of system tests
that mostly wont notice because they use the same method to establish what is
expected), and likely more understandable because there are now specific tests
for it showing what its expected to do, and quickly pointing out if changed
inappropriately.
--
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]
