gnodet commented on code in PR #24393:
URL: https://github.com/apache/camel/pull/24393#discussion_r3522974554
##########
components/camel-vertx/camel-vertx-websocket/src/test/java/org/apache/camel/component/vertx/websocket/VertxWebsocketConsumerAsClientMaxReconnectTest.java:
##########
@@ -42,26 +45,35 @@ void testMaxReconnect() throws Exception {
context.getRouteController().stopRoute("server");
- // Verify that we cannot send messages
- Exchange exchange = template.send(uri, new Processor() {
- @Override
- public void process(Exchange exchange) throws Exception {
- exchange.getMessage().setBody("Hello World Again");
- }
- });
- Exception exception = exchange.getException();
- Assertions.assertNotNull(exception);
- Assertions.assertInstanceOf(ConnectException.class,
exception.getCause());
+ // Verify that the server is fully down by waiting until sends fail.
+ // The producer endpoint's cached WebSocket may still appear open
briefly
+ // after stopRoute returns, until the Vert.x event loop processes the
+ // TCP close frame and isClosed() starts returning true.
+ await().atMost(10, TimeUnit.SECONDS)
+ .untilAsserted(() -> {
+ Exchange exchange = template.send(uri, new Processor() {
+ @Override
+ public void process(Exchange exchange) throws
Exception {
+ exchange.getMessage().setBody("Hello World Again");
+ }
+ });
+ Assertions.assertNotNull(exchange.getException());
+ Assertions.assertInstanceOf(ConnectException.class,
exchange.getException().getCause());
+ });
// Wait for client consumer reconnect max attempts to be exhausted
- Thread.sleep(300);
+ // (maxReconnectAttempts=1, reconnectInterval=10ms).
+ // Use pollDelay to give reconnect attempts time to complete before
checking.
+ await().atMost(10, TimeUnit.SECONDS)
+ .pollDelay(2, TimeUnit.SECONDS)
+ .untilAsserted(() -> mockEndpoint.assertIsSatisfied());
// Restart server
context.getRouteController().startRoute("server");
// Verify that the client consumer gave up reconnecting
template.sendBody(uri, "Hello World Again");
- mockEndpoint.assertIsSatisfied();
+ mockEndpoint.assertIsSatisfied(1000);
Review Comment:
_Claude Code on behalf of Arnaud Thimel_
Addressed in 2eb8739. Replaced `assertIsSatisfied(1000)` with
`setAssertPeriod(1000)` + `assertIsSatisfied()`. The `setAssertPeriod` causes
MockEndpoint to sleep the specified period after the initial assertion passes
and then re-assert, which properly catches any late deliveries that arrive
after restart.
##########
components/camel-vertx/camel-vertx-websocket/src/test/java/org/apache/camel/component/vertx/websocket/VertxWebsocketConsumerAsClientReconnectTest.java:
##########
@@ -42,26 +45,40 @@ void testReconnect() throws Exception {
context.getRouteController().stopRoute("server");
- // Verify that we cannot send messages
- Exchange exchange = template.send(uri, new Processor() {
- @Override
- public void process(Exchange exchange) throws Exception {
- exchange.getMessage().setBody("Hello World Again");
- }
- });
- Exception exception = exchange.getException();
- Assertions.assertNotNull(exception);
- Assertions.assertInstanceOf(ConnectException.class,
exception.getCause());
+ // Verify that the server is fully down by waiting until sends fail.
+ // The producer endpoint's cached WebSocket may still appear open
briefly
+ // after stopRoute returns, until the Vert.x event loop processes the
+ // TCP close frame and isClosed() starts returning true.
+ await().atMost(10, TimeUnit.SECONDS)
+ .untilAsserted(() -> {
+ Exchange exchange = template.send(uri, new Processor() {
+ @Override
+ public void process(Exchange exchange) throws
Exception {
+ exchange.getMessage().setBody("Hello World Again");
+ }
+ });
+ Assertions.assertNotNull(exchange.getException());
+ Assertions.assertInstanceOf(ConnectException.class,
exchange.getException().getCause());
+ });
// Restart server
context.getRouteController().startRoute("server");
- // Wait for client consumer reconnect
- Thread.sleep(300);
-
- // Verify that the client consumer reconnected
- template.sendBody(uri, "Hello World Again");
- mockEndpoint.assertIsSatisfied();
+ // Wait for client consumer to reconnect and verify end-to-end message
flow.
+ // After the server stops, the client consumer's close handler fires
+ // asynchronously on the Vert.x event loop, starting the reconnect
timer
+ // that connects to the restarted server.
+ // Use ignoreExceptions() to retry through transient failures while
both
+ // the producer and client consumer re-establish their connections.
+ await().atMost(20, TimeUnit.SECONDS)
+ .pollInterval(500, TimeUnit.MILLISECONDS)
+ .ignoreExceptions()
+ .untilAsserted(() -> {
+ mockEndpoint.reset();
+ mockEndpoint.expectedBodiesReceived("Hello World Again");
+ template.sendBody(uri, "Hello World Again");
+ mockEndpoint.assertIsSatisfied(500);
Review Comment:
_Claude Code on behalf of Arnaud Thimel_
Addressed in 2eb8739. Added `mockEndpoint.setResultWaitTime(500)` after
`reset()`. When `expectedCount > 0`, `assertIsSatisfied(long)` ignores the
parameter — the actual latch wait is controlled by `resultWaitTime`. With
`setResultWaitTime(500)`, the inner latch correctly waits 500ms per Awaitility
poll attempt instead of falling back to the 10s default.
##########
components/camel-vertx/camel-vertx-websocket/src/test/java/org/apache/camel/component/vertx/websocket/VertxWebsocketConsumerAsClientMaxReconnectTest.java:
##########
@@ -42,26 +45,35 @@ void testMaxReconnect() throws Exception {
context.getRouteController().stopRoute("server");
- // Verify that we cannot send messages
- Exchange exchange = template.send(uri, new Processor() {
- @Override
- public void process(Exchange exchange) throws Exception {
- exchange.getMessage().setBody("Hello World Again");
- }
- });
- Exception exception = exchange.getException();
- Assertions.assertNotNull(exception);
- Assertions.assertInstanceOf(ConnectException.class,
exception.getCause());
+ // Verify that the server is fully down by waiting until sends fail.
+ // The producer endpoint's cached WebSocket may still appear open
briefly
+ // after stopRoute returns, until the Vert.x event loop processes the
+ // TCP close frame and isClosed() starts returning true.
+ await().atMost(10, TimeUnit.SECONDS)
+ .untilAsserted(() -> {
+ Exchange exchange = template.send(uri, new Processor() {
+ @Override
+ public void process(Exchange exchange) throws
Exception {
+ exchange.getMessage().setBody("Hello World Again");
+ }
+ });
+ Assertions.assertNotNull(exchange.getException());
+ Assertions.assertInstanceOf(ConnectException.class,
exchange.getException().getCause());
+ });
// Wait for client consumer reconnect max attempts to be exhausted
- Thread.sleep(300);
+ // (maxReconnectAttempts=1, reconnectInterval=10ms).
+ // Use pollDelay to give reconnect attempts time to complete before
checking.
+ await().atMost(10, TimeUnit.SECONDS)
+ .pollDelay(2, TimeUnit.SECONDS)
+ .untilAsserted(() -> mockEndpoint.assertIsSatisfied());
Review Comment:
_Claude Code on behalf of Arnaud Thimel_
Addressed in 2eb8739. Replaced the Awaitility `pollDelay` wrapper with
direct `mockEndpoint.assertIsSatisfied(2000)`. Since `expectedMessageCount(0)`
is set, `assertIsSatisfied(2000)` sleeps for 2 seconds then verifies the count
is still zero — exactly the semantics we need, no Awaitility overhead.
--
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]
