Github user gemmellr commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2490#discussion_r246403272
  
    --- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/amq/QueueConsumerPriorityTest.java
 ---
    @@ -0,0 +1,65 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.tests.integration.openwire.amq;
    +
    +import javax.jms.JMSException;
    +import javax.jms.Message;
    +import javax.jms.MessageConsumer;
    +import javax.jms.MessageProducer;
    +import javax.jms.Session;
    +
    +
    +import 
org.apache.activemq.artemis.tests.integration.openwire.BasicOpenWireTest;
    +import org.apache.activemq.command.ActiveMQQueue;
    +import org.junit.Before;
    +import org.junit.Test;
    +
    +public class QueueConsumerPriorityTest extends BasicOpenWireTest {
    +
    +
    +   @Override
    +   @Before
    +   public void setUp() throws Exception {
    +      super.setUp();
    +      this.makeSureCoreQueueExist("QUEUE.A");
    +   }
    +   @Test
    +   public void testQueueConsumerPriority() throws JMSException, 
InterruptedException {
    +      connection.start();
    +      Session consumerLowPriority = connection.createSession(false, 
Session.AUTO_ACKNOWLEDGE);
    +      Session consumerHighPriority = connection.createSession(false, 
Session.AUTO_ACKNOWLEDGE);
    +      assertNotNull(consumerHighPriority);
    +      Session senderSession = connection.createSession(false, 
Session.AUTO_ACKNOWLEDGE);
    +      String queueName = "QUEUE.A";
    +      ActiveMQQueue low = new ActiveMQQueue(queueName + 
"?consumer.priority=1");
    +      MessageConsumer lowConsumer = 
consumerLowPriority.createConsumer(low);
    +
    +      ActiveMQQueue high = new ActiveMQQueue(queueName + 
"?consumer.priority=2");
    +      MessageConsumer highConsumer = 
consumerLowPriority.createConsumer(high);
    +
    +      ActiveMQQueue senderQueue = new ActiveMQQueue(queueName);
    +
    +      MessageProducer producer = senderSession.createProducer(senderQueue);
    +
    +      Message msg = senderSession.createTextMessage("test");
    +      for (int i = 0; i < 1000; i++) {
    +         producer.send(msg);
    +         assertNotNull("null on iteration: " + i, 
highConsumer.receive(1000));
    +      }
    +      assertNull(lowConsumer.receive(2000));
    --- End diff --
    
    I don't personally think this is a case which warrants keeping poor 
behaviour 'for consistency' when there are various essentially equivalent 
checks/assertions the test could do which don't require wasting 2 seconds. 
Having maybe run it once to ensure it worked, I'd change it.
    
    The ActiveMQ 5 test suite is an even better example of a test suite so slow 
(due to things like this) that folks don't actually want to run it.


---

Reply via email to