mw/com: Add methods integration tests by bemerybmw · Pull Request #485 · eclipse-score/communication

bemerybmw · 2026-05-29T08:18:56Z

Depends-on: #479

The signature_variations is the exact same as the mixed_criticality tests, except the latter runs the test for all combinations of asil levels of consumers and providers. Therefore, we remove signature_variations.

gdadunashvili · 2026-06-11T13:35:59Z

+void check_result(std::future<score::Result<MethodReturnTypePtr<std::int32_t>>>&& result_future,
+                  std::int32_t expected_value)
+{
+    std::cout << "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" << std::endl;


Suggested change

std::cout << "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" << std::endl;

gdadunashvili · 2026-06-11T13:41:56Z

+    if (!find_result.has_value() || find_result->size() != 1)
+    {
+        FailTest("FindService failed or returned wrong count. Error: ", find_result.error());
+    }


find_result.error() will not exist if find_result->size() != 1 is the failing condition.

gdadunashvili · 2026-06-11T13:45:40Z

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


This file is never built. could be a duplicate of stop_offer_during_call.cpp?

gdadunashvili · 2026-06-11T13:56:13Z

+    {
+        std::cout << "Could not create Instance specifier!.\n"
+                  << "Here is why: " << instance_specifier_result.error() << "\n\n"
+                  << std::flush;


FailTest should be added here

gdadunashvili · 2026-06-11T13:58:20Z

+
+        auto async_result =
+            std::async(std::launch::async, [&proxy, test_input]() -> score::Result<MethodReturnTypePtr<std::int32_t>> {
+                while (true)


Maybe we should hardcode an upper limit of 1 Million iteration or sth. I know the test framework will interrupt long before that but this might get copy pasted to be executed in some other context

gdadunashvili · 2026-06-11T14:00:21Z

+    return target.wrap_exec("./bin/stop_offer_during_call",args,
+                           cwd="/opt/StopOfferDuringCallApp/", wait_on_exit=True, **kwargs)
+
+def test_multiple_proxies(target, **kwargs):


Suggested change

def test_multiple_proxies(target, **kwargs):

def test_stop_offer_during_call(target, **kwargs):

gdadunashvili · 2026-06-11T14:05:10Z

+load("@score_baselibs//score/language/safecpp:toolchain_features.bzl", "COMPILER_WARNING_FEATURES")
+load("//bazel/tools:json_schema_validator.bzl", "validate_json_schema_test")


this should be after the copyright?

gdadunashvili · 2026-06-11T14:07:05Z

+namespace
+{
+using namespace score::mw::com;
+using namespace score::mw::com::test;


this could just be turned into
namespace score::mw::com::test {

gdadunashvili · 2026-06-11T14:14:11Z

+        skeleton.StopOfferService();
+        std::cout << "Step 4.4: StopOfferService was called while the method was still running\n" << std::flush;
+
+        ++repetiction_count;


Suggested change

++repetiction_count;

++repetition_count;

gdadunashvili · 2026-06-11T14:20:04Z

+        offer_service(skeleton);
+    }
+
+    if (repetiction_count == 0)


maybe this should be successful_repetition_count?

gdadunashvili · 2026-06-12T07:56:14Z

+}
+
+template <typename ResultType>
+void FailIfMethodCallDidNotFail(const std::string& method_name, const ResultType& call_result)


This is a nitpick, but the name has too much logic in it. Maybe name it sth without a negation, like FailOnUnexpectedSuccess?

gdadunashvili · 2026-06-12T07:56:53Z

+    if (was_successfully_offered)
+    {
+        FailTest("ERROR: OfferService succeeded when it should have failed!");
+        return;


Suggested change

return;

gdadunashvili · 2026-06-12T08:20:19Z

+        std::cout << "\nConsumer: Step 10.1: Wait until proxy has reconnected to recreated skeleton" << std::endl;
+        // Since we have no way of being notified of the proxy being reconnected to the new skeleton, we keep calling a
+        // method in a loop until it succeeds. If it succeeded then it indicates that the proxy reconnected.
+        while (!consumer.GetProxy().without_args_or_return().has_value())


should we add an upper bound to this loop just to be safe?

gdadunashvili · 2026-06-12T08:24:55Z

+int main(int argc, const char** argv)
+{
+    score::mw::com::test::SetupAssertHandler();
+    score::mw::com::runtime::InitializeRuntime(argc, argv);


This is the old runtime initializer, should we use the new one everywhere? Also in other tests...

gdadunashvili · 2026-06-12T08:26:44Z

+
+        std::cout << "\nConsumer: Step 11: Call the same method again" << std::endl;
+        consumer.CallMethodWithInArgsAndReturn(
+            kTestValueA, kTestValueB, AllSignaturesMethodConsumer::CopyMode::WITH_COPY, kFailureMessagePrefix);


We could make one of these zero copy?

gdadunashvili · 2026-06-12T08:28:37Z

+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+def test_edge_cases(target):
+    with target.wrap_exec("bin/incomplete_handlers_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:


Suggested change

with target.wrap_exec("bin/incomplete_handlers_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:

with target.wrap_exec("bin/incomplete_handlers_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True):

gdadunashvili · 2026-06-12T08:28:51Z

+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+def test_edge_cases(target):
+    with target.wrap_exec("bin/proxy_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:


Suggested change

with target.wrap_exec("bin/proxy_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:

with target.wrap_exec("bin/proxy_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True):

gdadunashvili · 2026-06-12T08:29:04Z

+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+def test_edge_cases(target):
+    with target.wrap_exec("bin/skeleton_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:


Suggested change

with target.wrap_exec("bin/skeleton_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:

with target.wrap_exec("bin/skeleton_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True):

gdadunashvili · 2026-06-12T08:30:30Z

+    if (call_result.has_value())
+    {
+        FailTest(kFailureMessagePrefix, " Consumer: ", method_name, " unexpectedly succeeded.");
+        return;


Suggested change

return;

gdadunashvili · 2026-06-12T08:47:43Z

+        return;
+    }
+
+    std::cout << "Consumer: " << method_name << " failed as expected: " << call_result.error() << std::endl;


Technically we would need to disambiguate at call_result.error().
A method call could fail "propperly" with kBindingFailiure which is what we expect here. Or it could fail on this assertion, when we are trying to geth the queue_position.

gdadunashvili · 2026-06-12T13:02:54Z

@@ -0,0 +1,90 @@
+{


this is identical to consumer_asil_b_to_asil_b_mw_com_config.json

I think it is confusing to have towo identical configurations that are named differently. we could have oine that is called asil_b_to_asil_b_mw_com_config.json

gdadunashvili · 2026-06-12T13:04:50Z

@@ -0,0 +1,90 @@
+{


same comment as with provider_asil_b...
this file is identical to consumer_qm_to_qm... and should be deleted.

gdadunashvili · 2026-06-12T13:05:55Z

+Verifies that method calls are sent from consumer to provider with the correct values.
+Verifies that the provider executes the correct handler on the provided values. 
+Verifies that the consumer receives the correct return values.


Suggested change

Verifies that method calls are sent from consumer to provider with the correct values.

Verifies that the provider executes the correct handler on the provided values.

Verifies that the consumer receives the correct return values.

We should probably not have this here. It is not clear from the test name that these particular behaviors are verified. So someone could easily change the test behavior without knowing that these comments need to be updated

gdadunashvili · 2026-06-12T13:07:25Z

+Verifies that method calls are sent from consumer to provider with the correct values.
+Verifies that the provider executes the correct handler on the provided values. 
+Verifies that the consumer receives the correct return values.


Suggested change

Verifies that method calls are sent from consumer to provider with the correct values.

Verifies that the provider executes the correct handler on the provided values.

Verifies that the consumer receives the correct return values.

gdadunashvili · 2026-06-12T13:09:19Z

+"""
+def test_mixed_criticality_consumer_provider(target):
+    call_consumer_and_provider(
+        target, Criticality.QM, Criticality.QM, Criticality.QM)


Suggested change

target, Criticality.QM, Criticality.QM, Criticality.QM)

target, consumer_criticality=Criticality.QM, consumer_expects_criticality=Criticality.QM, provider_criticality=Criticality.QM)

gdadunashvili · 2026-06-12T13:31:35Z

+        FailTest(
+            "Methods mixed_criticality provider failed: WaitForProxyTestToFinish was stopped by "
+            "stop_token instead of notification");
+        return;


Suggested change

return;

gdadunashvili · 2026-06-12T13:34:07Z

+
+/// \brief ASIL-B provider main entry point for mixed criticality method tests.
+


Suggested change

/// \brief ASIL-B provider main entry point for mixed criticality method tests.

I think this information is contained in the file name

gdadunashvili · 2026-06-12T15:01:25Z


 int main(int argc, const char** argv)
 {
+    auto service_instance_manifest_path = score::mw::com::test::ParseServiceInstanceManifest(argc, argv);


this is never used

gdadunashvili · 2026-06-12T15:01:57Z

+    auto service_instance_manifest_path = score::mw::com::test::ParseServiceInstanceManifest(argc, argv);
+
    score::mw::com::test::SetupAssertHandler();
    score::mw::com::runtime::InitializeRuntime(argc, argv);


instead of the legacy initializer we should use the new one which makes use of the service_instance_manifest_path

gdadunashvili · 2026-06-12T15:07:28Z

+                               provider_criticality: Criticality,
+                               timeout_sec=60):


Suggested change

provider_criticality: Criticality,

timeout_sec=60):

provider_criticality: Criticality):

this is no longer necessary with the current test framework

github-code-quality Bot found potential problems May 29, 2026

View reviewed changes

Comment thread score/mw/com/test/methods/multiple_proxies/integration_test/test_fixture.py Fixed

bemerybmw mentioned this pull request May 29, 2026

Implement integration tests for fields with set and notifier #463

Draft

bemerybmw added 2 commits June 9, 2026 15:39

mw/com: Remove signature_variations

27e7a7f

The signature_variations is the exact same as the mixed_criticality tests, except the latter runs the test for all combinations of asil levels of consumers and providers. Therefore, we remove signature_variations.

mw/com: Add edge_cases_test

2832a9a

bemerybmw force-pushed the brem_methods_sctfs branch 2 times, most recently from 118eb76 to ff33ccc Compare June 9, 2026 15:04

bemerybmw added 3 commits June 10, 2026 09:13

mw/com: Add mixed_criticality test

60cbc88

mw/com: Add stop_offer_during_call test

6380cb2

mw/com: Add multiple_proxies test

dd28657

bemerybmw force-pushed the brem_methods_sctfs branch from ff33ccc to dd28657 Compare June 10, 2026 07:14

gdadunashvili reviewed Jun 11, 2026

View reviewed changes

gdadunashvili reviewed Jun 12, 2026

View reviewed changes

gdadunashvili approved these changes Jun 12, 2026

View reviewed changes

gdadunashvili self-requested a review June 12, 2026 15:09

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

	def test_multiple_proxies(target, **kwargs):
	def test_stop_offer_during_call(target, **kwargs):

		load("@score_baselibs//score/language/safecpp:toolchain_features.bzl", "COMPILER_WARNING_FEATURES")
		load("//bazel/tools:json_schema_validator.bzl", "validate_json_schema_test")

	with target.wrap_exec("bin/incomplete_handlers_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:
	with target.wrap_exec("bin/incomplete_handlers_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True):

	with target.wrap_exec("bin/proxy_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:
	with target.wrap_exec("bin/proxy_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True):

	with target.wrap_exec("bin/skeleton_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True) as process:
	with target.wrap_exec("bin/skeleton_recreation_test", cwd="/opt/EdgeCasesTestApp", wait_on_exit=True):

	Verifies that method calls are sent from consumer to provider with the correct values.
	Verifies that the provider executes the correct handler on the provided values.
	Verifies that the consumer receives the correct return values.

	target, Criticality.QM, Criticality.QM, Criticality.QM)
	target, consumer_criticality=Criticality.QM, consumer_expects_criticality=Criticality.QM, provider_criticality=Criticality.QM)


		/// \brief ASIL-B provider main entry point for mixed criticality method tests.

	provider_criticality: Criticality,
	timeout_sec=60):
	provider_criticality: Criticality):

Conversation

bemerybmw commented May 29, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants