gemmellr commented on code in PR #1303:
URL: https://github.com/apache/activemq/pull/1303#discussion_r1760933317
##########
activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java:
##########
@@ -296,24 +296,57 @@ public void testProducerDeliveryDelaySet() throws
JMSException {
messageProducer.setDeliveryDelay(1000l);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, (CompletionListener)null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
Review Comment:
You are going to want to actually have a CompletionListner in this test. The
test originally didnt have one (though could have) since it was only verifying
the method threw UnsupportedOperationException.
This test makes little sense as it stands now, in fact it violates the
specification (see later comment).
##########
activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java:
##########
@@ -296,24 +296,57 @@ public void testProducerDeliveryDelaySet() throws
JMSException {
messageProducer.setDeliveryDelay(1000l);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, (CompletionListener)null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageQoSParamsCompletionListener() throws
JMSException {
- messageProducer.send(null, 1, 4, 0l, null);
+ messageProducer.send(session.createTextMessage("test message"), 1, 4,
0l, null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendDestinationMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void
testProducerSendDestinationMessageQosParamsCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, 1, 4, 0l, null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), 1, 4, 0l, null);
Review Comment:
As previous comment
##########
activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java:
##########
@@ -296,24 +296,57 @@ public void testProducerDeliveryDelaySet() throws
JMSException {
messageProducer.setDeliveryDelay(1000l);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, (CompletionListener)null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageQoSParamsCompletionListener() throws
JMSException {
- messageProducer.send(null, 1, 4, 0l, null);
+ messageProducer.send(session.createTextMessage("test message"), 1, 4,
0l, null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendDestinationMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
Review Comment:
As previous comment
##########
activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java:
##########
@@ -296,24 +296,57 @@ public void testProducerDeliveryDelaySet() throws
JMSException {
messageProducer.setDeliveryDelay(1000l);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, (CompletionListener)null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageQoSParamsCompletionListener() throws
JMSException {
- messageProducer.send(null, 1, 4, 0l, null);
+ messageProducer.send(session.createTextMessage("test message"), 1, 4,
0l, null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendDestinationMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void
testProducerSendDestinationMessageQosParamsCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, 1, 4, 0l, null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), 1, 4, 0l, null);
+ }
+
+ @Test
+ public void testProducerSendMessageWithNonNullCompletionListener() throws
JMSException {
Review Comment:
This test doesnt make sense, as the spec requires the CompletionListener be
non-null, noting it throws "IllegalArgumentException - if the specified
CompletionListener is null"
https://jakarta.ee/specifications/messaging/3.1/apidocs/jakarta.messaging/jakarta/jms/messageproducer#send(jakarta.jms.Destination,jakarta.jms.Message,int,int,long,jakarta.jms.CompletionListener)
##########
activemq-client/src/main/java/org/apache/activemq/ActiveMQConnection.java:
##########
@@ -222,6 +224,7 @@ public class ActiveMQConnection implements Connection,
TopicConnection, QueueCon
* @param factoryStats
* @throws Exception
*/
+ // Mark
Review Comment:
Doesnt seem needed
##########
activemq-unit-tests/src/test/java/org/apache/activemq/jms2/ActiveMQJMS2ContextTest.java:
##########
@@ -296,24 +296,57 @@ public void testProducerDeliveryDelaySet() throws
JMSException {
messageProducer.setDeliveryDelay(1000l);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageCompletionListener() throws
JMSException {
- messageProducer.send(session.createQueue(methodNameDestinationName),
null, (CompletionListener)null);
+ messageProducer.send(session.createQueue(methodNameDestinationName),
session.createTextMessage("test message"), null);
}
- @Test(expected = UnsupportedOperationException.class)
+ @Test
public void testProducerSendMessageQoSParamsCompletionListener() throws
JMSException {
- messageProducer.send(null, 1, 4, 0l, null);
+ messageProducer.send(session.createTextMessage("test message"), 1, 4,
0l, null);
Review Comment:
As previous comment
##########
activemq-client/src/main/java/org/apache/activemq/ActiveMQConnection.java:
##########
@@ -45,6 +46,7 @@
import jakarta.jms.IllegalStateException;
import jakarta.jms.InvalidDestinationException;
import jakarta.jms.JMSException;
+//import jakarta.jms.Message;
Review Comment:
If not using it, dont add it commented-out...
--
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
