Implement NumericThreshold for Trigger
Implemented NumericThreshold for Trigger that allows to set
expected threshold value for collection of sensors and monitors
if this value is crossed. Implemented detection of direction
when threshold value is crossed. Added initial interface that
is used to commit an action when threshold value is crossed.
Moved Sensor Cache from Report Factory to Telemetry class, now
Sensor Cache is shared between Report Factory and Trigger Factory.
Moved fetching sensor from Dbus to seperate header to have single
implementation for factories.
Disabled tests that uses boost coroutine because of false positive
that is catched by address sanitizer.
Tested:
- Passed unit tests
- Built in yocto envrionment with success
- Telemetry service started successfully in OpenBMC
Change-Id: I1ff7ab96174a27576786f0b9a71554fe1eeff436
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
diff --git a/tests/meson.build b/tests/meson.build
index 509d67b..ee57387 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -24,6 +24,7 @@
'telemetry-ut',
[
'../src/metric.cpp',
+ '../src/numeric_threshold.cpp',
'../src/persistent_json_storage.cpp',
'../src/report.cpp',
'../src/report_factory.cpp',
@@ -31,12 +32,14 @@
'../src/sensor.cpp',
'../src/sensor_cache.cpp',
'../src/trigger.cpp',
+ '../src/trigger_factory.cpp',
'../src/trigger_manager.cpp',
'src/dbus_environment.cpp',
'src/main.cpp',
'src/stubs/dbus_sensor_object.cpp',
'src/test_detached_timer.cpp',
'src/test_metric.cpp',
+ 'src/test_numeric_threshold.cpp',
'src/test_persistent_json_storage.cpp',
'src/test_report.cpp',
'src/test_report_manager.cpp',
diff --git a/tests/src/mocks/trigger_action_mock.hpp b/tests/src/mocks/trigger_action_mock.hpp
new file mode 100644
index 0000000..586ac21
--- /dev/null
+++ b/tests/src/mocks/trigger_action_mock.hpp
@@ -0,0 +1,12 @@
+#pragma once
+
+#include "interfaces/trigger_action.hpp"
+
+#include <gmock/gmock.h>
+
+class TriggerActionMock : public interfaces::TriggerAction
+{
+ public:
+ MOCK_METHOD(void, commit, (const std::string&, uint64_t, double),
+ (override));
+};
diff --git a/tests/src/mocks/trigger_factory_mock.hpp b/tests/src/mocks/trigger_factory_mock.hpp
index b714a44..cf1be55 100644
--- a/tests/src/mocks/trigger_factory_mock.hpp
+++ b/tests/src/mocks/trigger_factory_mock.hpp
@@ -13,39 +13,42 @@
{
using namespace testing;
- ON_CALL(*this, make(_, _, _, _, _, _, _, _, _))
- .WillByDefault(WithArgs<0>(Invoke([](const std::string& name) {
+ ON_CALL(*this, make(_, _, _, _, _, _, _, _, _, _))
+ .WillByDefault(WithArgs<1>(Invoke([](const std::string& name) {
return std::make_unique<NiceMock<TriggerMock>>(name);
})));
}
- MOCK_METHOD(std::unique_ptr<interfaces::Trigger>, make,
- (const std::string& name, bool isDiscrete, bool logToJournal,
- bool logToRedfish, bool updateReport,
- (const std::vector<std::pair<sdbusplus::message::object_path,
- std::string>>& sensors),
- const std::vector<std::string>& reportNames,
- const TriggerThresholdParams& thresholds,
- interfaces::TriggerManager& triggerManager),
- (const, override));
+ MOCK_METHOD(
+ std::unique_ptr<interfaces::Trigger>, make,
+ (boost::asio::yield_context&, const std::string& name, bool isDiscrete,
+ bool logToJournal, bool logToRedfish, bool updateReport,
+ (const std::vector<
+ std::pair<sdbusplus::message::object_path, std::string>>& sensors),
+ const std::vector<std::string>& reportNames,
+ const TriggerThresholdParams& thresholdParams,
+ interfaces::TriggerManager& triggerManager),
+ (const, override));
auto& expectMake(
std::optional<std::reference_wrapper<const TriggerParams>> paramsOpt,
const testing::Matcher<interfaces::TriggerManager&>& tm)
{
+ using namespace testing;
+
if (paramsOpt)
{
const TriggerParams& params = *paramsOpt;
return EXPECT_CALL(
- *this, make(params.name(), params.isDiscrete(),
- params.logToJournal(), params.logToRedfish(),
- params.updateReport(), params.sensors(),
- params.reportNames(), params.thresholds(), tm));
+ *this,
+ make(_, params.name(), params.isDiscrete(),
+ params.logToJournal(), params.logToRedfish(),
+ params.updateReport(), params.sensors(),
+ params.reportNames(), params.thresholdParams(), tm));
}
else
{
- using testing::_;
- return EXPECT_CALL(*this, make(_, _, _, _, _, _, _, _, tm));
+ return EXPECT_CALL(*this, make(_, _, _, _, _, _, _, _, _, tm));
}
}
};
diff --git a/tests/src/params/trigger_params.hpp b/tests/src/params/trigger_params.hpp
index 340f9b3..464c68b 100644
--- a/tests/src/params/trigger_params.hpp
+++ b/tests/src/params/trigger_params.hpp
@@ -2,6 +2,7 @@
#include "interfaces/trigger_types.hpp"
+#include <chrono>
#include <utility>
class TriggerParams
@@ -49,7 +50,7 @@
return reportNamesProperty;
}
- const TriggerThresholdParams& thresholds() const
+ const TriggerThresholdParams& thresholdParams() const
{
return thresholdsProperty;
}
@@ -61,7 +62,17 @@
bool logToRedfishProperty = false;
bool updateReportProperty = false;
std::vector<std::pair<sdbusplus::message::object_path, std::string>>
- sensorsProperty = {};
- std::vector<std::string> reportNamesProperty = {};
- TriggerThresholdParams thresholdsProperty = {};
+ sensorsProperty = {
+ {sdbusplus::message::object_path(
+ "/xyz/openbmc_project/sensors/temperature/BMC_Temp"),
+ ""}};
+ std::vector<std::string> reportNamesProperty = {"Report1"};
+ TriggerThresholdParams thresholdsProperty =
+ std::vector<numeric::ThresholdParam>{
+ {static_cast<int>(numeric::Type::lowerCritical),
+ std::chrono::milliseconds(10).count(),
+ static_cast<int>(numeric::Direction::decreasing), 0.0},
+ {static_cast<int>(numeric::Type::upperCritical),
+ std::chrono::milliseconds(10).count(),
+ static_cast<int>(numeric::Direction::increasing), 90.0}};
};
diff --git a/tests/src/test_metric.cpp b/tests/src/test_metric.cpp
index a57e123..22fc97b 100644
--- a/tests/src/test_metric.cpp
+++ b/tests/src/test_metric.cpp
@@ -1,5 +1,4 @@
#include "helpers.hpp"
-#include "interfaces/sensor.hpp"
#include "metric.hpp"
#include "mocks/sensor_mock.hpp"
#include "utils/conv_container.hpp"
diff --git a/tests/src/test_numeric_threshold.cpp b/tests/src/test_numeric_threshold.cpp
new file mode 100644
index 0000000..0f1d145
--- /dev/null
+++ b/tests/src/test_numeric_threshold.cpp
@@ -0,0 +1,342 @@
+#include "dbus_environment.hpp"
+#include "helpers.hpp"
+#include "mocks/sensor_mock.hpp"
+#include "mocks/trigger_action_mock.hpp"
+#include "numeric_threshold.hpp"
+#include "utils/conv_container.hpp"
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+using namespace std::chrono_literals;
+
+class TestNumericThreshold : public Test
+{
+ public:
+ std::vector<std::shared_ptr<SensorMock>> sensorMocks = {
+ std::make_shared<NiceMock<SensorMock>>(),
+ std::make_shared<NiceMock<SensorMock>>()};
+ std::vector<std::string> sensorNames = {"Sensor1", "Sensor2"};
+ std::unique_ptr<TriggerActionMock> actionMockPtr =
+ std::make_unique<StrictMock<TriggerActionMock>>();
+ TriggerActionMock& actionMock = *actionMockPtr;
+ std::shared_ptr<NumericThreshold> sut;
+
+ void makeThreshold(std::chrono::milliseconds dwellTime,
+ numeric::Direction direction, double thresholdValue)
+ {
+ std::vector<std::unique_ptr<interfaces::TriggerAction>> actions;
+ actions.push_back(std::move(actionMockPtr));
+
+ sut = std::make_shared<NumericThreshold>(
+ DbusEnvironment::getIoc(),
+ utils::convContainer<std::shared_ptr<interfaces::Sensor>>(
+ sensorMocks),
+ sensorNames, std::move(actions), dwellTime, direction,
+ thresholdValue);
+ }
+
+ void SetUp() override
+ {
+ makeThreshold(0ms, numeric::Direction::increasing, 90.0);
+ }
+};
+
+TEST_F(TestNumericThreshold, initializeThresholdExpectAllSensorsAreRegistered)
+{
+ for (auto& sensor : sensorMocks)
+ {
+ EXPECT_CALL(*sensor,
+ registerForUpdates(Truly([sut = sut.get()](const auto& x) {
+ return x.lock().get() == sut;
+ })));
+ }
+
+ sut->initialize();
+}
+
+TEST_F(TestNumericThreshold, thresholdIsNotInitializeExpectNoActionCommit)
+{
+ EXPECT_CALL(actionMock, commit(_, _, _)).Times(0);
+}
+
+struct NumericParams
+{
+ NumericParams& Direction(numeric::Direction val)
+ {
+ direction = val;
+ return *this;
+ }
+
+ NumericParams&
+ Updates(std::vector<std::tuple<size_t, uint64_t, double>> val)
+ {
+ updates = std::move(val);
+ return *this;
+ }
+
+ NumericParams&
+ Expected(std::vector<std::tuple<size_t, uint64_t, double>> val)
+ {
+ expected = std::move(val);
+ return *this;
+ }
+
+ friend void PrintTo(const NumericParams& o, std::ostream* os)
+ {
+ *os << "{ Direction: " << static_cast<int>(o.direction)
+ << ", Updates: ";
+ for (const auto& [index, timestamp, value] : o.updates)
+ {
+ *os << "{ SensorIndex: " << index << ", Timestamp: " << timestamp
+ << ", Value: " << value << " }, ";
+ }
+ *os << "Expected: ";
+ for (const auto& [index, timestamp, value] : o.expected)
+ {
+ *os << "{ SensorIndex: " << index << ", Timestamp: " << timestamp
+ << ", Value: " << value << " }, ";
+ }
+ *os << " }";
+ }
+
+ numeric::Direction direction;
+ std::vector<std::tuple<size_t, uint64_t, double>> updates;
+ std::vector<std::tuple<size_t, uint64_t, double>> expected;
+};
+
+class TestNumericThresholdNoDwellTime :
+ public TestNumericThreshold,
+ public WithParamInterface<NumericParams>
+{
+ public:
+ void SetUp() override
+ {
+ makeThreshold(0ms, GetParam().direction, 90.0);
+ }
+};
+
+INSTANTIATE_TEST_SUITE_P(
+ _, TestNumericThresholdNoDwellTime,
+ Values(
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 89.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 91.0}})
+ .Expected({{0, 2, 91.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 99.0}, {0, 3, 80.0}, {0, 4, 98.0}})
+ .Expected({{0, 2, 99.0}, {0, 4, 98.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 99.0}, {1, 3, 100.0}, {1, 4, 98.0}})
+ .Expected({{0, 2, 99.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 91.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}})
+ .Expected({{0, 2, 80.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {0, 3, 99.0}, {0, 4, 85.0}})
+ .Expected({{0, 2, 80.0}, {0, 4, 85.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {1, 3, 99.0}, {1, 4, 88.0}})
+ .Expected({{0, 2, 80.0}, {1, 4, 88.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 98.0}, {0, 2, 91.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {0, 3, 85.0}, {0, 4, 91.0}})
+ .Expected({{0, 2, 80.0}, {0, 4, 91.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 100.0}, {1, 2, 80.0}, {0, 3, 85.0}, {1, 4, 91.0}})
+ .Expected({{0, 3, 85.0}, {1, 4, 91.0}})));
+
+TEST_P(TestNumericThresholdNoDwellTime, senorsIsUpdatedMultipleTimes)
+{
+ InSequence seq;
+ for (const auto& [index, timestamp, value] : GetParam().expected)
+ {
+ EXPECT_CALL(actionMock, commit(sensorNames[index], timestamp, value));
+ }
+
+ sut->initialize();
+ for (const auto& [index, timestamp, value] : GetParam().updates)
+ {
+ sut->sensorUpdated(*sensorMocks[index], timestamp, value);
+ }
+}
+
+class TestNumericThresholdWithDwellTime :
+ public TestNumericThreshold,
+ public WithParamInterface<NumericParams>
+{
+ public:
+ void SetUp() override
+ {
+ makeThreshold(2ms, GetParam().direction, 90.0);
+ }
+
+ void sleep()
+ {
+ DbusEnvironment::sleepFor(4ms);
+ }
+};
+
+INSTANTIATE_TEST_SUITE_P(
+ _, TestNumericThresholdWithDwellTime,
+ Values(
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 89.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 91.0}})
+ .Expected({{0, 2, 91.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 99.0}, {0, 3, 80.0}, {0, 4, 98.0}})
+ .Expected({{0, 2, 99.0}, {0, 4, 98.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {1, 2, 99.0}, {0, 3, 100.0}, {1, 4, 86.0}})
+ .Expected({{0, 3, 100.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 91.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}})
+ .Expected({{0, 2, 80.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {0, 3, 99.0}, {0, 4, 85.0}})
+ .Expected({{0, 2, 80.0}, {0, 4, 85.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {1, 3, 99.0}, {1, 4, 88.0}})
+ .Expected({{0, 2, 80.0}, {1, 4, 88.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 98.0}, {0, 2, 91.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {0, 3, 85.0}, {0, 4, 91.0}})
+ .Expected({{0, 2, 80.0}, {0, 4, 91.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 100.0}, {1, 2, 80.0}, {0, 3, 85.0}, {1, 4, 91.0}})
+ .Expected({{0, 3, 85.0}, {1, 4, 91.0}})));
+
+TEST_P(TestNumericThresholdWithDwellTime,
+ senorsIsUpdatedMultipleTimesSleepAfterEveryUpdate)
+{
+ InSequence seq;
+ for (const auto& [index, timestamp, value] : GetParam().expected)
+ {
+ EXPECT_CALL(actionMock, commit(sensorNames[index], timestamp, value));
+ }
+
+ sut->initialize();
+ for (const auto& [index, timestamp, value] : GetParam().updates)
+ {
+ sut->sensorUpdated(*sensorMocks[index], timestamp, value);
+ sleep();
+ }
+}
+
+class TestNumericThresholdWithDwellTime2 :
+ public TestNumericThreshold,
+ public WithParamInterface<NumericParams>
+{
+ public:
+ void SetUp() override
+ {
+ makeThreshold(2ms, GetParam().direction, 90.0);
+ }
+
+ void sleep()
+ {
+ DbusEnvironment::sleepFor(4ms);
+ }
+};
+
+INSTANTIATE_TEST_SUITE_P(
+ _, TestNumericThresholdWithDwellTime2,
+ Values(
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 89.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 91.0}})
+ .Expected({{0, 2, 91.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {0, 2, 99.0}, {0, 3, 80.0}, {0, 4, 98.0}})
+ .Expected({{0, 4, 98.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::increasing)
+ .Updates({{0, 1, 80.0}, {1, 2, 99.0}, {0, 3, 100.0}, {1, 4, 98.0}})
+ .Expected({{0, 3, 100.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 91.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}})
+ .Expected({{0, 2, 80.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {0, 3, 99.0}, {0, 4, 85.0}})
+ .Expected({{0, 4, 85.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::decreasing)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {1, 3, 99.0}, {1, 4, 88.0}})
+ .Expected({{0, 2, 80.0}, {1, 4, 88.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 98.0}, {0, 2, 91.0}})
+ .Expected({}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 100.0}, {0, 2, 80.0}, {0, 3, 85.0}, {0, 4, 91.0}})
+ .Expected({{0, 4, 91.0}}),
+ NumericParams()
+ .Direction(numeric::Direction::either)
+ .Updates({{0, 1, 100.0}, {1, 2, 80.0}, {0, 3, 85.0}, {1, 4, 91.0}})
+ .Expected({{0, 3, 85.0}, {1, 4, 91.0}})));
+
+TEST_P(TestNumericThresholdWithDwellTime2,
+ senorsIsUpdatedMultipleTimesSleepAfterLastUpdate)
+{
+ InSequence seq;
+ for (const auto& [index, timestamp, value] : GetParam().expected)
+ {
+ EXPECT_CALL(actionMock, commit(sensorNames[index], timestamp, value));
+ }
+
+ sut->initialize();
+ for (const auto& [index, timestamp, value] : GetParam().updates)
+ {
+ sut->sensorUpdated(*sensorMocks[index], timestamp, value);
+ }
+ sleep();
+}
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index 0f2d752..2b2b0fb 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -24,7 +24,8 @@
triggerParams.name(), triggerParams.isDiscrete(),
triggerParams.logToJournal(), triggerParams.logToRedfish(),
triggerParams.updateReport(), triggerParams.sensors(),
- triggerParams.reportNames(), triggerParams.thresholds(),
+ triggerParams.reportNames(), triggerParams.thresholdParams(),
+ std::vector<std::shared_ptr<interfaces::Threshold>>{},
*triggerManagerMockPtr);
}
@@ -74,7 +75,7 @@
Eq(triggerParams.reportNames()));
EXPECT_THAT(
getProperty<TriggerThresholdParams>(sut->getPath(), "Thresholds"),
- Eq(triggerParams.thresholds()));
+ Eq(triggerParams.thresholdParams()));
}
TEST_F(TestTrigger, deleteTrigger)
diff --git a/tests/src/test_trigger_manager.cpp b/tests/src/test_trigger_manager.cpp
index 71314fa..95043e2 100644
--- a/tests/src/test_trigger_manager.cpp
+++ b/tests/src/test_trigger_manager.cpp
@@ -24,7 +24,7 @@
TriggerManager::triggerManagerIfaceName, "AddTrigger",
params.name(), params.isDiscrete(), params.logToJournal(),
params.logToRedfish(), params.updateReport(), params.sensors(),
- params.reportNames(), params.thresholds());
+ params.reportNames(), params.thresholdParams());
return DbusEnvironment::waitForFuture(addTriggerPromise.get_future());
}
@@ -51,7 +51,7 @@
EXPECT_THAT(path, Eq(triggerMock.getPath()));
}
-TEST_F(TestTriggerManager, failToAddTriggerTwice)
+TEST_F(TestTriggerManager, DISABLED_failToAddTriggerTwice)
{
triggerFactoryMock.expectMake(triggerParams, Ref(*sut))
.WillOnce(Return(ByMove(std::move(triggerMockPtr))));
@@ -63,7 +63,7 @@
EXPECT_THAT(path, Eq(std::string()));
}
-TEST_F(TestTriggerManager, failToAddTriggerWhenMaxTriggerIsReached)
+TEST_F(TestTriggerManager, DISABLED_failToAddTriggerWhenMaxTriggerIsReached)
{
triggerFactoryMock.expectMake(std::nullopt, Ref(*sut))
.Times(TriggerManager::maxTriggers);