ARTEMIS-4548 refactor MQTT tests

Many MQTT tests are run twice - once using TCP and once using
WebSockets. This is essentially a big waste of time since once the
connection is established to the broker the tests are identical. The
tests should be refactored to run just once and then there can be a
small number of tests specifically for WebSockets.

This should knock several minutes off the test-suite.

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5Test.java

@@ -61,10 +61,6 @@ public class MQTT5Test extends MQTT5TestSupport {
 
    private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-   public MQTT5Test(String protocol) {
-      super(protocol);
-   }
-
    @Test(timeout = DEFAULT_TIMEOUT)
    public void testSimpleSendReceive() throws Exception {
       String topic = RandomUtil.randomString();

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5TestSupport.java

@@ -21,8 +21,6 @@
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.security.ProtectionDomain;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -71,15 +69,12 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.rules.TestName;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static java.util.Collections.singletonList;
 import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME;
 
-@RunWith(Parameterized.class)
 public class MQTT5TestSupport extends ActiveMQTestBase {
    protected static final String TCP = "tcp";
    protected static final String WS = "ws";
@@ -88,34 +83,28 @@ public class MQTT5TestSupport extends ActiveMQTestBase {
    protected static final SimpleString DEAD_LETTER_ADDRESS = new SimpleString("DLA");
    protected static final SimpleString EXPIRY_ADDRESS = new SimpleString("EXPIRY");
 
-   @Parameterized.Parameters(name = "protocol={0}")
-   public static Collection<Object[]> getParams() {
-      return Arrays.asList(new Object[][] {
-         {TCP},
-         {WS}
-      });
+   protected MqttClient createPahoClient(String clientId) throws MqttException {
+      return createPahoClient(TCP, clientId);
    }
 
-   protected String protocol;
-
-   public MQTT5TestSupport(String protocol) {
-      this.protocol = protocol;
+   protected MqttClient createPahoClient(String protocol, String clientId) throws MqttException {
+      return createPahoClient(protocol, clientId, (isUseSsl() ? getSslPort() : getPort()));
    }
 
-   protected MqttClient createPahoClient(String clientId) throws MqttException {
-      return new MqttClient(protocol + "://localhost:" + (isUseSsl() ? getSslPort() : getPort()), clientId, new MemoryPersistence());
+   protected MqttClient createPahoClient(String clientId, int port) throws MqttException {
+      return createPahoClient(TCP, clientId, port);
    }
 
-   protected MqttClient createPahoClient(String clientId, int port) throws MqttException {
+   protected MqttClient createPahoClient(String protocol, String clientId, int port) throws MqttException {
       return new MqttClient(protocol + "://localhost:" + port, clientId, new MemoryPersistence());
    }
 
    protected org.eclipse.paho.client.mqttv3.MqttClient createPaho3_1_1Client(String clientId) throws org.eclipse.paho.client.mqttv3.MqttException {
-      return new org.eclipse.paho.client.mqttv3.MqttClient(protocol + "://localhost:" + (isUseSsl() ? getSslPort() : getPort()), clientId, new org.eclipse.paho.client.mqttv3.persist.MemoryPersistence());
+      return new org.eclipse.paho.client.mqttv3.MqttClient(TCP + "://localhost:" + (isUseSsl() ? getSslPort() : getPort()), clientId, new org.eclipse.paho.client.mqttv3.persist.MemoryPersistence());
    }
 
    protected MqttAsyncClient createAsyncPahoClient(String clientId) throws MqttException {
-      return new MqttAsyncClient(protocol + "://localhost:" + (isUseSsl() ? getSslPort() : getPort()), clientId, new MemoryPersistence());
+      return new MqttAsyncClient(TCP + "://localhost:" + (isUseSsl() ? getSslPort() : getPort()), clientId, new MemoryPersistence());
    }
 
    private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTTRetainMessageManagerTest.java

@@ -50,10 +50,6 @@ public class MQTTRetainMessageManagerTest extends MQTT5TestSupport {
    private final int numberOfMessages = 1000;
    private final int numberOfTests = 10;
 
-   public MQTTRetainMessageManagerTest(String protocol) {
-      super(protocol);
-   }
-
    @Before
    public void beforeEach() throws MqttException {
       mqttPublisher = createPahoClient("publisher");

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/ControlPacketFormatTests.java

@@ -49,10 +49,6 @@
 
 public class ControlPacketFormatTests extends MQTT5TestSupport {
 
-   public ControlPacketFormatTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-2.2.1-2] A PUBLISH packet MUST NOT contain a Packet Identifier if its QoS value is set to 0.
     */

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/DataFormatTests.java

@@ -31,8 +31,4 @@
 
 @Ignore
 public class DataFormatTests extends MQTT5TestSupport {
-
-   public DataFormatTests(String protocol) {
-      super(protocol);
-   }
 }

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/EnhancedAuthenticationTests.java

@@ -43,10 +43,6 @@
 
 public class EnhancedAuthenticationTests extends MQTT5TestSupport {
 
-   public EnhancedAuthenticationTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-4.12.0-1] If the Server does not support the Authentication Method supplied by the Client, it MAY send a
     * CONNACK with a Reason Code of 0x8C (Bad authentication method) or 0x87 (Not Authorized) as described in section

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/FlowControlTests.java

@@ -33,8 +33,4 @@
 
 @Ignore
 public class FlowControlTests extends MQTT5TestSupport {
-
-   public FlowControlTests(String protocol) {
-      super(protocol);
-   }
 }

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/HandlingErrorTests.java

@@ -22,7 +22,6 @@
 import org.eclipse.paho.mqttv5.client.MqttConnectionOptions;
 import org.eclipse.paho.mqttv5.client.MqttConnectionOptionsBuilder;
 import org.eclipse.paho.mqttv5.common.MqttException;
-import org.junit.Assume;
 import org.junit.Test;
 
 /**
@@ -33,10 +32,6 @@
 
 public class HandlingErrorTests extends MQTT5TestSupport {
 
-   public HandlingErrorTests(String protocol) {
-      this.protocol = protocol;
-   }
-
    /*
     * [MQTT-4.13.2-1] The CONNACK and DISCONNECT packets allow a Reason Code of 0x80 or greater to indicate that the
     * Network Connection will be closed. If a Reason Code of 0x80 or greater is specified, then the Network Connection
@@ -46,9 +41,6 @@ public HandlingErrorTests(String protocol) {
     */
    @Test(timeout = DEFAULT_TIMEOUT)
    public void testEmptyClientIDWithoutCleanStart() throws Exception {
-      // This is apparently broken with the Paho client + web socket. The broker never even receives a CONNECT packet.
-      Assume.assumeTrue(protocol.equals(TCP));
-
       MqttClient client = createPahoClient("");
       MqttConnectionOptions options = new MqttConnectionOptionsBuilder()
          .cleanStart(false)

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/MessageDeliveryRetryTests.java

@@ -29,10 +29,6 @@
 
 public class MessageDeliveryRetryTests extends MQTT5TestSupport {
 
-   public MessageDeliveryRetryTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-4.4.0-1] When a Client reconnects with Clean Start set to 0 and a session is present, both the Client and
     * Server MUST resend any unacknowledged PUBLISH packets (where QoS > 0) and PUBREL packets using their original

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/MessageOrderingTests.java

@@ -36,8 +36,4 @@
 
 @Ignore
 public class MessageOrderingTests extends MQTT5TestSupport {
-
-   public MessageOrderingTests(String protocol) {
-      super(protocol);
-   }
 }

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/MessageReceiptTests.java

@@ -34,10 +34,6 @@
 
 public class MessageReceiptTests extends MQTT5TestSupport {
 
-   public MessageReceiptTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-4.5.0-1] When a Server takes ownership of an incoming Application Message it MUST add it to the Session
     * State for those Clients that have matching Subscriptions.

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/NetworkConnectionTests.java

@@ -27,8 +27,4 @@
 
 @Ignore
 public class NetworkConnectionTests extends MQTT5TestSupport {
-
-   public NetworkConnectionTests(String protocol) {
-      super(protocol);
-   }
 }

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/QoSTests.java

@@ -60,10 +60,6 @@
 
 public class QoSTests extends MQTT5TestSupport {
 
-   public QoSTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-4.3.2-2] In the QoS 1 delivery protocol, the sender MUST send a PUBLISH packet containing this Packet
     * Identifier with QoS 1 and DUP flag set to 0.

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/SessionStateTests.java

@@ -27,8 +27,4 @@
 
 @Ignore
 public class SessionStateTests extends MQTT5TestSupport {
-
-   public SessionStateTests(String protocol) {
-      super(protocol);
-   }
 }

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/SubscriptionTests.java

@@ -43,10 +43,6 @@
 
 public class SubscriptionTests extends MQTT5TestSupport {
 
-   public SubscriptionTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-4.8.2-3] The Server MUST respect the granted QoS for the Client's subscription.
     */

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/TopicNameAndFilterTests.java

@@ -49,10 +49,6 @@
 
 public class TopicNameAndFilterTests extends MQTT5TestSupport {
 
-   public TopicNameAndFilterTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-4.7.2-1] The Server MUST NOT match Topic Filters starting with a wildcard character (# or +) with Topic
     * Names beginning with a $ character.

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/WebSocketTests.java

@@ -39,8 +39,4 @@
 
 @Ignore
 public class WebSocketTests extends MQTT5TestSupport {
-
-   public WebSocketTests(String protocol) {
-      super(protocol);
-   }
 }

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/AuthTests.java

@@ -39,8 +39,4 @@
 
 @Ignore
 public class AuthTests  extends MQTT5TestSupport {
-
-   public AuthTests(String protocol) {
-      this.protocol = protocol;
-   }
 }

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnAckTests.java

@@ -41,7 +41,6 @@
 import org.eclipse.paho.mqttv5.client.MqttDisconnectResponse;
 import org.eclipse.paho.mqttv5.common.MqttException;
 import org.eclipse.paho.mqttv5.common.packet.MqttReturnCode;
-import org.junit.Assume;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -82,10 +81,6 @@
 
 public class ConnAckTests  extends MQTT5TestSupport {
 
-   public ConnAckTests(String protocol) {
-      this.protocol = protocol;
-   }
-
    /*
     * [MQTT-3.1.3-6] A Server MAY allow a Client to supply a ClientID that has a length of zero bytes, however if it
     * does so the Server MUST treat this as a special case and assign a unique ClientID to that Client.
@@ -99,8 +94,6 @@ public ConnAckTests(String protocol) {
     */
    @Test(timeout = DEFAULT_TIMEOUT)
    public void testEmptyClientID() throws Exception {
-      // This is apparently broken with the Paho client + web socket. The broker never even receives a CONNECT packet.
-      Assume.assumeTrue(protocol.equals(TCP));
 
       // no session should exist
       assertEquals(0, getSessionStates().size());
@@ -268,8 +261,6 @@ public void testConnackSentFirst() throws Exception {
     */
    @Test(timeout = DEFAULT_TIMEOUT)
    public void testSessionPresentWithNonZeroConnackReasonCode() throws Exception {
-      // This is apparently broken with the Paho client + web socket. The broker never even receives a CONNECT packet.
-      Assume.assumeTrue(protocol.equals(TCP));
       CountDownLatch latch = new CountDownLatch(1);
 
       MQTTInterceptor outgoingInterceptor = (packet, connection) -> {

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnectTests.java

@@ -16,6 +16,7 @@
  */
 package org.apache.activemq.artemis.tests.integration.mqtt5.spec.controlpackets;
 
+import java.lang.invoke.MethodHandles;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
@@ -42,11 +43,9 @@
 import org.eclipse.paho.mqttv5.common.packet.MqttProperties;
 import org.eclipse.paho.mqttv5.common.packet.MqttReturnCode;
 import org.eclipse.paho.mqttv5.common.packet.UserProperty;
-import org.junit.Assume;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import java.lang.invoke.MethodHandles;
 
 /**
  * Fulfilled by client or Netty codec (i.e. not tested here):
@@ -104,10 +103,6 @@ public class ConnectTests extends MQTT5TestSupport {
 
    private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-   public ConnectTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-3.1.2-7] If the Will Flag is set to 1 this indicates that, a Will Message MUST be stored on the Server and
     * associated with the Session.
@@ -632,9 +627,6 @@ public void testClientID() throws Exception {
     */
    @Test(timeout = DEFAULT_TIMEOUT)
    public void testEmptyClientID() throws Exception {
-      // This is apparently broken with the Paho client + web socket. The broker never even receives a CONNECT packet.
-      Assume.assumeTrue(protocol.equals(TCP));
-
       // no session should exist
       assertEquals(0, getSessionStates().size());
 
@@ -658,9 +650,6 @@ public void testEmptyClientID() throws Exception {
     */
    @Test(timeout = DEFAULT_TIMEOUT)
    public void testEmptyClientIDWithoutCleanStart() throws Exception {
-      // This is apparently broken with the Paho client + web socket. The broker never even receives a CONNECT packet.
-      Assume.assumeTrue(protocol.equals(TCP));
-
       // no session should exist
       assertEquals(0, getSessionStates().size());
 

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnectTestsWithSecurity.java

@@ -29,10 +29,6 @@
 
 public class ConnectTestsWithSecurity extends MQTT5TestSupport {
 
-   public ConnectTestsWithSecurity(String protocol) {
-      super(protocol);
-   }
-
    @Override
    public boolean isSecurityEnabled() {
       return true;

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/DisconnectTests.java

@@ -53,10 +53,6 @@
 
 public class DisconnectTests  extends MQTT5TestSupport {
 
-   public DisconnectTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-3.14.2-1] The Client or Server sending the DISCONNECT packet MUST use one of the DISCONNECT Reason Codes.
     *

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PingReqTests.java

@@ -29,10 +29,6 @@
 
 public class PingReqTests  extends MQTT5TestSupport {
 
-   public PingReqTests(String protocol) {
-      super(protocol);
-   }
-
    /*
     * [MQTT-3.12.4-1] The Server MUST send a PINGRESP packet in response to a PINGREQ packet.
     */

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PingRespTests.java

@@ -26,8 +26,4 @@
 
 @Ignore
 public class PingRespTests  extends MQTT5TestSupport {
-
-   public PingRespTests(String protocol) {
-      super(protocol);
-   }
 }