added Enabled to AddReportFutureVersion API
- New parameter added to API
- Changed API to be more flexible in the future
Tested:
- Unit tests are passing
- Tested together with:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/49796/23
POST, PATCH, PUT for MetricReportDefinitions are working correctly
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: Ic988446d283a581dc6866418b1b27a989c3bc9a0
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 8be6397..579c714 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -22,6 +22,8 @@
#include <sdbusplus/exception.hpp>
+#include <ranges>
+
using namespace testing;
using namespace std::literals::string_literals;
using namespace std::chrono_literals;
@@ -969,11 +971,30 @@
public TestReport,
public WithParamInterface<ReportUpdatesReportParams>
{
+ public:
void SetUp() override
+ {}
+
+ void changeReport(ReportingType rt, Milliseconds interval)
{
- sut = makeReport(ReportParams(GetParam().reportParams)
- .reportingType(ReportingType::periodic)
- .interval(std::chrono::hours(1000)));
+ setProperty<std::string>(sut->getPath(), "ReportingType",
+ utils::enumToString(rt));
+ setProperty<uint64_t>(sut->getPath(), "Interval", interval.count());
+ }
+
+ auto readings()
+ {
+ auto [timestamp, readings] =
+ getProperty<Readings>(sut->getPath(), "Readings");
+ return readings;
+ }
+
+ void updateReportFourTimes()
+ {
+ for (int i = 0; i < 4; i++)
+ {
+ messanger.send(messages::UpdateReportInd{{sut->getId()}});
+ }
}
};
@@ -1067,22 +1088,57 @@
std::vector<ReadingData>{
{std::make_tuple("a"s, "b"s, 17.1, 114u),
std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
- true}));
+ true},
+ ReportUpdatesReportParams{
+ defaultParams()
+ .reportUpdates(ReportUpdates::appendStopsWhenFull)
+ .appendLimit(std::numeric_limits<uint64_t>::max()),
+ std::vector<ReadingData>{
+ {std::make_tuple("a"s, "b"s, 17.1, 114u),
+ std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ false}));
TEST_P(TestReportWithReportUpdatesAndLimit,
readingsAreUpdatedAfterIntervalExpires)
{
- for (int i = 0; i < 4; i++)
- {
- messanger.send(messages::UpdateReportInd{{sut->getId()}});
- }
+ sut = makeReport(ReportParams(GetParam().reportParams)
+ .reportingType(ReportingType::periodic)
+ .interval(std::chrono::hours(1000)));
- const auto [timestamp, readings] =
- getProperty<Readings>(sut->getPath(), "Readings");
- const auto enabled = getProperty<bool>(sut->getPath(), "Enabled");
+ updateReportFourTimes();
- EXPECT_THAT(readings, ElementsAreArray(GetParam().expectedReadings));
- EXPECT_EQ(enabled, GetParam().expectedEnabled);
+ EXPECT_THAT(readings(), ElementsAreArray(GetParam().expectedReadings));
+ EXPECT_THAT(getProperty<bool>(sut->getPath(), "Enabled"),
+ Eq(GetParam().expectedEnabled));
+}
+
+TEST_P(TestReportWithReportUpdatesAndLimit,
+ appendLimitIsRespectedAfterChangingToPeriodic)
+{
+ sut = makeReport(ReportParams(GetParam().reportParams)
+ .reportingType(ReportingType::onRequest)
+ .interval(std::chrono::hours(0)));
+
+ changeReport(ReportingType::periodic, std::chrono::hours(1000));
+ updateReportFourTimes();
+
+ EXPECT_THAT(readings(), ElementsAreArray(GetParam().expectedReadings));
+ EXPECT_THAT(getProperty<bool>(sut->getPath(), "Enabled"),
+ Eq(GetParam().expectedEnabled));
+}
+
+TEST_P(TestReportWithReportUpdatesAndLimit,
+ appendLimitIsIgnoredAfterChangingToOnRequest)
+{
+ sut = makeReport(ReportParams(GetParam().reportParams)
+ .reportingType(ReportingType::periodic)
+ .interval(std::chrono::hours(1000)));
+
+ changeReport(ReportingType::onRequest, Milliseconds{0});
+ updateReportFourTimes();
+
+ EXPECT_THAT(readings(), SizeIs(2u));
+ EXPECT_THAT(getProperty<bool>(sut->getPath(), "Enabled"), Eq(true));
}
class TestReportInitialization : public TestReport
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 6ce0f1f..f593fd4 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -12,11 +12,15 @@
#include "utils/string_utils.hpp"
#include "utils/transform.hpp"
#include "utils/tstring.hpp"
+#include "utils/variant_utils.hpp"
using namespace testing;
using namespace std::string_literals;
using namespace std::chrono_literals;
+using AddReportFutureVersionVariantForSet =
+ utils::WithoutMonostate<AddReportFutureVersionVariant>;
+
class TestReportManager : public Test
{
public:
@@ -53,10 +57,10 @@
DbusEnvironment::synchronizeIoc();
}
- template <class... Args>
- requires(sizeof...(Args) > 1)
- std::pair<boost::system::error_code, std::string> addReport(
- Args&&... args)
+ std::pair<boost::system::error_code, std::string>
+ addReport(const std::vector<
+ std::pair<std::string, AddReportFutureVersionVariantForSet>>&
+ properties)
{
std::promise<std::pair<boost::system::error_code, std::string>>
addReportPromise;
@@ -67,22 +71,33 @@
},
DbusEnvironment::serviceName(), ReportManager::reportManagerPath,
ReportManager::reportManagerIfaceName, "AddReportFutureVersion",
- std::forward<Args>(args)...);
+ properties);
return DbusEnvironment::waitForFuture(addReportPromise.get_future());
}
auto addReport(const ReportParams& params)
{
- 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()));
+ 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("MetricParams",
+ toReadingParameters(params.metricParameters()));
+
+ return addReport(properties);
}
template <class T>
@@ -124,6 +139,36 @@
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
+TEST_F(TestReportManager, addDisabledReport)
+{
+ reportParams.enabled(false);
+
+ EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
+ .WillOnce(Return(ByMove(std::move(reportMockPtr))));
+
+ auto [ec, path] = addReport(reportParams);
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq(reportMock.getPath()));
+}
+
+TEST_F(TestReportManager, addReportWithOnlyDefaultParams)
+{
+ EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
+ EXPECT_CALL(reportFactoryMock,
+ make("Report"s, "Report"s, ReportingType::onRequest,
+ std::vector<ReportAction>{}, Milliseconds{}, 0,
+ ReportUpdates::overwrite, _, _,
+ std::vector<LabeledMetricParameters>{}, true, Readings{}))
+ .WillOnce(Return(ByMove(std::move(reportMockPtr))));
+
+ auto [ec, path] = addReport(
+ std::vector<
+ std::pair<std::string, AddReportFutureVersionVariantForSet>>{});
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq(reportMock.getPath()));
+}
+
TEST_F(TestReportManager, addOnChangeReport)
{
EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
@@ -350,12 +395,8 @@
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
- auto [ec, path] = addReport(
- reportParams.reportName(), "InvalidReportingType",
- utils::transform(reportParams.reportActions(),
- [](const auto v) { return utils::enumToString(v); }),
- reportParams.interval().count(),
- toReadingParameters(reportParams.metricParameters()));
+ auto [ec, path] = addReport({{"Name", reportParams.reportName()},
+ {"ReportingType", "InvalidReportingType"}});
EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(path, Eq(std::string()));