Development/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

tests/src/tests/TestStructsJsonRpc.cpp

+    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

tests/src/tests/TestPrimitivesJsonRpc.cpp

+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

tests/src/tests/TestEnumsJsonRpc.cpp

+
+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

tests/CMakeLists.txt

+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

tests/src/Module.h


 #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

tests/src/tests/TestRestrictionsJsonRpc.cpp

+    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

tests/src/tests/TestRestrictionsJsonRpc.cpp

+    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

tests/src/tests/TestEnumsJsonRpc.cpp

+    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

tests/src/tests/TestEnumsJsonRpc.cpp

+    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

tests/src/JsonRpcTestHarness.h

+
+    // 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

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 5 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Development/json rpc functional tests#248

Development/json rpc functional tests#248
smanes0213 wants to merge 21 commits intomasterfrom
development/json-rpc-functional-tests

smanes0213 commented Mar 24, 2026

Uh oh!

rdkcmf-jenkins commented Mar 24, 2026

Uh oh!

rdkcmf-jenkins commented Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 24, 2026

Uh oh!

bramoosterhuis Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants