Github user michaelandrepearce commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/2116#discussion_r191793991 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/SessionMetadataAddExceptionTest.java --- @@ -75,6 +93,37 @@ public void testInvalidClientIdSetFactory() throws Exception { cf.createConnection(); } + @Test(timeout = 5000, expected = JMSException.class) + public void testDuplicateClientIdSet() throws Exception { + ActiveMQConnectionFactory activeMQConnectionFactory = (ActiveMQConnectionFactory) cf; + Connection con = cf.createConnection(); + Connection con2 = cf.createConnection(); + try { + con.setClientID("valid"); + con2.setClientID("valid"); + fail("Should have failed for duplicate clientId"); + } catch (InvalidClientIDException e) { + assertEquals(1, duplicateCount.get()); + throw e; --- End diff -- Problem here is assert will be bypassed and test will still pass if another exception thrown as tests expects JMSException. Expects should be explicit and need a catch all with and assert fail
---