From 49f255a9fc3dc0e05f88f7b2eb4b634fbbd817d6 Mon Sep 17 00:00:00 2001 From: bobra200 Date: Wed, 3 Jun 2026 06:21:11 -0700 Subject: [PATCH 1/3] Address PR #66 review comments on params and test robustness --- src/metrics_impl.cpp | 6 +++--- test/unit/mock_helper.h | 18 ++++++++++++++++-- test/utils.cpp | 4 ++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/metrics_impl.cpp b/src/metrics_impl.cpp index 9f29027..0e04139 100644 --- a/src/metrics_impl.cpp +++ b/src/metrics_impl.cpp @@ -30,17 +30,17 @@ MetricsImpl::MetricsImpl(Firebolt::Helpers::IHelper& helper) Result MetricsImpl::ready() const { - return helper_.invoke("Metrics.ready", nlohmann::json({})); + return helper_.invoke("Metrics.ready", nlohmann::json()); } Result MetricsImpl::signIn() const { - return helper_.invoke("Metrics.signIn", nlohmann::json({})); + return helper_.invoke("Metrics.signIn", nlohmann::json()); } Result MetricsImpl::signOut() const { - return helper_.invoke("Metrics.signOut", nlohmann::json({})); + return helper_.invoke("Metrics.signOut", nlohmann::json()); } Result MetricsImpl::startContent(const std::optional& entityId, diff --git a/test/unit/mock_helper.h b/test/unit/mock_helper.h index f2a9ffb..e29342c 100644 --- a/test/unit/mock_helper.h +++ b/test/unit/mock_helper.h @@ -107,8 +107,22 @@ class MockBase void mockInvoke(const std::string& methodName) { EXPECT_CALL(mockHelper, invoke(methodName, _)) - .WillOnce(Invoke([](const std::string& /*methodName*/, const nlohmann::json& /*parameters*/) - { return Firebolt::Result{Firebolt::Error::None}; })); + .WillOnce(Invoke([&](const std::string& methodName, const nlohmann::json& parameters) + { + nlohmann::json message = { + {"jsonrpc", "2.0"}, + {"id", "0"}, + {"method", methodName}, + }; + + if (!parameters.is_null()) + { + message["params"] = parameters; + } + + Firebolt::Error err = jsonEngine.MockResponse(message); + return Firebolt::Result{err}; + })); } void mockSubscribe(const std::string& eventName) diff --git a/test/utils.cpp b/test/utils.cpp index 60de1be..cc75f38 100644 --- a/test/utils.cpp +++ b/test/utils.cpp @@ -136,9 +136,9 @@ void verifyEventReceived(std::mutex& mtx, std::condition_variable& cv, const boo void verifyEventNotReceived(std::mutex& mtx, std::condition_variable& cv, const bool& eventReceived) { - // Wait for the event to be received or timeout after 5 seconds + // Wait for the event to be received or timeout after EventWaitTime. std::unique_lock lock(mtx); - if (cv.wait_for(lock, std::chrono::seconds(EventWaitTime), [&] { return eventReceived; })) + if (cv.wait_for(lock, EventWaitTime, [&] { return eventReceived; })) { FAIL() << "Unexpectedly received event"; } From ed57b7ccf20de0eb5769634f0b5a69e83e34010f Mon Sep 17 00:00:00 2001 From: bobra200 Date: Wed, 3 Jun 2026 06:39:14 -0700 Subject: [PATCH 2/3] Apply clang-format to mock_helper after review fixes --- test/unit/mock_helper.h | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/test/unit/mock_helper.h b/test/unit/mock_helper.h index e29342c..d5d5580 100644 --- a/test/unit/mock_helper.h +++ b/test/unit/mock_helper.h @@ -107,22 +107,23 @@ class MockBase void mockInvoke(const std::string& methodName) { EXPECT_CALL(mockHelper, invoke(methodName, _)) - .WillOnce(Invoke([&](const std::string& methodName, const nlohmann::json& parameters) - { - nlohmann::json message = { - {"jsonrpc", "2.0"}, - {"id", "0"}, - {"method", methodName}, - }; - - if (!parameters.is_null()) - { - message["params"] = parameters; - } - - Firebolt::Error err = jsonEngine.MockResponse(message); - return Firebolt::Result{err}; - })); + .WillOnce(Invoke( + [&](const std::string& methodName, const nlohmann::json& parameters) + { + nlohmann::json message = { + {"jsonrpc", "2.0"}, + {"id", "0"}, + {"method", methodName}, + }; + + if (!parameters.is_null()) + { + message["params"] = parameters; + } + + Firebolt::Error err = jsonEngine.MockResponse(message); + return Firebolt::Result{err}; + })); } void mockSubscribe(const std::string& eventName) From 51e0df3f7a6c53847bf77d12b5092f0927e59933 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Wed, 3 Jun 2026 11:21:09 -0400 Subject: [PATCH 3/3] Address copilot comments --- test/unit/mock_helper.h | 4 ++-- test/utils.cpp | 4 ++-- test/utils.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/mock_helper.h b/test/unit/mock_helper.h index d5d5580..00598e5 100644 --- a/test/unit/mock_helper.h +++ b/test/unit/mock_helper.h @@ -108,12 +108,12 @@ class MockBase { EXPECT_CALL(mockHelper, invoke(methodName, _)) .WillOnce(Invoke( - [&](const std::string& methodName, const nlohmann::json& parameters) + [&](const std::string& invokedMethodName, const nlohmann::json& parameters) { nlohmann::json message = { {"jsonrpc", "2.0"}, {"id", "0"}, - {"method", methodName}, + {"method", invokedMethodName}, }; if (!parameters.is_null()) diff --git a/test/utils.cpp b/test/utils.cpp index cc75f38..c24a4e1 100644 --- a/test/utils.cpp +++ b/test/utils.cpp @@ -125,7 +125,7 @@ void verifyUnsubscribeResult(const Firebolt::Result& result) FAIL() << "Unsubscribe failed." + toError(result); } } -void verifyEventReceived(std::mutex& mtx, std::condition_variable& cv, const bool& eventReceived) +void verifyEventReceived(std::mutex& mtx, std::condition_variable& cv, bool& eventReceived) { std::unique_lock lock(mtx); if (!cv.wait_for(lock, EventWaitTime, [&] { return eventReceived; })) @@ -134,7 +134,7 @@ void verifyEventReceived(std::mutex& mtx, std::condition_variable& cv, const boo } } -void verifyEventNotReceived(std::mutex& mtx, std::condition_variable& cv, const bool& eventReceived) +void verifyEventNotReceived(std::mutex& mtx, std::condition_variable& cv, bool& eventReceived) { // Wait for the event to be received or timeout after EventWaitTime. std::unique_lock lock(mtx); diff --git a/test/utils.h b/test/utils.h index b5d0693..bd1e2e3 100644 --- a/test/utils.h +++ b/test/utils.h @@ -32,8 +32,8 @@ void triggerRaw(const std::string& payload); void verifyEventSubscription(const Firebolt::Result& id); void verifyUnsubscribeResult(const Firebolt::Result& result); -void verifyEventReceived(std::mutex& mtx, std::condition_variable& cv, const bool& eventReceived); -void verifyEventNotReceived(std::mutex& mtx, std::condition_variable& cv, const bool& eventReceived); +void verifyEventReceived(std::mutex& mtx, std::condition_variable& cv, bool& eventReceived); +void verifyEventNotReceived(std::mutex& mtx, std::condition_variable& cv, bool& eventReceived); template inline std::string toError(const Firebolt::Result& result) {