Api changes in AddReportFuture version
Added support for CollectionFunction, CollectionDuration,
CollectionTimeScope, ReportUpdates, AppendLimit.
New API separates Id and Name, user can decide to pass only Name
to auto generate Id or pass Id which needs to be unique.
Tested:
- No functional changes to old API, everything works as before
- All use cases can be replaced with new API to achieve same results
- New features which require new API work as expected
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I647efab36d90a548754f89968223e162a087481e
diff --git a/tests/meson.build b/tests/meson.build
index 3059953..27cdb42 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,12 +28,14 @@
'../src/trigger_manager.cpp',
'../src/types/report_types.cpp',
'../src/utils/conversion_trigger.cpp',
+ '../src/utils/generate_id.cpp',
'src/dbus_environment.cpp',
'src/main.cpp',
'src/stubs/dbus_sensor_object.cpp',
'src/test_conversion.cpp',
'src/test_detached_timer.cpp',
'src/test_discrete_threshold.cpp',
+ 'src/test_generate_id.cpp',
'src/test_metric.cpp',
'src/test_numeric_threshold.cpp',
'src/test_on_change_threshold.cpp',
diff --git a/tests/src/mocks/report_factory_mock.hpp b/tests/src/mocks/report_factory_mock.hpp
index cc0bc8e..58c71d6 100644
--- a/tests/src/mocks/report_factory_mock.hpp
+++ b/tests/src/mocks/report_factory_mock.hpp
@@ -15,14 +15,15 @@
return utils::transform(readingParams, [](const auto& params) {
return LabeledMetricParameters(
utils::transform(std::get<0>(params),
- [](const auto& sensorPath) {
- return LabeledSensorParameters("Service",
- sensorPath);
+ [](const auto& sensorData) {
+ return LabeledSensorParameters(
+ "Service", std::get<0>(sensorData),
+ std::get<1>(sensorData));
}),
utils::toOperationType(std::get<1>(params)),
- std::get<2>(params), std::get<3>(params),
- utils::toCollectionTimeScope(std::get<4>(params)),
- CollectionDuration(Milliseconds(std::get<5>(params))));
+ std::get<2>(params),
+ utils::toCollectionTimeScope(std::get<3>(params)),
+ CollectionDuration(Milliseconds(std::get<4>(params))));
});
}
@@ -35,9 +36,10 @@
.WillByDefault(
WithArgs<1>(Invoke(&ReportFactoryMock::convertToLabeled)));
- ON_CALL(*this, make(A<const std::string&>(), _, _, _, _, _, _, _, _, _))
- .WillByDefault(WithArgs<0>(Invoke([](const std::string& name) {
- return std::make_unique<NiceMock<ReportMock>>(name);
+ ON_CALL(*this,
+ make(A<const std::string&>(), _, _, _, _, _, _, _, _, _, _))
+ .WillByDefault(WithArgs<0>(Invoke([](const std::string& id) {
+ return std::make_unique<NiceMock<ReportMock>>(id);
})));
}
@@ -46,7 +48,7 @@
(const, override));
MOCK_METHOD(std::unique_ptr<interfaces::Report>, make,
- (const std::string&, const ReportingType,
+ (const std::string&, const std::string&, const ReportingType,
const std::vector<ReportAction>&, Milliseconds, uint64_t,
const ReportUpdates, interfaces::ReportManager&,
interfaces::JsonStorage&, std::vector<LabeledMetricParameters>,
@@ -62,15 +64,16 @@
{
const ReportParams& params = *paramsRef;
return EXPECT_CALL(
- *this, make(params.reportName(), params.reportingType(),
- params.reportActions(), params.interval(),
- params.appendLimit(), params.reportUpdates(), rm,
- js, params.metricParameters(), params.enabled()));
+ *this, make(params.reportId(), params.reportName(),
+ params.reportingType(), params.reportActions(),
+ params.interval(), params.appendLimit(),
+ params.reportUpdates(), rm, js,
+ params.metricParameters(), params.enabled()));
}
else
{
using testing::_;
- return EXPECT_CALL(*this, make(_, _, _, _, _, _, rm, js, _, _));
+ return EXPECT_CALL(*this, make(_, _, _, _, _, _, _, rm, js, _, _));
}
}
};
diff --git a/tests/src/mocks/report_mock.hpp b/tests/src/mocks/report_mock.hpp
index 726d53c..6a6e259 100644
--- a/tests/src/mocks/report_mock.hpp
+++ b/tests/src/mocks/report_mock.hpp
@@ -7,12 +7,12 @@
class ReportMock : public interfaces::Report
{
public:
- ReportMock(std::string name)
+ ReportMock(const std::string& id)
{
using namespace testing;
- ON_CALL(*this, getName).WillByDefault([name] { return name; });
- ON_CALL(*this, getPath).WillByDefault([name] { return "/" + name; });
+ ON_CALL(*this, getId).WillByDefault([id] { return id; });
+ ON_CALL(*this, getPath).WillByDefault([id] { return "/" + id; });
EXPECT_CALL(*this, Die).Times(AnyNumber());
}
@@ -21,7 +21,7 @@
Die();
}
- MOCK_METHOD(std::string, getName, (), (override, const));
+ MOCK_METHOD(std::string, getId, (), (override, const));
MOCK_METHOD(std::string, getPath, (), (override, const));
MOCK_METHOD(void, updateReadings, (), (override));
MOCK_METHOD(void, Die, ());
diff --git a/tests/src/mocks/sensor_mock.hpp b/tests/src/mocks/sensor_mock.hpp
index b86a26e..9ff72e2 100644
--- a/tests/src/mocks/sensor_mock.hpp
+++ b/tests/src/mocks/sensor_mock.hpp
@@ -24,6 +24,7 @@
}
MOCK_METHOD(Id, id, (), (const, override));
+ MOCK_METHOD(std::string, metadata, (), (const, override));
MOCK_METHOD(void, registerForUpdates,
(const std::weak_ptr<interfaces::SensorListener>&), (override));
MOCK_METHOD(void, unregisterFromUpdates,
diff --git a/tests/src/mocks/trigger_factory_mock.hpp b/tests/src/mocks/trigger_factory_mock.hpp
index ab7510d..9fa2f48 100644
--- a/tests/src/mocks/trigger_factory_mock.hpp
+++ b/tests/src/mocks/trigger_factory_mock.hpp
@@ -24,7 +24,7 @@
MOCK_METHOD(std::unique_ptr<interfaces::Trigger>, make,
(const std::string& id, const std::string& name,
const std::vector<std::string>& triggerActions,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
interfaces::TriggerManager& triggerManager,
interfaces::JsonStorage& triggerStorage,
const LabeledTriggerThresholdParams& labeledThresholdParams,
@@ -54,7 +54,7 @@
return EXPECT_CALL(
*this, make(params.id(), params.name(), params.triggerActions(),
- params.reportNames(), tm, triggerStorage,
+ params.reportIds(), tm, triggerStorage,
params.thresholdParams(), params.sensors()));
}
else
diff --git a/tests/src/params/metric_params.hpp b/tests/src/params/metric_params.hpp
index 4654060..90db484 100644
--- a/tests/src/params/metric_params.hpp
+++ b/tests/src/params/metric_params.hpp
@@ -35,17 +35,6 @@
return idProperty;
}
- MetricParams& metadata(std::string val)
- {
- metadataProperty = std::move(val);
- return *this;
- }
-
- const std::string& metadata() const
- {
- return metadataProperty;
- }
-
MetricParams& collectionTimeScope(CollectionTimeScope val)
{
collectionTimeScopeProperty = val;
@@ -93,7 +82,6 @@
private:
OperationType operationTypeProperty = {};
std::string idProperty = "MetricId";
- std::string metadataProperty = "MetricMetadata";
CollectionTimeScope collectionTimeScopeProperty = {};
CollectionDuration collectionDurationProperty =
CollectionDuration(Milliseconds(0u));
diff --git a/tests/src/params/report_params.hpp b/tests/src/params/report_params.hpp
index 44ae901..47e74e3 100644
--- a/tests/src/params/report_params.hpp
+++ b/tests/src/params/report_params.hpp
@@ -9,6 +9,17 @@
class ReportParams final
{
public:
+ ReportParams& reportId(std::string val)
+ {
+ reportIdProperty = std::move(val);
+ return *this;
+ }
+
+ const std::string& reportId() const
+ {
+ return reportIdProperty;
+ }
+
ReportParams& reportName(std::string val)
{
reportNameProperty = std::move(val);
@@ -98,6 +109,7 @@
}
private:
+ std::string reportIdProperty = "TestId";
std::string reportNameProperty = "TestReport";
ReportingType reportingTypeProperty = ReportingType::onRequest;
std::vector<ReportAction> reportActionsProperty;
@@ -107,18 +119,18 @@
std::vector<LabeledMetricParameters> metricParametersProperty{
{LabeledMetricParameters{
{LabeledSensorParameters{"Service",
- "/xyz/openbmc_project/sensors/power/p1"}},
+ "/xyz/openbmc_project/sensors/power/p1",
+ "metadata1"}},
OperationType::single,
"MetricId1",
- "Metadata1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))},
LabeledMetricParameters{
{LabeledSensorParameters{"Service",
- "/xyz/openbmc_project/sensors/power/p2"}},
+ "/xyz/openbmc_project/sensors/power/p2",
+ "metadata2"}},
OperationType::single,
"MetricId2",
- "Metadata2",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}};
bool enabledProperty = true;
diff --git a/tests/src/params/trigger_params.hpp b/tests/src/params/trigger_params.hpp
index 62acd7a..ea309e6 100644
--- a/tests/src/params/trigger_params.hpp
+++ b/tests/src/params/trigger_params.hpp
@@ -49,9 +49,9 @@
return labeledSensorsProperty;
}
- const std::vector<std::string>& reportNames() const
+ const std::vector<std::string>& reportIds() const
{
- return reportNamesProperty;
+ return reportIdsProperty;
}
TriggerParams& thresholdParams(LabeledTriggerThresholdParams val)
@@ -73,7 +73,7 @@
std::vector<LabeledSensorInfo> labeledSensorsProperty = {
{"service1", "/xyz/openbmc_project/sensors/temperature/BMC_Temp",
"metadata1"}};
- std::vector<std::string> reportNamesProperty = {"Report1"};
+ std::vector<std::string> reportIdsProperty = {"Report1"};
LabeledTriggerThresholdParams labeledThresholdsProperty =
std::vector<numeric::LabeledThresholdParam>{
numeric::LabeledThresholdParam{numeric::Type::lowerCritical,
diff --git a/tests/src/test_conversion.cpp b/tests/src/test_conversion.cpp
index a61ff1b..2b7249c 100644
--- a/tests/src/test_conversion.cpp
+++ b/tests/src/test_conversion.cpp
@@ -43,10 +43,10 @@
EXPECT_EQ(toEnum(2), Enum::two);
}
-TEST_F(TestConversion, passInvalidValueExpectToThrowOutOfRangeException)
+TEST_F(TestConversion, passInvalidValueExpectToThrowException)
{
- EXPECT_THROW(toEnum(-1), std::out_of_range);
- EXPECT_THROW(toEnum(3), std::out_of_range);
+ EXPECT_THROW(toEnum(-1), sdbusplus::exception::SdBusError);
+ EXPECT_THROW(toEnum(3), sdbusplus::exception::SdBusError);
}
TEST_F(TestConversion, convertsToUnderlyingType)
@@ -72,10 +72,11 @@
TEST_F(TestConversion, enumToStringThrowsWhenUknownEnumPassed)
{
- EXPECT_THROW(enumToString(static_cast<Enum>(77)), std::out_of_range);
+ EXPECT_THROW(enumToString(static_cast<Enum>(77)),
+ sdbusplus::exception::SdBusError);
}
TEST_F(TestConversion, toEnumThrowsWhenUknownStringPassed)
{
- EXPECT_THROW(toEnum("four"), std::out_of_range);
+ EXPECT_THROW(toEnum("four"), sdbusplus::exception::SdBusError);
}
diff --git a/tests/src/test_generate_id.cpp b/tests/src/test_generate_id.cpp
new file mode 100644
index 0000000..0bfabf5
--- /dev/null
+++ b/tests/src/test_generate_id.cpp
@@ -0,0 +1,67 @@
+#include "utils/generate_id.hpp"
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+
+class TestGenerateId : public Test
+{
+ public:
+ std::vector<std::string> conflicts;
+};
+
+TEST_F(TestGenerateId, returnPrefixWhenPrefixIsId)
+{
+ constexpr size_t maxSize = 32;
+ EXPECT_THAT(utils::generateId("prefix", "name", conflicts, maxSize),
+ Eq("prefix"));
+ EXPECT_THAT(utils::generateId("pre", "name123", conflicts, maxSize),
+ Eq("pre"));
+ EXPECT_THAT(utils::generateId("prefix/abc", "name", conflicts, maxSize),
+ Eq("prefix/abc"));
+ EXPECT_THAT(utils::generateId("prefix/abc", "name",
+ {"conflicts", "prefix/abc"}, maxSize),
+ Eq("prefix/abc"));
+}
+
+TEST_F(TestGenerateId, returnsNameWithPrefixWhenPrefixIsNamesapce)
+{
+ constexpr size_t maxSize = 32;
+ EXPECT_THAT(utils::generateId("prefix/", "name", conflicts, maxSize),
+ Eq("prefix/name"));
+ EXPECT_THAT(utils::generateId("pre/", "name", conflicts, maxSize),
+ Eq("pre/name"));
+}
+
+TEST_F(TestGenerateId, returnsOriginalNameWithoutInvalidCharacters)
+{
+ constexpr size_t maxSize = 32;
+ EXPECT_THAT(utils::generateId("", "n#a$m@e", conflicts, maxSize),
+ Eq("name"));
+ EXPECT_THAT(utils::generateId("/", "n!^aŹme", conflicts, maxSize),
+ Eq("/name"));
+ EXPECT_THAT(utils::generateId("p/", "n!^aŹm*(e", conflicts, maxSize),
+ Eq("p/name"));
+}
+
+TEST_F(TestGenerateId, returnsUniqueIdWhenConflictExist)
+{
+ constexpr size_t maxSize = 32;
+
+ EXPECT_THAT(utils::generateId("p/", "name",
+ {"conflicts", "p/name", "p/name1"}, maxSize),
+ Eq("p/name0"));
+ EXPECT_THAT(utils::generateId("p/", "name",
+ {"conflicts", "p/name", "p/name0", "p/name1"},
+ maxSize),
+ Eq("p/name2"));
+}
+
+TEST_F(TestGenerateId, returnsUniqueIdWithingMaxSize)
+{
+ constexpr size_t maxSize = 4;
+
+ EXPECT_THAT(utils::generateId("", "trigger", {""}, maxSize), Eq("trig"));
+ EXPECT_THAT(utils::generateId("", "trigger", {"trig"}, maxSize),
+ Eq("tri0"));
+}
diff --git a/tests/src/test_metric.cpp b/tests/src/test_metric.cpp
index a8c3d86..e19446c 100644
--- a/tests/src/test_metric.cpp
+++ b/tests/src/test_metric.cpp
@@ -25,7 +25,9 @@
std::vector<std::shared_ptr<SensorMock>> result;
for (size_t i = 0; i < amount; ++i)
{
- result.emplace_back(std::make_shared<NiceMock<SensorMock>>());
+ auto& metricMock =
+ result.emplace_back(std::make_shared<NiceMock<SensorMock>>());
+ ON_CALL(*metricMock, metadata()).WillByDefault(Return("metadata"));
}
return result;
}
@@ -35,13 +37,12 @@
return std::make_shared<Metric>(
utils::convContainer<std::shared_ptr<interfaces::Sensor>>(
sensorMocks),
- p.operationType(), p.id(), p.metadata(), p.collectionTimeScope(),
+ p.operationType(), p.id(), p.collectionTimeScope(),
p.collectionDuration(), std::move(clockFakePtr));
}
MetricParams params = MetricParams()
.id("id")
- .metadata("metadata")
.operationType(OperationType::avg)
.collectionTimeScope(CollectionTimeScope::point)
.collectionDuration(CollectionDuration(0ms));
@@ -83,51 +84,6 @@
ElementsAre(MetricValue({"id", "metadata", 0., 0u})));
}
-TEST_F(TestMetric, parsesSensorMetadata)
-{
- using ReadingMetadata =
- utils::LabeledTuple<std::tuple<std::string, std::string>,
- utils::tstring::SensorDbusPath,
- utils::tstring::SensorRedfishUri>;
-
- nlohmann::json metadata;
- metadata["MetricProperties"] = {"sensor1", "sensor2"};
-
- sensorMocks = makeSensorMocks(2);
- sut = makeSut(params.metadata(metadata.dump()));
-
- EXPECT_THAT(
- sut->getReadings(),
- ElementsAre(
- MetricValue{"id", ReadingMetadata("", "sensor1").dump(), 0., 0u},
- MetricValue{"id", ReadingMetadata("", "sensor2").dump(), 0., 0u}));
-}
-
-TEST_F(TestMetric, parsesSensorMetadataWhenMoreMetadataThanSensors)
-{
- nlohmann::json metadata;
- metadata["MetricProperties"] = {"sensor1", "sensor2"};
-
- sensorMocks = makeSensorMocks(1);
- sut = makeSut(params.metadata(metadata.dump()));
-
- EXPECT_THAT(sut->getReadings(),
- ElementsAre(MetricValue{"id", metadata.dump(), 0., 0u}));
-}
-
-TEST_F(TestMetric, parsesSensorMetadataWhenMoreSensorsThanMetadata)
-{
- nlohmann::json metadata;
- metadata["MetricProperties"] = {"sensor1"};
-
- sensorMocks = makeSensorMocks(2);
- sut = makeSut(params.metadata(metadata.dump()));
-
- EXPECT_THAT(sut->getReadings(),
- ElementsAre(MetricValue{"id", metadata.dump(), 0., 0u},
- MetricValue{"id", metadata.dump(), 0., 0u}));
-}
-
class TestMetricAfterInitialization : public TestMetric
{
public:
@@ -167,17 +123,18 @@
ON_CALL(*sensorMocks.front(), id())
.WillByDefault(Return(SensorMock::makeId("service1", "path1")));
+ ON_CALL(*sensorMocks.front(), metadata())
+ .WillByDefault(Return("metadata1"));
const auto conf = sut->dumpConfiguration();
LabeledMetricParameters expected = {};
expected.at_label<ts::Id>() = params.id();
- expected.at_label<ts::MetricMetadata>() = params.metadata();
expected.at_label<ts::OperationType>() = params.operationType();
expected.at_label<ts::CollectionTimeScope>() = params.collectionTimeScope();
expected.at_label<ts::CollectionDuration>() = params.collectionDuration();
expected.at_label<ts::SensorPath>() = {
- LabeledSensorParameters("service1", "path1")};
+ LabeledSensorParameters("service1", "path1", "metadata1")};
EXPECT_THAT(conf, Eq(expected));
}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 7a56dac..1beb15f 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -6,6 +6,7 @@
#include "params/report_params.hpp"
#include "report.hpp"
#include "report_manager.hpp"
+#include "utils/clock.hpp"
#include "utils/contains.hpp"
#include "utils/conv_container.hpp"
#include "utils/transform.hpp"
@@ -58,10 +59,10 @@
sut = makeReport(ReportParams());
}
- static interfaces::JsonStorage::FilePath to_file_path(std::string name)
+ static interfaces::JsonStorage::FilePath to_file_path(std::string id)
{
return interfaces::JsonStorage::FilePath(
- std::to_string(std::hash<std::string>{}(name)));
+ std::to_string(std::hash<std::string>{}(id)));
}
std::unique_ptr<Report> makeReport(const ReportParams& params)
@@ -70,9 +71,9 @@
return std::make_unique<Report>(
DbusEnvironment::getIoc(), DbusEnvironment::getObjServer(),
- params.reportName(), params.reportingType(), params.reportActions(),
- params.interval(), params.appendLimit(), params.reportUpdates(),
- *reportManagerMock, storageMock,
+ params.reportId(), params.reportName(), params.reportingType(),
+ params.reportActions(), params.interval(), params.appendLimit(),
+ params.reportUpdates(), *reportManagerMock, storageMock,
utils::convContainer<std::shared_ptr<interfaces::Metric>>(
metricMocks),
params.enabled());
@@ -118,6 +119,11 @@
}
};
+TEST_F(TestReport, returnsId)
+{
+ EXPECT_THAT(sut->getId(), Eq(defaultParams.reportId()));
+}
+
TEST_F(TestReport, verifyIfPropertiesHaveValidValue)
{
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Enabled"),
@@ -146,6 +152,8 @@
EXPECT_THAT(getProperty<ReadingParameters>(
sut->getPath(), "ReadingParametersFutureVersion"),
Eq(toReadingParameters(defaultParams.metricParameters())));
+ EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"),
+ Eq(defaultParams.reportName()));
}
TEST_F(TestReport, readingsAreInitialyEmpty)
@@ -204,7 +212,7 @@
TEST_F(TestReport, settingPersistencyToFalseRemovesReportFromStorage)
{
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
bool persistency = false;
EXPECT_THAT(setProperty(sut->getPath(), "Persistency", persistency).value(),
@@ -228,7 +236,7 @@
TEST_F(TestReport, deleteReportExpectThatFileIsRemoveFromStorage)
{
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
auto ec = deleteReport(sut->getPath());
EXPECT_THAT(ec, Eq(boost::system::errc::success));
}
@@ -248,6 +256,7 @@
_, TestReportStore,
Values(std::make_pair("Enabled"s, nlohmann::json(ReportParams().enabled())),
std::make_pair("Version"s, nlohmann::json(6)),
+ std::make_pair("Id"s, nlohmann::json(ReportParams().reportId())),
std::make_pair("Name"s, nlohmann::json(ReportParams().reportName())),
std::make_pair("ReportingType",
nlohmann::json(ReportParams().reportingType())),
@@ -265,20 +274,20 @@
{{{tstring::SensorPath::str(),
{{{tstring::Service::str(), "Service"},
{tstring::Path::str(),
- "/xyz/openbmc_project/sensors/power/p1"}}}},
+ "/xyz/openbmc_project/sensors/power/p1"},
+ {tstring::Metadata::str(), "metadata1"}}}},
{tstring::OperationType::str(), OperationType::single},
{tstring::Id::str(), "MetricId1"},
- {tstring::MetricMetadata::str(), "Metadata1"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}},
{{tstring::SensorPath::str(),
{{{tstring::Service::str(), "Service"},
{tstring::Path::str(),
- "/xyz/openbmc_project/sensors/power/p2"}}}},
+ "/xyz/openbmc_project/sensors/power/p2"},
+ {tstring::Metadata::str(), "metadata2"}}}},
{tstring::OperationType::str(), OperationType::single},
{tstring::Id::str(), "MetricId2"},
- {tstring::MetricMetadata::str(), "Metadata2"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}}}))));
@@ -289,9 +298,9 @@
{
InSequence seq;
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
EXPECT_CALL(checkPoint, Call());
- EXPECT_CALL(storageMock, store(to_file_path(sut->getName()), _))
+ EXPECT_CALL(storageMock, store(to_file_path(sut->getId()), _))
.WillOnce(SaveArg<1>(&storedConfiguration));
}
@@ -306,8 +315,7 @@
TEST_P(TestReportStore, reportIsSavedToStorageAfterCreated)
{
- EXPECT_CALL(storageMock,
- store(to_file_path(ReportParams().reportName()), _))
+ EXPECT_CALL(storageMock, store(to_file_path(ReportParams().reportId()), _))
.WillOnce(SaveArg<1>(&storedConfiguration));
sut = makeReport(ReportParams());
@@ -337,7 +345,7 @@
EXPECT_NO_THROW(makeReport(GetParam()));
}
-class TestReportInvalidNames :
+class TestReportInvalidIds :
public TestReport,
public WithParamInterface<ReportParams>
{
@@ -346,23 +354,18 @@
{}
};
-INSTANTIATE_TEST_SUITE_P(InvalidNames, TestReportInvalidNames,
- Values(ReportParams().reportName("/"),
- ReportParams().reportName("/Invalid"),
- ReportParams().reportName("Invalid/"),
- ReportParams().reportName("Invalid/Invalid/"),
- ReportParams().reportName("Invalid?")));
+INSTANTIATE_TEST_SUITE_P(InvalidNames, TestReportInvalidIds,
+ Values(ReportParams().reportId("/"),
+ ReportParams().reportId("/Invalid"),
+ ReportParams().reportId("Invalid/"),
+ ReportParams().reportId("Invalid/Invalid/"),
+ ReportParams().reportId("Invalid?")));
-TEST_P(TestReportInvalidNames, reportCtorThrowOnInvalidName)
-{
- EXPECT_THROW(makeReport(GetParam()), sdbusplus::exception::SdBusError);
-}
-
-TEST_F(TestReportInvalidNames, reportCtorThrowOnInvalidNameAndNoStoreIsCalled)
+TEST_P(TestReportInvalidIds, failsToCreateReportWithInvalidName)
{
EXPECT_CALL(storageMock, store).Times(0);
- EXPECT_THROW(makeReport(ReportParams().reportName("/Invalid")),
- sdbusplus::exception::SdBusError);
+
+ EXPECT_THROW(makeReport(GetParam()), sdbusplus::exception::SdBusError);
}
class TestReportAllReportTypes :
@@ -391,7 +394,7 @@
TEST_P(TestReportAllReportTypes, updateReadingsCallEnabledPropertyOff)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
setProperty(sut->getPath(), "Enabled", false);
sut->updateReadings();
@@ -404,7 +407,7 @@
TEST_P(TestReportAllReportTypes, updateReadingsCallEnabledPropertyOn)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
sut->updateReadings();
const auto [timestamp, readings] =
@@ -425,7 +428,7 @@
TEST_F(TestReportOnRequestType, updatesReadingTimestamp)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
ASSERT_THAT(update(sut->getPath()), Eq(boost::system::errc::success));
@@ -504,7 +507,7 @@
TEST_F(TestReportPeriodicReport, readingTimestampIsUpdatedAfterIntervalExpires)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
DbusEnvironment::sleepFor(ReportManager::minInterval + 1ms);
const auto [timestamp, readings] =
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 1bf886c..05a00ed 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -26,7 +26,7 @@
StorageMock& storageMock = *storageMockPtr;
std::unique_ptr<ReportMock> reportMockPtr =
- std::make_unique<NiceMock<ReportMock>>(reportParams.reportName());
+ std::make_unique<NiceMock<ReportMock>>(reportParams.reportId());
ReportMock& reportMock = *reportMockPtr;
std::unique_ptr<ReportManager> sut;
@@ -68,14 +68,16 @@
auto addReport(const ReportParams& params)
{
- return addReport(
- 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()));
+ 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()));
}
template <class T>
@@ -110,16 +112,42 @@
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
-TEST_F(TestReportManager, addReportWithMaxLengthName)
+TEST_F(TestReportManager, nameIsUsedToGenerateIdWhenIdIsEmptyInAddReport)
{
- std::string reportName(ReportManager::maxReportNameLength, 'z');
- reportParams.reportName(reportName);
+ reportParams.reportId("ReportName");
+ reportParams.reportName("ReportName");
+
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
+
+ auto [ec, path] = addReport(reportParams.reportId(""));
+
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq("/ReportName"));
+}
+
+TEST_F(TestReportManager, nameIsUsedToGenerateIdWhenIdIsNamespace)
+{
+ reportParams.reportId("Prefix/ReportName");
+ reportParams.reportName("ReportName");
+
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
+
+ auto [ec, path] = addReport(reportParams.reportId("Prefix/"));
+
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq("/Prefix/ReportName"));
+}
+
+TEST_F(TestReportManager, addReportWithMaxLengthId)
+{
+ std::string reportId(ReportManager::maxReportIdLength, 'z');
+ reportParams.reportId(reportId);
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 + reportName));
+ EXPECT_THAT(path, Eq("/"s + reportId));
}
TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongName)
@@ -127,8 +155,8 @@
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
- reportParams.reportName(
- std::string(ReportManager::maxReportNameLength + 1, 'z'));
+ reportParams.reportId(
+ std::string(ReportManager::maxReportIdLength + 1, 'z'));
auto [ec, path] = addReport(reportParams);
@@ -204,14 +232,14 @@
for (size_t i = 0; i < ReportManager::maxReports; i++)
{
- reportParams.reportName(reportParams.reportName() + std::to_string(i));
+ reportParams.reportId(reportParams.reportName() + std::to_string(i));
auto [ec, path] = addReport(reportParams);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
}
- reportParams.reportName(reportParams.reportName() +
- std::to_string(ReportManager::maxReports));
+ reportParams.reportId(reportParams.reportName() +
+ std::to_string(ReportManager::maxReports));
auto [ec, path] = addReport(reportParams);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::too_many_files_open));
@@ -270,7 +298,7 @@
EXPECT_CALL(reportMock, updateReadings());
addReport(reportParams);
- sut->updateReport(reportParams.reportName());
+ sut->updateReport(reportParams.reportId());
}
TEST_F(TestReportManager, updateReportDoNothingIfReportDoesNotExist)
@@ -302,10 +330,10 @@
reportParams.metricParameters(
std::vector<LabeledMetricParameters>{{LabeledMetricParameters{
{LabeledSensorParameters{"Service",
- "/xyz/openbmc_project/sensors/power/p1"}},
+ "/xyz/openbmc_project/sensors/power/p1",
+ "Metadata1"}},
operationType,
"MetricId1",
- "Metadata1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}});
@@ -315,7 +343,7 @@
auto [ec, path] = addReport(reportParams);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
- EXPECT_THAT(path, Eq("/"s + reportParams.reportName()));
+ EXPECT_THAT(path, Eq("/"s + reportParams.reportId()));
}
class TestReportManagerStorage : public TestReportManager
@@ -344,6 +372,7 @@
nlohmann::json data = nlohmann::json{
{"Enabled", reportParams.enabled()},
{"Version", Report::reportVersion},
+ {"Id", reportParams.reportId()},
{"Name", reportParams.reportName()},
{"ReportingType", utils::toUnderlying(reportParams.reportingType())},
{"ReportActions", reportParams.reportActions()},
diff --git a/tests/src/test_sensor.cpp b/tests/src/test_sensor.cpp
index 5547cf0..ffa73b1 100644
--- a/tests/src/test_sensor.cpp
+++ b/tests/src/test_sensor.cpp
@@ -4,6 +4,7 @@
#include "sensor.hpp"
#include "sensor_cache.hpp"
#include "stubs/dbus_sensor_object.hpp"
+#include "utils/clock.hpp"
#include <sdbusplus/asio/property.hpp>
@@ -51,9 +52,9 @@
std::unique_ptr<stubs::DbusSensorObject> sensorObject = makeSensorObject();
SensorCache sensorCache;
- uint64_t timestamp = std::time(0);
+ uint64_t timestamp = Clock().timestamp();
std::shared_ptr<Sensor> sut = sensorCache.makeSensor<Sensor>(
- DbusEnvironment::serviceName(), sensorObject->path(),
+ DbusEnvironment::serviceName(), sensorObject->path(), "metadata",
DbusEnvironment::getIoc(), DbusEnvironment::getBus());
std::shared_ptr<SensorListenerMock> listenerMock =
std::make_shared<StrictMock<SensorListenerMock>>();
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index a074e7a..66e9bf0 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -57,7 +57,7 @@
return std::make_unique<Trigger>(
DbusEnvironment::getIoc(), DbusEnvironment::getObjServer(),
params.id(), params.name(), params.triggerActions(),
- params.reportNames(), params.sensors(), params.thresholdParams(),
+ params.reportIds(), params.sensors(), params.thresholdParams(),
std::vector<std::shared_ptr<interfaces::Threshold>>{},
*triggerManagerMockPtr, storageMock);
}
@@ -109,7 +109,7 @@
Eq(utils::fromLabeledSensorsInfo(triggerParams.sensors())));
EXPECT_THAT(
getProperty<std::vector<std::string>>(sut->getPath(), "ReportNames"),
- Eq(triggerParams.reportNames()));
+ Eq(triggerParams.reportIds()));
EXPECT_THAT(
getProperty<TriggerThresholdParams>(sut->getPath(), "Thresholds"),
Eq(std::visit(utils::FromLabeledThresholdParamConversion(),
@@ -268,10 +268,10 @@
Eq(triggerParams.triggerActions()));
}
-TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerReportNames)
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerReportIds)
{
- ASSERT_THAT(storedConfiguration.at("ReportNames"),
- Eq(triggerParams.reportNames()));
+ ASSERT_THAT(storedConfiguration.at("ReportIds"),
+ Eq(triggerParams.reportIds()));
}
TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerSensors)
@@ -280,7 +280,7 @@
expectedItem["service"] = "service1";
expectedItem["sensorPath"] =
"/xyz/openbmc_project/sensors/temperature/BMC_Temp";
- expectedItem["sensorMetadata"] = "metadata1";
+ expectedItem["metadata"] = "metadata1";
ASSERT_THAT(storedConfiguration.at("Sensors"), ElementsAre(expectedItem));
}
diff --git a/tests/src/test_trigger_manager.cpp b/tests/src/test_trigger_manager.cpp
index c813e97..182c58b 100644
--- a/tests/src/test_trigger_manager.cpp
+++ b/tests/src/test_trigger_manager.cpp
@@ -30,7 +30,7 @@
DbusEnvironment::serviceName(), TriggerManager::triggerManagerPath,
TriggerManager::triggerManagerIfaceName, "AddTrigger", params.id(),
params.name(), params.triggerActions(), sensorInfos,
- params.reportNames(),
+ params.reportIds(),
std::visit(utils::FromLabeledThresholdParamConversion(),
params.thresholdParams()));
return DbusEnvironment::waitForFuture(addTriggerPromise.get_future());
@@ -294,7 +294,7 @@
{"TriggerActions", TriggerParams().triggerActions()},
{"ThresholdParams", utils::labeledThresholdParamsToJson(
TriggerParams().thresholdParams())},
- {"ReportNames", TriggerParams().reportNames()},
+ {"ReportIds", TriggerParams().reportIds()},
{"Sensors", TriggerParams().sensors()}};
nlohmann::json data2 = data1;