CAMEL-23903: Fix flaky test RouteIdTransactedIT in camel-jms#24420
Conversation
Replace MockEndpoint.assertIsSatisfied(context) with Awaitility's untilAsserted() in both testRouteId() and testRouteIdFailed() to provide a generous 30-second timeout for transacted JMS message processing, which can exceed the default 10-second latch timeout under CI load on JDK 17. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Guillaume Nodet <gnodet@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 12 all testedMaveniverse Scalpel detected 1 affected modules (current approach: 12). Modules only in current approach (11)
Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules) Modules Scalpel would test (1)
All tested modules (12 modules)
|
There was a problem hiding this comment.
Pull request overview
This PR addresses flakiness in the RouteIdTransactedIT JMS transacted integration tests by replacing a single-shot MockEndpoint.assertIsSatisfied(context) assertion with an Awaitility-based retry window, aiming to better tolerate CI timing variability around transacted processing and onException handling.
Changes:
- Add Awaitility
untilAsserted()around mock assertions intestRouteIdandtestRouteIdFailed. - Introduce
TimeUnitand Awaitility static import to support the new wait logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await().atMost(30, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> MockEndpoint.assertIsSatisfied(context)); |
There was a problem hiding this comment.
Done — switched to MockEndpoint.assertIsSatisfied(context, 30, TimeUnit.SECONDS) in e38a078.
| await().atMost(30, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> MockEndpoint.assertIsSatisfied(context)); |
There was a problem hiding this comment.
Done — same fix applied here.
| import org.junit.jupiter.api.TestMethodOrder; | ||
| import org.springframework.context.support.ClassPathXmlApplicationContext; | ||
|
|
||
| import static org.awaitility.Awaitility.await; |
There was a problem hiding this comment.
Done — Awaitility import removed.
Address Copilot review: use MockEndpoint.assertIsSatisfied(context, 30, TimeUnit.SECONDS) which sets the timeout directly on the mock's internal latch, rather than wrapping with Awaitility which only retries the default 10-second wait. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Guillaume Nodet <gnodet@gmail.com>
Summary
Fix flaky
RouteIdTransactedIT.testRouteIdFailed(andtestRouteIdfor consistency) by replacingMockEndpoint.assertIsSatisfied(context)withMockEndpoint.assertIsSatisfied(context, 30, TimeUnit.SECONDS).Root cause: The default
MockEndpointlatch timeout is 10 seconds (waitForCompleteLatchdefaults to 10s whenresultWaitTime=0). In transacted JMS routes under CI load on JDK 17, the message processing (including transaction handling and exception routing viaonException) can exceed this timeout, causing intermittent test failures.Fix: Use the
MockEndpoint.assertIsSatisfied(context, 30, TimeUnit.SECONDS)overload which setsresultWaitTimedirectly on each mock's internal latch, giving transacted routes a generous 30-second window. This follows the same pattern already used in other camel-jms tests (e.g.,JmsProducerConcurrentWithReplyTest,JmsInOnlyWithReplyToHeaderTopicTest).Test plan
RouteIdTransactedITpasses (2 tests, 0 failures)Claude Code on behalf of Guillaume Nodet
🤖 Generated with Claude Code