Skip to content

CAMEL-24026: Fix flaky XsltFromFileExceptionTest - #25334

Merged
davsclaus merged 2 commits into
apache:mainfrom
atiaomar1978-hub:cursor/CAMEL-24026-xslt-flaky-test-c587
Aug 5, 2026
Merged

CAMEL-24026: Fix flaky XsltFromFileExceptionTest#25334
davsclaus merged 2 commits into
apache:mainfrom
atiaomar1978-hub:cursor/CAMEL-24026-xslt-flaky-test-c587

Conversation

@atiaomar1978-hub

Copy link
Copy Markdown
Contributor

Summary

Fixes CAMEL-24026: XsltFromFileExceptionTest is flaky on slow CI when the file consumer has not finished before assertions run.

Why #24625 was reverted: reordering to wait on oneExchangeDone first still failed — the default 10s notify latch timed out (Exchange should have completed was false). Simply swapping assertion order was insufficient.

This fix (different approach):

  • Use assertMockEndpointsSatisfied(30, SECONDS) so mock waits generously for the file consumer route to complete
  • Poll file move completion with Awaitility (ok/ / error/ directories) instead of oneExchangeDone
  • Drop reliance on the global oneExchangeDone notify latch, which races with producer vs consumer exchanges

Test plan

  • ./mvnw -pl core/camel-core -am test -Dtest=XsltFromFileExceptionTest

AI-generated PR description on behalf of atiaomar1978-hub

…file poll

PR apache#24625 reordered oneExchangeDone before mock assertions but still
flaked when the 10s notify latch timed out on slow CI. Use a 30-second
MockEndpoint assertion for route completion and Awaitility to poll until
files are moved to ok/error, instead of relying on oneExchangeDone.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@atiaomar1978-hub

Copy link
Copy Markdown
Contributor Author

Fix summary

Addresses CAMEL-24026 — flaky XsltFromFileExceptionTest.

Why #24625 was reverted

PR #24625 reordered to wait on oneExchangeDone before mock assertions. It was reverted in #24945 because CI still saw failures (Exchange should have completed was false). Simply swapping order was not enough.

Why this fix is different

Approach Problem
Original 10s mock wait before file consumer finishes
#24625 oneExchangeDone unreliable — producer write to fileUri() can satisfy the global whenDone(1) notify before the consumer route runs
This PR 30s assertMockEndpointsSatisfied for consumer route + Awaitility polling for file move to ok//error/

Verified

./mvnw -pl core/camel-core -am test -Dtest=XsltFromFileExceptionTest

AI-generated comment on behalf of atiaomar1978-hub

Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
@atiaomar1978-hub

Copy link
Copy Markdown
Contributor Author

Review follow-up (BugBot + Grok)

BugBot: No issues in this test change (unrelated TUI findings are on fork main, not this branch).

Grok: Fix approach is sound — timed mock wait + Awaitility for post-route file move. Applied feedback: added inline comment explaining why oneExchangeDone must not be used here.

Note: XPathFromFileExceptionTest has the same legacy pattern; out of scope for this ticket but could be hardened similarly in a follow-up.


AI-generated comment on behalf of atiaomar1978-hub

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions github-actions Bot added the core label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • core/camel-core

🔬 Scalpel shadow comparison — Scalpel: 24 tested, 0 compile-only — current: 0 all tested

Maveniverse Scalpel detected 24 affected modules (current approach: 0).

⚠️ Modules only in Scalpel (24)
  • camel-console
  • camel-core
  • camel-csimple-joor
  • camel-datasonnet
  • camel-groovy
  • camel-hl7
  • camel-jactl
  • camel-javascript
  • camel-joor
  • camel-jq
  • camel-jsonpath
  • camel-main
  • camel-management
  • camel-mvel
  • camel-ognl
  • camel-platform-http-jolokia
  • camel-python
  • camel-saxon
  • camel-spring-xml
  • camel-stax
  • camel-xml-io-dsl
  • camel-xml-jaxb-dsl
  • camel-xml-jaxb-dsl-test-definition
  • camel-xml-jaxb-dsl-test-spring

Skip-tests mode would test 24 modules (1 direct + 23 downstream), skip tests for 0 (generated code, meta-modules)

Modules Scalpel would test (24)
  • camel-console
  • camel-core
  • camel-csimple-joor
  • camel-datasonnet
  • camel-groovy
  • camel-hl7
  • camel-jactl
  • camel-javascript
  • camel-joor
  • camel-jq
  • camel-jsonpath
  • camel-main
  • camel-management
  • camel-mvel
  • camel-ognl
  • camel-platform-http-jolokia
  • camel-python
  • camel-saxon
  • camel-spring-xml
  • camel-stax
  • camel-xml-io-dsl
  • camel-xml-jaxb-dsl
  • camel-xml-jaxb-dsl-test-definition
  • camel-xml-jaxb-dsl-test-spring

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

⚠️ Some tests are disabled on GitHub Actions (@DisabledIfSystemProperty(named = "ci.env.name")) and require manual verification:

  • core/camel-core: 2 test(s) disabled on GitHub Actions
All tested modules (24 modules)
  • Camel :: Console
  • Camel :: Core
  • Camel :: DataSonnet
  • Camel :: Groovy
  • Camel :: HL7
  • Camel :: JQ
  • Camel :: JSon Path
  • Camel :: Jactl
  • Camel :: JavaScript
  • Camel :: MVEL
  • Camel :: Main
  • Camel :: Management
  • Camel :: OGNL (deprecated)
  • Camel :: Platform HTTP :: Jolokia
  • Camel :: Python
  • Camel :: Saxon
  • Camel :: Spring XML
  • Camel :: StAX
  • Camel :: XML DSL Jaxb :: Test :: Definition
  • Camel :: XML DSL Jaxb :: Test :: Spring
  • Camel :: XML DSL with camel-xml-io
  • Camel :: XML DSL with camel-xml-jaxb
  • Camel :: csimple jOOR (deprecated)
  • Camel :: jOOR

⚙️ View full build and test results

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-reasoned fix for a flaky test with clear prior-attempt analysis.

Root cause: oneExchangeDone is a global NotifyBuilder that fires on the first completed exchange of any kind. Since template.sendBodyAndHeader(fileUri(), ...) itself completes an exchange (the producer write), oneExchangeDone can fire before the file consumer route even picks up the file — making the previous assertion order racy.

Fix: Eliminates the latch entirely and uses the right synchronization:

  1. assertMockEndpointsSatisfied(30, SECONDS) — confirms route processing via MockEndpoint's internal CountDownLatch
  2. Awaitility — polls for the asynchronous file move that occurs after route completion

This two-phase wait correctly models the two asynchronous steps.

Conventions followed correctly:

  • assertMockEndpointsSatisfied(timeout, unit) used directly, not wrapped in Awaitility
  • Awaitility reserved for the non-mock filesystem condition
  • No Thread.sleep introduced
  • public removed from test class and methods per JUnit 5 convention
  • Import ordering correct per impsort-maven-plugin configuration

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of @gnodet

@davsclaus davsclaus added the test label Aug 5, 2026
@davsclaus
davsclaus merged commit f8beba3 into apache:main Aug 5, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants