Remove redundant Awaitility wrapping around MockEndpoint assertions#24509
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
compilation error: |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 35 tested, 0 compile-only — current: 54 all testedMaveniverse Scalpel detected 35 affected modules (current approach: 54).
|
MockEndpoint.assertIsSatisfied() already uses an internal CountDownLatch that waits up to 10 seconds by default, making Awaitility wrapping redundant. This replaces all 58 instances across 49 test files with native MockEndpoint timed assertions: - For timeouts <= 10s: plain assertIsSatisfied() (uses built-in 10s default) - For timeouts > 10s on instance calls: setResultWaitTime() + assertIsSatisfied() - For timeouts > 10s on static calls: assertIsSatisfied(context, N, TimeUnit) Awaitility/Duration/TimeUnit imports are cleaned up where no longer used elsewhere in the file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Awaitility's untilAsserted() internally caught InterruptedException, so test methods didn't need a throws clause. After unwrapping to direct MockEndpoint.assertIsSatisfied() calls, the checked exception must be declared. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert files where Awaitility provided essential semantics beyond wrapping assertIsSatisfied: - 12 file consumer tests: pollDelay is critical for negative assertions (expectedMessageCount(0)). Without the delay, assertIsSatisfied() returns immediately when 0 messages are expected, making the test pass vacuously without proving the consumer didn't pick up the file. - JmsReplyToLoopIssueTest: atMost(250ms) is deliberately short for loop detection — fast fail if an endless reply loop keeps producing messages. - MllpIdleTimeoutStrategyTest: pollInterval(500ms) retry semantics were part of the test design. - MulticastParallelStreamingTimeoutTest: rapid-poll design with setResultWaitTime(100) + Awaitility retry was intentional (CAMEL-23901) to avoid blocking 20s on a single attempt. Also fixes missing throws Exception on JmsDurableTopicIT.testDurableTopic(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Additional reverts after thorough audit of all remaining modified files. These tests use Awaitility with expectedMessageCount(0) (negative tests) or atLeast semantics where removing Awaitility makes the test vacuous: - FromFtpNoFilesIT: atMost(3s) with expectedCount(0) - verifies no files consumed - FtpConsumerDoneFileNameFixedIT: atMost(1s) with expectedCount(0) - done-file filter - JmsAutoStartupTest: atMost(2s) with expectedCount(0) - consumer stopped - LevelDBAggregateDiscardOnTimeoutTest: atMost(3s) with expectedCount(0) - discard timeout - MongoDbTailableCursorConsumerIT: atMost(1s) with expectedCount(0) - empty collection - MyBatisConsumerIdleMessageTest: atLeast(110ms) - minimum wait for idle polling - QuartzAutoStartTest: atMost(2s) with expectedCount(0) - scheduler not started - QuartzRouteRestartTest: atMost(2s) with expectedCount(0) - route stopped - SpringQuartzPersistentStoreRestartRouteTest: atMost(2s) with expectedCount(0) - same Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
assertIsSatisfied() throws InterruptedException which was previously handled internally by Awaitility's untilAsserted(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a886cfc to
68bac0f
Compare
Summary
Claude Code on behalf of gnodet
MockEndpoint.assertIsSatisfied()already uses an internalCountDownLatchthat waits up to 10 seconds by default (seeMockEndpoint.javaline 1997:waitTime = timeout == 0 ? 10000L : timeout), making Awaitility wrapping redundant when it only wrapsassertIsSatisfied()without additional Awaitility features.This PR replaces redundant Awaitility-wrapped MockEndpoint assertions across 25 test files with native MockEndpoint timed assertions:
assertIsSatisfied()(uses built-in 10s default)setResultWaitTime(millis)+assertIsSatisfied()assertIsSatisfied(context, N, TimeUnit.SECONDS)Awaitility/Duration/TimeUnit imports are cleaned up where no longer used elsewhere in the file. Also adds missing
throws Exceptionclauses on test methods that previously relied on Awaitility's internal exception handling.Preserved Awaitility usages (24 files NOT changed)
Files were explicitly excluded where Awaitility provides essential semantics beyond wrapping assertIsSatisfied:
expectedMessageCount(0)):pollDelay/atMostprovides mandatory wait before checking that a consumer did NOT pick up a file/message — without it,assertIsSatisfied()returns immediately and the test passes vacuously. Affects 19 file consumer, FTP, JMS, Quartz, LevelDB, and MongoDB tests.atMost(250ms)deliberately short for fast-fail (JmsReplyToLoopIssueTest)atLeast(110ms)for idle polling semantics (MyBatisConsumerIdleMessageTest)pollInterval(500ms)retry design (MllpIdleTimeoutStrategyTest)setResultWaitTime(100)+ Awaitility retry to avoid 20s blocking (MulticastParallelStreamingTimeoutTest, CAMEL-23901)Related: follows the guidance documented in #24496.
Affected modules
camel-core,camel-file-watch,camel-jms,camel-sjms,camel-kafka,camel-ftp,camel-mina,camel-netty,camel-snmp,camel-chatscript,camel-zeebe,camel-aws2-s3,camel-aws2-kinesisTest plan
core/camel-coretests pass (file, processor, throttle tests)camel-quartz— 3 tests passcamel-snmp— 3 tests passcamel-leveldb— 2 tests passcamel-netty— 1 test passescamel-file-watch— tests passcamel-mybatis— tests pass🤖 Generated with Claude Code