jeanouii commented on code in PR #1814:
URL: https://github.com/apache/activemq/pull/1814#discussion_r2974538752
##########
activemq-ra/src/test/java/org/apache/activemq/ra/MessageEndpointProxyTest.java:
##########
@@ -66,27 +60,14 @@ public void testInvalidConstruction() {
@Test(timeout = 60000)
public void testSuccessfulCallSequence() throws Exception {
- setupBeforeDeliverySuccessful();
Review Comment:
Missing equivalents for this?
verify(mockEndpointAndListener).beforeDelivery(any(Method.class));
verify(mockEndpointAndListener).onMessage(stubMessage);
verify(mockEndpointAndListener).afterDelivery();
##########
activemq-ra/src/test/java/org/apache/activemq/ra/MessageEndpointProxyTest.java:
##########
@@ -18,6 +18,8 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.Mockito.*;
Review Comment:
Avoid star imports?
##########
activemq-ra/src/test/java/org/apache/activemq/ra/ServerSessionImplTest.java:
##########
@@ -125,83 +104,41 @@ public void testCloseCanStopActiveSession() throws
Exception {
final int maxMessages = 4000;
final CountDownLatch messageCount = new CountDownLatch(maxMessages);
- final MessageEndpointFactory messageEndpointFactory =
context.mock(MessageEndpointFactory.class);
- final MessageResourceAdapter resourceAdapter =
context.mock(MessageResourceAdapter.class);
- final ActiveMQEndpointActivationKey key =
context.mock(ActiveMQEndpointActivationKey.class);
- messageEndpoint = context.mock(MessageEndpointProxy.class);
- workManager = context.mock(WorkManager.class);
- final MessageActivationSpec messageActivationSpec =
context.mock(MessageActivationSpec.class);
- final BootstrapContext boostrapContext =
context.mock(BootstrapContext.class);
- context.checking(new Expectations() {
- {
- allowing(boostrapContext).getWorkManager();
- will(returnValue(workManager));
- allowing(resourceAdapter).getBootstrapContext();
- will(returnValue(boostrapContext));
-
allowing(messageEndpointFactory).isDeliveryTransacted(with(any(Method.class)));
- will(returnValue(Boolean.FALSE));
- allowing(key).getMessageEndpointFactory();
- will(returnValue(messageEndpointFactory));
- allowing(key).getActivationSpec();
- will(returnValue(messageActivationSpec));
- allowing(messageActivationSpec).isUseJndi();
- will(returnValue(Boolean.FALSE));
- allowing(messageActivationSpec).getDestinationType();
- will(returnValue("jakarta.jms.Queue"));
- allowing(messageActivationSpec).getDestination();
- will(returnValue("Queue"));
- allowing(messageActivationSpec).getAcknowledgeModeForSession();
- will(returnValue(1));
- allowing(messageActivationSpec).getMaxSessionsIntValue();
- will(returnValue(1));
- allowing(messageActivationSpec).getEnableBatchBooleanValue();
- will(returnValue(Boolean.FALSE));
-
allowing(messageActivationSpec).isUseRAManagedTransactionEnabled();
- will(returnValue(Boolean.TRUE));
-
allowing(messageEndpointFactory).createEndpoint(with(nullValue(XAResource.class)));
- will(returnValue(messageEndpoint));
-
- allowing(workManager).scheduleWork((Work)
with(any(Work.class)), with(any(long.class)), with(any(ExecutionContext.class)),
- with(any(WorkListener.class)));
- will(new Action() {
- @Override
- public Object invoke(Invocation invocation) throws
Throwable {
- return null;
- }
-
- @Override
- public void describeTo(Description description) {
- }
- });
-
- allowing(messageEndpoint).beforeDelivery((Method)
with(any(Method.class)));
-
allowing(messageEndpoint).onMessage(with(any(jakarta.jms.Message.class)));
- will(new Action() {
- @Override
- public Object invoke(Invocation invocation) throws
Throwable {
- messageCount.countDown();
- if (messageCount.getCount() < maxMessages - 11) {
- TimeUnit.MILLISECONDS.sleep(200);
- }
- return null;
- }
-
- @Override
- public void describeTo(Description description) {
- description.appendText("Keep message count");
- }
- });
- allowing(messageEndpoint).afterDelivery();
- allowing(messageEndpoint).release();
-
- allowing(workManager).scheduleWork(
- with(any(Work.class)),
- with(any(Long.TYPE)),
- with(nullValue(ExecutionContext.class)),
- with(nullValue(WorkListener.class)));
-
+ final MessageEndpointFactory messageEndpointFactory =
mock(MessageEndpointFactory.class);
+ final MessageResourceAdapter resourceAdapter =
mock(MessageResourceAdapter.class);
+ final ActiveMQEndpointActivationKey key =
mock(ActiveMQEndpointActivationKey.class);
+ messageEndpoint = mock(MessageEndpointProxy.class);
+ workManager = mock(WorkManager.class);
+ final MessageActivationSpec messageActivationSpec =
mock(MessageActivationSpec.class);
+ final BootstrapContext boostrapContext = mock(BootstrapContext.class);
+
+
lenient().when(boostrapContext.getWorkManager()).thenReturn(workManager);
Review Comment:
Similar to what was done with JMock, but some of this block could be
factorized in a helper method
##########
activemq-ra/src/test/java/org/apache/activemq/ra/MessageEndpointProxyTest.java:
##########
@@ -66,27 +60,14 @@ public void testInvalidConstruction() {
@Test(timeout = 60000)
public void testSuccessfulCallSequence() throws Exception {
- setupBeforeDeliverySuccessful();
- setupOnMessageSuccessful();
- setupAfterDeliverySuccessful();
-
doBeforeDeliveryExpectSuccess();
doOnMessageExpectSuccess();
doAfterDeliveryExpectSuccess();
}
@Test(timeout = 60000)
public void testBeforeDeliveryFailure() throws Exception {
- context.checking(new Expectations() {{
- oneOf
(mockEndpointAndListener).beforeDelivery(with(any(Method.class)));
- will(throwException(new ResourceException()));
- }});
- context.checking(new Expectations() {{
- never (mockEndpointAndListener).onMessage(null);
- never (mockEndpointAndListener).afterDelivery();
- }});
-
- setupExpectRelease();
+ doThrow(new
ResourceException()).when(mockEndpointAndListener).beforeDelivery(any(Method.class));
Review Comment:
A few checks not ported here I believe
- the 2 never in the checking block
verify(mockEndpointAndListener, never()).onMessage(any());
verify(mockEndpointAndListener, never()).afterDelivery();
- the release in the setupExpectRelease() method
verify(mockEndpointAndListener).release();
##########
activemq-ra/src/test/java/org/apache/activemq/ra/MessageEndpointProxyTest.java:
##########
@@ -102,16 +83,10 @@ public void testBeforeDeliveryFailure() throws Exception {
@Test(timeout = 60000)
public void testOnMessageFailure() throws Exception {
- setupBeforeDeliverySuccessful();
-
- context.checking(new Expectations() {{
- oneOf (mockEndpointAndListener).onMessage(with(same(stubMessage)));
- will(throwException(new RuntimeException()));
- }});
+ doBeforeDeliveryExpectSuccess();
- setupAfterDeliverySuccessful();
Review Comment:
Missing the verify for Mockito
##########
activemq-ra/src/test/java/org/apache/activemq/ra/ServerSessionImplTest.java:
##########
@@ -60,21 +49,14 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
-import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.*;
Review Comment:
I think Matt told me to avoid this. It's a test class, so I think it's ok to
have it
##########
activemq-ra/src/test/java/org/apache/activemq/ra/ServerSessionImplTest.java:
##########
@@ -60,21 +49,14 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
-import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.*;
+import static org.mockito.ArgumentMatchers.*;
@Ignore
Review Comment:
Outside of the scope of this PR. This test is @Ignore, so you could have
also remove it
Or fix it.
But that's fully equivalent
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
