Use gmock on property_change_listener
1. Mock property_change_listener;
2. Make unit test link against gmock
3. Update TestManager to use mocked property change listener.
4. Fix an issue found by the updated test case, that when the properties
are changed during host is on, and changed back to the same as before,
when host becomes off, the properties shall remain the same and listeners
shall not be notified.
Change-Id: I815b59cb23edfcac910c2a8c26ea5a71e872d92f
Signed-off-by: Lei YU <mine260309@gmail.com>
diff --git a/manager.cpp b/manager.cpp
index 56e0d0b..bcc581c 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -134,20 +134,12 @@
if (key == PROPERTY_TIME_MODE)
{
setCurrentTimeMode(value);
- for (const auto listener : listeners)
- {
- listener->onModeChanged(timeMode);
- }
- // When time_mode is updated, update the NTP setting
- updateNtpSetting(value);
+ onTimeModeChanged(value);
}
else if (key == PROPERTY_TIME_OWNER)
{
setCurrentTimeOwner(value);
- for (const auto listener : listeners)
- {
- listener->onOwnerChanged(timeOwner);
- }
+ onTimeOwnerChanged();
}
}
}
@@ -259,20 +251,17 @@
}
if (!requestedMode.empty())
{
- setCurrentTimeMode(requestedMode);
- for (const auto& listener : listeners)
+ if (setCurrentTimeMode(requestedMode))
{
- listener->onModeChanged(timeMode);
+ onTimeModeChanged(requestedMode);
}
- updateNtpSetting(requestedMode);
setRequestedMode({}); // Clear requested mode
}
if (!requestedOwner.empty())
{
- setCurrentTimeOwner(requestedOwner);
- for (const auto& listener : listeners)
+ if (setCurrentTimeOwner(requestedOwner))
{
- listener->onOwnerChanged(timeOwner);
+ onTimeOwnerChanged();
}
setRequestedOwner({}); // Clear requested owner
}
@@ -300,20 +289,56 @@
return 0;
}
-void Manager::setCurrentTimeMode(const std::string& mode)
+bool Manager::setCurrentTimeMode(const std::string& mode)
{
- log<level::INFO>("Time mode is changed",
- entry("MODE=%s", mode.c_str()));
- timeMode = convertToMode(mode);
- utils::writeData(modeFile, mode);
+ auto newMode = convertToMode(mode);
+ if (newMode != timeMode)
+ {
+ log<level::INFO>("Time mode is changed",
+ entry("MODE=%s", mode.c_str()));
+ timeMode = newMode;
+ utils::writeData(modeFile, mode);
+ return true;
+ }
+ else
+ {
+ return false;
+ }
}
-void Manager::setCurrentTimeOwner(const std::string& owner)
+bool Manager::setCurrentTimeOwner(const std::string& owner)
{
- log<level::INFO>("Time owner is changed",
- entry("OWNER=%s", owner.c_str()));
- timeOwner = convertToOwner(owner);
- utils::writeData(ownerFile, owner);
+ auto newOwner = convertToOwner(owner);
+ if (newOwner != timeOwner)
+ {
+ log<level::INFO>("Time owner is changed",
+ entry("OWNER=%s", owner.c_str()));
+ timeOwner = newOwner;
+ utils::writeData(ownerFile, owner);
+ return true;
+ }
+ else
+ {
+ return false;
+ }
+}
+
+void Manager::onTimeModeChanged(const std::string& mode)
+{
+ for (const auto listener : listeners)
+ {
+ listener->onModeChanged(timeMode);
+ }
+ // When time_mode is updated, update the NTP setting
+ updateNtpSetting(mode);
+}
+
+void Manager::onTimeOwnerChanged()
+{
+ for (const auto& listener : listeners)
+ {
+ listener->onOwnerChanged(timeOwner);
+ }
}
std::string Manager::getSettings(const char* value) const
diff --git a/manager.hpp b/manager.hpp
index 7c00122..172105b 100644
--- a/manager.hpp
+++ b/manager.hpp
@@ -81,17 +81,37 @@
*/
std::string getSettings(const char* setting) const;
- /** @brief Set current time mode
+ /** @brief Set current time mode from the time mode string
+ *
+ * @param[in] mode - The string of time mode
+ *
+ * @return - true if the mode is updated
+ * false if it's the same as before
+ */
+ bool setCurrentTimeMode(const std::string& mode);
+
+ /** @brief Set current time owner from the time owner string
+ *
+ * @param[in] owner - The string of time owner
+ *
+ * @return - true if the owner is updated
+ * false if it's the same as before
+ */
+ bool setCurrentTimeOwner(const std::string& owner);
+
+ /** @brief Called on time mode is changed
+ *
+ * Notify listeners that time mode is changed and update ntp setting
*
* @param[in] mode - The string of time mode
*/
- void setCurrentTimeMode(const std::string& mode);
+ void onTimeModeChanged(const std::string& mode);
- /** @brief Set current time owner
+ /** @brief Called on time owner is changed
*
- * @param[in] owner - The string of time owner
+ * Notify listeners that time owner is changed
*/
- void setCurrentTimeOwner(const std::string& owner);
+ void onTimeOwnerChanged();
/** @brief Notified on settings property changed
*
diff --git a/test/TestManager.cpp b/test/TestManager.cpp
index 1e4096e..bbaca96 100644
--- a/test/TestManager.cpp
+++ b/test/TestManager.cpp
@@ -3,6 +3,9 @@
#include "types.hpp"
#include "manager.hpp"
+#include "mocked_property_change_listener.hpp"
+
+using ::testing::_;
namespace phosphor
{
@@ -14,12 +17,17 @@
public:
sdbusplus::bus::bus bus;
Manager manager;
+ MockPropertyChangeListner listener1;
+ MockPropertyChangeListner listener2;
TestManager()
: bus(sdbusplus::bus::new_default()),
manager(bus)
{
- // Empty
+ // Add two mocked listeners so that we can test
+ // the behavior related to listeners
+ manager.addListener(&listener1);
+ manager.addListener(&listener2);
}
// Proxies for Manager's private members and functions
@@ -104,28 +112,48 @@
EXPECT_FALSE(hostOn());
}
-TEST_F(TestManager, propertyChange)
+TEST_F(TestManager, propertyChanged)
{
// When host is off, property change will be notified to listners
EXPECT_FALSE(hostOn());
+
+ // Check mocked listeners shall receive notifications on property changed
+ EXPECT_CALL(listener1, onModeChanged(Mode::MANUAL)).Times(1);
+ EXPECT_CALL(listener1, onOwnerChanged(Owner::HOST)).Times(1);
+ EXPECT_CALL(listener2, onModeChanged(Mode::MANUAL)).Times(1);
+ EXPECT_CALL(listener2, onOwnerChanged(Owner::HOST)).Times(1);
+
notifyPropertyChanged("time_mode", "MANUAL");
notifyPropertyChanged("time_owner", "HOST");
+
EXPECT_EQ("", getRequestedMode());
EXPECT_EQ("", getRequestedOwner());
- // TODO: if gmock is ready, check mocked listners shall receive notifies
- notifyPgoodChanged(true);
// When host is on, property changes are saved as requested ones
- notifyPropertyChanged("time_mode", "MANUAL");
- notifyPropertyChanged("time_owner", "HOST");
- EXPECT_EQ("MANUAL", getRequestedMode());
- EXPECT_EQ("HOST", getRequestedOwner());
+ notifyPgoodChanged(true);
+
+ // Check mocked listeners shall not receive notifications
+ EXPECT_CALL(listener1, onModeChanged(Mode::MANUAL)).Times(0);
+ EXPECT_CALL(listener1, onOwnerChanged(Owner::HOST)).Times(0);
+ EXPECT_CALL(listener2, onModeChanged(Mode::MANUAL)).Times(0);
+ EXPECT_CALL(listener2, onOwnerChanged(Owner::HOST)).Times(0);
+
+ notifyPropertyChanged("time_mode", "NTP");
+ notifyPropertyChanged("time_owner", "SPLIT");
+
+ EXPECT_EQ("NTP", getRequestedMode());
+ EXPECT_EQ("SPLIT", getRequestedOwner());
// When host becomes off, the requested mode/owner shall be notified
// to listners, and be cleared
+ EXPECT_CALL(listener1, onModeChanged(Mode::NTP)).Times(1);
+ EXPECT_CALL(listener1, onOwnerChanged(Owner::SPLIT)).Times(1);
+ EXPECT_CALL(listener2, onModeChanged(Mode::NTP)).Times(1);
+ EXPECT_CALL(listener2, onOwnerChanged(Owner::SPLIT)).Times(1);
+
notifyPgoodChanged(false);
- // TODO: if gmock is ready, check mocked listners shall receive notifies
+
EXPECT_EQ("", getRequestedMode());
EXPECT_EQ("", getRequestedOwner());
@@ -135,6 +163,50 @@
ASSERT_DEATH(notifyPropertyChanged("invalid property", "whatever"), "");
}
+TEST_F(TestManager, propertyChangedAndChangedbackWhenHostOn)
+{
+ // Property is now MANUAL/HOST
+ notifyPropertyChanged("time_mode", "MANUAL");
+ notifyPropertyChanged("time_owner", "HOST");
+
+ // Set host on
+ notifyPgoodChanged(true);
+
+ // Check mocked listeners shall not receive notifications
+ EXPECT_CALL(listener1, onModeChanged(_)).Times(0);
+ EXPECT_CALL(listener1, onOwnerChanged(_)).Times(0);
+ EXPECT_CALL(listener2, onModeChanged(_)).Times(0);
+ EXPECT_CALL(listener2, onOwnerChanged(_)).Times(0);
+
+ notifyPropertyChanged("time_mode", "NTP");
+ notifyPropertyChanged("time_owner", "SPLIT");
+
+ // Saved as requested mode/owner
+ EXPECT_EQ("NTP", getRequestedMode());
+ EXPECT_EQ("SPLIT", getRequestedOwner());
+
+ // Property changed back to MANUAL/HOST
+ notifyPropertyChanged("time_mode", "MANUAL");
+ notifyPropertyChanged("time_owner", "HOST");
+
+ // Requested mode/owner shall be updated
+ EXPECT_EQ("MANUAL", getRequestedMode());
+ EXPECT_EQ("HOST", getRequestedOwner());
+
+ // Because the latest mode/owner is the same as when host is off,
+ // The listeners shall not be notified, and requested mode/owner
+ // shall be cleared
+ EXPECT_CALL(listener1, onModeChanged(_)).Times(0);
+ EXPECT_CALL(listener1, onOwnerChanged(_)).Times(0);
+ EXPECT_CALL(listener2, onModeChanged(_)).Times(0);
+ EXPECT_CALL(listener2, onOwnerChanged(_)).Times(0);
+
+ notifyPgoodChanged(false);
+
+ EXPECT_EQ("", getRequestedMode());
+ EXPECT_EQ("", getRequestedOwner());
+}
+
// TODO: if gmock is ready, add case to test
// updateNtpSetting() and updateNetworkSetting()
diff --git a/test/mocked_property_change_listener.hpp b/test/mocked_property_change_listener.hpp
new file mode 100644
index 0000000..d581dd3
--- /dev/null
+++ b/test/mocked_property_change_listener.hpp
@@ -0,0 +1,17 @@
+#pragma once
+#include <gmock/gmock.h>
+#include "property_change_listener.hpp"
+
+namespace phosphor {
+namespace time {
+
+class MockPropertyChangeListner : public PropertyChangeListner {
+ public:
+ MOCK_METHOD1(onModeChanged,
+ void(Mode mode));
+ MOCK_METHOD1(onOwnerChanged,
+ void(Owner owner));
+};
+
+} // namespace time
+} // namespace phosphor