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:
This method 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 the 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]
