Changes related to service method integration tests by gdadunashvili · Pull Request #277 · eclipse-score/communication

gdadunashvili · 2026-04-08T13:20:36Z

These are changes that are related to integration tests for methods.
Either bugfixes that were discovered through methods tests, or infrastructure code that will be used by integration tests

bemerybmw

The final merge commit can be removed?

score/mw/com/types.h

score/mw/com/test/common_test_resources/command_line_parser.cpp

bemerybmw · 2026-04-13T11:14:44Z

score/mw/com/test/common_test_resources/command_line_parser.cpp

+            param_name.data(), po::value<std::string>(), ("specify " + std::string{param_doc}).data());
+    }
+
+    po::variables_map args;


This should use the CommandLineArgsMapType alias.

Still not done?

Now CommandLineArgsMapType is a std::unordered map and boost command line options does not escape the body of this function

bemerybmw · 2026-04-13T11:15:15Z

score/mw/com/test/common_test_resources/command_line_parser.h

+using CommandLineArgsMapType = boost::program_options::variables_map;
+
+template <typename ParsedType>
+auto GetValueIfProvided(const boost::program_options::variables_map& args, const std::string& arg_string)


please add docstrings to these functions and mention when they should be used.

bemerybmw · 2026-04-13T11:17:09Z

score/mw/com/test/common_test_resources/command_line_parser.cpp

+
+namespace score::mw::com::test
+{
+auto ParseCommandLineArguments(int argc,


This code is extracted from SctfTestRunner? IMO, SctfTestRunner should be updated to use this, otherwise we have two ways of parsing command line arguments. And if we then want to for example remove the boost dependency in the future, we only have that dependency in this target and so only have to refactor this code and not the SctfTestRunner.

I addressed this issue

bemerybmw · 2026-04-13T11:18:49Z

score/mw/com/test/common_test_resources/command_line_parser.h

+    std::string error_msg = "could not find the requested parameter: " + arg_string;
+    if constexpr (std::is_same_v<ParsedType, std::string>)
+    {
+        return (args.count(arg_string) > 0U) ? args[arg_string].as<std::string>()


This check is duplicated in each if. Can't we just check if args.count(arg_string) == 0U once at the start?

bemerybmw · 2026-04-13T11:20:55Z

score/mw/com/test/common_test_resources/command_line_parser.h

+
+using CommandLineArgsMapType = boost::program_options::variables_map;
+
+template <typename ParsedType>


Instead of templating with ParsedType and then checking within the function that it's a string, int or float, why not template with:
enum class ParsedType
{
INVALID,
INT,
STRING,
FLOAT
}

Doing this is not possible since the return type can not be deduced anymore. Also since I changed the implementation to from_chars there are not internal branches anymore

bemerybmw · 2026-04-13T11:22:04Z

score/mw/com/test/common_test_resources/fail_test.h

+}  // namespace detail
+
+template <typename... Args>
+void fail_test(Args&&... args)


Please add function docs

bemerybmw · 2026-04-13T11:22:21Z

score/mw/com/test/common_test_resources/fail_test.h

+
+namespace detail
+{
+void fail_test_(std::stringstream&& strstr);


Why lowercase and trailing underscore? Why not FailTest or FailTestImpl or something like that?

bemerybmw · 2026-04-13T11:27:42Z

score/mw/com/test/common_test_resources/fail_test.cpp

+
+void fail_test_(std::stringstream&& strstr)
+{
+    std::move(strstr) << "\033[0m  \033[1m\033[41mTEST FAILED\033[0m\n";


Why are you moving here?

I was mistaken

bemerybmw · 2026-04-15T06:26:08Z

score/mw/com/test/common_test_resources/command_line_parser.h

+
+using CommandLineArgsMapType = std::unordered_map<std::string, std::string>;
+
+/// \brief Get a value reperesednted as a string, from an arguments map and parse it into the appropriate type


Suggested change

/// \brief Get a value reperesednted as a string, from an arguments map and parse it into the appropriate type

/// \brief Get a value represented as a string, from an arguments map and parse it into the appropriate type

bemerybmw · 2026-04-15T06:26:33Z

score/mw/com/test/common_test_resources/command_line_parser.cpp

+            param_name.data(), po::value<std::string>(), ("specify " + std::string{param_doc}).data());
+    }
+
+    po::variables_map args;


Still not done?

bemerybmw · 2026-04-15T06:27:25Z

score/mw/com/test/common_test_resources/command_line_parser.h

+
+/// \brief Get a value reperesednted as a string, from an arguments map and parse it into the appropriate type
+///
+/// \tparam ParsedType Target type of the parsed argument. Must be std::string, integral or floating point type.


Suggested change

/// \tparam ParsedType Target type of the parsed argument. Must be std::string, integral or floating point type.

/// \tparam ReturnType Target type of the parsed argument. Must be std::string, integral or floating point type.

This function can safely terminate a test and offers the ability to consume variadic number of printable arguments to print them as an error message

methods tests reuse the same configuration with slight variations, like changing the applicationID or asil_level, but leavidng the rest unchanged. This can lead to many duplicated json files which only slightly differ and are hard to maintain. This config_generator bazel rule together with the config_generator.py and base_mw_com_config.json provides a way to generate config files on the fly during the build step.

Handler scopes need to be reinitiated by PrepareOffer in case they were expired by StopOffer before

gdadunashvili requested review from LittleHuba, bemerybmw, castler, crimson11, hoe-jo and limdor as code owners April 8, 2026 13:20

gdadunashvili added the test-qnx Enables the checks with QNX toolchains. Checks require committer approval to start execution. label Apr 9, 2026

gdadunashvili temporarily deployed to workflow-approval April 9, 2026 14:41 — with GitHub Actions Inactive

bemerybmw requested changes Apr 13, 2026

View reviewed changes

gdadunashvili force-pushed the dadu_bugfixes_from_sctfs branch from b526473 to 2cb49ec Compare April 14, 2026 17:20

gdadunashvili had a problem deploying to workflow-approval April 14, 2026 17:20 — with GitHub Actions Error

bemerybmw reviewed Apr 15, 2026

View reviewed changes

gdadunashvili and others added 9 commits April 15, 2026 15:28

mw/com: Add MethodReturnTypePtr to the public API

9e76daa

mw/com: Add MethodInArgTypePtr to the public API

3787b4c

mw/com: Create a generic command line parser

818b8d2

mw/com: Add FailTest helper function to test resources

6e3e5eb

This function can safely terminate a test and offers the ability to consume variadic number of printable arguments to print them as an error message

com/test: change all command line arguments to long form

3f63518

com/test: update sctf test runner to use the generic command line parser

f38d52a

com/test: Add todo for a missing test

209ff3c

mw/com: Fix handler scope expiration bug

8da71a5

Handler scopes need to be reinitiated by PrepareOffer in case they were expired by StopOffer before

gdadunashvili force-pushed the dadu_bugfixes_from_sctfs branch from 2cb49ec to 8da71a5 Compare April 15, 2026 13:29

gdadunashvili requested a deployment to workflow-approval April 15, 2026 13:29 — with GitHub Actions Waiting


		using CommandLineArgsMapType = boost::program_options::variables_map;

		template <typename ParsedType>


		using CommandLineArgsMapType = std::unordered_map<std::string, std::string>;

		/// \brief Get a value reperesednted as a string, from an arguments map and parse it into the appropriate type

	/// \brief Get a value reperesednted as a string, from an arguments map and parse it into the appropriate type
	/// \brief Get a value represented as a string, from an arguments map and parse it into the appropriate type

	/// \tparam ParsedType Target type of the parsed argument. Must be std::string, integral or floating point type.
	/// \tparam ReturnType Target type of the parsed argument. Must be std::string, integral or floating point type.

Conversation

gdadunashvili commented Apr 8, 2026

Uh oh!

bemerybmw left a comment

Choose a reason for hiding this comment

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