mw/com: Add the integration tests for moving proxyEvent by sahithi-nukala · Pull Request #607 · eclipse-score/communication

sahithi-nukala · 2026-06-29T04:57:39Z

Adding the integration tests that verify a Proxy (and its ProxyEvent) can be moved without disrupting service discovery, subscription, or sample reception.

Issue: #483

bemerybmw · 2026-07-15T17:10:42Z

+                 const std::size_t num_send_iterations,
+                 const score::cpp::stop_token& stop_token)
+{
+    const auto moved_to_name = filesystem::Path{kInstanceSpecifierMovedTo.ToString()}.Filename().Native();


Do we need to add moved_to_name to kInterprocessNotificationShmPath?

bemerybmw · 2026-07-15T17:18:37Z

+        const auto initial_value = static_cast<std::uint32_t>(iteration * num_samples_to_send) + 1U;
+        std::cout << "\nProvider: Iteration " << (iteration + 1U) << " of " << num_send_iterations << " - Send "
+                  << num_samples_to_send << " samples" << std::endl;
+        SendSamples(skeleton_container.GetSkeleton(), num_samples_to_send, initial_value);


If you want to extract this into a function, then the name should be more descriptive IMO. Maybe something like SendIncrementingSequenceOfSamples. Otherwise passing in "initial_value" is a bit strange to me

yes updated to SendIncrementingSequenceOfSamples

SendIncrementingSequenceOfSamples is used in both proxy_event/provider.cpp and skeleton_event/provider.cpp with identical logic, only the skeleton type differs. To avoid the duplication, I've extracted it as a header-only template function into common_test_resources/send_incrementing_sequence_of_samples.h

bemerybmw · 2026-07-15T17:24:09Z

+
+    // Step 2. Move construct proxy before subscribe
+    std::cout << "\nConsumer: Step 2 - Move construct proxy before subscribe" << std::endl;
+    auto moved_proxy = proxy_container.Extract();


IMO, it's better to make the move construction explicit like in the skeleton move test:

skeleton_container.CreateSkeleton(kInstanceSpecifierMovedTo, "skeleton_event_move_semantics"); auto original_skeleton = skeleton_container.Extract(); // Step 2. Move construct skeleton std::cout << "\nProvider: Step 2 - Move construct skeleton" << std::endl; auto moved_to_skeleton = std::move(original_skeleton);

bemerybmw · 2026-07-15T17:25:29Z

+namespace score::mw::com::test
+{
+
+inline auto MakeSampleSequenceCallback(std::optional<std::uint32_t>& latest_value, const char* failure_message_prefix)


IMO, these functions are quite specific to the move semantic tests, so I wouldn't put them in the test resources.

correct, I'm moving this MakeSampleSequenceCallback to consumer.cpp under proxy_event

bemerybmw · 2026-07-15T17:33:55Z

+    for (std::size_t iteration = 0U; iteration < num_iterations; ++iteration)
+    {
+        std::cout << "\nConsumer: Iteration " << (iteration + 1U) << " of " << num_iterations << std::endl;
+        const bool subscribed =


Why are you waiting for a state change on every iteration?

in all 4 scenarios ReceiveAndNotify is always called with num_iterations=1 so the loop only ever runs once per call WaitForStateChange is not actually being called repeatedly across iterations. the call is needed because ReceiveAndNotify is also invoked after ResubscribeAcrossReoffer, where the consumer has re-subscribed but the provider hasn't re-offered yet, so the state is still kSubscriptionPending and we need to wait for kSubscribed before receiving samples. For the earlier calls where the service is already offered, WaitForStateChange returns immediately since it checks the current state first

bemerybmw · 2026-07-15T17:41:16Z

+
+    // Step 6. Wait for provider to send the first batch of values and notify
+    std::cout << "\nConsumer: Step 6 - Receive first batch of samples" << std::endl;
+    ReceiveAndNotify(proxy_event_receiver,


In general, I don't think it's a good idea to extract too much actual test logic. We want the test sequence to be as clear as possible. Something like proxy_state_change_notifier.WaitForStateChange or proxy_event_receiver.WaitForSamples are basically doing one thing and it's completely clear from the name what that is. Here, looking at the name and the args, it's not really clear what it does.
e.g. the name doesn't hint that it's also checking the subscription state (which I'm not sure it should be there at all), it's not really clear that it will receive samples, notify and then reset the process_synchronizer and then repeat that multiple times. IMO, that logic should be here in the main test sequence or it for example you could make it simpler like ReceiveSamplesAndNotify but do the loop here and reset the process_synchronizer here. But tbh at that point, I don't see much point in the function. I also think the fact that the function takes 6 params and has 3 template params is a bit of a code smell.

This is also not an exact science but is a bit of developer preference. We can also get an outside opinion. @limdor what do you think?

bemerybmw · 2026-07-15T17:43:06Z

+
+    // Step 7. Unsubscribe and subscribe again across the provider's re-offer
+    std::cout << "\nConsumer: Step 7 - Unsubscribe and subscribe again" << std::endl;
+    ResubscribeAcrossReoffer(moved_proxy.moved_event_,


similar to my comments above. This should either be made clearer or put here in the main.

bemerybmw · 2026-07-15T17:53:41Z

+{
+    std::cout << "\nConsumer: Waiting for provider to withdraw its offer" << std::endl;
+    const bool withdrawn =
+        proxy_state_change_notifier.WaitForStateChange(stop_token, SubscriptionState::kSubscriptionPending);


kSubscriptionPending shouldn't really be used to determine whether a provider is alive or not. I think you should probably use a ProcessSynchronizer instead. Although to be honest, I think it would also be fine to unsubscribe and subscribe without the provider reoffering. Is there a particular reason why you want to wait for the skeleton to reoffer?

bemerybmw · 2026-07-15T17:56:50Z

+        ProxyEventReceiver proxy_event_receiver{
+            original_proxy.moved_event_,
+            MakeSampleSequenceCallback(latest_value, "proxy_event_move_semantics consumer failed:")};
+        ProxyEventStateChangeNotifier proxy_state_change_notifier{original_proxy.moved_event_};


We want to make sure that the set receive handler is not affected when moving. so this should not be destroyed and we shouldn't create a new one.

bemerybmw · 2026-07-15T17:57:40Z

+    auto process_synchronizer_result =
+        ProcessSynchronizer::CreateUniquePtr(kInterprocessNotificationShmPath + std::string{moved_to_name});
+
+    // Step 1. Create and offer the skeleton the consumer receives samples from. A second instance is also offered so


You could have both proxy instances connect to the same skeleton?

The second skeleton (MovedFrom) is necessary it gives the placeholder proxy its own shm so the MovedTo lifecycle is unaffected

github-project-automation Bot added this to COM - Communication FT Jun 29, 2026

github-project-automation Bot moved this to Backlog in COM - Communication FT Jun 29, 2026

sahithi-nukala force-pushed the sah_proxy_event_move_tests branch from 544058d to 49a2440 Compare June 29, 2026 07:10

bemerybmw reviewed Jun 29, 2026

View reviewed changes

Comment thread score/mw/com/test/common_test_resources/proxy_container.h Outdated

bemerybmw reviewed Jun 29, 2026

View reviewed changes

Comment thread score/mw/com/test/proxy_event_move_semantics/consumer.cpp Outdated

bemerybmw reviewed Jun 29, 2026

View reviewed changes

Comment thread score/mw/com/test/proxy_event_move_semantics/consumer.cpp Outdated

sahithi-nukala force-pushed the sah_proxy_event_move_tests branch 7 times, most recently from f486d86 to c755a1c Compare July 3, 2026 06:08

sahithi-nukala marked this pull request as ready for review July 3, 2026 06:24

sahithi-nukala requested review from LittleHuba, castler, crimson11, hoe-jo and limdor as code owners July 3, 2026 06:24

sahithi-nukala requested a review from bemerybmw July 3, 2026 09:26

bemerybmw requested changes Jul 15, 2026

View reviewed changes

sahithi-nukala marked this pull request as draft July 16, 2026 04:27

sahithi-nukala force-pushed the sah_proxy_event_move_tests branch 7 times, most recently from 47c99b8 to f9277ff Compare July 27, 2026 06:07

sahithi-nukala force-pushed the sah_proxy_event_move_tests branch from f9277ff to b3d31a0 Compare July 27, 2026 06:50

sahithi-nukala force-pushed the sah_proxy_event_move_tests branch from b3d31a0 to 2909973 Compare July 27, 2026 08:55

sahithi-nukala added the test-qnx Enables the checks with QNX toolchains. Checks require committer approval to start execution. label Jul 27, 2026

sahithi-nukala requested a deployment to workflow-approval July 27, 2026 09:53 — with GitHub Actions Waiting

sahithi-nukala added 2 commits July 27, 2026 16:26

mw/com: Extend common test helpers to support proxy move-semantics tests

d2350a7

mw/com: Add proxyEvent move-semantics integration tests

66e4c14

sahithi-nukala force-pushed the sah_proxy_event_move_tests branch from 2909973 to 66e4c14 Compare July 27, 2026 10:57

Uh oh!

Conversation

sahithi-nukala commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sahithi-nukala commented Jun 29, 2026 •

edited

Loading