Removed FutureVersion from API
Instead of using FutureVersion API currently used version is updated.
This change needs to be bumped together with [1]. API that utilized map
of variants to be more flexible and backwards compatible was reverted.
It was decided that straight forward API that is commonly used should be
used instead.
Removed MetricId property from Metric. In telemetry it was implemented
as a name for Metric, but it was supposed to work as described in [2].
Currently MetricId is not supported by telemetry service and property
was removed.
Tested:
- After chaging bmcweb to use new API old and new features are working
as expected
[1]: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/44270
[2]: https://redfish.dmtf.org/schemas/v1/MetricReportDefinition.v1_4_2.json
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I5930a466a370f268d68f575a4a3db5ee9655e574
diff --git a/tests/src/dbus_environment.hpp b/tests/src/dbus_environment.hpp
index 558445e..1006323 100644
--- a/tests/src/dbus_environment.hpp
+++ b/tests/src/dbus_environment.hpp
@@ -131,6 +131,23 @@
return DbusEnvironment::waitForFuture(std::move(future));
}
+ template <class... Args>
+ static boost::system::error_code
+ callMethod(const std::string& path, const std::string& interface,
+ const std::string& method, Args&&... args)
+ {
+ auto promise = std::promise<boost::system::error_code>();
+ auto future = promise.get_future();
+ DbusEnvironment::getBus()->async_method_call(
+ [promise = std::move(promise)](
+ boost::system::error_code ec) mutable {
+ promise.set_value(ec);
+ },
+ DbusEnvironment::serviceName(), path, interface, method,
+ std::forward<Args>(args)...);
+ return DbusEnvironment::waitForFuture(std::move(future));
+ }
+
private:
static std::future<bool> getFuture(std::string_view name);
diff --git a/tests/src/helpers/metric_value_helpers.hpp b/tests/src/helpers/metric_value_helpers.hpp
index 9634de3..dee48bd 100644
--- a/tests/src/helpers/metric_value_helpers.hpp
+++ b/tests/src/helpers/metric_value_helpers.hpp
@@ -6,12 +6,12 @@
inline void PrintTo(const MetricValue& o, std::ostream* os)
{
- (*os) << "{ id: " << o.id << ", metadata: " << o.metadata
- << ", value: " << o.value << ", timestamp: " << o.timestamp << " }";
+ (*os) << "{ metadata: " << o.metadata << ", value: " << o.value
+ << ", timestamp: " << o.timestamp << " }";
}
inline bool operator==(const MetricValue& left, const MetricValue& right)
{
- return std::tie(left.id, left.metadata, left.value, left.timestamp) ==
- std::tie(right.id, right.metadata, right.value, right.timestamp);
+ return std::tie(left.metadata, left.value, left.timestamp) ==
+ std::tie(right.metadata, right.value, right.timestamp);
}
diff --git a/tests/src/mocks/report_factory_mock.hpp b/tests/src/mocks/report_factory_mock.hpp
index 8af6d5c..deed82d 100644
--- a/tests/src/mocks/report_factory_mock.hpp
+++ b/tests/src/mocks/report_factory_mock.hpp
@@ -21,9 +21,8 @@
std::get<1>(sensorData));
}),
utils::toOperationType(std::get<1>(params)),
- std::get<2>(params),
- utils::toCollectionTimeScope(std::get<3>(params)),
- CollectionDuration(Milliseconds(std::get<4>(params))));
+ utils::toCollectionTimeScope(std::get<2>(params)),
+ CollectionDuration(Milliseconds(std::get<3>(params))));
});
}
diff --git a/tests/src/params/metric_params.hpp b/tests/src/params/metric_params.hpp
index cc5519d..cef5f87 100644
--- a/tests/src/params/metric_params.hpp
+++ b/tests/src/params/metric_params.hpp
@@ -24,17 +24,6 @@
return operationTypeProperty;
}
- MetricParams& id(std::string val)
- {
- idProperty = std::move(val);
- return *this;
- }
-
- const std::string& id() const
- {
- return idProperty;
- }
-
MetricParams& collectionTimeScope(CollectionTimeScope val)
{
collectionTimeScopeProperty = val;
@@ -92,7 +81,6 @@
private:
OperationType operationTypeProperty = {};
- std::string idProperty = "MetricId";
CollectionTimeScope collectionTimeScopeProperty = {};
CollectionDuration collectionDurationProperty =
CollectionDuration(Milliseconds(0u));
diff --git a/tests/src/params/report_params.hpp b/tests/src/params/report_params.hpp
index 458e11b..a10c783 100644
--- a/tests/src/params/report_params.hpp
+++ b/tests/src/params/report_params.hpp
@@ -134,7 +134,6 @@
"/xyz/openbmc_project/sensors/power/p1",
"metadata1"}},
OperationType::avg,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))},
LabeledMetricParameters{
@@ -142,7 +141,6 @@
"/xyz/openbmc_project/sensors/power/p2",
"metadata2"}},
OperationType::avg,
- "MetricId2",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}};
bool enabledProperty = true;
diff --git a/tests/src/test_metric.cpp b/tests/src/test_metric.cpp
index 952651c..6410337 100644
--- a/tests/src/test_metric.cpp
+++ b/tests/src/test_metric.cpp
@@ -45,12 +45,11 @@
return std::make_shared<Metric>(
utils::convContainer<std::shared_ptr<interfaces::Sensor>>(
sensorMocks),
- p.operationType(), p.id(), p.collectionTimeScope(),
- p.collectionDuration(), std::move(clockFakePtr));
+ p.operationType(), p.collectionTimeScope(), p.collectionDuration(),
+ std::move(clockFakePtr));
}
MetricParams params = MetricParams()
- .id("id")
.operationType(OperationType::avg)
.collectionTimeScope(CollectionTimeScope::point)
.collectionDuration(CollectionDuration(0ms));
@@ -141,7 +140,7 @@
ASSERT_THAT(
sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata0", 31.2,
+ ElementsAre(MetricValue{"metadata0", 31.2,
std::chrono::duration_cast<Milliseconds>(
clockFake.system.timestamp())
.count()}));
@@ -167,7 +166,6 @@
const auto conf = sut->dumpConfiguration();
LabeledMetricParameters expected = {};
- expected.at_label<ts::Id>() = params.id();
expected.at_label<ts::OperationType>() = params.operationType();
expected.at_label<ts::CollectionTimeScope>() = params.collectionTimeScope();
expected.at_label<ts::CollectionDuration>() = params.collectionDuration();
@@ -357,9 +355,8 @@
expectedReading] = GetParam().expectedReading();
const auto readings = sut->getUpdatedReadings();
- EXPECT_THAT(readings,
- ElementsAre(MetricValue{"id", "metadata0", expectedReading,
- expectedTimestamp.count()}));
+ EXPECT_THAT(readings, ElementsAre(MetricValue{"metadata0", expectedReading,
+ expectedTimestamp.count()}));
}
TEST_P(TestMetricCalculationFunctions,
@@ -377,9 +374,8 @@
expectedReading] = GetParam().expectedReading();
const auto readings = sut->getUpdatedReadings();
- EXPECT_THAT(readings,
- ElementsAre(MetricValue{"id", "metadata0", expectedReading,
- expectedTimestamp.count()}));
+ EXPECT_THAT(readings, ElementsAre(MetricValue{"metadata0", expectedReading,
+ expectedTimestamp.count()}));
}
TEST_P(TestMetricCalculationFunctions, returnsIsTimerRequired)
@@ -411,13 +407,13 @@
clockFake.system.set(Milliseconds{72});
EXPECT_THAT(sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata0", 10.0, 72},
- MetricValue{"id", "metadata1", 11.0, 72},
- MetricValue{"id", "metadata2", 12.0, 72},
- MetricValue{"id", "metadata3", 13.0, 72},
- MetricValue{"id", "metadata4", 14.0, 72},
- MetricValue{"id", "metadata5", 15.0, 72},
- MetricValue{"id", "metadata6", 16.0, 72}));
+ ElementsAre(MetricValue{"metadata0", 10.0, 72},
+ MetricValue{"metadata1", 11.0, 72},
+ MetricValue{"metadata2", 12.0, 72},
+ MetricValue{"metadata3", 13.0, 72},
+ MetricValue{"metadata4", 14.0, 72},
+ MetricValue{"metadata5", 15.0, 72},
+ MetricValue{"metadata6", 16.0, 72}));
}
TEST_F(TestMetricWithMultipleSensors, readingsContainOnlyAvailableSensors)
@@ -431,10 +427,10 @@
clockFake.system.set(Milliseconds{62});
EXPECT_THAT(sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata5", 15.0, 62},
- MetricValue{"id", "metadata3", 13.0, 62},
- MetricValue{"id", "metadata6", 16.0, 62},
- MetricValue{"id", "metadata0", 10.0, 62}));
+ ElementsAre(MetricValue{"metadata5", 15.0, 62},
+ MetricValue{"metadata3", 13.0, 62},
+ MetricValue{"metadata6", 16.0, 62},
+ MetricValue{"metadata0", 10.0, 62}));
}
TEST_F(TestMetricWithMultipleSensors, readingsContainsAllReadingsOutOfOrder)
@@ -448,11 +444,11 @@
clockFake.system.set(Milliseconds{52});
EXPECT_THAT(sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata6", 16.0, 52},
- MetricValue{"id", "metadata5", 15.0, 52},
- MetricValue{"id", "metadata3", 13.0, 52},
- MetricValue{"id", "metadata4", 14.0, 52},
- MetricValue{"id", "metadata0", 10.0, 52},
- MetricValue{"id", "metadata2", 12.0, 52},
- MetricValue{"id", "metadata1", 11.0, 52}));
+ ElementsAre(MetricValue{"metadata6", 16.0, 52},
+ MetricValue{"metadata5", 15.0, 52},
+ MetricValue{"metadata3", 13.0, 52},
+ MetricValue{"metadata4", 14.0, 52},
+ MetricValue{"metadata0", 10.0, 52},
+ MetricValue{"metadata2", 12.0, 52},
+ MetricValue{"metadata1", 11.0, 52}));
}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 1d0f0bb..accd112 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -83,8 +83,8 @@
}
metricMocks.resize(metricParameters.size());
- std::vector<MetricValue> readings{{MetricValue{"a", "b", 17.1, 114},
- MetricValue{"aa", "bb", 42.0, 74}}};
+ std::vector<MetricValue> readings{
+ {MetricValue{"b", 17.1, 114}, MetricValue{"bb", 42.0, 74}}};
ASSERT_THAT(readings.size(), Ge(metricParameters.size()));
@@ -161,6 +161,16 @@
property, newValue);
}
+ template <class... Args>
+ static boost::system::error_code callMethod(const std::string& path,
+ const std::string& method,
+ Args&&... args)
+ {
+ return DbusEnvironment::callMethod(path, Report::reportIfaceName,
+ "SetReportingProperties",
+ std::forward<Args>(args)...);
+ }
+
template <class T>
struct ChangePropertyParams
{
@@ -242,17 +252,14 @@
getProperty<bool>(sut->getPath(), "LogToMetricReportsCollection"),
Eq(utils::contains(defaultParams().reportActions(),
ReportAction::logToMetricReportsCollection)));
- EXPECT_THAT(getProperty<ReadingParameters>(
- sut->getPath(), "ReadingParametersFutureVersion"),
- Eq(toReadingParameters(defaultParams().metricParameters())));
+ EXPECT_THAT(
+ getProperty<ReadingParameters>(sut->getPath(), "ReadingParameters"),
+ Eq(toReadingParameters(defaultParams().metricParameters())));
EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"),
Eq(defaultParams().reportName()));
EXPECT_THAT(
getProperty<std::vector<object_path>>(sut->getPath(), "Triggers"),
IsEmpty());
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(sut->getPath(), "ErrorMessages"),
- IsEmpty());
}
TEST_F(TestReport, readingsAreInitialyEmpty)
@@ -269,7 +276,6 @@
"/xyz/openbmc_project/sensors/power/psu",
"NewMetadata123"}},
OperationType::avg,
- "NewMetricId123",
CollectionTimeScope::startup,
CollectionDuration(250ms)}}});
auto metrics = getMetricsFromReadingParams(newParams);
@@ -277,61 +283,17 @@
EXPECT_CALL(*reportFactoryMock, updateMetrics(_, _, _))
.WillOnce(SetArgReferee<0>(metrics));
EXPECT_THAT(
- setProperty(sut->getPath(), "ReadingParametersFutureVersion", newParams)
- .value(),
+ setProperty(sut->getPath(), "ReadingParameters", newParams).value(),
Eq(boost::system::errc::success));
- EXPECT_THAT(getProperty<ReadingParameters>(
- sut->getPath(), "ReadingParametersFutureVersion"),
- Eq(newParams));
-}
-
-TEST_F(TestReport, setReadingParametersWithTooLongMetricId)
-{
- const ReadingParameters currentValue =
- toReadingParameters(defaultParams().metricParameters());
-
- ReadingParameters newParams = toReadingParameters(
- std::vector<LabeledMetricParameters>{{LabeledMetricParameters{
- {LabeledSensorInfo{"Service",
- "/xyz/openbmc_project/sensors/power/psu",
- "NewMetadata123"}},
- OperationType::avg,
- utils::string_utils::getTooLongId(),
- CollectionTimeScope::startup,
- CollectionDuration(250ms)}}});
-
- changeProperty<ReadingParameters>(
- sut->getPath(), "ReadingParametersFutureVersion",
- {.valueBefore = Eq(currentValue),
- .newValue = newParams,
- .ec = Eq(boost::system::errc::invalid_argument),
- .valueAfter = Eq(currentValue)});
-}
-
-TEST_F(TestReport, setReportingTypeWithValidNewType)
-{
- changeProperty<std::string>(
- sut->getPath(), "ReportingType",
- {.valueBefore = Not(Eq(utils::enumToString(ReportingType::onRequest))),
- .newValue = utils::enumToString(ReportingType::onRequest)});
-}
-
-TEST_F(TestReport, setReportingTypeWithInvalidType)
-{
- const std::string currentValue =
- utils::enumToString(defaultParams().reportingType());
-
- changeProperty<std::string>(
- sut->getPath(), "ReportingType",
- {.valueBefore = Eq(currentValue),
- .newValue = "Periodic_ABC",
- .ec = Eq(boost::system::errc::invalid_argument),
- .valueAfter = Eq(currentValue)});
+ EXPECT_THAT(
+ getProperty<ReadingParameters>(sut->getPath(), "ReadingParameters"),
+ Eq(newParams));
}
TEST_F(TestReport, setReportActionsWithValidNewActions)
{
- std::vector<std::string> newActions = {"EmitsReadingsUpdate"};
+ std::vector<std::string> newActions = {
+ utils::enumToString(ReportAction::emitsReadingsUpdate)};
std::vector<std::string> currActions =
utils::transform(defaultParams().reportActions(),
[](const auto v) { return utils::enumToString(v); });
@@ -342,16 +304,19 @@
Eq(boost::system::errc::success));
EXPECT_THAT(
getProperty<std::vector<std::string>>(sut->getPath(), "ReportActions"),
- UnorderedElementsAre("EmitsReadingsUpdate",
- "LogToMetricReportsCollection"));
+ UnorderedElementsAre(
+ utils::enumToString(ReportAction::emitsReadingsUpdate),
+ utils::enumToString(ReportAction::logToMetricReportsCollection)));
}
TEST_F(TestReport, setReportActionsWithValidUnsortedActions)
{
- std::vector<std::string> newActions = {"LogToMetricReportsCollection",
- "EmitsReadingsUpdate"};
- std::vector<std::string> expectedActions = {"EmitsReadingsUpdate",
- "LogToMetricReportsCollection"};
+ std::vector<std::string> newActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection),
+ utils::enumToString(ReportAction::emitsReadingsUpdate)};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::emitsReadingsUpdate),
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
std::vector<std::string> currActions =
utils::transform(defaultParams().reportActions(),
[](const auto v) { return utils::enumToString(v); });
@@ -368,7 +333,8 @@
TEST_F(TestReport, setReportActionsWithEmptyActions)
{
std::vector<std::string> newActions = {};
- std::vector<std::string> expectedActions = {"LogToMetricReportsCollection"};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
std::vector<std::string> currActions =
utils::transform(defaultParams().reportActions(),
[](const auto v) { return utils::enumToString(v); });
@@ -397,7 +363,8 @@
TEST_F(TestReport, createReportWithEmptyActions)
{
- std::vector<std::string> expectedActions = {"LogToMetricReportsCollection"};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
sut = makeReport(ReportParams().reportId("TestId_1").reportActions({}));
EXPECT_THAT(
@@ -407,10 +374,12 @@
TEST_F(TestReport, createReportWithValidUnsortedActions)
{
- std::vector<std::string> newActions = {"LogToMetricReportsCollection",
- "EmitsReadingsUpdate"};
- std::vector<std::string> expectedActions = {"EmitsReadingsUpdate",
- "LogToMetricReportsCollection"};
+ std::vector<std::string> newActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection),
+ utils::enumToString(ReportAction::emitsReadingsUpdate)};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::emitsReadingsUpdate),
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
sut = makeReport(
ReportParams()
@@ -431,113 +400,75 @@
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Enabled"), Eq(newValue));
}
-TEST_F(TestReport, setIntervalWithValidValue)
+TEST_F(TestReport, setReportingPropertiesWithValidValues)
{
uint64_t newValue = ReportManager::minInterval.count() * 42;
- EXPECT_THAT(setProperty(sut->getPath(), "Interval", newValue).value(),
+ EXPECT_THAT(callMethod(sut->getPath(), "SetReportingProperties",
+ utils::enumToString(ReportingType::periodic),
+ newValue),
Eq(boost::system::errc::success));
EXPECT_THAT(getProperty<uint64_t>(sut->getPath(), "Interval"),
Eq(newValue));
}
-TEST_F(
- TestReport,
- settingIntervalWithInvalidValueDoesNotChangePropertyAndReturnsInvalidArgument)
+TEST_F(TestReport, failsToSetInvalidInterval)
{
uint64_t newValue = ReportManager::minInterval.count() - 1;
- EXPECT_THAT(setProperty(sut->getPath(), "Interval", newValue).value(),
- Eq(boost::system::errc::invalid_argument));
+
+ EXPECT_THAT(
+ callMethod(sut->getPath(), "SetReportingProperties", "", newValue),
+ Eq(boost::system::errc::invalid_argument));
+
EXPECT_THAT(getProperty<uint64_t>(sut->getPath(), "Interval"),
Eq(defaultParams().interval().count()));
}
-TEST_F(TestReport, settingInvalidReportingTypeCreatesErrorMessage)
+TEST_F(TestReport, failsToSetIncompatibleInterval)
{
auto report = makeReport(defaultParams()
.reportId("report2")
.reportingType(ReportingType::onRequest)
.interval(Milliseconds{0}));
- EXPECT_THAT(
- setProperty<std::string>(report->getPath(), "ReportingType", "Periodic")
- .value(),
- Eq(boost::system::errc::success));
-
- EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
- Eq("Periodic"));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- UnorderedElementsAre(
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict), "Interval"),
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict),
- "ReportingType")));
-}
-
-TEST_F(TestReport, settingValidReportingTypeRemovesErrors)
-{
- auto report = makeReport(defaultParams()
- .reportId("report2")
- .reportingType(ReportingType::onRequest)
- .interval(Milliseconds{0}));
+ uint64_t newValue = ReportManager::minInterval.count();
EXPECT_THAT(
- setProperty<std::string>(report->getPath(), "ReportingType", "Periodic")
- .value(),
- Eq(boost::system::errc::success));
- EXPECT_THAT(setProperty<std::string>(report->getPath(), "ReportingType",
- "OnRequest")
- .value(),
- Eq(boost::system::errc::success));
-
- EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
- Eq("OnRequest"));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- IsEmpty());
-}
-
-TEST_F(TestReport, settingInvalidIntervalDisablesReport)
-{
- auto report = makeReport(defaultParams()
- .reportId("report2")
- .reportingType(ReportingType::periodic)
- .interval(ReportManager::minInterval));
-
- EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval", 0).value(),
- Eq(boost::system::errc::success));
-
- EXPECT_THAT(getProperty<uint64_t>(report->getPath(), "Interval"), Eq(0u));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- UnorderedElementsAre(
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict), "Interval"),
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict),
- "ReportingType")));
-}
-
-TEST_F(TestReport, settingValidIntervalEnablesReport)
-{
- auto report = makeReport(defaultParams()
- .reportId("report2")
- .reportingType(ReportingType::periodic)
- .interval(ReportManager::minInterval));
-
- EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval", 0).value(),
- Eq(boost::system::errc::success));
- EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval",
- ReportManager::minInterval.count())
- .value(),
- Eq(boost::system::errc::success));
+ callMethod(report->getPath(), "SetReportingProperties", "", newValue),
+ Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(getProperty<uint64_t>(report->getPath(), "Interval"),
- Eq(ReportManager::minInterval.count()));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- IsEmpty());
+ Eq(defaultParams().interval().count()));
+}
+
+TEST_F(TestReport, failsToSetInvalidReportingType)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::onRequest)
+ .interval(Milliseconds{0}));
+
+ EXPECT_THAT(callMethod(sut->getPath(), "SetReportingProperties", "XYZ",
+ std::numeric_limits<uint64_t>::max()),
+ Eq(boost::system::errc::invalid_argument));
+
+ EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
+ Eq(utils::enumToString(ReportingType::onRequest)));
+}
+
+TEST_F(TestReport, failsToSetIncompatibleReportingType)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::onRequest)
+ .interval(Milliseconds{0}));
+
+ EXPECT_THAT(callMethod(sut->getPath(), "SetReportingProperties",
+ utils::enumToString(ReportingType::periodic),
+ std::numeric_limits<uint64_t>::max()),
+ Eq(boost::system::errc::invalid_argument));
+
+ EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
+ Eq(utils::enumToString(ReportingType::onRequest)));
}
TEST_F(TestReport, settingEmitsReadingsUpdateHaveNoEffect)
@@ -701,7 +632,6 @@
"/xyz/openbmc_project/sensors/power/p1"},
{tstring::Metadata::str(), "metadata1"}}}},
{tstring::OperationType::str(), OperationType::avg},
- {tstring::Id::str(), "MetricId1"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}},
@@ -711,7 +641,6 @@
"/xyz/openbmc_project/sensors/power/p2"},
{tstring::Metadata::str(), "metadata2"}}}},
{tstring::OperationType::str(), OperationType::avg},
- {tstring::Id::str(), "MetricId2"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}}}))));
@@ -877,9 +806,8 @@
const auto [timestamp, readings] = getProperty<Readings>(sut->getPath(),
"Readings");
- EXPECT_THAT(readings,
- ElementsAre(std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)));
+ EXPECT_THAT(readings, ElementsAre(std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)));
}
class TestReportNonOnRequestType :
@@ -892,10 +820,13 @@
}
};
-INSTANTIATE_TEST_SUITE_P(
- _, TestReportNonOnRequestType,
- Values(defaultParams().reportingType(ReportingType::periodic),
- defaultParams().reportingType(ReportingType::onChange)));
+INSTANTIATE_TEST_SUITE_P(_, TestReportNonOnRequestType,
+ Values(defaultParams()
+ .reportingType(ReportingType::periodic)
+ .interval(ReportManager::minInterval * 10),
+ defaultParams()
+ .reportingType(ReportingType::onChange)
+ .interval(Milliseconds(0))));
TEST_P(TestReportNonOnRequestType, readingsAreNotUpdateOnUpdateCall)
{
@@ -958,9 +889,8 @@
const auto [timestamp, readings] = getProperty<Readings>(sut->getPath(),
"Readings");
- EXPECT_THAT(readings,
- ElementsAre(std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)));
+ EXPECT_THAT(readings, ElementsAre(std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)));
}
struct ReportUpdatesReportParams
@@ -979,9 +909,8 @@
void changeReport(ReportingType rt, Milliseconds interval)
{
- setProperty<std::string>(sut->getPath(), "ReportingType",
- utils::enumToString(rt));
- setProperty<uint64_t>(sut->getPath(), "Interval", interval.count());
+ callMethod(sut->getPath(), "SetReportingProperties",
+ utils::enumToString(rt), interval.count());
}
auto readings()
@@ -1007,21 +936,20 @@
defaultParams()
.reportUpdates(ReportUpdates::appendWrapsWhenFull)
.appendLimit(5),
- std::vector<ReadingData>{{std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u)}},
+ std::vector<ReadingData>{{std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendWrapsWhenFull)
.appendLimit(4),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
@@ -1032,35 +960,33 @@
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(10),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(5),
- std::vector<ReadingData>{{std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u)}},
false},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(4),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
false},
ReportUpdatesReportParams{
defaultParams()
@@ -1071,33 +997,29 @@
defaultParams()
.reportUpdates(ReportUpdates::overwrite)
.appendLimit(500),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::overwrite)
.appendLimit(1),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::overwrite)
.appendLimit(0),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(2u),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
false}));
TEST_P(TestReportWithReportUpdatesAndLimit,
@@ -1270,7 +1192,8 @@
sut = makeReport(defaultParams()
.reportingType(ReportingType::periodic)
- .reportActions({}));
+ .reportActions({})
+ .interval(Milliseconds(1000)));
makeMonitor();
DbusEnvironment::sleepFor(defaultParams().interval() * 2);
}
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index fc394ae..295d966 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -18,8 +18,7 @@
using namespace std::string_literals;
using namespace std::chrono_literals;
-using AddReportFutureVersionVariantForSet =
- utils::WithoutMonostate<AddReportFutureVersionVariant>;
+using AddReportVariantForSet = utils::WithoutMonostate<AddReportVariant>;
class TestReportManager : public Test
{
@@ -57,10 +56,9 @@
DbusEnvironment::synchronizeIoc();
}
- std::pair<boost::system::error_code, std::string>
- addReport(const std::vector<
- std::pair<std::string, AddReportFutureVersionVariantForSet>>&
- properties)
+ template <class... Args>
+ requires(sizeof...(Args) > 1)
+ std::pair<boost::system::error_code, std::string> addReport(Args&&... args)
{
std::promise<std::pair<boost::system::error_code, std::string>>
addReportPromise;
@@ -70,34 +68,22 @@
addReportPromise.set_value({ec, path});
},
DbusEnvironment::serviceName(), ReportManager::reportManagerPath,
- ReportManager::reportManagerIfaceName, "AddReportFutureVersion",
- properties);
+ ReportManager::reportManagerIfaceName, "AddReport",
+ std::forward<Args>(args)...);
return DbusEnvironment::waitForFuture(addReportPromise.get_future());
}
auto addReport(const ReportParams& params)
{
- std::vector<std::pair<std::string, AddReportFutureVersionVariantForSet>>
- properties;
-
- properties.emplace_back("Id", params.reportId());
- properties.emplace_back("Name", params.reportName());
- properties.emplace_back("ReportingType",
- utils::enumToString(params.reportingType()));
- properties.emplace_back("ReportUpdates",
- utils::enumToString(params.reportUpdates()));
- properties.emplace_back("AppendLimit", params.appendLimit());
- properties.emplace_back("Enabled", params.enabled());
- properties.emplace_back("ReportActions",
- utils::transform(params.reportActions(),
- [](const auto v) {
- return utils::enumToString(v);
- }));
- properties.emplace_back("Interval", params.interval().count());
- properties.emplace_back("ReadingParameters",
- toReadingParameters(params.metricParameters()));
-
- return addReport(properties);
+ return addReport(
+ params.reportId(), params.reportName(),
+ utils::enumToString(params.reportingType()),
+ utils::enumToString(params.reportUpdates()), params.appendLimit(),
+ utils::transform(
+ params.reportActions(),
+ [](const auto v) { return utils::enumToString(v); }),
+ params.interval().count(),
+ toReadingParameters(params.metricParameters()), params.enabled());
}
template <class T>
@@ -125,7 +111,10 @@
{
EXPECT_THAT(
getProperty<std::vector<std::string>>("SupportedOperationTypes"),
- UnorderedElementsAre("Maximum", "Minimum", "Average", "Summation"));
+ UnorderedElementsAre(utils::enumToString(OperationType::max),
+ utils::enumToString(OperationType::min),
+ utils::enumToString(OperationType::avg),
+ utils::enumToString(OperationType::sum)));
}
TEST_F(TestReportManager, addReport)
@@ -163,8 +152,10 @@
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
auto [ec, path] = addReport(
- std::vector<
- std::pair<std::string, AddReportFutureVersionVariantForSet>>{});
+ "", "", "", "", std::numeric_limits<uint64_t>::max(),
+ std::vector<std::string>(), std::numeric_limits<uint64_t>::max(),
+ ReadingParameters(), true);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
@@ -243,28 +234,6 @@
EXPECT_THAT(path, Eq("/"s + reportParams.reportId()));
}
-TEST_F(TestReportManager, addReportWithMaxLengthMetricId)
-{
- namespace ts = utils::tstring;
- std::vector<LabeledMetricParameters> newMetricParams{
- {LabeledMetricParameters{
- {LabeledSensorInfo{"Service",
- "/xyz/openbmc_project/sensors/power/p1",
- "metadata1"}},
- OperationType::avg,
- utils::string_utils::getMaxId(),
- CollectionTimeScope::point,
- CollectionDuration(Milliseconds(0u))}}};
-
- reportParams.metricParameters(newMetricParams);
- reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
-
- auto [ec, path] = addReport(reportParams);
-
- EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
- EXPECT_THAT(path, Eq("/"s + reportParams.reportId()));
-}
-
TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongFullId)
{
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
@@ -338,31 +307,6 @@
EXPECT_THAT(path, Eq(std::string()));
}
-TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongMetricId)
-{
- namespace ts = utils::tstring;
-
- std::vector<LabeledMetricParameters> newMetricParams{
- {LabeledMetricParameters{
- {LabeledSensorInfo{"Service",
- "/xyz/openbmc_project/sensors/power/p1",
- "metadata1"}},
- OperationType::avg,
- utils::string_utils::getTooLongId(),
- CollectionTimeScope::point,
- CollectionDuration(Milliseconds(0u))}}};
-
- reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
- .Times(0);
-
- reportParams.metricParameters(newMetricParams);
-
- auto [ec, path] = addReport(reportParams);
-
- EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
- EXPECT_THAT(path, Eq(std::string()));
-}
-
TEST_F(TestReportManager, DISABLED_failToAddReportTwice)
{
reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
@@ -395,8 +339,10 @@
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
- auto [ec, path] = addReport({{"Name", reportParams.reportName()},
- {"ReportingType", "InvalidReportingType"}});
+ auto [ec, path] = addReport(
+ "", "", "InvalidReportingType", "",
+ std::numeric_limits<uint64_t>::max(), std::vector<std::string>(),
+ std::numeric_limits<uint64_t>::max(), ReadingParameters(), false);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(path, Eq(std::string()));
@@ -414,7 +360,6 @@
"/xyz/openbmc_project/sensors/power/p1",
"Metadata1"}},
OperationType::avg,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}});
@@ -448,7 +393,6 @@
metricParams.emplace_back(
LabeledMetricParameters{{},
OperationType::avg,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))});
}
@@ -476,13 +420,14 @@
TEST_F(TestReportManager, addReportWithAppendLimitEqualToUint64MaxIsAllowed)
{
- reportParams.appendLimit(std::numeric_limits<uint64_t>::max());
-
EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
- reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
+ reportFactoryMock
+ .expectMake(reportParams.appendLimit(ReportManager::maxAppendLimit),
+ Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
- auto [ec, path] = addReport(reportParams);
+ auto [ec, path] = addReport(
+ reportParams.appendLimit(std::numeric_limits<uint64_t>::max()));
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
@@ -574,7 +519,6 @@
"/xyz/openbmc_project/sensors/power/p1",
"Metadata1"}},
operationType,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}});
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index f55dec1..6cccb4a 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -315,8 +315,9 @@
{
EXPECT_CALL(*triggerFactoryMockPtr, updateThresholds(_, _, _, _, _, _));
TriggerThresholdParams newThresholds =
- std::vector<discrete::ThresholdParam>(
- {std::make_tuple("discrete threshold", "OK", 10, "12.3")});
+ std::vector<discrete::ThresholdParam>({std::make_tuple(
+ "discrete threshold", utils::enumToString(discrete::Severity::ok),
+ 10, "12.3")});
EXPECT_THAT(setProperty(sut->getPath(), "Thresholds", newThresholds),
Eq(boost::system::errc::success));
}
@@ -329,7 +330,8 @@
TriggerThresholdParams newThresholds =
std::vector<discrete::ThresholdParam>({std::make_tuple(
- utils::string_utils::getTooLongName(), "OK", 10, "12.3")});
+ utils::string_utils::getTooLongName(),
+ utils::enumToString(discrete::Severity::ok), 10, "12.3")});
changeProperty<TriggerThresholdParams>(
sut->getPath(), "Thresholds",