gemmellr commented on code in PR #5485:
URL: https://github.com/apache/activemq-artemis/pull/5485#discussion_r1941117633
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java:
##########
@@ -1432,11 +1432,11 @@ public void messageArrived(String topic, MqttMessage
message) throws Exception {
int sentAs = Integer.parseInt(new String(message.getPayload(),
StandardCharsets.UTF_8));
logger.info("QoS of publish: {}; QoS of subscription: {}; QoS of
receive: {}", sentAs, qosOfSubscription, message.getQos());
if (sentAs == 0) {
- assertTrue(message.getQos() == 0);
+ assertEquals(0, message.getQos());
} else if (sentAs == 1) {
- assertTrue(message.getQos() == (qosOfSubscription == 0 ? 0 : 1));
+ assertEquals(message.getQos(), (qosOfSubscription == 0 ? 0 : 1));
} else if (sentAs == 2) {
- assertTrue(message.getQos() == qosOfSubscription);
+ assertEquals(message.getQos(), qosOfSubscription);
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/interop/CompressedInteropTest.java:
##########
@@ -133,7 +133,7 @@ private void receiveStreamMessage(boolean useCore) throws
Exception {
byte[] bytesVal = new byte[originVal.length];
streamMessage.readBytes(bytesVal);
for (int i = 0; i < bytesVal.length; i++) {
- assertTrue(bytesVal[i] == originVal[i]);
+ assertEquals(bytesVal[i], originVal[i]);
Review Comment:
Args are reversed, here and most of the other ones changed below (except the
last one)
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithLargeMessagesTest.java:
##########
@@ -140,8 +139,8 @@ public void testSendReceiveLargePersistentMessages() throws
Exception {
for (int i = 0; i < count; i++) {
ClientStompFrame frame = conn.receiveFrame(60000);
assertNotNull(frame);
- assertTrue(frame.getCommand().equals("MESSAGE"));
- assertTrue(frame.getHeader("destination").equals(getQueuePrefix() +
getQueueName()));
+ assertEquals("MESSAGE", frame.getCommand());
+ assertEquals(frame.getHeader("destination"), getQueuePrefix() +
getQueueName());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v11/StompV11Test.java:
##########
@@ -1616,7 +1617,7 @@ public void testMessagesAreInOrder() throws Exception {
for (int i = 0; i < ctr; ++i) {
frame = conn.receiveFrame();
- assertTrue(frame.getBody().equals(data[i]), "Message not in order");
+ assertEquals(frame.getBody(), data[i], "Message not in order");
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v11/StompV11Test.java:
##########
@@ -1878,9 +1879,9 @@ public void testSubscribeToTopic() throws Exception {
ClientStompFrame frame = conn.receiveFrame();
- assertTrue(frame.getCommand().equals(Stomp.Responses.MESSAGE));
-
assertTrue(frame.getHeader(Stomp.Headers.Message.DESTINATION).equals(getTopicPrefix()
+ getTopicName()));
- assertTrue(frame.getBody().equals(getName()));
+ assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
+ assertEquals(frame.getHeader(Stomp.Headers.Message.DESTINATION),
getTopicPrefix() + getTopicName());
+ assertEquals(frame.getBody(), getName());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java:
##########
@@ -1432,11 +1432,11 @@ public void messageArrived(String topic, MqttMessage
message) throws Exception {
int sentAs = Integer.parseInt(new String(message.getPayload(),
StandardCharsets.UTF_8));
logger.info("QoS of publish: {}; QoS of subscription: {}; QoS of
receive: {}", sentAs, qosOfSubscription, message.getQos());
if (sentAs == 0) {
- assertTrue(message.getQos() == 0);
+ assertEquals(0, message.getQos());
} else if (sentAs == 1) {
- assertTrue(message.getQos() == (qosOfSubscription == 0 ? 0 : 1));
+ assertEquals(message.getQos(), (qosOfSubscription == 0 ? 0 : 1));
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/amq/JmsCreateConsumerInOnMessageTest.java:
##########
@@ -84,7 +85,7 @@ public void onMessage(Message message) {
}
} catch (Exception ex) {
ex.printStackTrace();
- assertTrue(false);
+ fail();
Review Comment:
This is entirely broken; since it is inside onMessage, it specifically isnt
on the test thread and as a result cant fail the test directly.
It would need to set a marker (e.g atomic ref to the exception), notify the
lock, and then check the marker on the test thread to have it fail.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/SimpleOpenWireTest.java:
##########
@@ -964,7 +964,7 @@ public void testAutoDestinationCreationOnProducerSend()
throws JMSException {
MessageConsumer consumer = session.createConsumer(queue);
TextMessage message1 = (TextMessage) consumer.receive(1000);
- assertTrue(message1.getText().equals(message.getText()));
+ assertEquals(message1.getText(), message.getText());
Review Comment:
Args are reversed.
(The message.getText() would also be better replaced as a variable, also
passed instead of a literal in the earlier `session.createTextMessage` )
##########
tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/brokerConnection/mirror/ClusteredMirrorSoakTest.java:
##########
@@ -292,7 +292,7 @@ public void testSimpleQueue() throws Exception {
final int numberOfMessages = 200;
- assertTrue(numberOfMessages % 2 == 0, "numberOfMessages must be even");
+ assertEquals(numberOfMessages % 2, 0, "numberOfMessages must be even");
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompConnectionCleanupTest.java:
##########
@@ -72,8 +72,8 @@ public void testConnectionCleanup() throws Exception {
ClientStompFrame frame = conn.receiveFrame(10000);
- assertTrue(frame.getCommand().equals("MESSAGE"));
- assertTrue(frame.getHeader("destination").equals(getQueuePrefix() +
getQueueName()));
+ assertEquals("MESSAGE", frame.getCommand());
+ assertEquals(frame.getHeader("destination"), getQueuePrefix() +
getQueueName());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/OutgoingConnectionTest.java:
##########
@@ -305,12 +306,12 @@ public void testOutgoingXAResourceWrapper() throws
Exception {
XASession s = queueConnection.createXASession();
XAResource resource = s.getXAResource();
- assertTrue(resource instanceof ActiveMQXAResourceWrapper);
+ assertInstanceOf(ActiveMQXAResourceWrapper.class, resource);
ActiveMQXAResourceWrapperImpl xaResourceWrapper =
(ActiveMQXAResourceWrapperImpl) resource;
- assertTrue(xaResourceWrapper.getJndiName().equals("java://jmsXA NodeId:"
+ server.getNodeID()));
-
assertTrue(xaResourceWrapper.getProductVersion().equals(VersionLoader.getVersion().getFullVersion()));
-
assertTrue(xaResourceWrapper.getProductName().equals(ActiveMQResourceAdapter.PRODUCT_NAME));
+ assertEquals(xaResourceWrapper.getJndiName(), "java://jmsXA NodeId:" +
server.getNodeID());
+ assertEquals(xaResourceWrapper.getProductVersion(),
VersionLoader.getVersion().getFullVersion());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java:
##########
@@ -937,7 +938,7 @@ public void testMessagesAreInOrder() throws Exception {
for (int i = 0; i < ctr; ++i) {
ClientStompFrame frame = conn.receiveFrame(1000);
- assertTrue(frame.getBody().equals(data[i]), "Message not in order");
+ assertEquals(frame.getBody(), data[i], "Message not in order");
Review Comment:
Args are reversed. Here and next 3 changes.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingSendTest.java:
##########
@@ -712,7 +712,7 @@ public void testPageLimitBytesValidationOnRestart() throws
Exception {
assertTrue(isFull);
currentPages = queuePagingStore.getNumberOfPages();
// now current pages should be one more than maxPages
- assertTrue(currentPages == maxPages + 1);
+ assertEquals(currentPages, maxPages + 1);
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/LockManagerReplicationTest.java:
##########
@@ -677,7 +677,7 @@ private void receiveFrom(ClientSessionFactory
clientSessionFactory, String addr)
ClientConsumer consumer = clientSession.createConsumer(addr);
Message message = consumer.receive(4000);
assertNotNull(message);
- assertTrue(message.getStringProperty("K").equals(addr));
+ assertEquals(message.getStringProperty("K"), addr);
Review Comment:
Args are reversed
##########
tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/jms/ActiveMQDestinationTest.java:
##########
@@ -46,18 +46,18 @@ public void testEquals() throws Exception {
ActiveMQDestination sameDestination =
ActiveMQDestination.fromPrefixedName(address);
ActiveMQDestination differentDestination =
ActiveMQDestination.fromPrefixedName(address + RandomUtil.randomString());
- assertFalse(destination.equals(null));
- assertTrue(destination.equals(destination));
- assertTrue(destination.equals(sameDestination));
- assertFalse(destination.equals(differentDestination));
+ assertNotEquals(null, destination);
+ assertEquals(destination, destination);
+ assertEquals(destination, sameDestination);
+ assertNotEquals(destination, differentDestination);
Review Comment:
Broken equals(null) test. Would change all back as per earlier tests.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/SimpleOpenWireTest.java:
##########
@@ -993,7 +993,7 @@ public void
testAutoDestinationCreationAndDeletionOnConsumer() throws Exception
producer.send(queue, message);
TextMessage message1 = (TextMessage) consumer.receive(1000);
- assertTrue(message1.getText().equals(message.getText()));
+ assertEquals(message1.getText(), message.getText());
Review Comment:
Ditto
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v12/StompV12Test.java:
##########
@@ -2544,7 +2545,7 @@ public void testSameMessageHasDifferentAckIdPerConsumer()
throws Exception {
ClientStompFrame frame2 = conn.receiveFrame();
String secondAckID = frame2.getHeader(Stomp.Headers.Message.ACK);
assertNotNull(secondAckID);
- assertTrue(!firstAckID.equals(secondAckID), firstAckID + " must not
equal " + secondAckID);
+ assertNotEquals(firstAckID, secondAckID, firstAckID + " must not equal "
+ secondAckID);
Review Comment:
Duped message details
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithLargeMessagesTest.java:
##########
@@ -183,8 +182,8 @@ public void testReceiveLargePersistentMessagesFromCore()
throws Exception {
for (int i = 0; i < count; i++) {
ClientStompFrame frame = conn.receiveFrame(60000);
assertNotNull(frame);
- assertTrue(frame.getCommand().equals("MESSAGE"));
- assertTrue(frame.getHeader("destination").equals(getQueuePrefix() +
getQueueName()));
+ assertEquals("MESSAGE", frame.getCommand());
+ assertEquals(frame.getHeader("destination"), getQueuePrefix() +
getQueueName());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v11/StompV11Test.java:
##########
@@ -1414,9 +1415,9 @@ public void testClientAckNotPartOfTransaction() throws
Exception {
ClientStompFrame frame = conn.receiveFrame();
- assertTrue(frame.getCommand().equals(Stomp.Responses.MESSAGE));
+ assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
assertNotNull(frame.getHeader(Stomp.Headers.Message.DESTINATION));
- assertTrue(frame.getBody().equals(getName()));
+ assertEquals(frame.getBody(), getName());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java:
##########
@@ -2223,7 +2224,7 @@ public void
testSameMessageHasDifferentMessageIdPerConsumer() throws Exception {
ClientStompFrame frame2 = conn.receiveFrame();
String secondMessageID =
frame2.getHeader(Stomp.Headers.Message.MESSAGE_ID);
assertNotNull(secondMessageID);
- assertTrue(!firstMessageID.equals(secondMessageID), firstMessageID + "
must not equal " + secondMessageID);
+ assertNotEquals(firstMessageID, secondMessageID, firstMessageID + " must
not equal " + secondMessageID);
Review Comment:
The message is now just duplicating object value which the assert will cover
itself (unlike prevously where it would just have said expected true).
Could generalise it to say what was wrong, e.g "MessageIDs should not be
equal"
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithLargeMessagesTest.java:
##########
@@ -459,8 +458,8 @@ public void
testReceiveLargeCompressedToLargePersistentMessagesFromCore() throws
assertNotNull(frame);
logger.debug(frame.toString());
logger.debug("part of frame: {}", frame.getBody().substring(0,
250));
- assertTrue(frame.getCommand().equals("MESSAGE"));
- assertTrue(frame.getHeader("destination").equals(getQueuePrefix()
+ getQueueName()));
+ assertEquals("MESSAGE", frame.getCommand());
+ assertEquals(frame.getHeader("destination"), getQueuePrefix() +
getQueueName());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithLargeMessagesTest.java:
##########
@@ -103,7 +102,7 @@ public void testSendReceiveLargeMessage() throws Exception {
// Receive STOMP Message
ClientStompFrame frame = conn.receiveFrame();
- assertTrue(frame.getBody().equals(payload));
+ assertEquals(frame.getBody(), payload);
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTestBase.java:
##########
@@ -541,7 +540,7 @@ public static ClientStompFrame
subscribeTopic(StompClientConnection conn,
if (receipt) {
assertNotNull(frame, "Requested receipt, but response is null");
-
assertTrue(frame.getHeader(Stomp.Headers.Response.RECEIPT_ID).equals(uuid));
+ assertEquals(frame.getHeader(Stomp.Headers.Response.RECEIPT_ID),
uuid);
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v11/StompV11Test.java:
##########
@@ -1606,7 +1607,7 @@ public void testMessagesAreInOrder() throws Exception {
for (int i = 0; i < ctr; ++i) {
frame = conn.receiveFrame();
- assertTrue(frame.getBody().equals(data[i]), "Message not in order");
+ assertEquals(frame.getBody(), data[i], "Message not in order");
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v11/StompV11Test.java:
##########
@@ -2398,7 +2399,7 @@ public void
testSameMessageHasDifferentMessageIdPerConsumer() throws Exception {
ClientStompFrame frame2 = conn.receiveFrame();
String secondMessageID =
frame2.getHeader(Stomp.Headers.Message.MESSAGE_ID);
assertNotNull(secondMessageID);
- assertTrue(!firstMessageID.equals(secondMessageID), firstMessageID + "
must not equal " + secondMessageID);
+ assertNotEquals(firstMessageID, secondMessageID, firstMessageID + " must
not equal " + secondMessageID);
Review Comment:
Duped message detail
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v12/StompV12Test.java:
##########
@@ -1634,7 +1635,7 @@ public void testMessagesAreInOrder() throws Exception {
for (int i = 0; i < ctr; ++i) {
frame = conn.receiveFrame();
- assertTrue(frame.getBody().equals(data[i]), "Message not in order");
+ assertEquals(frame.getBody(), data[i], "Message not in order");
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v12/StompV12Test.java:
##########
@@ -1644,7 +1645,7 @@ public void testMessagesAreInOrder() throws Exception {
for (int i = 0; i < ctr; ++i) {
frame = conn.receiveFrame();
- assertTrue(frame.getBody().equals(data[i]), "Message not in order");
+ assertEquals(frame.getBody(), data[i], "Message not in order");
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v11/StompV11Test.java:
##########
@@ -1909,9 +1910,9 @@ public void testSubscribeToTopicWithNoLocal() throws
Exception {
frame = conn.receiveFrame();
- assertTrue(frame.getCommand().equals(Stomp.Responses.MESSAGE));
-
assertTrue(frame.getHeader(Stomp.Headers.Message.DESTINATION).equals(getTopicPrefix()
+ getTopicName()));
- assertTrue(frame.getBody().equals(getName()));
+ assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
+ assertEquals(frame.getHeader(Stomp.Headers.Message.DESTINATION),
getTopicPrefix() + getTopicName());
+ assertEquals(frame.getBody(), getName());
Review Comment:
Args are reversed
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v12/StompV12Test.java:
##########
@@ -1945,9 +1946,9 @@ public void testSubscribeToTopicWithNoLocal() throws
Exception {
frame = conn.receiveFrame();
- assertTrue(frame.getCommand().equals(Stomp.Responses.MESSAGE));
-
assertTrue(frame.getHeader(Stomp.Headers.Subscribe.DESTINATION).equals(getTopicPrefix()
+ getTopicName()));
- assertTrue(frame.getBody().equals(getName()));
+ assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
+ assertEquals(frame.getHeader(Stomp.Headers.Subscribe.DESTINATION),
getTopicPrefix() + getTopicName());
+ assertEquals(frame.getBody(), getName());
Review Comment:
Args are reversed
##########
tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ConnectionFactoryPropertiesTest.java:
##########
@@ -143,7 +143,7 @@ public void testEquality() throws Exception {
cfp2.setParsedConnectionParameters(connectionParameters2);
cfp2.setAutoGroup(true);
- assertTrue(cfp1.equals(cfp2));
+ assertEquals(cfp1, cfp2);
Review Comment:
Would change equals tests back as per others. Here and next 2 changes.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v12/StompV12Test.java:
##########
@@ -1376,9 +1377,9 @@ public void testClientAckNotPartOfTransaction() throws
Exception {
ClientStompFrame frame = conn.receiveFrame();
- assertTrue(frame.getCommand().equals(Stomp.Responses.MESSAGE));
+ assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
assertNotNull(frame.getHeader(Stomp.Headers.Message.DESTINATION));
- assertTrue(frame.getBody().equals(getName()));
+ assertEquals(frame.getBody(), getName());
Review Comment:
Args are reversed
##########
tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/brokerConnection/sender/SenderSoakTest.java:
##########
@@ -126,7 +126,7 @@ public void testSender(boolean mirror) throws Exception {
final int numberOfMessages = 1000;
- assertTrue(numberOfMessages % 2 == 0, "numberOfMessages must be even");
+ assertEquals(numberOfMessages % 2, 0, "numberOfMessages must be even");
Review Comment:
Args are reversed
##########
tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/brokerConnection/mirror/ClusteredMirrorSoakTest.java:
##########
@@ -515,7 +515,7 @@ public void testMirroredTopics() throws Exception {
final int numberOfMessages = 200;
- assertTrue(numberOfMessages % 2 == 0, "numberOfMessages must be even");
+ assertEquals(numberOfMessages % 2, 0, "numberOfMessages must be even");
Review Comment:
Args are reversed
##########
tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/brokerConnection/mirror/MultiMirrorSoakTest.java:
##########
@@ -145,7 +145,7 @@ public void testMultiMirror() throws Exception {
public void internalMirror(String producerURI, String consumerURi) throws
Exception {
final int numberOfMessages = 200;
- assertTrue(numberOfMessages % 2 == 0, "numberOfMessages must be even");
+ assertEquals(numberOfMessages % 2, 0, "numberOfMessages must be even");
Review Comment:
Args are reversed
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
