Implement integration tests for fields with set and notifier by sahithi-nukala · Pull Request #463 · eclipse-score/communication

sahithi-nukala · 2026-05-21T08:56:10Z

Move proxy and skeleton containers from methods to common test resources because it would be used by both fields and methods.
Update traits and traits tests for field get/set/notifier parameters.
Restructure field initial value integration tests and add set and notifier scenarios

bemerybmw · 2026-05-29T12:24:48Z

We usually consumer / provider instead of client / service. Could you please follow the same naming convention?

bemerybmw · 2026-05-29T12:35:50Z

+    using Parameters = score::mw::com::test::SctfTestRunner::RunParameters::Parameters;
+
+    const std::vector<Parameters> allowed_parameters{Parameters::CYCLE_TIME, Parameters::MODE};
+    score::mw::com::test::SctfTestRunner test_runner(argc, argv, allowed_parameters);


SctfTestRunner is very old and we don't use it for new tests. Please look at how we do config parsing here: https://github.com/eclipse-score/communication/blob/brem_methods_sctfs/score/mw/com/test/methods/multiple_proxies/main_provider.cpp

bemerybmw · 2026-05-29T12:36:20Z

+
+    const std::vector<Parameters> allowed_parameters{Parameters::CYCLE_TIME, Parameters::MODE};
+    score::mw::com::test::SctfTestRunner test_runner(argc, argv, allowed_parameters);
+    const auto& run_parameters = test_runner.GetRunParameters();


Need to register a sig term handler like in https://github.com/eclipse-score/communication/blob/brem_methods_sctfs/score/mw/com/test/methods/multiple_proxies/main_provider.cpp

bemerybmw · 2026-05-29T12:42:25Z

+        return -17;
+    }
+
+    while (!stop_token.stop_requested())


Use ProcessSynchronizer instead of a loop on the stop token like in

communication/score/mw/com/test/methods/basic_acceptance_test/provider.cpp

Line 43 in f6a14ef

auto process_synchronizer_result = ProcessSynchronizer::Create(kInterprocessNotificationShmPath);

bemerybmw · 2026-05-29T12:43:04Z

+    }
+    auto instance_specifier = std::move(instance_specifier_result).value();
+
+    auto service_result = SetEnabledSkeleton::Create(std::move(instance_specifier));


Use SkeletonContainer instead of doing this yourself

bemerybmw · 2026-05-29T12:45:29Z

+
+    while (!stop_token.stop_requested())
+    {
+        std::this_thread::sleep_for(cycle_time);


We don't need cycle_time to be passed in from python, we only want to pass in params that need to be changed when calling the binary from python (e.g. mode). Anyway, this won't be needed when using ProcessSynchronizer.

bemerybmw · 2026-05-29T12:46:51Z

+    if (!instance_specifier_result.has_value())
+    {
+        std::cerr << "Unable to create instance specifier, terminating\n";
+        return -3;


Instead of returning error codes and propagating this up through all the layers, we should instead use FailTest to immediately fail the test e.g.

communication/score/mw/com/test/methods/basic_acceptance_test/provider.cpp

Line 64 in f6a14ef

FailTest("Provider: Failed to register with_in_args_and_return handler");

Same applies for the consumer

bemerybmw · 2026-05-29T12:48:26Z

+    auto instance_specifier = std::move(instance_specifier_result).value();
+
+    std::promise<std::vector<InitialOnlyProxy::HandleType>> service_discovery_promise{};
+    auto service_discovery_future = service_discovery_promise.get_future();


Use ProxyContainer

bemerybmw · 2026-05-29T14:11:06Z

+};
+
+template <typename T>
+class SetEnabledInterface : public T::Base


We should have one class per file.

bemerybmw · 2026-05-29T14:11:32Z

+namespace score::mw::com::test
+{
+
+constexpr const char* const kInstanceSpecifierString = "test/fields/set_and_notifier";


The shared constants should be in a different file, not with the interface classes.

bemerybmw · 2026-05-29T14:13:19Z

+    return target.wrap_exec("bin/main_service", args, cwd="/opt/MainServiceApp", **kwargs)
+
+
+def test_field_notifier_initial_value(target):


We should have different python files per test configuration (e.g. https://github.com/eclipse-score/communication/tree/f6a14ef6f69482b404ec9392067fdd67c5126d51/score/mw/com/test/methods/mixed_criticality/integration_test). The configurations needed are mentioned in the ticket.

Separate files per configuration have been added for 2 cases from the ticket. and for the 4 configurations requiring getter support (test_get.py, test_get_and_notifier.py, test_set_and_get.py, test_set_get_and_notifier.py)

bemerybmw · 2026-06-11T05:54:15Z

@@ -0,0 +1,33 @@
+/*******************************************************************************


all header files should have a corresponding cpp file, even if it's empty.

bemerybmw · 2026-06-11T05:55:29Z

+  public:
+    using T::Base::Base;
+
+    typename T::template Field<std::int32_t> test_field{*this, "test_field"};


name it something like initial_only_field so it doesn't have the same name as the other field.

We then would also need a differenet configuration file per test.

bemerybmw · 2026-06-11T05:55:45Z

+  public:
+    using T::Base::Base;
+
+    typename T::template Field<std::int32_t, false, true> test_field{*this, "test_field"};


Name it set_enabled_field

bemerybmw · 2026-06-11T05:57:03Z

-    typename T::template Field<std::int32_t, false, true> test_field{*this, "test_field"};
-};
-
 using InitialOnlyProxy = score::mw::com::AsProxy<InitialOnlyInterface>;


This file should be removed and these aliases should go in the relevant interface files

bemerybmw · 2026-06-11T06:00:07Z


+const std::string kInterprocessNotificationShmPath{"/field_initial_value_interprocess_notification"};
+
 int run_client(const std::size_t num_retries, const std::chrono::milliseconds retry_backoff_time)


Also use FailTest in this file and run_client should return void?

bemerybmw · 2026-06-11T06:37:14Z

+        run_notifier_consumer(num_retries, retry_backoff_time);
+        return;
+    }
+    if (mode == "set")


This should be set_and_notifier?

bemerybmw · 2026-06-11T06:42:10Z

+
+    auto& service = skeleton_container.GetSkeleton();
+    const auto register_handler_result = service.test_field.RegisterSetHandler([](std::int32_t& value) noexcept {
+        if (value > kSetAcceptedValue)


I think that instead of doing clamping, we can simply have the set handler do some constant transformation, e.g. value * a + b; We can then check on proxy side that the value was transformed as expected. It means that we don't have to do 2 method calls.

i replaced the clamping logic with a transformation ([value * 2 + 1] in the set handler. this way the consumer only needs to make one set call - it sends [kSetRequestValue] and checks that the received notification equals [kSetRequestValue * 2 + 1].

bemerybmw · 2026-06-11T06:47:27Z

mains should have main_ prefix

bemerybmw · 2026-06-11T06:47:58Z

+    constexpr auto kModeArg = "mode";
+    constexpr auto kServiceInstanceManifestArg = "service-instance-manifest";
+
+    const std::vector<std::pair<std::string, std::string>> parameter_description_pairs{


please extract config parsing into a function like in the tests I sent you

bemerybmw · 2026-06-11T06:49:44Z

    ],
 )

 cc_library(


If this is now used not just in fields, it should be moved to common resources folder which is accessible by both. e.g. https://github.com/eclipse-score/communication/tree/main/score/mw/com/test/common_test_resources

moved the process_synchronizer target to common_test_resources

…on_test_resources

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch 4 times, most recently from 8306eb3 to 38982b6 Compare May 21, 2026 13:57

bemerybmw requested changes May 22, 2026

View reviewed changes

Comment thread score/mw/com/impl/traits.h

Comment thread score/mw/com/impl/traits_test.cpp

Comment thread score/mw/com/test/common_test_resources/BUILD

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch 6 times, most recently from 7bd4c90 to a4435a5 Compare May 27, 2026 11:52

sahithi-nukala marked this pull request as ready for review May 27, 2026 12:18

sahithi-nukala requested review from LittleHuba, castler, crimson11, hoe-jo and limdor as code owners May 27, 2026 12:18

sahithi-nukala requested a review from bemerybmw May 27, 2026 12:18

bemerybmw reviewed May 27, 2026

View reviewed changes

Comment thread score/mw/com/test/field_initial_value/basic_acceptance_test/BUILD

sahithi-nukala marked this pull request as draft May 28, 2026 05:04

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch 8 times, most recently from 0e981e5 to 5496cf0 Compare May 29, 2026 09:42

sahithi-nukala requested a review from bemerybmw May 29, 2026 10:17

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch from 5496cf0 to 4039570 Compare May 29, 2026 10:17

sahithi-nukala marked this pull request as ready for review May 29, 2026 11:06

bemerybmw requested changes May 29, 2026

View reviewed changes

bemerybmw reviewed May 29, 2026

View reviewed changes

sahithi-nukala marked this pull request as draft June 1, 2026 05:18

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch 3 times, most recently from 655b99c to e177601 Compare June 2, 2026 13:40

sahithi-nukala marked this pull request as ready for review June 2, 2026 14:15

sahithi-nukala marked this pull request as draft June 4, 2026 04:29

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch 2 times, most recently from 39f0cd4 to 02408e2 Compare June 4, 2026 04:42

sahithi-nukala marked this pull request as ready for review June 4, 2026 05:29

bemerybmw requested changes Jun 11, 2026

View reviewed changes

sahithi-nukala marked this pull request as draft June 11, 2026 08:38

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch from 7a0e916 to 02408e2 Compare June 12, 2026 05:00

sahithi-nukala added 7 commits June 12, 2026 10:35

Move proxy and skeleton containers from methods to common test resources

d2c6f36

Update traits and traits tests for field get/set/notifier parameters

c1a9ef0

mw/com: move field_initial_value test package under fields

13a53ec

mw/com: add fields test suite root package

8b7f8da

mw/com: add set_and_notifier field integration tests

8e8ac59

mw/com: Address review comments for fields integration tests

3afd498

mw/com: fix references after moving proxy/skeleton containers to comm…

0b7a081

…on_test_resources

sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch from 02408e2 to 0b7a081 Compare June 12, 2026 05:35

		return target.wrap_exec("bin/main_service", args, cwd="/opt/MainServiceApp", **kwargs)


		def test_field_notifier_initial_value(target):

		@@ -0,0 +1,33 @@
		/*******************************************************************************


		const std::string kInterprocessNotificationShmPath{"/field_initial_value_interprocess_notification"};

		int run_client(const std::size_t num_retries, const std::chrono::milliseconds retry_backoff_time)

Conversation

sahithi-nukala commented May 21, 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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

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!

sahithi-nukala commented May 21, 2026 •

edited

Loading