Copilot commented on code in PR #24393:
URL: https://github.com/apache/camel/pull/24393#discussion_r3522734858
##########
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:
This Awaitility block is effectively a fixed 2s delay followed by a single
`assertIsSatisfied()` check (the `atMost(10s)` and `untilAsserted` don't add
much here, since `pollDelay` forces the wait and `assertIsSatisfied()` has no
built-in wait after `reset()` for empty endpoints).
Given `expectedMessageCount(0)`, you can make the intent clearer and ensure
the empty-endpoint check actually covers the delay window by directly using
`mockEndpoint.assertIsSatisfied(2000)` (which sleeps and then asserts no
messages were received).
##########
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:
`MockEndpoint.assertIsSatisfied(long)` takes a timeout *only for empty
endpoints* (expectedCount == 0). Here `expectedBodiesReceived(...)` sets
expectedCount > 0, so the `500` value is ignored and the assert may still block
up to MockEndpoint's default wait time (10s) after each `reset()`. This makes
the Awaitility polling behavior misleading and can slow/flakify the test.
Set the mock's `resultWaitTime` after each reset and call
`assertIsSatisfied()` (or use the static `MockEndpoint.assertIsSatisfied(…,
TimeUnit, …)` helper).
--
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]
