From 4218912f495c3fe30f8c896d0a297566b9aaaaa7 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Fri, 15 May 2026 12:04:14 -0400 Subject: [PATCH 01/10] RDKEMW-14899 : Add unit tests for transport layer coverage gaps --- test/unit/gatewayTest.cpp | 683 +++++++++++++++++++++++++++++++++++ test/unit/helperTest.cpp | 379 +++++++++++++++++++ test/unit/json_typesTest.cpp | 32 ++ test/unit/loggerTest.cpp | 369 +++++++++++++++++++ test/unit/transportTest.cpp | 358 ++++++++++++++++++ 5 files changed, 1821 insertions(+) create mode 100644 test/unit/loggerTest.cpp diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index 02b7b93..9ebf851 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -700,3 +700,686 @@ TEST_F(GatewayUTest, DisconnectDoesNotHangWhenServerDisappearsWithActiveSubscrip EXPECT_LT(elapsed.count(), 200) << "unsubscribe() took " << elapsed.count() << " ms — blocked waiting for ACK from silent server (bug reproduced)"; } + +// --------------------------------------------------------------------------- +// Additional gateway tests for coverage gaps +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.ConnectAlreadyConnected +// Covers: src/gateway.cpp (transport returns AlreadyConnected) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, ConnectAlreadyConnected) +{ + IGateway& gateway = connectAndWait(); + + // Try to connect again + Firebolt::Error err = gateway.connect(getTestConfig(), [](bool, const Firebolt::Error&) {}); + EXPECT_EQ(err, Firebolt::Error::AlreadyConnected); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.DuplicateSubscribeToSameEvent +// Covers: src/gateway.cpp:server.subscribe returns Error::General on dup usercb +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, DuplicateSubscribeToSameEvent) +{ + IGateway& gateway = connectAndWait(); + + auto onEvent = [](void*, const nlohmann::json&) {}; + int dummyCb = 0; + + Firebolt::Error err = gateway.subscribe("test.onDup", onEvent, &dummyCb); + EXPECT_EQ(err, Firebolt::Error::None); + + // Duplicate subscribe with same usercb should fail + err = gateway.subscribe("test.onDup", onEvent, &dummyCb); + EXPECT_EQ(err, Firebolt::Error::General); + + gateway.unsubscribe("test.onDup", &dummyCb); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.UnsubscribeNonexistentEvent +// Covers: src/gateway.cpp:server.unsubscribe returns General when not found +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, UnsubscribeNonexistentEvent) +{ + IGateway& gateway = connectAndWait(); + + int dummyCb = 0; + Firebolt::Error err = gateway.unsubscribe("nonexistent.event", &dummyCb); + EXPECT_EQ(err, Firebolt::Error::General); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.NotificationWithValueWrapping +// Covers: src/gateway.cpp:325-340 (notify with "value" key unwrapping) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, NotificationWithValueWrapping) +{ + IGateway& gateway = connectAndWait(); + + std::promise eventPromise; + auto eventFuture = eventPromise.get_future(); + + auto onEvent = [](void* usercb, const nlohmann::json& params) + { static_cast*>(usercb)->set_value(params); }; + + Firebolt::Error err = gateway.subscribe("test.onValue", onEvent, &eventPromise); + EXPECT_EQ(err, Firebolt::Error::None); + + // Server sends notification with params: {"value": 42} (scalar → unwrapped) + m_onMessageAction = [](server* s, connection_hdl hdl) + { + nlohmann::json eventMsg; + eventMsg["jsonrpc"] = "2.0"; + eventMsg["method"] = "test.onValue"; + eventMsg["params"] = {{"value", 42}}; + s->send(hdl, eventMsg.dump(), websocketpp::frame::opcode::text); + }; + gateway.send("dummy", {}); + + auto status = eventFuture.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(status, std::future_status::ready); + + // When params has a single "value" key with non-object value, it's unwrapped + nlohmann::json received = eventFuture.get(); + EXPECT_EQ(received, 42); + + gateway.unsubscribe("test.onValue", &eventPromise); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.NotificationWithValueObjectNotUnwrapped +// Covers: src/gateway.cpp:330-336 (value IS object → keep full params) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, NotificationWithValueObjectNotUnwrapped) +{ + IGateway& gateway = connectAndWait(); + + std::promise eventPromise; + auto eventFuture = eventPromise.get_future(); + + auto onEvent = [](void* usercb, const nlohmann::json& params) + { static_cast*>(usercb)->set_value(params); }; + + Firebolt::Error err = gateway.subscribe("test.onObjValue", onEvent, &eventPromise); + EXPECT_EQ(err, Firebolt::Error::None); + + // Server sends notification with params: {"value": {"nested": true}} (object → NOT unwrapped) + m_onMessageAction = [](server* s, connection_hdl hdl) + { + nlohmann::json eventMsg; + eventMsg["jsonrpc"] = "2.0"; + eventMsg["method"] = "test.onObjValue"; + eventMsg["params"] = {{"value", {{"nested", true}}}}; + s->send(hdl, eventMsg.dump(), websocketpp::frame::opcode::text); + }; + gateway.send("dummy", {}); + + auto status = eventFuture.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(status, std::future_status::ready); + + // When value is an object, params are kept as-is + nlohmann::json received = eventFuture.get(); + EXPECT_TRUE(received.contains("value")); + EXPECT_TRUE(received["value"]["nested"].get()); + + gateway.unsubscribe("test.onObjValue", &eventPromise); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.NotificationNoSubscribers +// Covers: src/gateway.cpp:350-353 (no subscribers for event → warning log) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, NotificationNoSubscribers) +{ + IGateway& gateway = connectAndWait(); + + // Server sends a notification for an event nobody subscribed to + m_onMessageAction = [](server* s, connection_hdl hdl) + { + nlohmann::json eventMsg; + eventMsg["jsonrpc"] = "2.0"; + eventMsg["method"] = "test.onNobodyListens"; + eventMsg["params"] = {{"data", true}}; + s->send(hdl, eventMsg.dump(), websocketpp::frame::opcode::text); + }; + gateway.send("dummy", {}); + + // Give time for the notification to be processed (it should just log a warning) + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + // Verify the gateway still works after the orphan notification + auto responseFuture = gateway.request("test.method", {{"key", "val"}}); + auto responseStatus = responseFuture.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(responseStatus, std::future_status::ready); + EXPECT_TRUE(responseFuture.get()); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.ResponseWithNoReceiver +// Covers: src/gateway.cpp: Client::response out_of_range catch +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, ResponseWithNoReceiver) +{ + IGateway& gateway = connectAndWait(); + + // Server sends a response with an id nobody is waiting for + m_onMessageAction = [](server* s, connection_hdl hdl) + { + nlohmann::json orphanResponse; + orphanResponse["jsonrpc"] = "2.0"; + orphanResponse["id"] = 99999; + orphanResponse["result"] = {{"orphan", true}}; + s->send(hdl, orphanResponse.dump(), websocketpp::frame::opcode::text); + }; + gateway.send("dummy", {}); + + // Give time for the orphan to be processed + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + // Gateway should still function normally + auto responseFuture = gateway.request("test.method", {{"key", "val"}}); + auto responseStatus = responseFuture.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(responseStatus, std::future_status::ready); + EXPECT_TRUE(responseFuture.get()); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.SubscribeTimeout +// Covers: src/gateway.cpp:subscribe ACK timeout path +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, SubscribeTimeout) +{ + // Use a message handler that never responds to subscribe requests + m_messageHandler = [](connection_hdl, server::message_ptr) { /* drop everything */ }; + + IGateway& gateway = connectAndWait(); + + auto onEvent = [](void*, const nlohmann::json&) {}; + int dummyCb = 0; + + Firebolt::Error err = gateway.subscribe("test.onTimeout", onEvent, &dummyCb); + // Subscribe should return Timedout because the ACK never arrives + EXPECT_EQ(err, Firebolt::Error::Timedout); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.SendNotConnected +// Covers: src/gateway.cpp:Client::send → transport.send NotConnected +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, SendNotConnected) +{ + // Don't start the server — connect will fail + IGateway& gateway = GetGatewayInstance(); + // Don't connect, just try to send on a fresh instance (already disconnected from TearDown) + // The gateway instance is a singleton, so we need to rely on the state being disconnected + // after TearDown was called in a previous test + // Instead, let's connect to a non-existent server and verify send after connection failure + std::promise connPromise; + auto connFuture = connPromise.get_future(); + std::atomic promiseSet{false}; + + Firebolt::Config cfg = getTestConfig(); + cfg.wsUrl = "ws://localhost:49199"; // No server here + + Firebolt::Error err = + gateway.connect(cfg, + [&](bool connected, const Firebolt::Error&) + { + bool expected = false; + if (promiseSet.compare_exchange_strong(expected, true)) + { + connPromise.set_value(connected); + } + }); + ASSERT_EQ(err, Firebolt::Error::None); + + // Wait for connection failure + auto status = connFuture.wait_for(std::chrono::milliseconds(500)); + if (status == std::future_status::ready) + { + EXPECT_FALSE(connFuture.get()); + } + + // Send should fail with NotConnected + err = gateway.send("test.method", {}); + EXPECT_EQ(err, Firebolt::Error::NotConnected); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.DisconnectWithoutConnect +// Covers: src/gateway.cpp:disconnect when transport is in NotStarted state +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, DisconnectWithoutConnect) +{ + // After TearDown resets the gateway, calling disconnect again should be safe + // The singleton is already disconnected after the preceding test's TearDown + IGateway& gateway = GetGatewayInstance(); + Firebolt::Error err = gateway.disconnect(); + EXPECT_EQ(err, Firebolt::Error::None); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.NotificationMultipleParams +// Covers: src/gateway.cpp:340-344 (params with multiple keys → pass as-is) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, NotificationMultipleParams) +{ + IGateway& gateway = connectAndWait(); + + std::promise eventPromise; + auto eventFuture = eventPromise.get_future(); + + auto onEvent = [](void* usercb, const nlohmann::json& params) + { static_cast*>(usercb)->set_value(params); }; + + Firebolt::Error err = gateway.subscribe("test.onMultiParam", onEvent, &eventPromise); + EXPECT_EQ(err, Firebolt::Error::None); + + // Server sends notification with multiple params (not just "value") + m_onMessageAction = [](server* s, connection_hdl hdl) + { + nlohmann::json eventMsg; + eventMsg["jsonrpc"] = "2.0"; + eventMsg["method"] = "test.onMultiParam"; + eventMsg["params"] = {{"key1", "val1"}, {"key2", "val2"}}; + s->send(hdl, eventMsg.dump(), websocketpp::frame::opcode::text); + }; + gateway.send("dummy", {}); + + auto status = eventFuture.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(status, std::future_status::ready); + + nlohmann::json received = eventFuture.get(); + EXPECT_EQ(received["key1"], "val1"); + EXPECT_EQ(received["key2"], "val2"); + + gateway.unsubscribe("test.onMultiParam", &eventPromise); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.SubscribeErrorFromServer +// Covers: src/gateway.cpp:subscribe → server returns error → unsubscribe +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, SubscribeErrorFromServer) +{ + // Server responds to subscribe with an error + m_messageHandler = [this](connection_hdl hdl, server::message_ptr msg) + { + try + { + auto request = nlohmann::json::parse(msg->get_payload()); + nlohmann::json response; + response["jsonrpc"] = "2.0"; + response["id"] = request["id"]; + response["error"]["code"] = -32601; + response["error"]["message"] = "Method not found"; + m_server.send(hdl, response.dump(), msg->get_opcode()); + } + catch (...) + { + } + }; + + IGateway& gateway = connectAndWait(); + + auto onEvent = [](void*, const nlohmann::json&) {}; + int dummyCb = 0; + + Firebolt::Error err = gateway.subscribe("test.onInvalid", onEvent, &dummyCb); + // The error from the server should be propagated, and the local subscription cleaned up + EXPECT_NE(err, Firebolt::Error::None); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.LegacyRPCv1UnsubscribeCleanup +// Covers: src/gateway.cpp:unsubscribe legacy RPC v1 event map cleanup +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, LegacyRPCv1UnsubscribeCleanup) +{ + m_messageHandler = [this](connection_hdl hdl, server::message_ptr msg) + { + auto request = nlohmann::json::parse(msg->get_payload()); + if (request.contains("params") && request["params"].contains("listen")) + { + nlohmann::json response; + response["jsonrpc"] = "2.0"; + response["id"] = request["id"]; + response["result"]["listening"] = request["params"]["listen"]; + m_server.send(hdl, response.dump(), msg->get_opcode()); + } + }; + + startServer(); + IGateway& gateway = GetGatewayInstance(); + auto connectionFuture = m_connectionPromise.get_future(); + + Firebolt::Config cfg = getTestConfig(); + cfg.legacyRPCv1 = true; + Firebolt::Error connectErr = gateway.connect(cfg, [this](bool connected, const Firebolt::Error& err) + { onConnectionChange(connected, err); }); + ASSERT_EQ(connectErr, Firebolt::Error::None); + ASSERT_EQ(connectionFuture.wait_for(std::chrono::seconds(2)), std::future_status::ready); + + auto onEvent = [](void*, const nlohmann::json&) {}; + int dummyCb = 0; + + Firebolt::Error err = gateway.subscribe("test.onLegacyEvent", onEvent, &dummyCb); + EXPECT_EQ(err, Firebolt::Error::None); + + // Unsubscribe should clean up the rpcv1_eventMap entry + err = gateway.unsubscribe("test.onLegacyEvent", &dummyCb); + EXPECT_EQ(err, Firebolt::Error::None); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.LegacyRPCv1NonEventResult +// Covers: src/gateway.cpp:655-665 (legacy: result is present but id NOT in eventMap) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, LegacyRPCv1NonEventResult) +{ + m_messageHandler = [this](connection_hdl hdl, server::message_ptr msg) + { + auto request = nlohmann::json::parse(msg->get_payload()); + nlohmann::json response; + response["jsonrpc"] = "2.0"; + response["id"] = request["id"]; + response["result"] = {{"data", "not_listening"}}; + m_server.send(hdl, response.dump(), msg->get_opcode()); + }; + + startServer(); + IGateway& gateway = GetGatewayInstance(); + auto connectionFuture = m_connectionPromise.get_future(); + + Firebolt::Config cfg = getTestConfig(); + cfg.legacyRPCv1 = true; + Firebolt::Error connectErr = gateway.connect(cfg, [this](bool connected, const Firebolt::Error& err) + { onConnectionChange(connected, err); }); + ASSERT_EQ(connectErr, Firebolt::Error::None); + ASSERT_EQ(connectionFuture.wait_for(std::chrono::seconds(2)), std::future_status::ready); + + // A normal request in legacy mode — result does NOT have "listening" key + // and the id is not in the event map, so it falls through to client.response() + auto future = gateway.request("device.name", {}); + auto status = future.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(status, std::future_status::ready); + + auto result = future.get(); + ASSERT_TRUE(result); + EXPECT_EQ((*result)["data"], "not_listening"); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.WaitTimeConfiguration +// Covers: src/gateway.cpp:runtime_waitTime_ms configuration from Config +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, WaitTimeConfiguration) +{ + // Use a handler that never responds + m_messageHandler = [](connection_hdl, server::message_ptr) {}; + + startServer(); + IGateway& gateway = GetGatewayInstance(); + auto connectionFuture = m_connectionPromise.get_future(); + + Firebolt::Config cfg = getTestConfig(); + cfg.waitTime_ms = 200; // Short timeout + + Firebolt::Error err = gateway.connect(cfg, [this](bool connected, const Firebolt::Error& e) + { onConnectionChange(connected, e); }); + ASSERT_EQ(err, Firebolt::Error::None); + ASSERT_EQ(connectionFuture.wait_for(std::chrono::seconds(2)), std::future_status::ready); + + auto start = std::chrono::steady_clock::now(); + auto future = gateway.request("test.noReply", {}); + auto status = future.wait_for(std::chrono::milliseconds(1000)); + ASSERT_EQ(status, std::future_status::ready); + + auto result = future.get(); + auto elapsed = std::chrono::duration_cast(std::chrono::steady_clock::now() - start); + + EXPECT_FALSE(result); + EXPECT_EQ(result.error(), Firebolt::Error::Timedout); + // Should timeout around waitTime_ms + watchdog interval (200 + 500 = ~700ms max) + EXPECT_LT(elapsed.count(), 1500); +} + +// --------------------------------------------------------------------------- +// Branch-coverage tests +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.SendResponseIsIgnoredByClient +// Covers: gateway.cpp:153-154 (invokes.find(id) != end → erase + return) +// The response for a fire-and-forget send() ID should be silently dropped. +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, SendResponseIsIgnoredByClient) +{ + // Server echoes back a response for every message (including sends) + IGateway& gateway = connectAndWait(); + + // send() uses Client::send which adds the ID to the invokes set. + // When the echo comes back, client.response() should find it in invokes, + // erase it, and return without error. + nlohmann::json params = {{"key", "value"}}; + Firebolt::Error err = gateway.send("test.fire_and_forget", params); + EXPECT_EQ(err, Firebolt::Error::None); + + // Give time for the echo response to be processed + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + // Gateway should still be functional — the invoke response was silently dropped + auto future = gateway.request("test.method", {{"k", "v"}}); + auto status = future.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(status, std::future_status::ready); + EXPECT_TRUE(future.get()); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.RequestFailsWhenSendErrors +// Covers: gateway.cpp:136-141 (transport_.send returns error → promise set + erase) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, RequestFailsWhenSendErrors) +{ + // Don't start server — connect to a port nobody listens on + // so transport is in Disconnected state after connection failure + IGateway& gateway = GetGatewayInstance(); + std::promise connPromise; + auto connFuture = connPromise.get_future(); + std::atomic pset{false}; + + Firebolt::Config cfg = getTestConfig(); + cfg.wsUrl = "ws://localhost:49198"; + + Firebolt::Error err = + gateway.connect(cfg, + [&](bool connected, const Firebolt::Error&) + { + bool exp = false; + if (pset.compare_exchange_strong(exp, true)) + connPromise.set_value(connected); + }); + ASSERT_EQ(err, Firebolt::Error::None); + + connFuture.wait_for(std::chrono::milliseconds(500)); + + // Now request — transport.send will fail with NotConnected, + // which triggers the error branch in Client::request (lines 136-141) + auto future = gateway.request("test.method", {{"k", "v"}}); + auto status = future.wait_for(std::chrono::seconds(2)); + ASSERT_EQ(status, std::future_status::ready); + + auto result = future.get(); + EXPECT_FALSE(result); + EXPECT_EQ(result.error(), Firebolt::Error::NotConnected); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.LegacySubscribeFailureCleansEventMap +// Covers: gateway.cpp:565-567 (legacyRPCv1 && subscribe error → erase rpcv1_eventMap) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, LegacySubscribeFailureCleansEventMap) +{ + // Server never responds to subscribe → timeout + m_messageHandler = [](connection_hdl, server::message_ptr) {}; + + startServer(); + IGateway& gateway = GetGatewayInstance(); + auto connectionFuture = m_connectionPromise.get_future(); + + Firebolt::Config cfg = getTestConfig(); + cfg.legacyRPCv1 = true; + Firebolt::Error connectErr = gateway.connect(cfg, [this](bool connected, const Firebolt::Error& err) + { onConnectionChange(connected, err); }); + ASSERT_EQ(connectErr, Firebolt::Error::None); + ASSERT_EQ(connectionFuture.wait_for(std::chrono::seconds(2)), std::future_status::ready); + + auto onEvent = [](void*, const nlohmann::json&) {}; + int dummyCb = 0; + + // Subscribe will timeout (50ms ACK timeout), hitting the error cleanup path + // that also cleans the rpcv1_eventMap (line 565-567) + Firebolt::Error err = gateway.subscribe("test.onLegacyTimeout", onEvent, &dummyCb); + EXPECT_EQ(err, Firebolt::Error::Timedout); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.UnsubscribeAckWithError +// Covers: gateway.cpp:621 (!result → status = result.error() in unsubscribe ACK) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, UnsubscribeAckWithError) +{ + // Server responds to subscribe with success, but to unsubscribe with error + m_messageHandler = [this](connection_hdl hdl, server::message_ptr msg) + { + try + { + auto request = nlohmann::json::parse(msg->get_payload()); + nlohmann::json response; + response["jsonrpc"] = "2.0"; + response["id"] = request["id"]; + + if (request.contains("params") && request["params"].contains("listen")) + { + if (request["params"]["listen"].get()) + { + // Subscribe ACK — success + response["result"] = {{"listening", true}}; + } + else + { + // Unsubscribe ACK — error + response["error"]["code"] = -32601; + response["error"]["message"] = "Unsubscribe failed"; + } + } + else + { + response["result"] = nlohmann::json::object(); + } + m_server.send(hdl, response.dump(), msg->get_opcode()); + } + catch (...) + { + } + }; + + IGateway& gateway = connectAndWait(); + + auto onEvent = [](void*, const nlohmann::json&) {}; + int dummyCb = 0; + + Firebolt::Error err = gateway.subscribe("test.onErrUnsub", onEvent, &dummyCb); + EXPECT_EQ(err, Firebolt::Error::None); + + // Unsubscribe should get the error from the ACK (line 621) + err = gateway.unsubscribe("test.onErrUnsub", &dummyCb); + EXPECT_EQ(err, Firebolt::Error::MethodNotFound); +} + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.LegacyUnsubscribeIteratesPastNonMatchingEntry +// Covers: gateway.cpp:600 (++it in rpcv1_eventMap loop — else branch when +// it->second != event, advancing the iterator past non-matching entries) +// Scenario: In legacy RPC v1 mode, subscribe to two events (eventA gets a +// lower message ID, eventB gets a higher one). When unsubscribing +// eventB, the loop iterates past eventA's entry (++it) before +// finding eventB. This is a real scenario when multiple concurrent +// subscriptions exist. +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, LegacyUnsubscribeIteratesPastNonMatchingEntry) +{ + m_messageHandler = [this](connection_hdl hdl, server::message_ptr msg) + { + try + { + auto request = nlohmann::json::parse(msg->get_payload()); + if (request.contains("params") && request["params"].contains("listen")) + { + nlohmann::json response; + response["jsonrpc"] = "2.0"; + response["id"] = request["id"]; + response["result"]["listening"] = request["params"]["listen"]; + m_server.send(hdl, response.dump(), msg->get_opcode()); + } + } + catch (...) + { + } + }; + + startServer(); + IGateway& gateway = GetGatewayInstance(); + auto connectionFuture = m_connectionPromise.get_future(); + + Firebolt::Config cfg = getTestConfig(); + cfg.legacyRPCv1 = true; + Firebolt::Error connectErr = gateway.connect(cfg, [this](bool connected, const Firebolt::Error& err) + { onConnectionChange(connected, err); }); + ASSERT_EQ(connectErr, Firebolt::Error::None); + ASSERT_EQ(connectionFuture.wait_for(std::chrono::seconds(2)), std::future_status::ready); + + auto onEventA = [](void*, const nlohmann::json&) {}; + auto onEventB = [](void*, const nlohmann::json&) {}; + int cbA = 0; + int cbB = 0; + + // Subscribe to two events — eventA gets a lower message ID in rpcv1_eventMap + Firebolt::Error err = gateway.subscribe("test.onEventA", onEventA, &cbA); + EXPECT_EQ(err, Firebolt::Error::None); + + err = gateway.subscribe("test.onEventB", onEventB, &cbB); + EXPECT_EQ(err, Firebolt::Error::None); + + // Unsubscribe eventB — the loop must skip eventA's entry (++it at line 600) + // before finding eventB's entry + err = gateway.unsubscribe("test.onEventB", &cbB); + EXPECT_EQ(err, Firebolt::Error::None); + + // Clean up eventA + err = gateway.unsubscribe("test.onEventA", &cbA); + EXPECT_EQ(err, Firebolt::Error::None); +} diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index 486ec86..cb82c28 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -218,3 +218,382 @@ TEST(OnPropertyChangedCallbackUTest, InvalidJson) onPropertyChangedCallback(&subData, jsonResponse); } + +// --------------------------------------------------------------------------- +// Additional helper tests for coverage gaps +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.SetWithNonObjectParams +// Covers: src/helpers_impl.h:49-54 (non-object params wrapped in "value") +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, SetWithNonObjectParams) +{ + const std::string methodName = "test.set"; + const nlohmann::json params = 42; // scalar, not object + + // The helper wraps non-object params as {"value": params} + nlohmann::json expectedParams; + expectedParams["value"] = 42; + + std::promise> promise; + promise.set_value(Result{nlohmann::json{}}); + + EXPECT_CALL(mockGateway, request(methodName, expectedParams)).WillOnce(Return(ByMove(promise.get_future()))); + + auto result = helper.set(methodName, params); + EXPECT_TRUE(result); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.SetWithArrayParams +// Covers: src/helpers_impl.h:49-54 (array is not object → wrap in "value") +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, SetWithArrayParams) +{ + const std::string methodName = "test.set"; + const nlohmann::json params = nlohmann::json::array({1, 2, 3}); + + nlohmann::json expectedParams; + expectedParams["value"] = params; + + std::promise> promise; + promise.set_value(Result{nlohmann::json{}}); + + EXPECT_CALL(mockGateway, request(methodName, expectedParams)).WillOnce(Return(ByMove(promise.get_future()))); + + auto result = helper.set(methodName, params); + EXPECT_TRUE(result); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.InvokeFailure +// Covers: src/helpers_impl.h:63-65 (invoke returns gateway error) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, InvokeFailure) +{ + const std::string methodName = "test.invoke"; + const nlohmann::json params = {{"key", "value"}}; + + EXPECT_CALL(mockGateway, send(methodName, params)).WillOnce(Return(Error::NotConnected)); + + auto result = helper.invoke(methodName, params); + EXPECT_FALSE(result); + EXPECT_EQ(result.error(), Error::NotConnected); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.GetWithParameters +// Covers: src/helpers_impl.h:103 (getJson called with explicit parameters) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, GetWithParameters) +{ + const std::string methodName = "test.get"; + const nlohmann::json params = {{"param1", "abc"}}; + const nlohmann::json responseJson = {{"value", 99}}; + std::promise> promise; + promise.set_value(Result{responseJson}); + + EXPECT_CALL(mockGateway, request(methodName, params)).WillOnce(Return(ByMove(promise.get_future()))); + + auto result = helper.get(methodName, params); + ASSERT_TRUE(result); + EXPECT_EQ(*result, 99); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.GetWithErrorInfo +// Covers: src/helpers_impl.h:105-106 (error propagation with errorInfo) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, GetWithErrorInfo) +{ + const std::string methodName = "test.get"; + Firebolt::ErrorInfo errorInfo(-32601, "Method not found"); + std::promise> promise; + promise.set_value(Result{Error::MethodNotFound, errorInfo}); + + EXPECT_CALL(mockGateway, request(methodName, _)).WillOnce(Return(ByMove(promise.get_future()))); + + auto result = helper.get(methodName); + ASSERT_FALSE(result); + EXPECT_EQ(result.error(), Error::MethodNotFound); + EXPECT_EQ(result.errorInfo().error(), -32601); + EXPECT_EQ(result.errorInfo().message(), "Method not found"); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.SubscribeSuccess +// Covers: src/helpers_impl.h:119-133 (subscribe success flow) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, SubscribeSuccess) +{ + std::function notification = [](int) {}; + + EXPECT_CALL(mockGateway, subscribe("test.onEvent", _, _)).WillOnce(Return(Error::None)); + + IHelper& ihelper = helper; + auto result = ihelper.subscribe(this, "test.onEvent", std::move(notification), + onPropertyChangedCallback); + ASSERT_TRUE(result); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.SubscribeGatewayError +// Covers: src/helpers_impl.h:127-131 (subscribe gateway error → erase + return) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, SubscribeGatewayError) +{ + std::function notification = [](int) {}; + + EXPECT_CALL(mockGateway, subscribe("test.onEvent", _, _)).WillOnce(Return(Error::NotConnected)); + + IHelper& ihelper = helper; + auto result = ihelper.subscribe(this, "test.onEvent", std::move(notification), + onPropertyChangedCallback); + ASSERT_FALSE(result); + EXPECT_EQ(result.error(), Error::NotConnected); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.UnsubscribeNotFound +// Covers: src/helpers_impl.h:71-74 (unsubscribe with invalid id) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, UnsubscribeNotFound) +{ + auto result = helper.unsubscribe(9999); // non-existent id + EXPECT_FALSE(result); + EXPECT_EQ(result.error(), Error::General); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.UnsubscribeSuccess +// Covers: src/helpers_impl.h:75-79 (unsubscribe with valid id) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, UnsubscribeSuccess) +{ + // First subscribe to get a valid ID + std::function notification = [](int) {}; + EXPECT_CALL(mockGateway, subscribe("test.onEvent", _, _)).WillOnce(Return(Error::None)); + + IHelper& ihelper = helper; + auto subResult = ihelper.subscribe(this, "test.onEvent", std::move(notification), + onPropertyChangedCallback); + ASSERT_TRUE(subResult); + SubscriptionId id = *subResult; + + EXPECT_CALL(mockGateway, unsubscribe("test.onEvent", _)).WillOnce(Return(Error::None)); + + auto result = helper.unsubscribe(id); + EXPECT_TRUE(result); +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.UnsubscribeAllWithOwner +// Covers: src/helpers_impl.h:83-97 (unsubscribeAll matching owner) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, UnsubscribeAllWithOwner) +{ + void* owner1 = reinterpret_cast(0x1); + void* owner2 = reinterpret_cast(0x2); + + std::function notification1 = [](int) {}; + std::function notification2 = [](int) {}; + + EXPECT_CALL(mockGateway, subscribe("event1", _, _)).WillOnce(Return(Error::None)); + EXPECT_CALL(mockGateway, subscribe("event2", _, _)).WillOnce(Return(Error::None)); + + IHelper& ihelper = helper; + ihelper.subscribe(owner1, "event1", std::move(notification1), onPropertyChangedCallback); + ihelper.subscribe(owner2, "event2", std::move(notification2), onPropertyChangedCallback); + + // 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 name: HelperUTest.UnsubscribeAllNoMatch +// Covers: src/helpers_impl.h:83-97 (unsubscribeAll with no matching owner) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, UnsubscribeAllNoMatch) +{ + void* owner = reinterpret_cast(0x1); + void* otherOwner = reinterpret_cast(0x99); + + std::function notification = [](int) {}; + EXPECT_CALL(mockGateway, subscribe("event", _, _)).WillOnce(Return(Error::None)); + + IHelper& ihelper = helper; + ihelper.subscribe(owner, "event", std::move(notification), onPropertyChangedCallback); + + // Unsubscribe all for a different owner — no calls to gateway.unsubscribe + helper.unsubscribeAll(otherOwner); +} + +// --------------------------------------------------------------------------- +// Multi-arg callback test type for tuple apply path +// --------------------------------------------------------------------------- +struct TestMultiArgJson +{ + std::tuple vals; + void fromJson(const nlohmann::json& json) + { + vals = std::make_tuple(json.at("num").get(), json.at("str").get()); + } + auto value() const { return vals; } +}; + +// --------------------------------------------------------------------------- +// Test name: OnPropertyChangedCallbackUTest.MultiArgCallback +// Covers: src/helpers.h:51-52 (sizeof...(Args) > 1 → std::apply branch) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST(OnPropertyChangedCallbackUTest, MultiArgCallback) +{ + SubscriptionData subData; + subData.owner = nullptr; + subData.eventName = "test.multiEvent"; + + std::promise> promise; + auto future = promise.get_future(); + std::function notification = [&promise](int n, std::string s) + { promise.set_value({n, s}); }; + subData.notification = notification; + + nlohmann::json jsonResponse = {{"num", 7}, {"str", "hello"}}; + + onPropertyChangedCallback(&subData, jsonResponse); + + auto status = future.wait_for(std::chrono::seconds(1)); + ASSERT_EQ(status, std::future_status::ready); + auto [num, str] = future.get(); + EXPECT_EQ(num, 7); + EXPECT_EQ(str, "hello"); +} + +// --------------------------------------------------------------------------- +// Test name: OnPropertyChangedCallbackUTest.MultiArgInvalidJson +// Covers: helpers.h:59 (catch path for multi-arg template instantiation) +// Scenario: The multi-arg instantiation of onPropertyChangedCallback receives +// malformed JSON that fails fromJson(). Exercises the catch branch +// for the instantiation. +// --------------------------------------------------------------------------- +TEST(OnPropertyChangedCallbackUTest, MultiArgInvalidJson) +{ + SubscriptionData subData; + subData.owner = nullptr; + subData.eventName = "test.multiEvent"; + + std::function notification = [](int, std::string) + { FAIL() << "Notification should not be called"; }; + subData.notification = notification; + + // Missing "num" and "str" keys that TestMultiArgJson expects + nlohmann::json jsonResponse = {{"wrong_key", 42}}; + + // Should not throw — the catch block inside onPropertyChangedCallback logs and returns + onPropertyChangedCallback(&subData, jsonResponse); + // If we get here without crashing, the catch block handled the exception +} + +// --------------------------------------------------------------------------- +// Test name: SubscriptionManagerUTest.SubscribeFailure +// Covers: helpers_impl.cpp:38 (SubscriptionManager propagates error) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(SubscriptionManagerUTest, SubscribeFailure) +{ + std::function notification = [](int) {}; + EXPECT_CALL(mockHelper, subscribe(owner, "test.event", _, _)) + .WillOnce(Return(Result{Error::NotConnected})); + + auto result = subscriptionManager->subscribe("test.event", std::move(notification)); + ASSERT_FALSE(result); + EXPECT_EQ(result.error(), Error::NotConnected); +} + +// --------------------------------------------------------------------------- +// Test name: SubscriptionManagerUTest.DestructorCallsUnsubscribeAll +// Covers: helpers_impl.cpp:34 (destructor calls unsubscribeAll) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(SubscriptionManagerUTest, DestructorCallsUnsubscribeAll) +{ + EXPECT_CALL(mockHelper, unsubscribeAll(owner)).Times(1); + // Destroy the subscription manager — destructor should call unsubscribeAll + subscriptionManager.reset(); +} + +// --------------------------------------------------------------------------- +// Branch-coverage tests +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.DestructorCleansUpActiveSubscriptions +// Covers: helpers_impl.h:35-42 (destructor loop over subscriptions_) +// Also covers the false branch (empty subscriptions_ → skip loop) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, DestructorCleansUpActiveSubscriptions) +{ + // Create a HelperImpl with active subscriptions that are not + // manually unsubscribed — the destructor should clean them up. + auto* localHelper = new HelperImpl(mockGateway); + IHelper* ihelper = localHelper; + + std::function n1 = [](int) {}; + std::function n2 = [](int) {}; + + EXPECT_CALL(mockGateway, subscribe("evt1", _, _)).WillOnce(Return(Error::None)); + EXPECT_CALL(mockGateway, subscribe("evt2", _, _)).WillOnce(Return(Error::None)); + + ihelper->subscribe(this, "evt1", std::move(n1), onPropertyChangedCallback); + ihelper->subscribe(this, "evt2", std::move(n2), onPropertyChangedCallback); + + // Destructor should call gateway.unsubscribe for each active subscription + EXPECT_CALL(mockGateway, unsubscribe("evt1", _)).WillOnce(Return(Error::None)); + EXPECT_CALL(mockGateway, unsubscribe("evt2", _)).WillOnce(Return(Error::None)); + + delete localHelper; +} + +// --------------------------------------------------------------------------- +// Test name: HelperUTest.DestructorWithNoSubscriptions +// Covers: helpers_impl.h:35-42 (destructor with empty subscriptions_ → skip) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(HelperUTest, DestructorWithNoSubscriptions) +{ + // Destructor on a HelperImpl with no subscriptions should not call unsubscribe + auto* localHelper = new HelperImpl(mockGateway); + // No EXPECT_CALL for unsubscribe — none should be called + delete localHelper; +} + +// --------------------------------------------------------------------------- +// Test name: GetHelperInstanceTest.ReturnsSingleton +// Covers: helpers_impl.cpp:45,47-48 (GetHelperInstance singleton factory) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST(GetHelperInstanceTest, ReturnsSingleton) +{ + // GetHelperInstance() creates a static HelperImpl backed by the real gateway singleton. + IHelper& helper1 = GetHelperInstance(); + IHelper& helper2 = GetHelperInstance(); + // Same singleton reference + EXPECT_EQ(&helper1, &helper2); +} diff --git a/test/unit/json_typesTest.cpp b/test/unit/json_typesTest.cpp index 000923c..f5dd466 100644 --- a/test/unit/json_typesTest.cpp +++ b/test/unit/json_typesTest.cpp @@ -189,3 +189,35 @@ TEST(JsonTypesUTest, ArrayWithMixedTypes) nlohmann::json json = {1, "two", 3}; EXPECT_THROW(intArray.fromJson(json), nlohmann::json::type_error); } + +// --------------------------------------------------------------------------- +// Non-array input tests for each NL_Json_Array instantiation. +// Each instantiation generates its own copy of the throw at json_types.h:86. +// --------------------------------------------------------------------------- +TEST(JsonTypesUTest, IntegerArrayWithNonArrayPayload) +{ + NL_Json_Array arr; + nlohmann::json json = 42; + EXPECT_THROW(arr.fromJson(json), nlohmann::json::type_error); +} + +TEST(JsonTypesUTest, BooleanArrayWithNonArrayPayload) +{ + NL_Json_Array arr; + nlohmann::json json = true; + EXPECT_THROW(arr.fromJson(json), nlohmann::json::type_error); +} + +TEST(JsonTypesUTest, FloatArrayWithNonArrayPayload) +{ + NL_Json_Array arr; + nlohmann::json json = 3.14; + EXPECT_THROW(arr.fromJson(json), nlohmann::json::type_error); +} + +TEST(JsonTypesUTest, UnsignedArrayWithNonArrayPayload) +{ + NL_Json_Array arr; + nlohmann::json json = 99u; + EXPECT_THROW(arr.fromJson(json), nlohmann::json::type_error); +} diff --git a/test/unit/loggerTest.cpp b/test/unit/loggerTest.cpp new file mode 100644 index 0000000..5b4eadc --- /dev/null +++ b/test/unit/loggerTest.cpp @@ -0,0 +1,369 @@ +/** + * Copyright 2026 Comcast Cable Communications Management, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "firebolt/logger.h" +#include +#include +#include +#include + +using namespace Firebolt; + +// --------------------------------------------------------------------------- +// File: test/unit/loggerTest.cpp +// Covers: src/logger.cpp — format flag combinations (lines 119–153) +// --------------------------------------------------------------------------- + +class LoggerFormatUTest : public ::testing::Test +{ +protected: + void SetUp() override { Logger::setLogLevel(LogLevel::Debug); } + + void TearDown() override + { + // Restore defaults + Logger::setFormat(true, false, true, true); + Logger::setLogLevel(LogLevel::Error); + } + + // Captures stderr output from a Logger::log call via the FIREBOLT_LOG_ERROR macro style + std::string captureLogCall(LogLevel level, const std::string& module, const char* msg) + { + // Redirect stderr to a pipe + fflush(stderr); + int pipefd[2]; + EXPECT_EQ(pipe(pipefd), 0); + int savedStderr = dup(STDERR_FILENO); + dup2(pipefd[1], STDERR_FILENO); + + Logger::log(level, module, __FILE__, __func__, __LINE__, "%s", msg); + + fflush(stderr); + dup2(savedStderr, STDERR_FILENO); + close(savedStderr); + close(pipefd[1]); + + char buf[2048] = {0}; + ssize_t n = read(pipefd[0], buf, sizeof(buf) - 1); + close(pipefd[0]); + if (n > 0) + { + buf[n] = '\0'; + } + return std::string(buf); + } +}; + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.LocationTrue_FunctionTrue +// Covers: src/logger.cpp:141 (addLocation=true, addFunction=true branch) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LocationTrue_FunctionTrue) +{ + Logger::setFormat(false, true, true, false); + + std::string output = captureLogCall(LogLevel::Error, "Test", "hello"); + // Expect [filename:line,function] pattern + EXPECT_NE(output.find("[Firebolt|Test|Error]"), std::string::npos); + // Should contain file:line,function format (e.g. [loggerTest.cpp:72,captureLog]) + std::regex pattern(R"(\[.*\.cpp:\d+,\w+\])"); + EXPECT_TRUE(std::regex_search(output, pattern)) << "Output: " << output; + // Should NOT contain timestamp (disabled) + // Timestamp format is HH:MM:SS.mmm: + std::regex tsPattern(R"(\d{2}:\d{2}:\d{2}\.\d{3}:)"); + EXPECT_FALSE(std::regex_search(output, tsPattern)) << "Timestamp should not be present. Output: " << output; +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.LocationFalse_FunctionTrue +// Covers: src/logger.cpp:148 (only addFunction branch → [func()]) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LocationFalse_FunctionTrue) +{ + Logger::setFormat(false, false, true, false); + + std::string output = captureLogCall(LogLevel::Error, "Test", "msg"); + // Expect [function()] pattern + std::regex pattern(R"(\[\w+\(\)\])"); + EXPECT_TRUE(std::regex_search(output, pattern)) << "Output: " << output; + // Should NOT contain file:line + EXPECT_EQ(output.find(".cpp:"), std::string::npos) << "Should not contain file location. Output: " << output; +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.LocationTrue_FunctionFalse +// Covers: src/logger.cpp:145 (only addLocation branch → [file:line]) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LocationTrue_FunctionFalse) +{ + Logger::setFormat(false, true, false, false); + + std::string output = captureLogCall(LogLevel::Error, "Test", "msg"); + // Expect [filename:line] pattern without function name + std::regex pattern(R"(\[.*\.cpp:\d+\])"); + EXPECT_TRUE(std::regex_search(output, pattern)) << "Output: " << output; + // Should NOT contain function() pattern + std::regex funcPattern(R"(\[\w+\(\)\])"); + EXPECT_FALSE(std::regex_search(output, funcPattern)) << "Output: " << output; +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.LocationFalse_FunctionFalse +// Covers: src/logger.cpp:138 (neither location nor function → no [] block) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LocationFalse_FunctionFalse) +{ + Logger::setFormat(false, false, false, false); + + std::string output = captureLogCall(LogLevel::Error, "Test", "msg"); + // Should contain the module/level tag but no location/function brackets after it + EXPECT_NE(output.find("[Firebolt|Test|Error]"), std::string::npos); + // No file:line or function() + EXPECT_EQ(output.find(".cpp:"), std::string::npos) << "Output: " << output; + std::regex funcPattern(R"(\[\w+\(\)\])"); + EXPECT_FALSE(std::regex_search(output, funcPattern)) << "Output: " << output; + // No thread id + EXPECT_EQ(output.find(" + std::regex tidPattern(R"()"); + EXPECT_TRUE(std::regex_search(output, tidPattern)) << "Output: " << output; +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.ThreadIdDisabled +// Covers: src/logger.cpp:151 (formatter_addThreadId=false → skip) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, ThreadIdDisabled) +{ + Logger::setFormat(true, true, true, false); + + std::string output = captureLogCall(LogLevel::Error, "Test", "msg"); + EXPECT_EQ(output.find(")"); + EXPECT_TRUE(std::regex_search(output, tidPattern)) << "Output: " << output; + // Message + EXPECT_NE(output.find("hello world"), std::string::npos); +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.LogLevelFiltering +// Covers: src/logger.cpp:87 (logLevel > _logLevel early return) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LogLevelFiltering) +{ + Logger::setLogLevel(LogLevel::Error); + Logger::setFormat(false, false, false, false); + + // Debug should be filtered out when level is Error + std::string output = captureLogCall(LogLevel::Debug, "Test", "should not appear"); + EXPECT_TRUE(output.empty()) << "Debug message should be filtered. Output: " << output; +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.SetLogLevelMaxLevel +// Covers: src/logger.cpp:68-69 (logLevel == MaxLevel → set to Debug) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, SetLogLevelMaxLevel) +{ + Logger::setLogLevel(LogLevel::MaxLevel); + // MaxLevel maps to Debug internally + EXPECT_TRUE(Logger::isLogLevelEnabled(LogLevel::Debug)); +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.SetLogLevelBeyondMax +// Covers: src/logger.cpp:64-67 (logLevel < MaxLevel guard) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, SetLogLevelBeyondMax) +{ + Logger::setLogLevel(LogLevel::Error); + // Try to set a value beyond MaxLevel — should be ignored + Logger::setLogLevel(static_cast(static_cast(LogLevel::MaxLevel) + 1)); + // Level should still be Error (unchanged) + EXPECT_TRUE(Logger::isLogLevelEnabled(LogLevel::Error)); + EXPECT_FALSE(Logger::isLogLevelEnabled(LogLevel::Warning)); +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.MessageTruncation +// Covers: src/logger.cpp:96-99 (message truncation at MaxBufSize) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, MessageTruncation) +{ + Logger::setFormat(false, false, false, false); + // Generate a message larger than MaxBufSize (1024) + std::string longMsg(2000, 'X'); + std::string output = captureLogCall(LogLevel::Error, "Test", longMsg.c_str()); + // Should still produce output (truncated) without crashing + EXPECT_FALSE(output.empty()); + EXPECT_NE(output.find("[Firebolt|Test|Error]"), std::string::npos); +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.MessageWithTrailingNewline +// Covers: src/logger.cpp:100-103 (trailing newline removal) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, MessageWithTrailingNewline) +{ + Logger::setFormat(false, false, false, false); + + // The logger strips trailing newlines from the message + std::string output = captureLogCall(LogLevel::Error, "Test", "trailing newline\n"); + // The output should end with the message text (not double newline) + EXPECT_NE(output.find("trailing newline"), std::string::npos); + // There should be exactly one newline at the end (from fprintf) + size_t msgPos = output.find("trailing newline"); + EXPECT_NE(msgPos, std::string::npos); +} + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.LogLevelNames +// Covers: src/logger.cpp:43-49 (_logLevelNames map) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LogLevelNames) +{ + Logger::setFormat(false, false, false, false); + + std::string errOutput = captureLogCall(LogLevel::Error, "Mod", "e"); + EXPECT_NE(errOutput.find("|Error]"), std::string::npos); + + std::string warnOutput = captureLogCall(LogLevel::Warning, "Mod", "w"); + EXPECT_NE(warnOutput.find("|Warning]"), std::string::npos); + + std::string noticeOutput = captureLogCall(LogLevel::Notice, "Mod", "n"); + EXPECT_NE(noticeOutput.find("|Notice]"), std::string::npos); + + std::string infoOutput = captureLogCall(LogLevel::Info, "Mod", "i"); + EXPECT_NE(infoOutput.find("|Info]"), std::string::npos); + + std::string debugOutput = captureLogCall(LogLevel::Debug, "Mod", "d"); + EXPECT_NE(debugOutput.find("|Debug]"), std::string::npos); +} + +// --------------------------------------------------------------------------- +// ⚠️ PRODUCTION CODE FLAG +// File: src/logger.cpp:123 +// Symptom: strrchr(file.c_str(), '/') returns nullptr when file has no '/', +// and assigning nullptr to std::string via operator=(const char*) is +// undefined behavior (crash/segfault in libstdc++). +// Expected: Should check for nullptr before assigning to std::string. +// Risk: Crash when formatter_addLocation=true and __FILE__ has no slash +// (unlikely in production Docker builds, but possible in unit tests +// or embedded builds with relative paths). +// Action: Review before test is written. +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Branch-coverage tests +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Test name: LoggerFormatUTest.LocationWithSlashInPath +// Covers: logger.cpp:126 (fileName.empty() == false → substr(1) branch) +// When __FILE__ contains '/', strrchr returns a non-null pointer, +// fileName is non-empty, and the substr(1) path (line 130) is taken. +// This is the normal case for all Linux/Docker builds. +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LocationWithSlashInPath) +{ + Logger::setFormat(false, true, false, false); + + // Redirect stderr + fflush(stderr); + int pipefd[2]; + ASSERT_EQ(pipe(pipefd), 0); + int savedStderr = dup(STDERR_FILENO); + dup2(pipefd[1], STDERR_FILENO); + + // Pass a file path WITH a slash — triggers the substr(1) branch + Logger::log(LogLevel::Error, "Test", "/some/path/myfile.cpp", "testFunc", 99, "msg"); + + fflush(stderr); + dup2(savedStderr, STDERR_FILENO); + close(savedStderr); + close(pipefd[1]); + + char buf[2048] = {0}; + ssize_t n = read(pipefd[0], buf, sizeof(buf) - 1); + close(pipefd[0]); + + std::string output(buf, n > 0 ? n : 0); + // Should show just "myfile.cpp" (stripped the directory), not "/myfile.cpp" + EXPECT_NE(output.find("myfile.cpp:99"), std::string::npos) << "Output: " << output; + EXPECT_EQ(output.find("/myfile.cpp"), std::string::npos) << "Should strip leading slash. Output: " << output; +} diff --git a/test/unit/transportTest.cpp b/test/unit/transportTest.cpp index 6bd710b..5ad549b 100644 --- a/test/unit/transportTest.cpp +++ b/test/unit/transportTest.cpp @@ -27,6 +27,7 @@ #include #include #include +#include "firebolt/logger.h" using namespace Firebolt::Transport; @@ -696,3 +697,360 @@ TEST_F(TransportCustomServerUTest, MalformedMessageFromServer) err = transport.disconnect(); EXPECT_EQ(err, Firebolt::Error::None); } + +// --------------------------------------------------------------------------- +// Additional transport tests for coverage gaps +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Test name: TransportCustomServerUTest.NonTextMessageIgnored +// Covers: src/transport.cpp:305-308 (non-text opcode → warning + ignore) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(TransportCustomServerUTest, NonTextMessageIgnored) +{ + m_server.set_message_handler( + [this](connection_hdl hdl, server::message_ptr /*msg*/) + { + // Send a binary frame instead of text + m_server.send(hdl, "binary data", websocketpp::frame::opcode::binary); + // Then send a valid text message + nlohmann::json response; + response["jsonrpc"] = "2.0"; + response["id"] = 1; + response["result"] = {{"ok", true}}; + m_server.send(hdl, response.dump(), websocketpp::frame::opcode::text); + }); + + StartServer(); + + Transport transport; + std::promise connectionPromise; + auto connectionFuture = connectionPromise.get_future(); + std::promise validMessagePromise; + auto validMessageFuture = validMessagePromise.get_future(); + + auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) + { + if (connected) + { + connectionPromise.set_value(); + } + }; + + auto onMessage = [&](const nlohmann::json& msg) { validMessagePromise.set_value(msg); }; + + Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange); + ASSERT_EQ(err, Firebolt::Error::None); + ASSERT_EQ(connectionFuture.wait_for(std::chrono::milliseconds(150)), std::future_status::ready) + << "Connection timed out"; + + // Trigger the server to send a binary + text message + err = transport.send("trigger", {}, transport.getNextMessageID()); + ASSERT_EQ(err, Firebolt::Error::None); + + // Only the text message should arrive + auto msgStatus = validMessageFuture.wait_for(std::chrono::milliseconds(300)); + ASSERT_EQ(msgStatus, std::future_status::ready) << "Valid text message not received after binary was ignored"; + + nlohmann::json received = validMessageFuture.get(); + EXPECT_TRUE(received.contains("result")); + + transport.disconnect(); +} + +// --------------------------------------------------------------------------- +// Test name: TransportIntegrationUTest.DisconnectWhileConnected +// Covers: src/transport.cpp:212-246 (disconnect from Connected state) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(TransportIntegrationUTest, DisconnectWhileConnected) +{ + Transport transport; + std::promise connectionPromise; + auto connectionFuture = connectionPromise.get_future(); + + auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) + { + if (connected) + { + connectionPromise.set_value(true); + } + }; + + auto onMessage = [&](const nlohmann::json& /*msg*/) {}; + + Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange); + ASSERT_EQ(err, Firebolt::Error::None); + + auto status = connectionFuture.wait_for(std::chrono::milliseconds(150)); + ASSERT_EQ(status, std::future_status::ready) << "Connection timed out"; + + // Disconnect and verify state reset is clean + err = transport.disconnect(); + EXPECT_EQ(err, Firebolt::Error::None); + + // After disconnect, sending should fail with NotConnected + err = transport.send("test.method", {}, 1); + EXPECT_EQ(err, Firebolt::Error::NotConnected); +} + +// --------------------------------------------------------------------------- +// Test name: TransportIntegrationUTest.MultipleMessagesInSequence +// Covers: src/transport.cpp:100-120 (processQueuedMessages loop) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(TransportIntegrationUTest, MultipleMessagesInSequence) +{ + Transport transport; + std::promise connectionPromise; + auto connectionFuture = connectionPromise.get_future(); + std::atomic messageCount{0}; + std::promise allMessagesPromise; + auto allMessagesFuture = allMessagesPromise.get_future(); + + auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) + { + if (connected) + { + connectionPromise.set_value(true); + } + }; + + auto onMessage = [&](const nlohmann::json& /*msg*/) + { + if (++messageCount >= 3) + { + allMessagesPromise.set_value(); + } + }; + + Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange); + ASSERT_EQ(err, Firebolt::Error::None); + + auto status = connectionFuture.wait_for(std::chrono::milliseconds(150)); + ASSERT_EQ(status, std::future_status::ready) << "Connection timed out"; + + // Send multiple messages + for (int i = 0; i < 3; i++) + { + nlohmann::json params = {{"seq", i}}; + err = transport.send("test.method", params, transport.getNextMessageID()); + EXPECT_EQ(err, Firebolt::Error::None); + } + + auto msgStatus = allMessagesFuture.wait_for(std::chrono::milliseconds(500)); + ASSERT_EQ(msgStatus, std::future_status::ready) << "Not all messages were received"; + EXPECT_EQ(messageCount.load(), 3); + + transport.disconnect(); +} + +// --------------------------------------------------------------------------- +// Test name: TransportIntegrationUTest.ConnectWithTransportLogging +// Covers: src/transport.cpp:155-173 (transport logging include/exclude params) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(TransportIntegrationUTest, ConnectWithTransportLogging) +{ + Transport transport; + std::promise connectionPromise; + auto connectionFuture = connectionPromise.get_future(); + + auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) + { + if (connected) + { + connectionPromise.set_value(true); + } + }; + + auto onMessage = [&](const nlohmann::json& /*msg*/) {}; + + // Provide explicit transport logging masks + Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange, + static_cast(websocketpp::log::alevel::all), + static_cast(websocketpp::log::alevel::frame_payload)); + ASSERT_EQ(err, Firebolt::Error::None); + + auto status = connectionFuture.wait_for(std::chrono::milliseconds(150)); + ASSERT_EQ(status, std::future_status::ready) << "Connection timed out"; + + err = transport.disconnect(); + EXPECT_EQ(err, Firebolt::Error::None); +} + +// --------------------------------------------------------------------------- +// Test name: TransportUTest.GetNextMessageIDMonotonic +// Covers: src/transport.cpp:277 (atomic increment) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(TransportUTest, GetNextMessageIDMonotonic) +{ + unsigned prev = transport.getNextMessageID(); + for (int i = 0; i < 100; ++i) + { + unsigned next = transport.getNextMessageID(); + EXPECT_EQ(next, prev + 1); + prev = next; + } +} + +// --------------------------------------------------------------------------- +// Test name: TransportCustomServerUTest.DisconnectFromDisconnectedState +// Covers: src/transport.cpp:215-220 (disconnect when state is Disconnected) +// Scenario type: edge case +// --------------------------------------------------------------------------- +TEST_F(TransportCustomServerUTest, DisconnectFromDisconnectedState) +{ + m_server.set_open_handler( + [this](connection_hdl hdl) + { + m_server.close(hdl, websocketpp::close::status::normal, "Bye"); + }); + + StartServer(); + + Transport transport; + std::promise closedPromise; + auto closedFuture = closedPromise.get_future(); + + auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) + { + if (!connected) + { + closedPromise.set_value(); + } + }; + + auto onMessage = [](const nlohmann::json& /*msg*/) {}; + + Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange); + ASSERT_EQ(err, Firebolt::Error::None); + + // Wait for server to close connection + ASSERT_EQ(closedFuture.wait_for(std::chrono::milliseconds(300)), std::future_status::ready); + + // Now disconnect from already-disconnected state (no close() call, just cleanup) + err = transport.disconnect(); + EXPECT_EQ(err, Firebolt::Error::None); +} + +// --------------------------------------------------------------------------- +// Branch-coverage tests +// --------------------------------------------------------------------------- + +// --------------------------------------------------------------------------- +// Test name: TransportIntegrationUTest.DebugLoggingOnSendAndReceive +// Covers: transport.cpp:121 (debugEnabled_ true → log received msg) +// transport.cpp:293 (debugEnabled_ true → log sent msg) +// Scenario type: success +// --------------------------------------------------------------------------- +TEST_F(TransportIntegrationUTest, DebugLoggingOnSendAndReceive) +{ + // Set log level to Debug so debugEnabled_ is true when connect() is called + Firebolt::Logger::setLogLevel(Firebolt::LogLevel::Debug); + + Transport transport; + std::promise connectionPromise; + auto connectionFuture = connectionPromise.get_future(); + std::promise messagePromise; + auto messageFuture = messagePromise.get_future(); + + auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) + { + if (connected) + { + connectionPromise.set_value(true); + } + }; + + auto onMessage = [&](const nlohmann::json& msg) { messagePromise.set_value(msg); }; + + Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange); + ASSERT_EQ(err, Firebolt::Error::None); + + auto status = connectionFuture.wait_for(std::chrono::milliseconds(150)); + ASSERT_EQ(status, std::future_status::ready) << "Connection timed out"; + + nlohmann::json params = {{"key", "value"}}; + unsigned msgId = transport.getNextMessageID(); + err = transport.send("test.debug", params, msgId); + EXPECT_EQ(err, Firebolt::Error::None); + + auto msgStatus = messageFuture.wait_for(std::chrono::milliseconds(150)); + ASSERT_EQ(msgStatus, std::future_status::ready) << "Message response timed out"; + + nlohmann::json received = messageFuture.get(); + EXPECT_EQ(received["id"], msgId); + + transport.disconnect(); + Firebolt::Logger::setLogLevel(Firebolt::LogLevel::Error); +} + +// --------------------------------------------------------------------------- +// Test name: TransportCustomServerUTest.MessageDuringShutdownIgnored +// Covers: Resilience — verifies no crash or hang when an echo arrives +// during or immediately after disconnect(). The stopMessageWorker_ +// guard (transport.cpp:323-324) is unreachable from the public API, +// but this test validates safe behavior under rapid teardown. +// Scenario type: resilience +// --------------------------------------------------------------------------- +TEST_F(TransportCustomServerUTest, MessageDuringShutdownIgnored) +{ + std::atomic serverMsgCount{0}; + + m_server.set_message_handler( + [this, &serverMsgCount](connection_hdl hdl, server::message_ptr msg) + { + (void)this; + serverMsgCount++; + // Echo back + m_server.send(hdl, msg->get_payload(), msg->get_opcode()); + }); + + StartServer(); + + Transport transport; + std::promise connectionPromise; + auto connectionFuture = connectionPromise.get_future(); + std::atomic receivedCount{0}; + + auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) + { + if (connected) + { + connectionPromise.set_value(); + } + }; + + auto onMessage = [&](const nlohmann::json& /*msg*/) { receivedCount++; }; + + Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange); + ASSERT_EQ(err, Firebolt::Error::None); + ASSERT_EQ(connectionFuture.wait_for(std::chrono::milliseconds(150)), std::future_status::ready); + + // Send a message, then immediately disconnect + // The echo may arrive while the message worker is stopping + transport.send("test.method", {{"k", "v"}}, transport.getNextMessageID()); + transport.disconnect(); + + // No crash, no hang — that's the test + EXPECT_GE(serverMsgCount.load(), 1); +} + +// --------------------------------------------------------------------------- +// Test name: TransportUTest.ConnectWithInvalidUrl +// Covers: transport.cpp:191-192 (get_connection returns ec → NotConnected) +// Scenario type: failure +// --------------------------------------------------------------------------- +TEST_F(TransportUTest, ConnectWithInvalidUrl) +{ + Transport transport; + auto onMessage = [](const nlohmann::json&) {}; + auto onConnectionChange = [](bool, const Firebolt::Error&) {}; + + // An empty or completely invalid URL should cause get_connection to fail + Firebolt::Error err = transport.connect("", onMessage, onConnectionChange); + EXPECT_EQ(err, Firebolt::Error::NotConnected); +} From d7c5858a9e2e9ce325509ed2ff16500e5ddef8f1 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Fri, 15 May 2026 14:21:39 -0400 Subject: [PATCH 02/10] RDKEMW-14899 : Resolve nullptr UB in logger and address PR comments --- src/logger.cpp | 8 ++-- test/unit/gatewayTest.cpp | 4 +- test/unit/helperTest.cpp | 6 ++- test/unit/loggerTest.cpp | 93 ++++++++++++++++++++++++++++++------- test/unit/transportTest.cpp | 7 ++- 5 files changed, 90 insertions(+), 28 deletions(-) diff --git a/src/logger.cpp b/src/logger.cpp index 609199d..5bc6e08 100644 --- a/src/logger.cpp +++ b/src/logger.cpp @@ -120,14 +120,14 @@ void Logger::log(LogLevel logLevel, const std::string& module, const std::string std::string fileName; if (formatter_addLocation) { - fileName = strrchr(file.c_str(), '/'); - if (fileName.empty()) + const char* slash = strrchr(file.c_str(), '/'); + if (slash != nullptr) { - fileName = file; + fileName = slash + 1; } else { - fileName = fileName.substr(1); + fileName = file; } } diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index 9ebf851..0bc4807 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -1042,8 +1042,8 @@ TEST_F(GatewayUTest, SubscribeErrorFromServer) int dummyCb = 0; Firebolt::Error err = gateway.subscribe("test.onInvalid", onEvent, &dummyCb); - // The error from the server should be propagated, and the local subscription cleaned up - EXPECT_NE(err, Firebolt::Error::None); + // The server's JSON-RPC error (-32601) should be propagated as MethodNotFound + EXPECT_EQ(err, Firebolt::Error::MethodNotFound); } // --------------------------------------------------------------------------- diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index cb82c28..e64df37 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -440,7 +440,9 @@ TEST_F(HelperUTest, UnsubscribeAllNoMatch) IHelper& ihelper = helper; ihelper.subscribe(owner, "event", std::move(notification), onPropertyChangedCallback); - // Unsubscribe all for a different owner — no calls to gateway.unsubscribe + // 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); } @@ -454,7 +456,7 @@ struct TestMultiArgJson { vals = std::make_tuple(json.at("num").get(), json.at("str").get()); } - auto value() const { return vals; } + const auto& value() const { return vals; } }; // --------------------------------------------------------------------------- diff --git a/test/unit/loggerTest.cpp b/test/unit/loggerTest.cpp index 5b4eadc..35c1f51 100644 --- a/test/unit/loggerTest.cpp +++ b/test/unit/loggerTest.cpp @@ -17,6 +17,7 @@ */ #include "firebolt/logger.h" +#include #include #include #include @@ -46,9 +47,20 @@ class LoggerFormatUTest : public ::testing::Test { // Redirect stderr to a pipe fflush(stderr); - int pipefd[2]; - EXPECT_EQ(pipe(pipefd), 0); + int pipefd[2] = {-1, -1}; + if (pipe(pipefd) != 0) + { + ADD_FAILURE() << "pipe() failed while capturing logger output"; + return std::string(); + } int savedStderr = dup(STDERR_FILENO); + if (savedStderr < 0) + { + close(pipefd[0]); + close(pipefd[1]); + ADD_FAILURE() << "dup(STDERR_FILENO) failed while capturing logger output"; + return std::string(); + } dup2(pipefd[1], STDERR_FILENO); Logger::log(level, module, __FILE__, __func__, __LINE__, "%s", msg); @@ -266,9 +278,13 @@ TEST_F(LoggerFormatUTest, MessageTruncation) // Generate a message larger than MaxBufSize (1024) std::string longMsg(2000, 'X'); std::string output = captureLogCall(LogLevel::Error, "Test", longMsg.c_str()); - // Should still produce output (truncated) without crashing - EXPECT_FALSE(output.empty()); EXPECT_NE(output.find("[Firebolt|Test|Error]"), std::string::npos); + // The raw message is truncated to MaxBufSize-1 chars, and the formatted + // output buffer (also MaxBufSize) further limits it because the prefix + // consumes space. Verify truncation actually occurred: + size_t xCount = std::count(output.begin(), output.end(), 'X'); + EXPECT_GT(xCount, 0u) << "Truncated output should still contain message characters"; + EXPECT_LT(xCount, longMsg.size()) << "Message should have been truncated to fit MaxBufSize"; } // --------------------------------------------------------------------------- @@ -282,11 +298,13 @@ TEST_F(LoggerFormatUTest, MessageWithTrailingNewline) // The logger strips trailing newlines from the message std::string output = captureLogCall(LogLevel::Error, "Test", "trailing newline\n"); - // The output should end with the message text (not double newline) - EXPECT_NE(output.find("trailing newline"), std::string::npos); - // There should be exactly one newline at the end (from fprintf) + // The output should contain the message text size_t msgPos = output.find("trailing newline"); - EXPECT_NE(msgPos, std::string::npos); + ASSERT_NE(msgPos, std::string::npos); + // After stripping the user's \n, only fprintf's \n should remain: + // the text "trailing newline" should be followed by exactly "\n" (end of output) + std::string afterMsg = output.substr(msgPos + strlen("trailing newline")); + EXPECT_EQ(afterMsg, "\n") << "Expected exactly one trailing newline. Got: [" << afterMsg << "]"; } // --------------------------------------------------------------------------- @@ -315,17 +333,49 @@ TEST_F(LoggerFormatUTest, LogLevelNames) } // --------------------------------------------------------------------------- -// ⚠️ PRODUCTION CODE FLAG -// File: src/logger.cpp:123 -// Symptom: strrchr(file.c_str(), '/') returns nullptr when file has no '/', -// and assigning nullptr to std::string via operator=(const char*) is -// undefined behavior (crash/segfault in libstdc++). -// Expected: Should check for nullptr before assigning to std::string. -// Risk: Crash when formatter_addLocation=true and __FILE__ has no slash -// (unlikely in production Docker builds, but possible in unit tests -// or embedded builds with relative paths). -// Action: Review before test is written. +// Test name: LoggerFormatUTest.LocationWithoutSlashInPath +// Covers: src/logger.cpp:123 (strrchr returns nullptr → use file as-is) +// Regression: previously assigned nullptr to std::string (UB / crash) +// Scenario type: edge case // --------------------------------------------------------------------------- +TEST_F(LoggerFormatUTest, LocationWithoutSlashInPath) +{ + Logger::setFormat(false, true, false, false); + + // Redirect stderr + fflush(stderr); + int pipefd[2] = {-1, -1}; + if (pipe(pipefd) != 0) + { + ADD_FAILURE() << "pipe() failed while capturing logger output"; + return; + } + int savedStderr = dup(STDERR_FILENO); + if (savedStderr < 0) + { + close(pipefd[0]); + close(pipefd[1]); + ADD_FAILURE() << "dup(STDERR_FILENO) failed while capturing logger output"; + return; + } + dup2(pipefd[1], STDERR_FILENO); + + // Pass a file path WITHOUT a slash — previously caused nullptr UB + Logger::log(LogLevel::Error, "Test", "noSlashFile.cpp", "testFunc", 42, "msg"); + + fflush(stderr); + dup2(savedStderr, STDERR_FILENO); + close(savedStderr); + close(pipefd[1]); + + char buf[2048] = {0}; + ssize_t n = read(pipefd[0], buf, sizeof(buf) - 1); + close(pipefd[0]); + + std::string output(buf, n > 0 ? n : 0); + // Should use the bare filename as-is + EXPECT_NE(output.find("noSlashFile.cpp:42"), std::string::npos) << "Output: " << output; +} // --------------------------------------------------------------------------- // Branch-coverage tests @@ -348,6 +398,13 @@ TEST_F(LoggerFormatUTest, LocationWithSlashInPath) int pipefd[2]; ASSERT_EQ(pipe(pipefd), 0); int savedStderr = dup(STDERR_FILENO); + if (savedStderr < 0) + { + close(pipefd[0]); + close(pipefd[1]); + ADD_FAILURE() << "dup(STDERR_FILENO) failed while capturing logger output"; + return; + } dup2(pipefd[1], STDERR_FILENO); // Pass a file path WITH a slash — triggers the substr(1) branch diff --git a/test/unit/transportTest.cpp b/test/unit/transportTest.cpp index 5ad549b..b265509 100644 --- a/test/unit/transportTest.cpp +++ b/test/unit/transportTest.cpp @@ -17,6 +17,7 @@ */ #include "transport.h" +#include "firebolt/logger.h" #include #include #include @@ -27,7 +28,6 @@ #include #include #include -#include "firebolt/logger.h" using namespace Firebolt::Transport; @@ -950,6 +950,10 @@ TEST_F(TransportIntegrationUTest, DebugLoggingOnSendAndReceive) { // Set log level to Debug so debugEnabled_ is true when connect() is called Firebolt::Logger::setLogLevel(Firebolt::LogLevel::Debug); + struct LogLevelGuard + { + ~LogLevelGuard() { Firebolt::Logger::setLogLevel(Firebolt::LogLevel::Error); } + } logGuard; Transport transport; std::promise connectionPromise; @@ -985,7 +989,6 @@ TEST_F(TransportIntegrationUTest, DebugLoggingOnSendAndReceive) EXPECT_EQ(received["id"], msgId); transport.disconnect(); - Firebolt::Logger::setLogLevel(Firebolt::LogLevel::Error); } // --------------------------------------------------------------------------- From 20426b11d45e1861fcefea229ce6e60cce955919 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Mon, 18 May 2026 09:18:41 -0400 Subject: [PATCH 03/10] RDKEMW-14899 : Add develop branch to CI build and test workflow --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 21b6861..1ad8bb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI Build and Test on: push: - branches: [ main, '+([0-9])\.+([0-9])\.x-maintenance', '+([0-9])\.+([0-9])-rc' ] + branches: [ main, develop, '+([0-9])\.+([0-9])\.x-maintenance', '+([0-9])\.+([0-9])-rc' ] pull_request: - branches: [ main, '+([0-9])\.+([0-9])\.x-maintenance', '+([0-9])\.+([0-9])-rc' ] + branches: [ main, develop, '+([0-9])\.+([0-9])\.x-maintenance', '+([0-9])\.+([0-9])-rc' ] workflow_dispatch: inputs: protocol: From 31b209233d66a03de679ecad0ced979a36c2b5a6 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Mon, 18 May 2026 10:13:13 -0400 Subject: [PATCH 04/10] RDKEMW-14899 : Fix clang-format violations --- test/unit/gatewayTest.cpp | 38 ++++++++++++++++++------------------- test/unit/helperTest.cpp | 15 +++++++-------- test/unit/transportTest.cpp | 7 ++----- 3 files changed, 27 insertions(+), 33 deletions(-) diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index 0bc4807..f8ae4c1 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -934,16 +934,15 @@ TEST_F(GatewayUTest, SendNotConnected) Firebolt::Config cfg = getTestConfig(); cfg.wsUrl = "ws://localhost:49199"; // No server here - Firebolt::Error err = - gateway.connect(cfg, - [&](bool connected, const Firebolt::Error&) - { - bool expected = false; - if (promiseSet.compare_exchange_strong(expected, true)) - { - connPromise.set_value(connected); - } - }); + Firebolt::Error err = gateway.connect(cfg, + [&](bool connected, const Firebolt::Error&) + { + bool expected = false; + if (promiseSet.compare_exchange_strong(expected, true)) + { + connPromise.set_value(connected); + } + }); ASSERT_EQ(err, Firebolt::Error::None); // Wait for connection failure @@ -1144,8 +1143,8 @@ TEST_F(GatewayUTest, WaitTimeConfiguration) Firebolt::Config cfg = getTestConfig(); cfg.waitTime_ms = 200; // Short timeout - Firebolt::Error err = gateway.connect(cfg, [this](bool connected, const Firebolt::Error& e) - { onConnectionChange(connected, e); }); + Firebolt::Error err = + gateway.connect(cfg, [this](bool connected, const Firebolt::Error& e) { onConnectionChange(connected, e); }); ASSERT_EQ(err, Firebolt::Error::None); ASSERT_EQ(connectionFuture.wait_for(std::chrono::seconds(2)), std::future_status::ready); @@ -1212,14 +1211,13 @@ TEST_F(GatewayUTest, RequestFailsWhenSendErrors) Firebolt::Config cfg = getTestConfig(); cfg.wsUrl = "ws://localhost:49198"; - Firebolt::Error err = - gateway.connect(cfg, - [&](bool connected, const Firebolt::Error&) - { - bool exp = false; - if (pset.compare_exchange_strong(exp, true)) - connPromise.set_value(connected); - }); + Firebolt::Error err = gateway.connect(cfg, + [&](bool connected, const Firebolt::Error&) + { + bool exp = false; + if (pset.compare_exchange_strong(exp, true)) + connPromise.set_value(connected); + }); ASSERT_EQ(err, Firebolt::Error::None); connFuture.wait_for(std::chrono::milliseconds(500)); diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index e64df37..a371629 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -338,8 +338,8 @@ TEST_F(HelperUTest, SubscribeSuccess) EXPECT_CALL(mockGateway, subscribe("test.onEvent", _, _)).WillOnce(Return(Error::None)); IHelper& ihelper = helper; - auto result = ihelper.subscribe(this, "test.onEvent", std::move(notification), - onPropertyChangedCallback); + auto result = + ihelper.subscribe(this, "test.onEvent", std::move(notification), onPropertyChangedCallback); ASSERT_TRUE(result); } @@ -355,8 +355,8 @@ TEST_F(HelperUTest, SubscribeGatewayError) EXPECT_CALL(mockGateway, subscribe("test.onEvent", _, _)).WillOnce(Return(Error::NotConnected)); IHelper& ihelper = helper; - auto result = ihelper.subscribe(this, "test.onEvent", std::move(notification), - onPropertyChangedCallback); + auto result = + ihelper.subscribe(this, "test.onEvent", std::move(notification), onPropertyChangedCallback); ASSERT_FALSE(result); EXPECT_EQ(result.error(), Error::NotConnected); } @@ -385,8 +385,8 @@ TEST_F(HelperUTest, UnsubscribeSuccess) EXPECT_CALL(mockGateway, subscribe("test.onEvent", _, _)).WillOnce(Return(Error::None)); IHelper& ihelper = helper; - auto subResult = ihelper.subscribe(this, "test.onEvent", std::move(notification), - onPropertyChangedCallback); + auto subResult = + ihelper.subscribe(this, "test.onEvent", std::move(notification), onPropertyChangedCallback); ASSERT_TRUE(subResult); SubscriptionId id = *subResult; @@ -472,8 +472,7 @@ TEST(OnPropertyChangedCallbackUTest, MultiArgCallback) std::promise> promise; auto future = promise.get_future(); - std::function notification = [&promise](int n, std::string s) - { promise.set_value({n, s}); }; + std::function notification = [&promise](int n, std::string s) { promise.set_value({n, s}); }; subData.notification = notification; nlohmann::json jsonResponse = {{"num", 7}, {"str", "hello"}}; diff --git a/test/unit/transportTest.cpp b/test/unit/transportTest.cpp index b265509..0d689a4 100644 --- a/test/unit/transportTest.cpp +++ b/test/unit/transportTest.cpp @@ -903,11 +903,8 @@ TEST_F(TransportUTest, GetNextMessageIDMonotonic) // --------------------------------------------------------------------------- TEST_F(TransportCustomServerUTest, DisconnectFromDisconnectedState) { - m_server.set_open_handler( - [this](connection_hdl hdl) - { - m_server.close(hdl, websocketpp::close::status::normal, "Bye"); - }); + m_server.set_open_handler([this](connection_hdl hdl) + { m_server.close(hdl, websocketpp::close::status::normal, "Bye"); }); StartServer(); From 4063003a6c0ad23ad113bcca7147ca20b8eebbdd Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Mon, 18 May 2026 11:53:01 -0400 Subject: [PATCH 05/10] RDKEMW-14899 : Resolve Copilot review comments in unit tests --- test/unit/gatewayTest.cpp | 20 +++++++++++++------- test/unit/helperTest.cpp | 2 +- test/unit/loggerTest.cpp | 8 ++++---- test/unit/transportTest.cpp | 22 ++++++++++++++-------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index f8ae4c1..c41eb8a 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -945,16 +945,17 @@ TEST_F(GatewayUTest, SendNotConnected) }); ASSERT_EQ(err, Firebolt::Error::None); - // Wait for connection failure - auto status = connFuture.wait_for(std::chrono::milliseconds(500)); - if (status == std::future_status::ready) - { - EXPECT_FALSE(connFuture.get()); - } + // Wait for connection failure — assert so we don't proceed with dangling callback refs + auto status = connFuture.wait_for(std::chrono::milliseconds(2000)); + ASSERT_EQ(status, std::future_status::ready) << "Connection failure callback not received"; + EXPECT_FALSE(connFuture.get()); // Send should fail with NotConnected err = gateway.send("test.method", {}); EXPECT_EQ(err, Firebolt::Error::NotConnected); + + // Disconnect while locals are still alive to prevent use-after-scope + gateway.disconnect(); } // --------------------------------------------------------------------------- @@ -1220,7 +1221,9 @@ TEST_F(GatewayUTest, RequestFailsWhenSendErrors) }); ASSERT_EQ(err, Firebolt::Error::None); - connFuture.wait_for(std::chrono::milliseconds(500)); + // Assert callback arrives — prevents proceeding with dangling refs + auto connStatus = connFuture.wait_for(std::chrono::milliseconds(2000)); + ASSERT_EQ(connStatus, std::future_status::ready) << "Connection failure callback not received"; // Now request — transport.send will fail with NotConnected, // which triggers the error branch in Client::request (lines 136-141) @@ -1231,6 +1234,9 @@ TEST_F(GatewayUTest, RequestFailsWhenSendErrors) auto result = future.get(); EXPECT_FALSE(result); EXPECT_EQ(result.error(), Firebolt::Error::NotConnected); + + // Disconnect while locals are still alive to prevent use-after-scope + gateway.disconnect(); } // --------------------------------------------------------------------------- diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index a371629..56823de 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -581,7 +581,7 @@ TEST_F(HelperUTest, DestructorWithNoSubscriptions) { // Destructor on a HelperImpl with no subscriptions should not call unsubscribe auto* localHelper = new HelperImpl(mockGateway); - // No EXPECT_CALL for unsubscribe — none should be called + EXPECT_CALL(mockGateway, unsubscribe(_, _)).Times(0); delete localHelper; } diff --git a/test/unit/loggerTest.cpp b/test/unit/loggerTest.cpp index 35c1f51..d189b7b 100644 --- a/test/unit/loggerTest.cpp +++ b/test/unit/loggerTest.cpp @@ -303,7 +303,7 @@ TEST_F(LoggerFormatUTest, MessageWithTrailingNewline) ASSERT_NE(msgPos, std::string::npos); // After stripping the user's \n, only fprintf's \n should remain: // the text "trailing newline" should be followed by exactly "\n" (end of output) - std::string afterMsg = output.substr(msgPos + strlen("trailing newline")); + std::string afterMsg = output.substr(msgPos + sizeof("trailing newline") - 1); EXPECT_EQ(afterMsg, "\n") << "Expected exactly one trailing newline. Got: [" << afterMsg << "]"; } @@ -383,9 +383,9 @@ TEST_F(LoggerFormatUTest, LocationWithoutSlashInPath) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LocationWithSlashInPath -// Covers: logger.cpp:126 (fileName.empty() == false → substr(1) branch) -// When __FILE__ contains '/', strrchr returns a non-null pointer, -// fileName is non-empty, and the substr(1) path (line 130) is taken. +// Covers: logger.cpp:123-126 (slash != nullptr → fileName = slash + 1) +// When __FILE__ contains '/', strrchr returns a non-null pointer and +// the filename is extracted by advancing past the last slash. // This is the normal case for all Linux/Docker builds. // Scenario type: success // --------------------------------------------------------------------------- diff --git a/test/unit/transportTest.cpp b/test/unit/transportTest.cpp index 0d689a4..69dfd36 100644 --- a/test/unit/transportTest.cpp +++ b/test/unit/transportTest.cpp @@ -998,13 +998,18 @@ TEST_F(TransportIntegrationUTest, DebugLoggingOnSendAndReceive) // --------------------------------------------------------------------------- TEST_F(TransportCustomServerUTest, MessageDuringShutdownIgnored) { - std::atomic serverMsgCount{0}; + std::promise serverReceivedPromise; + auto serverReceivedFuture = serverReceivedPromise.get_future(); + std::atomic serverPromiseSet{false}; m_server.set_message_handler( - [this, &serverMsgCount](connection_hdl hdl, server::message_ptr msg) + [this, &serverReceivedPromise, &serverPromiseSet](connection_hdl hdl, server::message_ptr msg) { - (void)this; - serverMsgCount++; + bool expected = false; + if (serverPromiseSet.compare_exchange_strong(expected, true)) + { + serverReceivedPromise.set_value(); + } // Echo back m_server.send(hdl, msg->get_payload(), msg->get_opcode()); }); @@ -1014,7 +1019,6 @@ TEST_F(TransportCustomServerUTest, MessageDuringShutdownIgnored) Transport transport; std::promise connectionPromise; auto connectionFuture = connectionPromise.get_future(); - std::atomic receivedCount{0}; auto onConnectionChange = [&](bool connected, const Firebolt::Error& /*err*/) { @@ -1024,7 +1028,7 @@ TEST_F(TransportCustomServerUTest, MessageDuringShutdownIgnored) } }; - auto onMessage = [&](const nlohmann::json& /*msg*/) { receivedCount++; }; + auto onMessage = [&](const nlohmann::json& /*msg*/) {}; Firebolt::Error err = transport.connect(m_uri, onMessage, onConnectionChange); ASSERT_EQ(err, Firebolt::Error::None); @@ -1035,8 +1039,10 @@ TEST_F(TransportCustomServerUTest, MessageDuringShutdownIgnored) transport.send("test.method", {{"k", "v"}}, transport.getNextMessageID()); transport.disconnect(); - // No crash, no hang — that's the test - EXPECT_GE(serverMsgCount.load(), 1); + // No crash, no hang — that's the primary assertion. + // Verify the server did receive the message (wait to avoid racing the server thread). + auto serverStatus = serverReceivedFuture.wait_for(std::chrono::milliseconds(500)); + EXPECT_EQ(serverStatus, std::future_status::ready) << "Server never received the message"; } // --------------------------------------------------------------------------- From 21e53da07dc2a4b51a57e53a70025b4ac8dfe8d5 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Mon, 18 May 2026 13:16:38 -0400 Subject: [PATCH 06/10] RDKEMW-14899 : Resolve Copilot review comments in unit tests --- test/unit/gatewayTest.cpp | 1 + test/unit/helperTest.cpp | 3 +++ test/unit/loggerTest.cpp | 3 ++- test/unit/transportTest.cpp | 18 ++++++++++++++---- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index c41eb8a..864c5d1 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -19,6 +19,7 @@ #include "firebolt/gateway.h" #include "firebolt/logger.h" #include "utils.h" +#include #include #include #include diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index 56823de..c87e04e 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -359,6 +359,9 @@ TEST_F(HelperUTest, SubscribeGatewayError) ihelper.subscribe(this, "test.onEvent", std::move(notification), onPropertyChangedCallback); ASSERT_FALSE(result); EXPECT_EQ(result.error(), Error::NotConnected); + + // Verify the failed subscription was cleaned up — destructor should NOT call unsubscribe + EXPECT_CALL(mockGateway, unsubscribe("test.onEvent", _)).Times(0); } // --------------------------------------------------------------------------- diff --git a/test/unit/loggerTest.cpp b/test/unit/loggerTest.cpp index d189b7b..ee34989 100644 --- a/test/unit/loggerTest.cpp +++ b/test/unit/loggerTest.cpp @@ -18,6 +18,7 @@ #include "firebolt/logger.h" #include +#include #include #include #include @@ -407,7 +408,7 @@ TEST_F(LoggerFormatUTest, LocationWithSlashInPath) } dup2(pipefd[1], STDERR_FILENO); - // Pass a file path WITH a slash — triggers the substr(1) branch + // Pass a file path with slashes so the logger strips the directory and keeps only the filename Logger::log(LogLevel::Error, "Test", "/some/path/myfile.cpp", "testFunc", 99, "msg"); fflush(stderr); diff --git a/test/unit/transportTest.cpp b/test/unit/transportTest.cpp index 69dfd36..0d3d990 100644 --- a/test/unit/transportTest.cpp +++ b/test/unit/transportTest.cpp @@ -712,8 +712,12 @@ TEST_F(TransportCustomServerUTest, NonTextMessageIgnored) m_server.set_message_handler( [this](connection_hdl hdl, server::message_ptr /*msg*/) { - // Send a binary frame instead of text - m_server.send(hdl, "binary data", websocketpp::frame::opcode::binary); + // Send a binary frame with valid JSON — proves rejection is by opcode, not parse failure + nlohmann::json binaryPayload; + binaryPayload["jsonrpc"] = "2.0"; + binaryPayload["id"] = 999; + binaryPayload["result"] = {{"binary", true}}; + m_server.send(hdl, binaryPayload.dump(), websocketpp::frame::opcode::binary); // Then send a valid text message nlohmann::json response; response["jsonrpc"] = "2.0"; @@ -755,6 +759,8 @@ TEST_F(TransportCustomServerUTest, NonTextMessageIgnored) nlohmann::json received = validMessageFuture.get(); EXPECT_TRUE(received.contains("result")); + // Proves the binary frame (id=999) was dropped by opcode, not by JSON parse failure + EXPECT_EQ(received["id"], 1) << "Received binary frame's id instead of text frame's id"; transport.disconnect(); } @@ -1040,9 +1046,13 @@ TEST_F(TransportCustomServerUTest, MessageDuringShutdownIgnored) transport.disconnect(); // No crash, no hang — that's the primary assertion. - // Verify the server did receive the message (wait to avoid racing the server thread). + // Server receipt is best-effort: a rapid close may legitimately prevent delivery. auto serverStatus = serverReceivedFuture.wait_for(std::chrono::milliseconds(500)); - EXPECT_EQ(serverStatus, std::future_status::ready) << "Server never received the message"; + if (serverStatus != std::future_status::ready) + { + // Delivery was pre-empted by disconnect — acceptable for this resilience test. + SUCCEED() << "Message not delivered before disconnect (expected race outcome)"; + } } // --------------------------------------------------------------------------- From dfb0966e2d17322b6652eccde360950892220246 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Mon, 18 May 2026 14:07:51 -0400 Subject: [PATCH 07/10] RDKEMW-14899 : Use std::filesystem for filename extraction --- src/logger.cpp | 11 ++--------- test/unit/loggerTest.cpp | 7 +++---- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/logger.cpp b/src/logger.cpp index 5bc6e08..656d874 100644 --- a/src/logger.cpp +++ b/src/logger.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -120,15 +121,7 @@ void Logger::log(LogLevel logLevel, const std::string& module, const std::string std::string fileName; if (formatter_addLocation) { - const char* slash = strrchr(file.c_str(), '/'); - if (slash != nullptr) - { - fileName = slash + 1; - } - else - { - fileName = file; - } + fileName = std::filesystem::path(file).filename().string(); } #pragma GCC diagnostic push diff --git a/test/unit/loggerTest.cpp b/test/unit/loggerTest.cpp index ee34989..1de8054 100644 --- a/test/unit/loggerTest.cpp +++ b/test/unit/loggerTest.cpp @@ -335,7 +335,7 @@ TEST_F(LoggerFormatUTest, LogLevelNames) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LocationWithoutSlashInPath -// Covers: src/logger.cpp:123 (strrchr returns nullptr → use file as-is) +// Covers: src/logger.cpp (std::filesystem::path::filename() with no directory) // Regression: previously assigned nullptr to std::string (UB / crash) // Scenario type: edge case // --------------------------------------------------------------------------- @@ -384,9 +384,8 @@ TEST_F(LoggerFormatUTest, LocationWithoutSlashInPath) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LocationWithSlashInPath -// Covers: logger.cpp:123-126 (slash != nullptr → fileName = slash + 1) -// When __FILE__ contains '/', strrchr returns a non-null pointer and -// the filename is extracted by advancing past the last slash. +// Covers: logger.cpp (std::filesystem::path::filename() strips directory) +// When __FILE__ contains '/', the filename is extracted from the path. // This is the normal case for all Linux/Docker builds. // Scenario type: success // --------------------------------------------------------------------------- From 2f69e5dac8d69ecae6b2b742521c01daa4fb182f Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Mon, 18 May 2026 15:20:10 -0400 Subject: [PATCH 08/10] RDKEMW-14899 : Replace hardcoded line numbers with functional descriptions in comments --- test/unit/gatewayTest.cpp | 20 ++++++++++---------- test/unit/helperTest.cpp | 36 ++++++++++++++++++------------------ test/unit/loggerTest.cpp | 30 +++++++++++++++--------------- test/unit/transportTest.cpp | 18 +++++++++--------- 4 files changed, 52 insertions(+), 52 deletions(-) diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index 864c5d1..4ef355b 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -758,7 +758,7 @@ TEST_F(GatewayUTest, UnsubscribeNonexistentEvent) // --------------------------------------------------------------------------- // Test name: GatewayUTest.NotificationWithValueWrapping -// Covers: src/gateway.cpp:325-340 (notify with "value" key unwrapping) +// Covers: Server::notify value-unwrapping branch (params with single "value" key) // Scenario type: success // --------------------------------------------------------------------------- TEST_F(GatewayUTest, NotificationWithValueWrapping) @@ -797,7 +797,7 @@ TEST_F(GatewayUTest, NotificationWithValueWrapping) // --------------------------------------------------------------------------- // Test name: GatewayUTest.NotificationWithValueObjectNotUnwrapped -// Covers: src/gateway.cpp:330-336 (value IS object → keep full params) +// Covers: Server::notify value-is-object branch (params kept as-is) // Scenario type: success // --------------------------------------------------------------------------- TEST_F(GatewayUTest, NotificationWithValueObjectNotUnwrapped) @@ -837,7 +837,7 @@ TEST_F(GatewayUTest, NotificationWithValueObjectNotUnwrapped) // --------------------------------------------------------------------------- // Test name: GatewayUTest.NotificationNoSubscribers -// Covers: src/gateway.cpp:350-353 (no subscribers for event → warning log) +// Covers: Server::notify no-subscribers branch (warning log) // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(GatewayUTest, NotificationNoSubscribers) @@ -975,7 +975,7 @@ TEST_F(GatewayUTest, DisconnectWithoutConnect) // --------------------------------------------------------------------------- // Test name: GatewayUTest.NotificationMultipleParams -// Covers: src/gateway.cpp:340-344 (params with multiple keys → pass as-is) +// Covers: Server::notify multi-key params branch (pass as-is) // Scenario type: success // --------------------------------------------------------------------------- TEST_F(GatewayUTest, NotificationMultipleParams) @@ -1091,7 +1091,7 @@ TEST_F(GatewayUTest, LegacyRPCv1UnsubscribeCleanup) // --------------------------------------------------------------------------- // Test name: GatewayUTest.LegacyRPCv1NonEventResult -// Covers: src/gateway.cpp:655-665 (legacy: result is present but id NOT in eventMap) +// Covers: legacy RPC v1 response path (id not in eventMap → falls through to client.response) // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(GatewayUTest, LegacyRPCv1NonEventResult) @@ -1170,7 +1170,7 @@ TEST_F(GatewayUTest, WaitTimeConfiguration) // --------------------------------------------------------------------------- // Test name: GatewayUTest.SendResponseIsIgnoredByClient -// Covers: gateway.cpp:153-154 (invokes.find(id) != end → erase + return) +// Covers: Client::response invoke-set erase path (fire-and-forget response dropped) // The response for a fire-and-forget send() ID should be silently dropped. // Scenario type: edge case // --------------------------------------------------------------------------- @@ -1198,7 +1198,7 @@ TEST_F(GatewayUTest, SendResponseIsIgnoredByClient) // --------------------------------------------------------------------------- // Test name: GatewayUTest.RequestFailsWhenSendErrors -// Covers: gateway.cpp:136-141 (transport_.send returns error → promise set + erase) +// Covers: Client::request send-error path (promise set with error + erase) // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(GatewayUTest, RequestFailsWhenSendErrors) @@ -1242,7 +1242,7 @@ TEST_F(GatewayUTest, RequestFailsWhenSendErrors) // --------------------------------------------------------------------------- // Test name: GatewayUTest.LegacySubscribeFailureCleansEventMap -// Covers: gateway.cpp:565-567 (legacyRPCv1 && subscribe error → erase rpcv1_eventMap) +// Covers: legacy RPC v1 subscribe error cleanup (erase rpcv1_eventMap) // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(GatewayUTest, LegacySubscribeFailureCleansEventMap) @@ -1272,7 +1272,7 @@ TEST_F(GatewayUTest, LegacySubscribeFailureCleansEventMap) // --------------------------------------------------------------------------- // Test name: GatewayUTest.UnsubscribeAckWithError -// Covers: gateway.cpp:621 (!result → status = result.error() in unsubscribe ACK) +// Covers: unsubscribe ACK error propagation (!result → status = error) // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(GatewayUTest, UnsubscribeAckWithError) @@ -1327,7 +1327,7 @@ TEST_F(GatewayUTest, UnsubscribeAckWithError) // --------------------------------------------------------------------------- // Test name: GatewayUTest.LegacyUnsubscribeIteratesPastNonMatchingEntry -// Covers: gateway.cpp:600 (++it in rpcv1_eventMap loop — else branch when +// Covers: legacy RPC v1 unsubscribe loop (++it else branch — // it->second != event, advancing the iterator past non-matching entries) // Scenario: In legacy RPC v1 mode, subscribe to two events (eventA gets a // lower message ID, eventB gets a higher one). When unsubscribing diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index c87e04e..6775530 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -225,7 +225,7 @@ TEST(OnPropertyChangedCallbackUTest, InvalidJson) // --------------------------------------------------------------------------- // Test name: HelperUTest.SetWithNonObjectParams -// Covers: src/helpers_impl.h:49-54 (non-object params wrapped in "value") +// Covers: HelperImpl::set non-object params wrapped in "value" // Scenario type: success // --------------------------------------------------------------------------- TEST_F(HelperUTest, SetWithNonObjectParams) @@ -248,7 +248,7 @@ TEST_F(HelperUTest, SetWithNonObjectParams) // --------------------------------------------------------------------------- // Test name: HelperUTest.SetWithArrayParams -// Covers: src/helpers_impl.h:49-54 (array is not object → wrap in "value") +// Covers: HelperImpl::set array params wrapped in "value" // Scenario type: success // --------------------------------------------------------------------------- TEST_F(HelperUTest, SetWithArrayParams) @@ -270,7 +270,7 @@ TEST_F(HelperUTest, SetWithArrayParams) // --------------------------------------------------------------------------- // Test name: HelperUTest.InvokeFailure -// Covers: src/helpers_impl.h:63-65 (invoke returns gateway error) +// Covers: HelperImpl::invoke gateway error propagation // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(HelperUTest, InvokeFailure) @@ -287,7 +287,7 @@ TEST_F(HelperUTest, InvokeFailure) // --------------------------------------------------------------------------- // Test name: HelperUTest.GetWithParameters -// Covers: src/helpers_impl.h:103 (getJson called with explicit parameters) +// Covers: HelperImpl::getJson with explicit parameters // Scenario type: success // --------------------------------------------------------------------------- TEST_F(HelperUTest, GetWithParameters) @@ -307,7 +307,7 @@ TEST_F(HelperUTest, GetWithParameters) // --------------------------------------------------------------------------- // Test name: HelperUTest.GetWithErrorInfo -// Covers: src/helpers_impl.h:105-106 (error propagation with errorInfo) +// Covers: HelperImpl::get error propagation with errorInfo // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(HelperUTest, GetWithErrorInfo) @@ -328,7 +328,7 @@ TEST_F(HelperUTest, GetWithErrorInfo) // --------------------------------------------------------------------------- // Test name: HelperUTest.SubscribeSuccess -// Covers: src/helpers_impl.h:119-133 (subscribe success flow) +// Covers: HelperImpl::subscribe success flow // Scenario type: success // --------------------------------------------------------------------------- TEST_F(HelperUTest, SubscribeSuccess) @@ -345,7 +345,7 @@ TEST_F(HelperUTest, SubscribeSuccess) // --------------------------------------------------------------------------- // Test name: HelperUTest.SubscribeGatewayError -// Covers: src/helpers_impl.h:127-131 (subscribe gateway error → erase + return) +// Covers: HelperImpl::subscribe gateway error (erase + return) // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(HelperUTest, SubscribeGatewayError) @@ -366,7 +366,7 @@ TEST_F(HelperUTest, SubscribeGatewayError) // --------------------------------------------------------------------------- // Test name: HelperUTest.UnsubscribeNotFound -// Covers: src/helpers_impl.h:71-74 (unsubscribe with invalid id) +// Covers: HelperImpl::unsubscribe with invalid subscription id // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(HelperUTest, UnsubscribeNotFound) @@ -378,7 +378,7 @@ TEST_F(HelperUTest, UnsubscribeNotFound) // --------------------------------------------------------------------------- // Test name: HelperUTest.UnsubscribeSuccess -// Covers: src/helpers_impl.h:75-79 (unsubscribe with valid id) +// Covers: HelperImpl::unsubscribe with valid subscription id // Scenario type: success // --------------------------------------------------------------------------- TEST_F(HelperUTest, UnsubscribeSuccess) @@ -401,7 +401,7 @@ TEST_F(HelperUTest, UnsubscribeSuccess) // --------------------------------------------------------------------------- // Test name: HelperUTest.UnsubscribeAllWithOwner -// Covers: src/helpers_impl.h:83-97 (unsubscribeAll matching owner) +// Covers: HelperImpl::unsubscribeAll matching owner // Scenario type: success // --------------------------------------------------------------------------- TEST_F(HelperUTest, UnsubscribeAllWithOwner) @@ -429,7 +429,7 @@ TEST_F(HelperUTest, UnsubscribeAllWithOwner) // --------------------------------------------------------------------------- // Test name: HelperUTest.UnsubscribeAllNoMatch -// Covers: src/helpers_impl.h:83-97 (unsubscribeAll with no matching owner) +// Covers: HelperImpl::unsubscribeAll with no matching owner // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(HelperUTest, UnsubscribeAllNoMatch) @@ -464,7 +464,7 @@ struct TestMultiArgJson // --------------------------------------------------------------------------- // Test name: OnPropertyChangedCallbackUTest.MultiArgCallback -// Covers: src/helpers.h:51-52 (sizeof...(Args) > 1 → std::apply branch) +// Covers: onPropertyChangedCallback multi-arg std::apply branch // Scenario type: success // --------------------------------------------------------------------------- TEST(OnPropertyChangedCallbackUTest, MultiArgCallback) @@ -491,7 +491,7 @@ TEST(OnPropertyChangedCallbackUTest, MultiArgCallback) // --------------------------------------------------------------------------- // Test name: OnPropertyChangedCallbackUTest.MultiArgInvalidJson -// Covers: helpers.h:59 (catch path for multi-arg template instantiation) +// Covers: onPropertyChangedCallback multi-arg catch path // Scenario: The multi-arg instantiation of onPropertyChangedCallback receives // malformed JSON that fails fromJson(). Exercises the catch branch // for the instantiation. @@ -516,7 +516,7 @@ TEST(OnPropertyChangedCallbackUTest, MultiArgInvalidJson) // --------------------------------------------------------------------------- // Test name: SubscriptionManagerUTest.SubscribeFailure -// Covers: helpers_impl.cpp:38 (SubscriptionManager propagates error) +// Covers: SubscriptionManager::subscribe error propagation // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(SubscriptionManagerUTest, SubscribeFailure) @@ -532,7 +532,7 @@ TEST_F(SubscriptionManagerUTest, SubscribeFailure) // --------------------------------------------------------------------------- // Test name: SubscriptionManagerUTest.DestructorCallsUnsubscribeAll -// Covers: helpers_impl.cpp:34 (destructor calls unsubscribeAll) +// Covers: SubscriptionManager destructor calls unsubscribeAll // Scenario type: success // --------------------------------------------------------------------------- TEST_F(SubscriptionManagerUTest, DestructorCallsUnsubscribeAll) @@ -548,7 +548,7 @@ TEST_F(SubscriptionManagerUTest, DestructorCallsUnsubscribeAll) // --------------------------------------------------------------------------- // Test name: HelperUTest.DestructorCleansUpActiveSubscriptions -// Covers: helpers_impl.h:35-42 (destructor loop over subscriptions_) +// Covers: HelperImpl destructor loop over active subscriptions // Also covers the false branch (empty subscriptions_ → skip loop) // Scenario type: edge case // --------------------------------------------------------------------------- @@ -577,7 +577,7 @@ TEST_F(HelperUTest, DestructorCleansUpActiveSubscriptions) // --------------------------------------------------------------------------- // Test name: HelperUTest.DestructorWithNoSubscriptions -// Covers: helpers_impl.h:35-42 (destructor with empty subscriptions_ → skip) +// Covers: HelperImpl destructor with empty subscriptions (skip loop) // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(HelperUTest, DestructorWithNoSubscriptions) @@ -590,7 +590,7 @@ TEST_F(HelperUTest, DestructorWithNoSubscriptions) // --------------------------------------------------------------------------- // Test name: GetHelperInstanceTest.ReturnsSingleton -// Covers: helpers_impl.cpp:45,47-48 (GetHelperInstance singleton factory) +// Covers: GetHelperInstance singleton factory // Scenario type: success // --------------------------------------------------------------------------- TEST(GetHelperInstanceTest, ReturnsSingleton) diff --git a/test/unit/loggerTest.cpp b/test/unit/loggerTest.cpp index 1de8054..460c4ef 100644 --- a/test/unit/loggerTest.cpp +++ b/test/unit/loggerTest.cpp @@ -28,7 +28,7 @@ using namespace Firebolt; // --------------------------------------------------------------------------- // File: test/unit/loggerTest.cpp -// Covers: src/logger.cpp — format flag combinations (lines 119–153) +// Covers: src/logger.cpp — format flag combinations and level filtering // --------------------------------------------------------------------------- class LoggerFormatUTest : public ::testing::Test @@ -84,7 +84,7 @@ class LoggerFormatUTest : public ::testing::Test // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LocationTrue_FunctionTrue -// Covers: src/logger.cpp:141 (addLocation=true, addFunction=true branch) +// Covers: logger.cpp format branch (addLocation=true, addFunction=true) // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, LocationTrue_FunctionTrue) @@ -105,7 +105,7 @@ TEST_F(LoggerFormatUTest, LocationTrue_FunctionTrue) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LocationFalse_FunctionTrue -// Covers: src/logger.cpp:148 (only addFunction branch → [func()]) +// Covers: logger.cpp format branch (addFunction only → [func()]) // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, LocationFalse_FunctionTrue) @@ -122,7 +122,7 @@ TEST_F(LoggerFormatUTest, LocationFalse_FunctionTrue) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LocationTrue_FunctionFalse -// Covers: src/logger.cpp:145 (only addLocation branch → [file:line]) +// Covers: logger.cpp format branch (addLocation only → [file:line]) // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, LocationTrue_FunctionFalse) @@ -140,7 +140,7 @@ TEST_F(LoggerFormatUTest, LocationTrue_FunctionFalse) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LocationFalse_FunctionFalse -// Covers: src/logger.cpp:138 (neither location nor function → no [] block) +// Covers: logger.cpp format branch (no location/function → no [] block) // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, LocationFalse_FunctionFalse) @@ -160,7 +160,7 @@ TEST_F(LoggerFormatUTest, LocationFalse_FunctionFalse) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.ThreadIdEnabled -// Covers: src/logger.cpp:151 (formatter_addThreadId path) +// Covers: logger.cpp formatter_addThreadId true branch // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, ThreadIdEnabled) @@ -175,7 +175,7 @@ TEST_F(LoggerFormatUTest, ThreadIdEnabled) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.ThreadIdDisabled -// Covers: src/logger.cpp:151 (formatter_addThreadId=false → skip) +// Covers: logger.cpp formatter_addThreadId false branch // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, ThreadIdDisabled) @@ -188,7 +188,7 @@ TEST_F(LoggerFormatUTest, ThreadIdDisabled) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.TimestampEnabled -// Covers: src/logger.cpp:109-117 (formatter_addTs branch) +// Covers: logger.cpp formatter_addTs timestamp formatting branch // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, TimestampEnabled) @@ -203,7 +203,7 @@ TEST_F(LoggerFormatUTest, TimestampEnabled) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.AllFlagsEnabled -// Covers: src/logger.cpp:109-153 (all format branches active) +// Covers: logger.cpp all format branches active simultaneously // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, AllFlagsEnabled) @@ -228,7 +228,7 @@ TEST_F(LoggerFormatUTest, AllFlagsEnabled) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LogLevelFiltering -// Covers: src/logger.cpp:87 (logLevel > _logLevel early return) +// Covers: logger.cpp logLevel filtering early return // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, LogLevelFiltering) @@ -243,7 +243,7 @@ TEST_F(LoggerFormatUTest, LogLevelFiltering) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.SetLogLevelMaxLevel -// Covers: src/logger.cpp:68-69 (logLevel == MaxLevel → set to Debug) +// Covers: logger.cpp setLogLevel MaxLevel → Debug mapping // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, SetLogLevelMaxLevel) @@ -255,7 +255,7 @@ TEST_F(LoggerFormatUTest, SetLogLevelMaxLevel) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.SetLogLevelBeyondMax -// Covers: src/logger.cpp:64-67 (logLevel < MaxLevel guard) +// Covers: logger.cpp setLogLevel out-of-range guard // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, SetLogLevelBeyondMax) @@ -270,7 +270,7 @@ TEST_F(LoggerFormatUTest, SetLogLevelBeyondMax) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.MessageTruncation -// Covers: src/logger.cpp:96-99 (message truncation at MaxBufSize) +// Covers: logger.cpp message truncation at MaxBufSize // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, MessageTruncation) @@ -290,7 +290,7 @@ TEST_F(LoggerFormatUTest, MessageTruncation) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.MessageWithTrailingNewline -// Covers: src/logger.cpp:100-103 (trailing newline removal) +// Covers: logger.cpp trailing newline removal // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, MessageWithTrailingNewline) @@ -310,7 +310,7 @@ TEST_F(LoggerFormatUTest, MessageWithTrailingNewline) // --------------------------------------------------------------------------- // Test name: LoggerFormatUTest.LogLevelNames -// Covers: src/logger.cpp:43-49 (_logLevelNames map) +// Covers: logger.cpp _logLevelNames map entries // Scenario type: success // --------------------------------------------------------------------------- TEST_F(LoggerFormatUTest, LogLevelNames) diff --git a/test/unit/transportTest.cpp b/test/unit/transportTest.cpp index 0d3d990..c24d210 100644 --- a/test/unit/transportTest.cpp +++ b/test/unit/transportTest.cpp @@ -704,7 +704,7 @@ TEST_F(TransportCustomServerUTest, MalformedMessageFromServer) // --------------------------------------------------------------------------- // Test name: TransportCustomServerUTest.NonTextMessageIgnored -// Covers: src/transport.cpp:305-308 (non-text opcode → warning + ignore) +// Covers: transport.cpp non-text opcode branch (warning + ignore) // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(TransportCustomServerUTest, NonTextMessageIgnored) @@ -767,7 +767,7 @@ TEST_F(TransportCustomServerUTest, NonTextMessageIgnored) // --------------------------------------------------------------------------- // Test name: TransportIntegrationUTest.DisconnectWhileConnected -// Covers: src/transport.cpp:212-246 (disconnect from Connected state) +// Covers: transport.cpp disconnect from Connected state // Scenario type: success // --------------------------------------------------------------------------- TEST_F(TransportIntegrationUTest, DisconnectWhileConnected) @@ -803,7 +803,7 @@ TEST_F(TransportIntegrationUTest, DisconnectWhileConnected) // --------------------------------------------------------------------------- // Test name: TransportIntegrationUTest.MultipleMessagesInSequence -// Covers: src/transport.cpp:100-120 (processQueuedMessages loop) +// Covers: transport.cpp processQueuedMessages loop // Scenario type: success // --------------------------------------------------------------------------- TEST_F(TransportIntegrationUTest, MultipleMessagesInSequence) @@ -854,7 +854,7 @@ TEST_F(TransportIntegrationUTest, MultipleMessagesInSequence) // --------------------------------------------------------------------------- // Test name: TransportIntegrationUTest.ConnectWithTransportLogging -// Covers: src/transport.cpp:155-173 (transport logging include/exclude params) +// Covers: transport.cpp logging include/exclude params // Scenario type: success // --------------------------------------------------------------------------- TEST_F(TransportIntegrationUTest, ConnectWithTransportLogging) @@ -888,7 +888,7 @@ TEST_F(TransportIntegrationUTest, ConnectWithTransportLogging) // --------------------------------------------------------------------------- // Test name: TransportUTest.GetNextMessageIDMonotonic -// Covers: src/transport.cpp:277 (atomic increment) +// Covers: transport.cpp getNextMessageID atomic increment // Scenario type: success // --------------------------------------------------------------------------- TEST_F(TransportUTest, GetNextMessageIDMonotonic) @@ -904,7 +904,7 @@ TEST_F(TransportUTest, GetNextMessageIDMonotonic) // --------------------------------------------------------------------------- // Test name: TransportCustomServerUTest.DisconnectFromDisconnectedState -// Covers: src/transport.cpp:215-220 (disconnect when state is Disconnected) +// Covers: transport.cpp disconnect when already Disconnected // Scenario type: edge case // --------------------------------------------------------------------------- TEST_F(TransportCustomServerUTest, DisconnectFromDisconnectedState) @@ -945,8 +945,8 @@ TEST_F(TransportCustomServerUTest, DisconnectFromDisconnectedState) // --------------------------------------------------------------------------- // Test name: TransportIntegrationUTest.DebugLoggingOnSendAndReceive -// Covers: transport.cpp:121 (debugEnabled_ true → log received msg) -// transport.cpp:293 (debugEnabled_ true → log sent msg) +// Covers: transport.cpp debugEnabled_ true branch (log send/receive) +// Exercises both inbound and outbound debug logging // Scenario type: success // --------------------------------------------------------------------------- TEST_F(TransportIntegrationUTest, DebugLoggingOnSendAndReceive) @@ -1057,7 +1057,7 @@ TEST_F(TransportCustomServerUTest, MessageDuringShutdownIgnored) // --------------------------------------------------------------------------- // Test name: TransportUTest.ConnectWithInvalidUrl -// Covers: transport.cpp:191-192 (get_connection returns ec → NotConnected) +// Covers: transport.cpp get_connection error → NotConnected // Scenario type: failure // --------------------------------------------------------------------------- TEST_F(TransportUTest, ConnectWithInvalidUrl) From 9ca9fb92979da6359bfefc8666f7eeaf32af87fc Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Tue, 19 May 2026 10:47:26 -0400 Subject: [PATCH 09/10] RDKEMW-14899 : Fix memory leak in SubscriptionManagerUTest::UnsubscribeAll --- test/unit/helperTest.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index 6775530..6bf67d7 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -180,9 +180,8 @@ TEST_F(SubscriptionManagerUTest, Unsubscribe) TEST_F(SubscriptionManagerUTest, UnsubscribeAll) { - EXPECT_CALL(mockHelper, unsubscribeAll(owner)); + EXPECT_CALL(mockHelper, unsubscribeAll(owner)).Times(2); subscriptionManager->unsubscribeAll(); - subscriptionManager.release(); } TEST(OnPropertyChangedCallbackUTest, Basic) From dfab6c0655305e5126790e0fd4ed1847e6c627e9 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Tue, 19 May 2026 11:02:29 -0400 Subject: [PATCH 10/10] RDKEMW-14899 : Refactor pointer identity variables to use real objects addresses --- test/unit/helperTest.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/unit/helperTest.cpp b/test/unit/helperTest.cpp index 6bf67d7..d89abee 100644 --- a/test/unit/helperTest.cpp +++ b/test/unit/helperTest.cpp @@ -405,8 +405,10 @@ TEST_F(HelperUTest, UnsubscribeSuccess) // --------------------------------------------------------------------------- TEST_F(HelperUTest, UnsubscribeAllWithOwner) { - void* owner1 = reinterpret_cast(0x1); - void* owner2 = reinterpret_cast(0x2); + int owner1Tag = 0; + int owner2Tag = 0; + void* owner1 = &owner1Tag; + void* owner2 = &owner2Tag; std::function notification1 = [](int) {}; std::function notification2 = [](int) {}; @@ -433,8 +435,10 @@ TEST_F(HelperUTest, UnsubscribeAllWithOwner) // --------------------------------------------------------------------------- TEST_F(HelperUTest, UnsubscribeAllNoMatch) { - void* owner = reinterpret_cast(0x1); - void* otherOwner = reinterpret_cast(0x99); + int ownerTag = 0; + int otherOwnerTag = 0; + void* owner = &ownerTag; + void* otherOwner = &otherOwnerTag; std::function notification = [](int) {}; EXPECT_CALL(mockGateway, subscribe("event", _, _)).WillOnce(Return(Error::None));