CAMEL-23889: Fix flaky camel-vertx-websocket surefire tests#24393
Merged
gnodet merged 4 commits intoJul 4, 2026
Merged
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 tested, 29 compile-only — current: 10 all testedMaveniverse Scalpel detected 39 affected modules (current approach: 10).
|
gnodet
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets intermittent surefire flakiness in the camel-vertx-websocket module by making asynchronous WebSocket/reconnect assertions deterministic and correcting an overly-long test timeout.
Changes:
- Replaced
Thread.sleep(...)-based timing with Awaitility-based polling in reconnect-related tests. - Corrected
VertxWebSocketEventTesttimeout from5000seconds to5seconds. - Added Awaitility as a test-scoped dependency for
camel-vertx-websocket.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| components/camel-vertx/camel-vertx-websocket/src/test/java/org/apache/camel/component/vertx/websocket/VertxWebSocketEventTest.java | Fixes an incorrectly large CompletableFuture.get(...) timeout to avoid masking failures. |
| components/camel-vertx/camel-vertx-websocket/src/test/java/org/apache/camel/component/vertx/websocket/VertxWebsocketConsumerAsClientReconnectTest.java | Replaces sleeps with Awaitility to stabilize “server down” and reconnect assertions. |
| components/camel-vertx/camel-vertx-websocket/src/test/java/org/apache/camel/component/vertx/websocket/VertxWebsocketConsumerAsClientMaxReconnectTest.java | Uses Awaitility instead of sleep around reconnect exhaustion, but the final “no messages after restart” check still needs a stronger assertion window. |
| components/camel-vertx/camel-vertx-websocket/pom.xml | Adds Awaitility as a test dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace Thread.sleep() calls with Awaitility-based async assertions in the vertx-websocket reconnect tests, and fix an incorrect timeout value in VertxWebSocketEventTest. Changes: - VertxWebsocketConsumerAsClientReconnectTest: Replace Thread.sleep(300) with Awaitility await().untilAsserted() using fresh WebSocket connections to bypass stale producer endpoint cache after server restart. - VertxWebsocketConsumerAsClientMaxReconnectTest: Replace Thread.sleep(300) with Awaitility during(2, SECONDS) pattern to reliably verify reconnect attempts are exhausted. - VertxWebSocketEventTest: Fix timeout from 5000 seconds (83 minutes) to 5 seconds in webSocketFuture.get() call. - Add Awaitility test dependency to camel-vertx-websocket pom.xml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The during().untilAsserted() pattern was correct but misleading — with expectedMessageCount(0), assertIsSatisfied() passes immediately on every poll, so during() was effectively just a timed wait. Use pollDelay() instead which more clearly expresses the intent: wait for reconnect attempts to exhaust, then verify. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After stopRoute("server"), the producer endpoint's cached WebSocket may
still appear open (isClosed() returns false) until the Vert.x event loop
processes the TCP close frame. Wrap the "verify that we cannot send
messages" assertion in Awaitility to wait until the close propagates,
and restore full end-to-end Camel flow in the reconnect test.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
force-pushed
the
fix-flaky-camel-vertx-websocket-surefire-tests-on
branch
from
665d667 to
f0d4776
Compare
davsclaus
approved these changes
Jul 4, 2026
- ReconnectTest: assertIsSatisfied(500) timeout was silently ignored when expectedCount > 0 (defaults to 10s latch wait). Use setResultWaitTime(500) to control the actual wait time. - MaxReconnectTest: Replace Awaitility pollDelay wrapper with direct assertIsSatisfied(2000) for the zero-count wait (simpler, equivalent). Use setAssertPeriod(1000) for final assertion to re-verify after a delay and catch late deliveries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
Claude Code on behalf of Guillaume Nodet Addressed all three Copilot review comments: ReconnectTest MaxReconnectTest Awaitility wrapper (line 69): Agreed the Awaitility MaxReconnectTest final assertion (line 76): Replaced |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix intermittent surefire test failures in
camel-vertx-websocketobserved on both JDK 17 and JDK 25 CI runs (see PR #22300 CI failures).JIRA: CAMEL-23889
Root causes
Stale WebSocket cache after server stop:
VertxWebsocketEndpoint.getWebSocket()caches the WebSocket connection. AfterstopRoute("server"),isClosed()may still returnfalseuntil the Vert.x event loop processes the TCP close frame. This causes two race conditions:template.send()silently drops the message through the stale WebSocket instead of throwingConnectException.template.sendBody()silently loses messages through the stale cache.Thread.sleep(300)for async operations: Both reconnect tests usedThread.sleep(300)to wait for asynchronous Vert.x operations (close handler, reconnect timer), which is inherently unreliable.Incorrect timeout value:
VertxWebSocketEventTestusedwebSocketFuture.get(5000, TimeUnit.SECONDS)— a 83-minute timeout that could mask test failures.Fixes
VertxWebsocketConsumerAsClientReconnectTest: ReplacedThread.sleep(300)with Awaitility. Wrapped the "verify send fails" check inawait().untilAsserted()to handle the stale WebSocket race. Usedawait().ignoreExceptions().untilAsserted()for reconnection verification, retrying through transient failures while both the producer and client consumer re-establish their connections. Tests full end-to-end Camel message flow.VertxWebsocketConsumerAsClientMaxReconnectTest: Same "verify send fails" Awaitility fix. ReplacedThread.sleep(300)withawait().pollDelay(2, SECONDS).untilAsserted()to reliably wait for reconnect attempts to exhaust before verifying the mock stays at 0 messages.VertxWebSocketEventTest: Fixed timeout from5000seconds to5seconds.awaitilityas a test dependency tocamel-vertx-websocket.Test plan
camel-vertx-websocketpass withrerunFailingTestsCount=0mvn formatter:format impsort:sortClaude Code on behalf of Guillaume Nodet
🤖 Generated with Claude Code