[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #2625: ARTEMIS-2306 Make group first off by default, unless configured

[GitBox <gi...@apache.org>]

gemmellr commented on a change in pull request #2625: ARTEMIS-2306 Make group first off by default, unless configured
URL: https://github.com/apache/activemq-artemis/pull/2625#discussion_r276181243
 
 

 ##########
 File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSMessageGroupsTest.java
 ##########
 @@ -53,6 +55,13 @@ protected String getConfiguredProtocols() {
       return "AMQP,OPENWIRE,CORE";
    }
 
+   @Override
+   protected void configureAddressPolicy(ActiveMQServer server) {
+      super.configureAddressPolicy(server);
+      AddressSettings addressSettings = server.getConfiguration().getAddressesSettings().get("#");
 
 Review comment:
   This seems like it enables the feature for all the groups tests. It would be nice to enable it only for those tests aimed at testing that functionality. Tests can/should use test specific addresses, so could it be configured only for those used by the specific tests verifying it works?
   
   A negative-test that its otherwise disabled might also be nice, there doesnt look to be one here or in the Core tests.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services