You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "mattrpav (via GitHub)" <gi...@apache.org> on 2026/09/08 14:45:32 UTC
[PR] Fix flaky ExceptionListenerTest: wait for transport-failed before createSession [activemq]
mattrpav opened a new pull request, #2544:
URL: https://github.com/apache/activemq/pull/2544
The security exception fires the ExceptionListener and marks the transport failed on two separate async tasks in ActiveMQConnection.onException. Waiting only for the listener let createSession race a disposing transport and get JMSException("Stopped.") instead of the expected ConnectionFailedException.
--
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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Fix flaky ExceptionListenerTest: wait for transport-failed before createSession [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #2544:
URL: https://github.com/apache/activemq/pull/2544#discussion_r4007257697
##########
activemq-unit-tests/src/test/java/org/apache/activemq/usecases/ExceptionListenerTest.java:
##########
@@ -107,6 +108,20 @@ public boolean isSatisified() throws Exception {
assertTrue("expected exception: " + expected, expected.getCause().getCause() instanceof SecurityException);
+ // ActiveMQConnection.onException delivers the security failure to the
+ // ExceptionListener and marks the transport failed on two independent
+ // async tasks. The listener firing above does not guarantee the transport
+ // is failed yet, so wait for the failed state; otherwise createSession can
+ // race a mid-dispose transport and see JMSException("Stopped.") instead of
+ // the expected ConnectionFailedException.
+ final ActiveMQConnection amqConnection = (ActiveMQConnection) connection;
+ Wait.waitFor(new Wait.Condition() {
Review Comment:
```suggestion
Wait.waitFor((Condition) amqConnection::isTransportFailed, 5000, 10);
```
It makes sense to speed this up, the default wait can go 30 seconds and checks every second (we should probably change the defaults in another PR). Also you can make it a lot smaller using lambdas
--
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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Fix flaky ExceptionListenerTest: wait for transport-failed before createSession [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #2544:
URL: https://github.com/apache/activemq/pull/2544#discussion_r4007266157
##########
activemq-unit-tests/src/test/java/org/apache/activemq/usecases/ExceptionListenerTest.java:
##########
@@ -107,6 +108,20 @@ public boolean isSatisified() throws Exception {
assertTrue("expected exception: " + expected, expected.getCause().getCause() instanceof SecurityException);
+ // ActiveMQConnection.onException delivers the security failure to the
+ // ExceptionListener and marks the transport failed on two independent
+ // async tasks. The listener firing above does not guarantee the transport
+ // is failed yet, so wait for the failed state; otherwise createSession can
+ // race a mid-dispose transport and see JMSException("Stopped.") instead of
+ // the expected ConnectionFailedException.
+ final ActiveMQConnection amqConnection = (ActiveMQConnection) connection;
+ Wait.waitFor(new Wait.Condition() {
Review Comment:
Also should assert true that and not just continue if it fails
--
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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Fix flaky ExceptionListenerTest: wait for transport-failed before createSession [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #2544:
URL: https://github.com/apache/activemq/pull/2544#discussion_r4007257697
##########
activemq-unit-tests/src/test/java/org/apache/activemq/usecases/ExceptionListenerTest.java:
##########
@@ -107,6 +108,20 @@ public boolean isSatisified() throws Exception {
assertTrue("expected exception: " + expected, expected.getCause().getCause() instanceof SecurityException);
+ // ActiveMQConnection.onException delivers the security failure to the
+ // ExceptionListener and marks the transport failed on two independent
+ // async tasks. The listener firing above does not guarantee the transport
+ // is failed yet, so wait for the failed state; otherwise createSession can
+ // race a mid-dispose transport and see JMSException("Stopped.") instead of
+ // the expected ConnectionFailedException.
+ final ActiveMQConnection amqConnection = (ActiveMQConnection) connection;
+ Wait.waitFor(new Wait.Condition() {
Review Comment:
```suggestion
assertTrue(Wait.waitFor((Condition) amqConnection::isTransportFailed, 5000, 10));
```
It makes sense to speed this up, the default wait can go 30 seconds and checks every second (we should probably change the defaults in another PR). Also you can make it a lot smaller using lambdas
--
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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] Fix flaky ExceptionListenerTest: wait for transport-failed before createSession [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav merged PR #2544:
URL: https://github.com/apache/activemq/pull/2544
--
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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact