RDKEMW-14899 : Add unit tests for transport layer coverage gaps

[swethasukumarr]

No description provided.

[Copilot]

Pull request overview

This PR expands unit test coverage across the Firebolt C++ transport layer, focusing on transport, gateway, helper, logger, and JSON type edge cases.

Changes:

• Adds new gateway and transport tests for connection states, notifications, timeouts, logging, and legacy RPC v1 paths.
• Adds helper/json type tests for subscription cleanup, callback parsing, and array error handling.
• Introduces a new logger test file covering formatting, filtering, level handling, and message formatting branches.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
test/unit/transportTest.cpp	Adds transport coverage for logging, disconnect paths, non-text messages, sequencing, and invalid URLs.
test/unit/loggerTest.cpp	Adds logger formatting, level filtering, and output-capture tests.
test/unit/json_typesTest.cpp	Adds non-array payload tests for additional NL_Json_Array instantiations.
test/unit/helperTest.cpp	Adds helper, subscription manager, unsubscribe, and multi-argument callback tests.
test/unit/gatewayTest.cpp	Adds gateway tests for subscriptions, notifications, timeouts, errors, disconnects, and legacy RPC v1 behavior.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

test/unit/gatewayTest.cpp:1225

• Ignoring the connection-failure wait result leaves the callback capturing local state by reference even if it has not fired yet. If the async failure is delivered after the test body exits, the callback can access destroyed promise/atomic objects during TearDown. Assert the future is ready or use shared callback state that outlives the disconnect.

    connFuture.wait_for(std::chrono::milliseconds(500));

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (6)

test/unit/gatewayTest.cpp:935

• This assumes port 49199 is unused on the CI host. If another local process happens to listen there, the connection can succeed and the test will fail for the wrong reason. Use a controlled test server/ephemeral port setup rather than a hard-coded “unused” port.

    cfg.wsUrl = "ws://localhost:49199"; // No server here

test/unit/gatewayTest.cpp:1213

• This hard-coded port is only presumed to be unused. If a listener exists on the CI machine, the connection-failure setup will not produce NotConnected, making the test flaky. Prefer reserving/controlling an ephemeral port or using a deterministic invalid endpoint strategy.

    cfg.wsUrl = "ws://localhost:49198";

test/unit/helperTest.cpp:446

• The expectation for unsubscribe("event", _) is installed before calling unsubscribeAll(otherOwner), so an incorrect immediate unsubscribe during this call would satisfy the expectation and the test would still pass. To verify the no-match behavior, assert no unsubscribe calls during unsubscribeAll(otherOwner) and only allow the destructor cleanup afterward.

    // Unsubscribe all for a different owner — no immediate calls to gateway.unsubscribe.
    // The original subscription remains active and is cleaned up by the HelperImpl destructor.
    EXPECT_CALL(mockGateway, unsubscribe("event", _)).WillOnce(Return(Error::None));
    helper.unsubscribeAll(otherOwner);

test/unit/helperTest.cpp:424

• These expectations remain active through fixture teardown, so the test would pass even if unsubscribeAll(owner1) did nothing until the destructor, or if it incorrectly unsubscribed owner2 immediately. Verify the expected calls immediately after unsubscribeAll(owner1) and only then allow destructor cleanup so the owner filtering is actually tested.

    // Unsubscribe all for owner1 — only event1 should be unsubscribed
    EXPECT_CALL(mockGateway, unsubscribe("event1", _)).WillOnce(Return(Error::None));
    // event2 (owner2) is cleaned up by the HelperImpl destructor
    EXPECT_CALL(mockGateway, unsubscribe("event2", _)).WillOnce(Return(Error::None));

    helper.unsubscribeAll(owner1);

test/unit/gatewayTest.cpp:1155

• The test later allows the timeout path to take up to 1500 ms, but this wait_for only waits 1000 ms. On a loaded CI runner, a valid timeout that completes between 1000 and 1500 ms will fail here before the elapsed-time assertion is reached. Use a wait duration at least as large as the accepted upper bound.

    auto future = gateway.request("test.noReply", {});
    auto status = future.wait_for(std::chrono::milliseconds(1000));
    ASSERT_EQ(status, std::future_status::ready);

test/unit/helperTest.cpp:373

• This does not assert that an invalid subscription id avoids calling the gateway. With the default (naggy) mock, an unexpected mockGateway.unsubscribe would only produce a warning, so the test could still pass even if the not-found path had side effects.

    auto result = helper.unsubscribe(9999); // non-existent id
    EXPECT_FALSE(result);
    EXPECT_EQ(result.error(), Error::General);

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.