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-22T06:13:08Z


    // Note : at the moment the SkeletonField::Get implementation is not in the branch which means the skeleton and
    // proxy side does not have same template parameters.
-    template <typename SampleType, bool EnableSet = false, bool EnableGet = false, bool EnableNotifier = false>


Why did you reorder these?

Please put this change in a separate commit with a good description of why you're changing traits / traits_test.

done. updated the traits file changes into a separate commit

bemerybmw · 2026-05-22T06:14:23Z

  public:
    using InterfaceTrait::Base::Base;

+    static constexpr bool kEnableSet{std::is_same_v<InterfaceTrait, SkeletonTrait>};


Why did you add this?

because this uses same templated interface for both proxy and skeleton, but the field should not have the same capabilities on both sides. so for this set needs to stay enabled on the skeleton side, while the proxy side should remain without set.

bemerybmw · 2026-05-22T06:16:46Z

 load("@score_baselibs//score/language/safecpp:toolchain_features.bzl", "COMPILER_WARNING_FEATURES")
 load("//quality/unit_testing:unit_testing.bzl", "cc_unit_test")

+cc_library(


Moving these should be a separate commit. You can read my comment here to better understand how commits should look / why we split them up: #385 (comment)

done. pushed these changes with separate commit.

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

+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+
+test_suite(


We decided not to use these component_test suites but we will instead run tests using wildcards e.g. bazel test //score/mw/com/test/.... So you can remove this.

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:34:50Z

Please use the tests in this pr as a guide for new integration tests: #485

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.

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

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

Move proxy and skeleton containers from methods to common test resources

1f9330a

sahithi-nukala added 4 commits May 29, 2026 15:47

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

bf10d92

mw/com: move field_initial_value test package under fields

40121fa

mw/com: add fields test suite root package

8a0bf06

mw/com: add set_and_notifier field integration tests

4039570

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

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


		def test_field_notifier_initial_value(target):

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!

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!

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 May 21, 2026 •

edited

Loading