chore: camel-smb - fix SmbConsumerStartingDirectoryMustExistIT for route-start exception wrapping - #25903
Conversation
…ute-start exception wrapping CAMEL-24404 (#25554) made InternalRouteStartupManager always wrap consumer startup failures in FailedToStartRouteException. This test still caught the raw GenericFileOperationFailedException around context.start(), so it no longer matched and the test failed on every CI run since that change merged. Assert on FailedToStartRouteException with the original exception as cause, matching how the other consumer-startup tests were updated for CAMEL-24404. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@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: 9 tested, 27 compile-only — current: 9 all testedMaveniverse Scalpel detected 36 affected modules (current approach: 9).
|
gnodet
left a comment
There was a problem hiding this comment.
Clean, correct test fix for a CI failure caused by CAMEL-24404's exception wrapping change (#25554). The assertion pattern matches what PR #25554 used for the seven other modules it updated, and the assertj-core dependency addition is properly scoped.
Observations (non-blocking):
- Verified that no other tests in
camel-smbare affected — the otherGenericFileOperationFailedExceptionreferences deal with exchange/producer-level exceptions (extracted fromCamelExecutionException.getCause()), not consumer startup failures fromcontext.start(). - The assertion chain checks
.cause().hasMessage(...)without also asserting the cause type isGenericFileOperationFailedException. Adding.cause().isInstanceOf(GenericFileOperationFailedException.class)would be marginally more precise, but the message string is specific enough to avoid false positives.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Labels | components |
→ test (test-only change, no component code modified) |
| Milestone | (none) | 4.23.0 |
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
Summary
SmbConsumerStartingDirectoryMustExistIT.testStartingDirectoryMustExisthas been failing onmainsince CAMEL-24404 (#25554) merged. That change madeInternalRouteStartupManager.doStartOrResumeRouteConsumers()always wrap consumer startup failures inFailedToStartRouteException.This test still did:
Since
context.start()now throwsFailedToStartRouteException(wrapping the originalGenericFileOperationFailedExceptionas its cause), thecatchclause no longer matched, so the exception propagated uncaught and the test failed with an error on every run — spotted as a CI failure on an unrelated PR (#25892).#25554already updated 7 other modules for this same wrapping change (DefaultSupervisingRouteControllerTest,MainSupervisingRouteControllerTest,AiToolEndpointLifecycleTest, etc.) but missed this SMB test since it lives outside that PR's scope.Fix
Rewritten with AssertJ's
assertThatThrownBy, assertingFailedToStartRouteExceptionas the outer type and checking the original message via.cause(), matching the pattern used for the other tests updated by CAMEL-24404.assertj-corewas added as a test dependency tocamel-smb/pom.xml(version centrally managed in the parent POM) since it wasn't previously present in this module.I checked the rest of
camel-smb's tests for the sametry { context.start(); fail(); } catch (...)pattern — this was the only one affected.Test plan
mvn -o compile test-compileincomponents/camel-smb— compiles cleanmvn -o verify -Dit.test=SmbConsumerStartingDirectoryMustExistIT— passesmvn formatter:format impsort:sort— no changes neededClaude Code on behalf of davsclaus