Add unit test for item_udpater
To test item_updater easier, mocking utils is necessary.
So add a UtilsInterface to make the mocking eaiser.
Due to the fact that there is templated member functions that could not
be virtual, adding an extra "Impl" virtual function makes it possible to
simulates the mock of templated functions.
See
https://stackoverflow.com/questions/7968023/c-virtual-template-method
for details.
However, using std::any in with googlemock has an issue on GCC9, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415
for details.
So this commit uses std::experimental::any as a workaround.
With the mocked utils, it's easy to test item_updater in different
cases.
This commit adds the test cases for creating PSU objects, when:
* There is no PSU present
* There is one PSU present
* There are two PSUs present with the same version;
* There are two PSUs present with different versions.
Tested: Verify the unit tests passes.
Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: I5d74ab26b344c5c40bc141f97b8aca42e74ee88e
diff --git a/README b/README
index 8f3921a..78308f4 100644
--- a/README
+++ b/README
@@ -20,7 +20,8 @@
```
meson -Doe-sdk=enabled -Dtests=enabled build/
ninja -C build/ test # Meson skips running the case due to it thinks it's cross compiling
- ./build/test/utest # Manually run the test
+ # Manually run the tests
+ for t in `find build/test/ -maxdepth 1 -name "test_*"`; do ./$t || break ; done
```
[1]: https://github.com/openbmc/docs/blob/master/local-ci-build.md
diff --git a/meson.build b/meson.build
index a630564..c56d09d 100644
--- a/meson.build
+++ b/meson.build
@@ -28,6 +28,8 @@
cdata.set_quoted('ACTIVE_REV_ASSOCIATION', 'software_version')
cdata.set_quoted('FUNCTIONAL_FWD_ASSOCIATION', 'functional')
cdata.set_quoted('FUNCTIONAL_REV_ASSOCIATION', 'software_version')
+cdata.set_quoted('VERSION', 'Version')
+cdata.set_quoted('PRESENT', 'Present')
cdata.set_quoted('SOFTWARE_OBJPATH', get_option('SOFTWARE_OBJPATH'))
cdata.set_quoted('MANIFEST_FILE', get_option('MANIFEST_FILE'))
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index 1f1af9d..6c55062 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -55,7 +55,7 @@
purpose = value;
}
}
- else if (propertyName == "Version")
+ else if (propertyName == VERSION)
{
version = variant_ns::get<std::string>(propertyValue);
}
@@ -304,13 +304,13 @@
// The code was expecting to get callback on mutliple properties changed.
// But in practice, the callback is received one-by-one for each property.
// So it has to handle Present and Version property separately.
- auto p = properties.find("Present");
+ auto p = properties.find(PRESENT);
if (p != properties.end())
{
present = sdbusplus::message::variant_ns::get<bool>(p->second);
psuStatusMap[psuPath].present = *present;
}
- p = properties.find("Version");
+ p = properties.find(VERSION);
if (p != properties.end())
{
version = sdbusplus::message::variant_ns::get<std::string>(p->second);
@@ -341,7 +341,7 @@
{
// If a PSU is plugged out, version property is update to empty as
// well, and we get callback here, but ignore that because it is
- // handled by "Present" callback.
+ // handled by PRESENT callback.
return;
}
// Remove object or association
@@ -357,9 +357,9 @@
// Assume the same service implement both Version and Item interface
auto service = utils::getService(bus, p.c_str(), VERSION_IFACE);
auto version = utils::getProperty<std::string>(
- bus, service.c_str(), p.c_str(), VERSION_IFACE, "Version");
+ bus, service.c_str(), p.c_str(), VERSION_IFACE, VERSION);
auto present = utils::getProperty<bool>(bus, service.c_str(), p.c_str(),
- ITEM_IFACE, "Present");
+ ITEM_IFACE, PRESENT);
if (present && !version.empty())
{
createPsuObject(p, version);
diff --git a/src/item_updater.hpp b/src/item_updater.hpp
index 7bd9f66..eeacdf8 100644
--- a/src/item_updater.hpp
+++ b/src/item_updater.hpp
@@ -12,6 +12,8 @@
#include <xyz/openbmc_project/Association/Definitions/server.hpp>
#include <xyz/openbmc_project/Collection/DeleteAll/server.hpp>
+class TestItemUpdater;
+
namespace phosphor
{
namespace software
@@ -31,6 +33,8 @@
*/
class ItemUpdater : public ItemUpdaterInherit
{
+ friend class ::TestItemUpdater;
+
public:
/** @brief Constructs ItemUpdater
*
diff --git a/src/utils.cpp b/src/utils.cpp
index f80776e..5e92f4a 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -5,6 +5,9 @@
#include <openssl/sha.h>
#include <fstream>
+#include <phosphor-logging/log.hpp>
+
+using namespace phosphor::logging;
namespace utils
{
@@ -16,7 +19,14 @@
constexpr auto MAPPER_INTERFACE = "xyz.openbmc_project.ObjectMapper";
} // namespace
-std::vector<std::string> getPSUInventoryPath(sdbusplus::bus::bus& bus)
+const UtilsInterface& getUtils()
+{
+ static Utils utils;
+ return utils;
+}
+
+std::vector<std::string>
+ Utils::getPSUInventoryPath(sdbusplus::bus::bus& bus) const
{
std::vector<std::string> paths;
auto method = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
@@ -30,8 +40,8 @@
return paths;
}
-std::string getService(sdbusplus::bus::bus& bus, const char* path,
- const char* interface)
+std::string Utils::getService(sdbusplus::bus::bus& bus, const char* path,
+ const char* interface) const
{
auto mapper = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
MAPPER_INTERFACE, "GetObject");
@@ -64,7 +74,7 @@
}
}
-std::string getVersionId(const std::string& version)
+std::string Utils::getVersionId(const std::string& version) const
{
if (version.empty())
{
@@ -88,4 +98,27 @@
return (hexId.substr(0, 8));
}
+any Utils::getPropertyImpl(sdbusplus::bus::bus& bus, const char* service,
+ const char* path, const char* interface,
+ const char* propertyName) const
+{
+ auto method = bus.new_method_call(service, path,
+ "org.freedesktop.DBus.Properties", "Get");
+ method.append(interface, propertyName);
+ try
+ {
+ PropertyType value{};
+ auto reply = bus.call(method);
+ reply.read(value);
+ return any(value);
+ }
+ catch (const sdbusplus::exception::SdBusError& ex)
+ {
+ log<level::ERR>("GetProperty call failed", entry("PATH=%s", path),
+ entry("INTERFACE=%s", interface),
+ entry("PROPERTY=%s", propertyName));
+ throw std::runtime_error("GetProperty call failed");
+ }
+}
+
} // namespace utils
diff --git a/src/utils.hpp b/src/utils.hpp
index fd111ff..dd1317f 100644
--- a/src/utils.hpp
+++ b/src/utils.hpp
@@ -1,6 +1,6 @@
#pragma once
-#include <phosphor-logging/log.hpp>
+#include <experimental/any>
#include <sdbusplus/bus.hpp>
#include <string>
#include <vector>
@@ -8,7 +8,18 @@
namespace utils
{
-using namespace phosphor::logging;
+class UtilsInterface;
+
+// Due to a libstdc++ bug, we got compile error using std::any with gmock.
+// A temporary workaround is to use std::experimental::any.
+// See details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415
+using std::experimental::any;
+using std::experimental::any_cast;
+
+/**
+ * @brief Get the implementation of UtilsInterface
+ */
+const UtilsInterface& getUtils();
/**
* @brief Get PSU inventory object path from DBus
@@ -38,26 +49,7 @@
*/
template <typename T>
T getProperty(sdbusplus::bus::bus& bus, const char* service, const char* path,
- const char* interface, const char* propertyName)
-{
- auto method = bus.new_method_call(service, path,
- "org.freedesktop.DBus.Properties", "Get");
- method.append(interface, propertyName);
- try
- {
- sdbusplus::message::variant<T> value{};
- auto reply = bus.call(method);
- reply.read(value);
- return sdbusplus::message::variant_ns::get<T>(value);
- }
- catch (const sdbusplus::exception::SdBusError& ex)
- {
- log<level::ERR>("GetProperty call failed", entry("PATH=%s", path),
- entry("INTERFACE=%s", interface),
- entry("PROPERTY=%s", propertyName));
- throw std::runtime_error("GetProperty call failed");
- }
-}
+ const char* interface, const char* propertyName);
/**
* @brief Calculate the version id from the version string.
@@ -71,4 +63,79 @@
*/
std::string getVersionId(const std::string& version);
+/**
+ * @brief The interface for utils
+ */
+class UtilsInterface
+{
+ public:
+ // For now the code needs to get property for Present and Version
+ using PropertyType = sdbusplus::message::variant<std::string, bool>;
+
+ virtual ~UtilsInterface() = default;
+
+ virtual std::vector<std::string>
+ getPSUInventoryPath(sdbusplus::bus::bus& bus) const = 0;
+
+ virtual std::string getService(sdbusplus::bus::bus& bus, const char* path,
+ const char* interface) const = 0;
+
+ virtual std::string getVersionId(const std::string& version) const = 0;
+
+ virtual any getPropertyImpl(sdbusplus::bus::bus& bus, const char* service,
+ const char* path, const char* interface,
+ const char* propertyName) const = 0;
+
+ template <typename T>
+ T getProperty(sdbusplus::bus::bus& bus, const char* service,
+ const char* path, const char* interface,
+ const char* propertyName) const
+ {
+ any result =
+ getPropertyImpl(bus, service, path, interface, propertyName);
+ auto value = any_cast<PropertyType>(result);
+ return sdbusplus::message::variant_ns::get<T>(value);
+ }
+};
+
+class Utils : public UtilsInterface
+{
+ public:
+ std::vector<std::string>
+ getPSUInventoryPath(sdbusplus::bus::bus& bus) const override;
+
+ std::string getService(sdbusplus::bus::bus& bus, const char* path,
+ const char* interface) const override;
+
+ std::string getVersionId(const std::string& version) const override;
+
+ any getPropertyImpl(sdbusplus::bus::bus& bus, const char* service,
+ const char* path, const char* interface,
+ const char* propertyName) const override;
+};
+
+inline std::string getService(sdbusplus::bus::bus& bus, const char* path,
+ const char* interface)
+{
+ return getUtils().getService(bus, path, interface);
+}
+
+inline std::vector<std::string> getPSUInventoryPath(sdbusplus::bus::bus& bus)
+{
+ return getUtils().getPSUInventoryPath(bus);
+}
+
+inline std::string getVersionId(const std::string& version)
+{
+ return getUtils().getVersionId(version);
+}
+
+template <typename T>
+T getProperty(sdbusplus::bus::bus& bus, const char* service, const char* path,
+ const char* interface, const char* propertyName)
+{
+ return getUtils().getProperty<T>(bus, service, path, interface,
+ propertyName);
+}
+
} // namespace utils
diff --git a/src/version.hpp b/src/version.hpp
index 102d26a..a67fc82 100644
--- a/src/version.hpp
+++ b/src/version.hpp
@@ -129,9 +129,6 @@
getValue(const std::string& filePath,
std::map<std::string, std::string> keys);
- /** @brief Persistent Delete D-Bus object */
- std::unique_ptr<Delete> deleteObject;
-
/** @brief The temUpdater's erase callback. */
eraseFunc eraseCallback;
@@ -147,6 +144,9 @@
/** @brief This Version's version string */
const std::string versionStr;
+
+ /** @brief Persistent Delete D-Bus object */
+ std::unique_ptr<Delete> deleteObject;
};
} // namespace updater
diff --git a/test/meson.build b/test/meson.build
index 6d82b5d..4ec6c0e 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -21,8 +21,8 @@
)
test_inc = include_directories('.')
-utest = executable(
- 'utest',
+test_util = executable(
+ 'test_util',
'../src/utils.cpp',
'test_utils.cpp',
include_directories: [psu_inc, test_inc],
@@ -37,4 +37,23 @@
ssl,
])
-test('all', utest)
+test_item_updater = executable(
+ 'test_item_updater',
+ '../src/activation.cpp',
+ '../src/item_updater.cpp',
+ '../src/version.cpp',
+ 'test_item_updater.cpp',
+ include_directories: [psu_inc, test_inc],
+ link_args: dynamic_linker,
+ build_rpath: oe_sdk.enabled() ? rpath : '',
+ dependencies: [
+ gtest,
+ gmock,
+ phosphor_logging,
+ phosphor_dbus_interfaces,
+ sdbusplus,
+ ssl,
+ ])
+
+test('util', test_util)
+test('item_updater', test_item_updater)
diff --git a/test/mocked_utils.hpp b/test/mocked_utils.hpp
new file mode 100644
index 0000000..e8a8f34
--- /dev/null
+++ b/test/mocked_utils.hpp
@@ -0,0 +1,34 @@
+#include "utils.hpp"
+
+#include <gmock/gmock.h>
+
+namespace utils
+{
+
+class MockedUtils : public UtilsInterface
+{
+ public:
+ virtual ~MockedUtils() = default;
+
+ MOCK_CONST_METHOD1(getPSUInventoryPath,
+ std::vector<std::string>(sdbusplus::bus::bus& bus));
+
+ MOCK_CONST_METHOD3(getService,
+ std::string(sdbusplus::bus::bus& bus, const char* path,
+ const char* interface));
+
+ MOCK_CONST_METHOD1(getVersionId, std::string(const std::string& version));
+
+ MOCK_CONST_METHOD5(getPropertyImpl,
+ any(sdbusplus::bus::bus& bus, const char* service,
+ const char* path, const char* interface,
+ const char* propertyName));
+};
+
+const UtilsInterface& getUtils()
+{
+ static MockedUtils utils;
+ return utils;
+}
+
+} // namespace utils
diff --git a/test/test_item_updater.cpp b/test/test_item_updater.cpp
new file mode 100644
index 0000000..2b9af43
--- /dev/null
+++ b/test/test_item_updater.cpp
@@ -0,0 +1,208 @@
+#include "item_updater.hpp"
+#include "mocked_utils.hpp"
+
+#include <sdbusplus/test/sdbus_mock.hpp>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+using namespace phosphor::software::updater;
+using ::testing::_;
+using ::testing::Return;
+using ::testing::ReturnArg;
+using ::testing::StrEq;
+
+using std::experimental::any;
+using PropertyType = utils::UtilsInterface::PropertyType;
+
+class TestItemUpdater : public ::testing::Test
+{
+ public:
+ TestItemUpdater() :
+ mockedUtils(
+ reinterpret_cast<const utils::MockedUtils&>(utils::getUtils()))
+ {
+ ON_CALL(mockedUtils, getVersionId(_)).WillByDefault(ReturnArg<0>());
+ }
+
+ ~TestItemUpdater()
+ {
+ }
+
+ const auto& GetActivations()
+ {
+ return itemUpdater->activations;
+ }
+
+ std::string getObjPath(const std::string& versionId)
+ {
+ return std::string(dBusPath) + "/" + versionId;
+ }
+
+ static constexpr auto dBusPath = SOFTWARE_OBJPATH;
+ sdbusplus::SdBusMock sdbusMock;
+ sdbusplus::bus::bus mockedBus = sdbusplus::get_mocked_new(&sdbusMock);
+ const utils::MockedUtils& mockedUtils;
+ std::unique_ptr<ItemUpdater> itemUpdater;
+};
+
+TEST_F(TestItemUpdater, ctordtor)
+{
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+}
+
+TEST_F(TestItemUpdater, NotCreateObjectOnNotPresent)
+{
+ constexpr auto psuPath = "/com/example/inventory/psu0";
+ constexpr auto service = "com.example.Software.Psu";
+ constexpr auto version = "version0";
+ std::string objPath = getObjPath(version);
+ EXPECT_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillOnce(Return(std::vector<std::string>({psuPath})));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(VERSION)))
+ .WillOnce(Return(any(PropertyType(std::string(version)))));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(false)))); // not present
+
+ // The item updater itself
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
+ .Times(1);
+
+ // No activation/version objects are created
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
+ .Times(0);
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+}
+
+TEST_F(TestItemUpdater, CreateOnePSUOnPresent)
+{
+ constexpr auto psuPath = "/com/example/inventory/psu0";
+ constexpr auto service = "com.example.Software.Psu";
+ constexpr auto version = "version0";
+ std::string objPath = getObjPath(version);
+ EXPECT_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillOnce(Return(std::vector<std::string>({psuPath})));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(VERSION)))
+ .WillOnce(Return(any(PropertyType(std::string(version)))));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+
+ // The item updater itself
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
+ .Times(1);
+
+ // activation and version object will be added
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
+ .Times(2);
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+}
+
+TEST_F(TestItemUpdater, CreateTwoPSUsWithSameVersion)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ constexpr auto psu1 = "/com/example/inventory/psu1";
+ constexpr auto service = "com.example.Software.Psu";
+ auto version0 = std::string("version0");
+ auto version1 = std::string("version0");
+ auto objPath0 = getObjPath(version0);
+ auto objPath1 = getObjPath(version1);
+
+ EXPECT_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillOnce(Return(std::vector<std::string>({psu0, psu1})));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu0), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu1), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(VERSION)))
+ .WillOnce(Return(any(PropertyType(version0))));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(VERSION)))
+ .WillOnce(Return(any(PropertyType(version1))));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+
+ // The item updater itself
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
+ .Times(1);
+
+ // activation and version object will be added
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath0)))
+ .Times(2);
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+
+ // Verify there is only one activation and it has two associations
+ const auto& activations = GetActivations();
+ EXPECT_EQ(1u, activations.size());
+ const auto& activation = activations.find(version0)->second;
+ const auto& assocs = activation->associations();
+ EXPECT_EQ(2u, assocs.size());
+ EXPECT_EQ(psu0, std::get<2>(assocs[0]));
+ EXPECT_EQ(psu1, std::get<2>(assocs[1]));
+}
+
+TEST_F(TestItemUpdater, CreateTwoPSUsWithDifferentVersion)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ constexpr auto psu1 = "/com/example/inventory/psu1";
+ constexpr auto service = "com.example.Software.Psu";
+ auto version0 = std::string("version0");
+ auto version1 = std::string("version1");
+ auto objPath0 = getObjPath(version0);
+ auto objPath1 = getObjPath(version1);
+
+ EXPECT_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillOnce(Return(std::vector<std::string>({psu0, psu1})));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu0), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu1), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(VERSION)))
+ .WillOnce(Return(any(PropertyType(version0))));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(VERSION)))
+ .WillOnce(Return(any(PropertyType(version1))));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+
+ // The item updater itself
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
+ .Times(1);
+
+ // two new activation and version objects will be added
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath0)))
+ .Times(2);
+ EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath1)))
+ .Times(2);
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+
+ // Verify there are two activations and each with one association
+ const auto& activations = GetActivations();
+ EXPECT_EQ(2u, activations.size());
+ const auto& activation0 = activations.find(version0)->second;
+ const auto& assocs0 = activation0->associations();
+ EXPECT_EQ(1u, assocs0.size());
+ EXPECT_EQ(psu0, std::get<2>(assocs0[0]));
+
+ const auto& activation1 = activations.find(version1)->second;
+ const auto& assocs1 = activation1->associations();
+ EXPECT_EQ(1u, assocs1.size());
+ EXPECT_EQ(psu1, std::get<2>(assocs1[0]));
+}