Repository navigation
CAMEL-25645: camel-milo - ServerCertificateValidationTest writes the value before the consumers start - #27999
Merged
Merged
Conversation
…value before the consumers start Since CAMEL-25626 a consumer also delivers a null value with a Bad status, so a client that connected before the test wrote Foo received the initial Bad value of the item as well, and mock:trustAll got 2 messages. The two changes were merged on main 26 seconds apart and neither PR build ran with the other. The consumer routes now start after the value is written, as MonitorItemTest does since CAMEL-25626, so Foo is the first notification of each client. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
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: 1 of 706 tested, 0 compile-only — current: 8 all testedMaveniverse Scalpel detected 1 affected modules (current approach: 8). Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules) Modules only in current approach (7)
Modules Scalpel would test (1)
All tested modules (8 modules, 1m 51s total)Total reactor time: 1m 51s
Top 20 slowest modules:
|
4 of 5 tasks
This was referenced Oct 18, 2026
gnodet
added a commit
to gnodet/camel
that referenced
this pull request
Oct 23, 2026
…dundant test changes - Add testAwaitedCallReturnsResult() to CallClientTest to verify that an awaited call returns a non-null CallMethodResult with a good status and the expected output argument (CAMEL-25694 regression test on the real server path) - Add upgrade guide entry for the two observable behaviour changes: * an awaited write/call/read that fails now fails the exchange * an awaited call that succeeds now returns CallMethodResult as the body - Revert comment-only changes to ServerCertificateValidationTest: main already has the autoStartup(false) fix from CAMEL-25645 (apache#27999), so these rewrites only add noise to the commit history
davsclaus
pushed a commit
that referenced
this pull request
Oct 23, 2026
…ted as success (#27956) * CAMEL-25694: camel-milo - fix write/call service failures being reported as success In SubscriptionManager.write(), call(), readValues() and browse(), the futures returned by the underlying Milo client were chained with handleAsync(). When the upstream future completed exceptionally (e.g. Bad_Timeout service fault), the handleAsync() callback ran with a non-null 'e' parameter but returned the result value (null), converting the exceptional completion into a normal completion with a null value. MiloClientProducer awaits the future with whenComplete() and only sets an exchange exception when the throwable is non-null. Since handleAsync() had already discarded the exception, the producer saw a null-completed future, reported success, and set the exchange body to null. The fix replaces handleAsync() with whenCompleteAsync() in all four methods. whenCompleteAsync() runs the side-effect (handleConnectionFailure) but propagates the original completion unchanged: exceptional stays exceptional, success stays success. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: ServerCertificateValidationTest - start consumer routes after initial write OPC UA consumers deliver the current server node value when they subscribe. In the original test, all routes started automatically before the write, so each consumer received two messages: the uninitialised value at subscription time, and the 'Foo' write. The mock expected exactly 1 message, causing a deterministic failure ('Expected: <1> but was: <2>'). Follow the pattern established in MonitorItemTest: mark consumer routes with autoStartup(false), write the initial value first via the server producer, then start all consumer routes. Each consumer then sees only one notification (the value written before it subscribed). * fix: update SubscriptionManager test constructor calls to match new signature * fix: address review - add awaited call test, upgrade guide, revert redundant test changes - Add testAwaitedCallReturnsResult() to CallClientTest to verify that an awaited call returns a non-null CallMethodResult with a good status and the expected output argument (CAMEL-25694 regression test on the real server path) - Add upgrade guide entry for the two observable behaviour changes: * an awaited write/call/read that fails now fails the exchange * an awaited call that succeeds now returns CallMethodResult as the body - Revert comment-only changes to ServerCertificateValidationTest: main already has the autoStartup(false) fix from CAMEL-25645 (#27999), so these rewrites only add noise to the commit history * fix: fix upgrade guide wording - Bad_TypeMismatch is a per-node status, not a transport failure * fix: use correct requestBodyAndHeader signature with endpoint URI in CallClientTest --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
ServerCertificateValidationTest.onlyClientsThatCanValidateTheServerCertificateConnectfails onmain:CAMEL-25626 (#27908) and CAMEL-25645 (#27824) were merged 26 seconds apart on 2026-10-16, and neither PR build ran with the other change. Since CAMEL-25626, a consumer also delivers a null value with a Bad status. So the
trustAllclient, which connects before the test writesFoo, first receives the initialDataValue[value=null, statusCode=Bad]ofmilo-server:myitem1. The main build does not run the camel-milo tests, so it did not catch this. The failure shows up on every PR that touches camel-milo (for example #27910 and #27906).The consumer routes are now
autoStartup(false), and the test starts them after it writes the value, asMonitorItemTestdoes since CAMEL-25626.Foois then the first notification of every client that connects, and the expectations do not change. Test-only change.Test plan
ServerCertificateValidationTestfails onmain(55ed20f) and passes with this change (3 runs)camel-milotest suite: 74 run, 0 failuresClaude Code on behalf of Claus Ibsen
🤖 Generated with Claude Code