mw/com: Add test for interface containg events, fields and methods by bemerybmw · Pull Request #1018 · eclipse-score/communication

bemerybmw · 2026-08-27T12:14:36Z

No description provided.

gdadunashvili · 2026-08-28T08:27:09Z

+ * terms of the Apache License 3.0 which is available at
+ * https://www.apache.org/licenses/LICENSE-1.0
+ *
+ * SPDX-License-Identifier: Apache-1.0


should be Apache-2.0?

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

-namespace score::mw::com::test
-{
-
-std::string ParseServiceInstanceManifest(int argc, const char** argv)


I do not think we should have this tiny function in the global common resources. There is already too much stuff in there and no one has the overview, which leads to less usage of the common resources. ParseCommandLineArguments already exists as a general purpose function and this minor specialization really does not deserve to be part of the common resources for all tests.

As discussed, will leave this for now. Although I agree in general and we need to find a better way to reduce boilerplate between tests.

gdadunashvili · 2026-08-28T08:40:27Z

+  "appDesc": "all_service_elements",
  "logLevel": "kDebug",
  "logLevelThresholdConsole": "kDebug",
  "logMode": "kRemote|kConsole"


Suggested change

"logMode": "kRemote|kConsole"

"logMode": "kRemote"

This will stop double printing of the loggs

gdadunashvili · 2026-08-28T08:44:31Z

+
+    typename Trait::template Method<void(TestType, TestType)> with_in_args_only{*this, "with_in_args_only"};
+
+    typename Trait::template Field<TestType, WithGetter> getter_only_enabled_field{*this, "get_only_enabled_field"};


if there is no good reason for it I would change getter to get, to have same naming in the config and code

gdadunashvili · 2026-08-28T08:46:23Z

 {

-void RunConsumer();
+void run_consumer(const score::cpp::stop_token& stop_token);


This snake case naming inconsistency started with me doing it wrong once, and now it's spreading 😆
I do not have strong opinions about it but RunConsumer would be compliant to our guidelines

gdadunashvili · 2026-08-28T08:48:19Z

+
+
+def test_basic_acceptance_same_process_test(target):
+    """Test LoLa functionality between provider and consumer which contain field, methods and events in the same process.


Suggested change

"""Test LoLa functionality between provider and consumer which contain field, methods and events in the same process.

"""Test LoLa functionality between provider and consumer which contain fields, methods and events in the same process.

gdadunashvili · 2026-08-28T08:49:31Z

    features = COMPILER_WARNING_FEATURES + [
        "aborts_upon_exception",
    ],
+    visibility = ["//score/mw/com/test/fields:__pkg__"],


Suggested change

visibility = ["//score/mw/com/test/fields:__pkg__"],

visibility = ["//score/mw/com/test/all_service_elements:__pkg__"],

I do not think cc_binaries need visibility at all though. Since they are packaged right away in this BUILD file

gdadunashvili · 2026-08-28T08:51:52Z

+        proxy.with_in_args_and_return(kInArgsAndReturnMethodTestValueA, kInArgsAndReturnMethodTestValueB);
+    if (!with_in_args_and_return_result.has_value())
+    {
+        FailTest("Consumer: with_in_args_and_return call failed: ", with_in_args_and_return_result.error());


Should all fail tests use kFailureMessagePrefix?

gdadunashvili · 2026-08-28T09:09:54Z

+    std::cout << "\nConsumer: Step 3 - Test events" << std::endl;
+    SubscribeAndReceiveEventsOrFail(proxy.event_1, kEvent1ValuesToSend, stop_token);
+    SubscribeAndReceiveEventsOrFail(proxy.event_2, kEvent2ValuesToSend, stop_token);
+    SubscribeAndReceiveEventsOrFail(proxy.get_and_notifier_enabled_field, kAllGetAndNotifierValues, stop_token);


You are subscribing and blocking on each Event sequentially same happens later with methods and fields. So you are synchronising every service element interaction by hand. Which negates some advantages of having multiple service elements in the same test. This way we will never detect if LoLa has any problems with handling multiple service elements.

calling them async now.

github-project-automation Bot added this to COM - Communication FT Aug 27, 2026

github-project-automation Bot moved this to Backlog in COM - Communication FT Aug 27, 2026

bemerybmw commented Aug 27, 2026

View reviewed changes

Comment thread score/mw/com/test/all_service_elements/all_service_elements_datatype.h Outdated

bemerybmw commented Aug 27, 2026

View reviewed changes

Comment thread score/mw/com/test/all_service_elements/common_resources.h

bemerybmw force-pushed the brem_all_service_elements_test branch from 0978078 to ea980ae Compare August 27, 2026 12:46

github-advanced-security Bot found potential problems Aug 27, 2026

View reviewed changes

Comment thread score/mw/com/test/all_service_elements/consumer.cpp Fixed

Comment thread score/mw/com/test/all_service_elements/consumer.cpp Fixed

bemerybmw force-pushed the brem_all_service_elements_test branch 3 times, most recently from cb67d56 to 6beb1db Compare August 28, 2026 06:52

bemerybmw marked this pull request as ready for review August 28, 2026 08:31

bemerybmw requested review from LittleHuba, castler, crimson11, hoe-jo and limdor as code owners August 28, 2026 08:31

gdadunashvili reviewed Aug 28, 2026

View reviewed changes

bemerybmw added 2 commits September 1, 2026 12:57

mw/com: Extract config file parser into shared directory

6e1d537

mw/com: Add test for interface containg events, fields and methods

e7481e3

bemerybmw force-pushed the brem_all_service_elements_test branch from 6beb1db to e7481e3 Compare September 1, 2026 13:01


		typename Trait::template Method<void(TestType, TestType)> with_in_args_only{*this, "with_in_args_only"};

		typename Trait::template Field<TestType, WithGetter> getter_only_enabled_field{*this, "get_only_enabled_field"};



		def test_basic_acceptance_same_process_test(target):
		"""Test LoLa functionality between provider and consumer which contain field, methods and events in the same process.

	visibility = ["//score/mw/com/test/fields:__pkg__"],
	visibility = ["//score/mw/com/test/all_service_elements:__pkg__"],

Uh oh!

Conversation

bemerybmw commented Aug 27, 2026

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants