Development/ com and json rpc functional tests by smanes0213 · Pull Request #248 · rdkcentral/ThunderTools

smanes0213 · 2026-03-24T11:07:01Z

No description provided.

@out

…tor fix The ProxyStubGenerator does not forward the caller's IsSet() state for @out OptionalType<T> parameters. The stub always constructs an unset optional, so the implementation never receives the caller's intent and the value is never written back. Affected: Divide_WithRemainder, ParseInt_ValidNumber, ParseInt_NegativeNumber, ParseInt_InvalidString

Signed-off-by: smanes0213 <sankalpmaneshwar46@outlook.com>

rdkcmf-jenkins · 2026-03-24T11:09:24Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 3 files pending identification.

Protex Server Path: /home/blackduck/github/ThunderTools/248/rdkcentral/ThunderTools
Commit: cf7b33b

Report detail: gist'

rdkcmf-jenkins · 2026-03-24T11:11:08Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 3 files pending identification.

Protex Server Path: /home/blackduck/github/ThunderTools/248/rdkcentral/ThunderTools
Commit: 377a3b0

Report detail: gist'

bramoosterhuis · 2026-03-24T20:18:38Z

+    EXPECT_NE(response.find("20"), string::npos) << "Response: " << response;
+}
+
+// NOTE: SetGetColor test removed - setColor/getColor methods fail with error 30 (not found)


Don't remove tests that expose known or possible issues — instead keep them and prefix the test name with DISABLED_ so the gap stays visible.

bramoosterhuis · 2026-03-24T20:23:16Z

+class TestPrimitivesJsonRpc : public JsonRpcTesting::JsonRpcTestHarness<ITestPrimitives> {};
+
+// ===== Signed integers =====
+// NOTE: Signed integer boundary tests fail due to JsonGenerator bug (overflow validation)


Don't remove tests that expose known or possible issues — instead keep them and prefix the test name with DISABLED_ so the gap stays visible.

bramoosterhuis · 2026-03-24T20:26:11Z

+
+class TestEnumsJsonRpc : public JsonRpcTesting::JsonRpcTestHarness<ITestEnums> {};
+
+// Keep only tests for methods that actually work


Don't remove tests that expose known or possible issues — instead keep them and prefix the test name with DISABLED_ so the gap stays visible.

bramoosterhuis · 2026-03-24T20:28:08Z

+endmacro()
+
+# Helper macro to also add interface to JSON-RPC generation
+macro(AddTestInterfaceWithJsonRpc TestName)


Why did you choose to use extra macros creating an inheritance chain? In cmake cmake_parse_arguments works fine for macros. If you would add JSON_RPC and HAS_ENUMS the call sites become self-documenting:

AddTestInterface("Primitives" JSON_RPC) AddTestInterface("Enums" JSON_RPC HAS_ENUMS) AddTestInterface("Structs" JSON_RPC) AddTestInterface("Restrictions" JSON_RPC) AddTestInterface("Buffers") # COM-RPC only — known generator bug AddTestInterface("Iterators") # COM-RPC only — no JSON mapping

Adding a new flag later (say NO_INSTALL or SKIP) touches one macro instead of the whole inheritance chain, and there's no risk of forgetting to call the base macro from a derived one.

bramoosterhuis · 2026-03-24T20:47:55Z


 #include <core/core.h>
 #include <com/com.h>
+#include <plugins/JSONRPC.h>  // Only need JSONRPC (includes IPlugin, IShell, IDispatcher)


The unconditional #include <plugins/JSONRPC.h> is included in every translation unit including the COM-RPC binary. external/thunder/CMakeLists.txt only builds PLUGINS when INCLUDE_JSON_RPC_INTERFACES is ON, so a plain COM-RPC build will fail to find the header. Needs a guard:

#ifdef BUILD_JSON_RPC_TESTS #include <plugins/JSONRPC.h> #endif

bramoosterhuis · 2026-03-24T20:57:56Z

+    EXPECT_NE(Core::ERROR_NONE, result) << "Should reject zero";
+}
+
+TEST_F(TestRestrictionsJsonRpc, SetRatio_OutOfRange) {


SetRatio_OutOfRange uses {"value":1.5} but IsValidRatio correctly uses {"ratio":0.5} — the interface parameter is ratio, so SetRatio should also use {"ratio":1.5}.

bramoosterhuis · 2026-03-24T20:58:43Z

+    EXPECT_NE(Core::ERROR_NONE, result) << "Should reject value > 1.0";
+}
+
+TEST_F(TestRestrictionsJsonRpc, SetName_EmptyRejected) {


SetName_EmptyRejected and SetName_TooLongRejected use {"value":"..."} but the parameter is name.

bramoosterhuis · 2026-03-24T21:06:23Z

+    string response;
+    // Read-only property
+    EXPECT_EQ(Core::ERROR_NONE, CallMethod("currentState", "{}", response));
+    // Should return current state value (IDLE, RUNNING, etc.)


Maybe we want to assert the expected state in the response and mark the test DISABLED_ if it doesn't work yet — a disabled test with a failing assertion is more useful than a passing test with no assertion.

bramoosterhuis · 2026-03-24T21:06:30Z

+    string response;
+    EXPECT_EQ(Core::ERROR_NONE, CallMethod("computeState", 
+        R"({"current":"RUNNING","desired":"STOPPED"})", response));
+    // Should return derived state


Maybe we want to assert the expected state in the response and mark the test DISABLED_ if it doesn't work yet — a disabled test with a failing assertion is more useful than a passing test with no assertion.

bramoosterhuis · 2026-03-25T05:19:23Z

+
+    // JSON-RPC test harness that invokes JSON-RPC methods directly via PluginHost::JSONRPC
+    // Tests the generated JSON stubs by sending JSON strings and validating responses
+    template <typename INTERFACE>


The INTERFACE template parameter is never used anywhere in the class body. Every fixture gets the exact same g_server singleton regardless of which interface type is passed. TestPrimitivesJsonRpc, TestEnumsJsonRp etc. all resolve to identical classes at compile time — the type parameter is purely decorative.

Two directions to consider:

Either drop the template entirely and name it JsonRpcTestHarness, which is honest about what it does.

Or actually use INTERFACE to make it meaningful — for example using INTERFACE::ID to look up the right impl instance from g_implementations, so each fixture is genuinely scoped to its interface. This would also be the right foundation for per-test state reset once that issue is addressed.

…tes non-existent Core::Int24

…ef successfully

…just test targets

github-actions · 2026-04-23T17:01:21Z

JsonGenerator Results

View Results

Changes detected.

…unctional tests

github-actions · 2026-04-23T17:13:52Z

JsonGenerator Results

View Results

Changes detected.

… for improved efficiency

…fy the build process

github-actions · 2026-04-23T17:25:51Z

JsonGenerator Results

View Results

Changes detected.

…s targets

github-actions · 2026-04-23T17:40:42Z

JsonGenerator Results

View Results

Changes detected.

github-actions · 2026-04-23T17:52:48Z

JsonGenerator Results

View Results

Changes detected.

…nctional tests

github-actions · 2026-04-24T08:36:24Z

JsonGenerator Results

View Results

Changes detected.

…ep duration in tests

github-actions · 2026-04-24T09:36:50Z

JsonGenerator Results

View Results

Changes detected.

bramoosterhuis · 2026-04-24T09:51:04Z

Refactor functional test infrastructure

The previous test setup had grown organically into a single monolithic tests/CMakeLists.txt
(349 lines) that mixed COM-RPC and JSON-RPC concerns, used #ifdef BUILD_JSON_RPC_TESTS guards
throughout the implementation files, and registered implementations through two separate parallel
mechanisms (TestRegistry::ImplementationRegistrar for COM-RPC and JsonRpcServer::JsonRpcRegistrar
for JSON-RPC).

Structure

The tests are reorganised into tests/FunctionalTests/ with three subtrees: common/ holds the
interfaces, implementations, and generated code; comrpc/ and jsonrpc/ each build their own
executable. This makes the scope of each component clear and removes the need for conditional
compilation in implementation files.

Unified factory

TestRegistry and ComRpcServer::ImplementationProvider are replaced by a single
TestImplementation::Factory in common/. Both COM-RPC (Acquire()) and JSON-RPC
(RegisterJsonRpcInterfaces) pull from the same factory. Implementation files register once via a
static Factory::Registrar<INTERFACE, IMPL> and have no knowledge of which transport will use them.

JSON-RPC dispatch

The old JsonRpcServer inherited from both PluginHost::IPlugin and PluginHost::JSONRPC and
wired registration through a separate JsonRpcRegistrationProvider. The new server inherits directly
from JsonRPCRegister (a thin JSONRPCSupportsEventStatus subclass) and calls
RegisterJsonRpcInterfaces from its constructor. Registration code is generated from
JsonRpcRegistrations.cpp.in via configure_file, driven by the AddTestInterface(... JSON_RPC)
flag in CMake. Tests call Invoke() directly on the server — no socket, no process boundary.

COM-RPC connection sharing

Each test previously opened and closed a socket connection in SetUp/TearDown, causing ~15s
runtime for 136 tests. The harness now uses SetUpTestSuite/TearDownTestSuite to share a single
engine, client, and proxy per test suite. Runtime dropped to ~2s. Tests that assumed a fresh impl
per connection (GetPoint_BeforeSet_ReturnsError) are removed as they are not testing serialisation
behaviour.

bramoosterhuis · 2026-04-24T09:52:23Z

Refactor functional test infrastructure

The previous test setup had grown organically into a single monolithic tests/CMakeLists.txt
(349 lines) that mixed COM-RPC and JSON-RPC concerns, used #ifdef BUILD_JSON_RPC_TESTS guards
throughout the implementation files, and registered implementations through two separate parallel
mechanisms (TestRegistry::ImplementationRegistrar for COM-RPC and JsonRpcServer::JsonRpcRegistrar
for JSON-RPC).

Structure

The tests are reorganised into tests/FunctionalTests/ with three subtrees: common/ holds the
interfaces, implementations, and generated code; comrpc/ and jsonrpc/ each build their own
executable. This makes the scope of each component clear and removes the need for conditional
compilation in implementation files.

Unified factory

TestRegistry and ComRpcServer::ImplementationProvider are replaced by a single
TestImplementation::Factory in common/. Both COM-RPC (Acquire()) and JSON-RPC
(RegisterJsonRpcInterfaces) pull from the same factory. Implementation files register once via a
static Factory::Registrar<INTERFACE, IMPL> and have no knowledge of which transport will use them.

JSON-RPC dispatch

The old JsonRpcServer inherited from both PluginHost::IPlugin and PluginHost::JSONRPC and
wired registration through a separate JsonRpcRegistrationProvider. The new server inherits directly
from JsonRPCRegister (a thin JSONRPCSupportsEventStatus subclass) and calls
RegisterJsonRpcInterfaces from its constructor. Registration code is generated from
JsonRpcRegistrations.cpp.in via configure_file, driven by the AddTestInterface(... JSON_RPC)
flag in CMake. Tests call Invoke() directly on the server — no socket, no process boundary.

COM-RPC connection sharing

Each test previously opened and closed a socket connection in SetUp/TearDown, causing ~15s
runtime for 136 tests. The harness now uses SetUpTestSuite/TearDownTestSuite to share a single
engine, client, and proxy per test suite. Runtime dropped to ~2s. Tests that assumed a fresh impl
per connection (GetPoint_BeforeSet_ReturnsError) are removed as they are not testing serialisation
behaviour.

rdkcmf-jenkins · 2026-04-24T09:55:29Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 3 files pending identification.

Protex Server Path: /home/blackduck/github/ThunderTools/248/rdkcentral/ThunderTools
Commit: 10f7e72

Report detail: gist'

bramoosterhuis · 2026-04-24T10:01:54Z

@smanes0213 I took the liberty of refactoring the tests to have a common base and removed some complexity from the JSON-RPC side, while keeping the focus on exercising the generated code. Could you take a look and make sure I haven't stepped on anything you had in mind?

mhughesacn · 2026-04-24T14:35:24Z

Hi @smanes0213 : Please add proper (i.e. full, not one-line) apache-2.0 headers to the new files.

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

github-actions · 2026-04-24T18:03:27Z

JsonGenerator Results

View Results

Changes detected.

github-actions · 2026-04-24T18:15:28Z

JsonGenerator Results

View Results

Changes detected.

bramoosterhuis and others added 13 commits March 4, 2026 09:16

test: adding initial funtional tests

2234ad8

cmake: include tests to be build

d77abd6

cmake: make sure we can overrule the default generator paths

e769893

actions: initial workflow file to build and run functional tests

7389ea1

tests: disambiguate ActiveColor getter via const cast

417ad9a

tests: Enable more tests

7388525

cmake: disable async tests

9fa6eb6

tests: fix optional tests

cf3004a

adding vscode environment files

d57a691

tests: fix 32/64bit build differences

1748b4c

tests: add override to AddRef/Release()

e5c02ba

Development : Add JSON-RPC functional test infrastructure

cf7b33b

Signed-off-by: smanes0213 <sankalpmaneshwar46@outlook.com>

Copilot AI review requested due to automatic review settings March 24, 2026 11:07

Copilot started reviewing on behalf of smanes0213 March 24, 2026 11:07 View session

Update ITestPrimitives.h

377a3b0

smanes0213 marked this pull request as draft March 24, 2026 11:10

smanes0213 review requested due to automatic review settings March 24, 2026 11:10

smanes0213 added 2 commits March 24, 2026 17:01

Update ITestPrimitives.h

33de376

Update ITestPrimitives.h

b22aabc

bramoosterhuis requested changes Mar 24, 2026

View reviewed changes

bramoosterhuis reviewed Mar 25, 2026

View reviewed changes

smanes0213 added 6 commits March 25, 2026 12:52

Resolve review comments

3b98b5a

Add @stubgen:omit to Int24/UInt24 methods - ProxyStubGenerator genera…

5d07b1b

…tes non-existent Core::Int24

Remove @stubgen:omit from Int24/UInt24 - generator uses int24_t typed…

85aa701

…ef successfully

Remove Events test for JSONRPC

616e509

Update ProxyStubFunctionalTests.yml

1a1ef05

Add handwritten JSON-RPC handler versioning coverage

8c251bf

bramoosterhuis added 3 commits April 23, 2026 18:47

Update ProxyStubFunctionalTests.yml to enable functional tests and ad…

409d7bd

…just test targets

Update launch and settings configurations for functional tests

1fad748

Merge branch 'master' into development/json-rpc-functional-tests

9f99154

Update ProxyStubFunctionalTests.yml to build targets separately for f…

e4c15b1

…unctional tests

bramoosterhuis added 2 commits April 23, 2026 19:16

Update ProxyStubFunctionalTests.yml to build all targets concurrently…

0468cd5

… for improved efficiency

Remove parallel build option for ProxyStub functional tests to simpli…

30dd64f

…fy the build process

Update ProxyStubFunctionalTests.yml to build specific functional test…

8c37242

…s targets

Update ProxyStubFunctionalTests.yml to specify generator paths for fu…

f396bfc

…nctional tests

bramoosterhuis added 3 commits April 24, 2026 11:21

Refactor TestIterators to use ASYNC_DRAIN_DELAY_MS for consistent sle…

66db269

…ep duration in tests

Remove GetPoint_BeforeSet test to streamline test suite

893f075

Speed up test execution by sharing COM-RPC connection per test suite

10f7e72

bramoosterhuis marked this pull request as ready for review April 24, 2026 09:52

Add license headers to functional and JSON-RPC test files

ceefce1

Copilot AI review requested due to automatic review settings April 24, 2026 17:50

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Revert the private declaration of Registrar class

3f4a606


		class TestEnumsJsonRpc : public JsonRpcTesting::JsonRpcTestHarness<ITestEnums> {};

		// Keep only tests for methods that actually work

Conversation

smanes0213 commented Mar 24, 2026

Uh oh!

rdkcmf-jenkins commented Mar 24, 2026

Uh oh!

rdkcmf-jenkins commented Mar 24, 2026

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!

github-actions Bot commented Apr 23, 2026

JsonGenerator Results

Uh oh!

github-actions Bot commented Apr 23, 2026

JsonGenerator Results

Uh oh!

github-actions Bot commented Apr 23, 2026

JsonGenerator Results

Uh oh!

github-actions Bot commented Apr 23, 2026

JsonGenerator Results

Uh oh!

github-actions Bot commented Apr 23, 2026

JsonGenerator Results

Uh oh!

github-actions Bot commented Apr 24, 2026

JsonGenerator Results

Uh oh!

github-actions Bot commented Apr 24, 2026

JsonGenerator Results

Uh oh!

bramoosterhuis commented Apr 24, 2026

Refactor functional test infrastructure

Structure

Unified factory

JSON-RPC dispatch

COM-RPC connection sharing

Uh oh!

bramoosterhuis commented Apr 24, 2026

Refactor functional test infrastructure

Structure

Unified factory

JSON-RPC dispatch

COM-RPC connection sharing

Uh oh!

rdkcmf-jenkins commented Apr 24, 2026

Uh oh!

bramoosterhuis commented Apr 24, 2026

Uh oh!

mhughesacn commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 24, 2026

JsonGenerator Results

Uh oh!

github-actions Bot commented Apr 24, 2026

JsonGenerator Results

Uh oh!

Reviewers

Assignees

Labels