Rework envMock
Instead of mocking all the functions, only mock std::getenv.
Now each test only needs to provide an EnvImpl that delegate calls to
mockEnv in order to inject dependencies on std::getenv. This for several
reasons:
1. Any call to env::getEnv() will be calling the real implementation of
the C++ code, and testing real code is better than testing mocks.
2. It is easier to write a fake class that takes a config string which
can greatly simplify test cases.
3. We can now write unit tests that ensure the number of times
std::getenv gets called (should be once, but multiple times right now).
Tested: unit tests still pass
Signed-off-by: Kun Yi <kunyi731@gmail.com>
Change-Id: I3e5aff7fa5d025de1b8ae798af43b97d31151ab9
diff --git a/env.cpp b/env.cpp
index 8dbb679..318009f 100644
--- a/env.cpp
+++ b/env.cpp
@@ -16,64 +16,18 @@
#include "env.hpp"
-#include "hwmon.hpp"
-
#include <cstdlib>
-#include <fstream>
+#include <string>
namespace env
{
-std::string getEnv(const char* key)
+const char* EnvImpl::get(const char* key) const
{
- auto value = std::getenv(key);
- return (value) ? std::string(value) : std::string();
+ return std::getenv(key);
}
-std::string getEnv(const char* prefix, const SensorSet::key_type& sensor)
-{
- std::string key;
-
- key.assign(prefix);
- key.append(1, '_');
- key.append(sensor.first);
- key.append(sensor.second);
-
- return getEnv(key.c_str());
-}
-
-std::string getEnv(const char* prefix, const std::string& type,
- const std::string& id)
-{
- SensorSet::key_type sensor{type, id};
- return getEnv(prefix, sensor);
-}
-
-std::string getIndirectID(std::string path, const std::string& fileSuffix,
- const SensorSet::key_type& sensor)
-{
- std::string content;
-
- path.append(sensor.first);
- path.append(sensor.second);
- path.append(1, '_');
- path.append(fileSuffix);
-
- std::ifstream handle(path.c_str());
- if (!handle.fail())
- {
- content.assign((std::istreambuf_iterator<char>(handle)),
- (std::istreambuf_iterator<char>()));
-
- if (!content.empty())
- {
- // remove the newline
- content.pop_back();
- }
- }
-
- return content;
-}
+EnvImpl env_impl;
} // namespace env
diff --git a/env.hpp b/env.hpp
index b3ddff5..b8c3815 100644
--- a/env.hpp
+++ b/env.hpp
@@ -2,20 +2,48 @@
#include "sensorset.hpp"
+#include <fstream>
#include <string>
namespace env
{
+/** @class Env
+ * @brief Overridable std::getenv interface
+ */
+struct Env
+{
+ virtual ~Env() = default;
+
+ virtual const char* get(const char* key) const = 0;
+};
+
+/** @class EnvImpl
+ * @brief Concrete implementation that calls std::getenv
+ */
+struct EnvImpl : public Env
+{
+ const char* get(const char* key) const override;
+};
+
+/** @brief Default instantiation of Env */
+extern EnvImpl env_impl;
+
/** @brief Reads an environment variable
*
* Reads the environment for that key
*
* @param[in] key - the key
+ * @param[in] env - env interface that defaults to calling std::getenv
*
* @return string - the env var value
*/
-std::string getEnv(const char* key);
+inline std::string getEnv(const char* key, const Env* env = &env_impl)
+{
+ // Be aware value could be nullptr
+ auto value = env->get(key);
+ return (value) ? std::string(value) : std::string();
+}
/** @brief Reads an environment variable
*
@@ -23,21 +51,38 @@
*
* @param[in] prefix - the variable prefix
* @param[in] sensor - Sensor details
+ * @param[in] env - env interface that defaults to calling std::getenv
*
* @return string - the env var value
*/
-std::string getEnv(const char* prefix, const SensorSet::key_type& sensor);
+inline std::string getEnv(const char* prefix, const SensorSet::key_type& sensor,
+ const Env* env = &env_impl)
+{
+ std::string key;
+
+ key.assign(prefix);
+ key.append(1, '_');
+ key.append(sensor.first);
+ key.append(sensor.second);
+
+ return getEnv(key.c_str(), env);
+}
/** @brief Reads an environment variable, and takes type and id separately
*
* @param[in] prefix - the variable prefix
* @param[in] type - sensor type, like 'temp'
* @param[in] id - sensor ID, like '5'
+ * @param[in] env - env interface that defaults to calling std::getenv
*
* @return string - the env var value
*/
-std::string getEnv(const char* prefix, const std::string& type,
- const std::string& id);
+inline std::string getEnv(const char* prefix, const std::string& type,
+ const std::string& id, const Env* env = &env_impl)
+{
+ SensorSet::key_type sensor{type, id};
+ return getEnv(prefix, sensor, env);
+}
/** @brief Gets the ID for the sensor with a level of indirection
*
@@ -48,7 +93,31 @@
* @param[in] fileSuffix - The file suffix
* @param[in] sensor - Sensor details
*/
-std::string getIndirectID(std::string path, const std::string& fileSuffix,
- const SensorSet::key_type& sensor);
+inline std::string getIndirectID(std::string path,
+ const std::string& fileSuffix,
+ const SensorSet::key_type& sensor)
+{
+ std::string content;
+
+ path.append(sensor.first);
+ path.append(sensor.second);
+ path.append(1, '_');
+ path.append(fileSuffix);
+
+ std::ifstream handle(path.c_str());
+ if (!handle.fail())
+ {
+ content.assign((std::istreambuf_iterator<char>(handle)),
+ (std::istreambuf_iterator<char>()));
+
+ if (!content.empty())
+ {
+ // remove the newline
+ content.pop_back();
+ }
+ }
+
+ return content;
+}
} // namespace env
diff --git a/test/Makefile.am b/test/Makefile.am
index dce609a..41cee60 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -24,7 +24,6 @@
TESTS = $(check_PROGRAMS)
env_unittest_SOURCES = env_unittest.cpp
-env_unittest_LDADD = env.o
average_unittest_SOURCES = average_unittest.cpp
average_unittest_LDADD = $(top_builddir)/average.o
@@ -47,7 +46,6 @@
$(STDPLUS_LIBS) \
$(top_builddir)/sensor.o \
$(top_builddir)/hwmon.o \
- env.o \
gpio.o
hwmonio_default_unittest_SOURCES = hwmonio_default_unittest.cpp
diff --git a/test/env.cpp b/test/env.cpp
deleted file mode 100644
index 934b9a0..0000000
--- a/test/env.cpp
+++ /dev/null
@@ -1,31 +0,0 @@
-#include "env_mock.hpp"
-
-// Set this before each test that hits a call to getEnv().
-EnvInterface* envIntf = nullptr;
-
-namespace env
-{
-
-std::string getEnv(const char* key)
-{
- return (envIntf) ? envIntf->getEnv(key) : "";
-}
-
-std::string getEnv(const char* prefix, const SensorSet::key_type& sensor)
-{
- return (envIntf) ? envIntf->getEnv(prefix, sensor) : "";
-}
-
-std::string getEnv(const char* prefix, const std::string& type,
- const std::string& id)
-{
- return (envIntf) ? envIntf->getEnv(prefix, type, id) : "";
-}
-
-std::string getIndirectID(std::string path, const std::string& fileSuffix,
- const SensorSet::key_type& sensor)
-{
- return (envIntf) ? envIntf->getIndirectID(path, fileSuffix, sensor) : "";
-}
-
-} // namespace env
diff --git a/test/env_mock.hpp b/test/env_mock.hpp
index 4cf232f..9aad8c1 100644
--- a/test/env_mock.hpp
+++ b/test/env_mock.hpp
@@ -1,40 +1,20 @@
#pragma once
-#include "sensorset.hpp"
+#include "env.hpp"
#include <string>
#include <gmock/gmock.h>
-class EnvInterface
+namespace env
+{
+
+class EnvMock : public Env
{
public:
- virtual ~EnvInterface() = default;
-
- virtual std::string getEnv(const char* key) const = 0;
- virtual std::string getEnv(const char* prefix,
- const SensorSet::key_type& sensor) const = 0;
- virtual std::string getEnv(const char* prefix, const std::string& type,
- const std::string& id) const = 0;
- virtual std::string
- getIndirectID(std::string path, const std::string& fileSuffix,
- const SensorSet::key_type& sensor) const = 0;
+ MOCK_CONST_METHOD1(get, const char*(const char*));
};
-class EnvMock : public EnvInterface
-{
- public:
- virtual ~EnvMock() = default;
+static inline EnvMock mockEnv;
- MOCK_CONST_METHOD1(getEnv, std::string(const char*));
- MOCK_CONST_METHOD2(getEnv,
- std::string(const char*, const SensorSet::key_type&));
- MOCK_CONST_METHOD3(getEnv, std::string(const char*, const std::string&,
- const std::string&));
- MOCK_CONST_METHOD3(getIndirectID,
- std::string(std::string, const std::string&,
- const SensorSet::key_type&));
-};
-
-// Set this before each test that hits a call to getEnv().
-extern EnvInterface* envIntf;
+} // namespace env
diff --git a/test/env_unittest.cpp b/test/env_unittest.cpp
index 665f8a5..8e12e0e 100644
--- a/test/env_unittest.cpp
+++ b/test/env_unittest.cpp
@@ -5,28 +5,46 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+using ::testing::_;
using ::testing::Return;
using ::testing::StrEq;
-using ::testing::StrictMock;
+
+namespace env
+{
+
+// Delegate all calls to getEnv() to the mock
+const char* EnvImpl::get(const char* key) const
+{
+ return mockEnv.get(key);
+}
+
+EnvImpl env_impl;
+
+} // namespace env
+
+TEST(EnvTest, NonExistingEnvReturnsEmptyString)
+{
+ EXPECT_CALL(env::mockEnv, get(_)).WillOnce(Return(nullptr));
+ EXPECT_EQ(std::string(), env::getEnv("NonExistingKey"));
+}
TEST(EnvTest, EmptyEnv)
{
+ EXPECT_CALL(env::mockEnv, get(StrEq("AVERAGE_power1")))
+ .WillOnce(Return(nullptr));
EXPECT_FALSE(
phosphor::utility::isAverageEnvSet(std::make_pair("power", "1")));
}
TEST(EnvTest, ValidAverageEnv)
{
- StrictMock<EnvMock> eMock;
- envIntf = &eMock;
-
std::string power = "power";
std::string one = "1";
std::string two = "2";
- EXPECT_CALL(eMock, getEnv(StrEq("AVERAGE"), power, one))
+ EXPECT_CALL(env::mockEnv, get(StrEq("AVERAGE_power1")))
.WillOnce(Return("true"));
- EXPECT_CALL(eMock, getEnv(StrEq("AVERAGE"), power, two))
+ EXPECT_CALL(env::mockEnv, get(StrEq("AVERAGE_power2")))
.WillOnce(Return("bar"));
EXPECT_TRUE(phosphor::utility::isAverageEnvSet(std::make_pair(power, one)));
diff --git a/test/sensor_unittest.cpp b/test/sensor_unittest.cpp
index 8271b54..3a446b2 100644
--- a/test/sensor_unittest.cpp
+++ b/test/sensor_unittest.cpp
@@ -10,12 +10,24 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+namespace env
+{
+
+// Delegate all calls to getEnv() to the mock
+const char* EnvImpl::get(const char* key) const
+{
+ return mockEnv.get(key);
+}
+
+EnvImpl env_impl;
+
+} // namespace env
+
class SensorTest : public ::testing::Test
{
protected:
void SetUp() override
{
- envIntf = nullptr;
gpioIntf = nullptr;
}
@@ -33,27 +45,20 @@
TEST_F(SensorTest, BasicConstructorTest)
{
/* Constructor test with nothing in an rcList or GPIO chip. */
-
- StrictMock<EnvMock> eMock;
- envIntf = &eMock;
-
auto sensorKey = std::make_pair(temp, five);
std::unique_ptr<hwmonio::HwmonIOInterface> hwmonio_mock =
std::make_unique<hwmonio::HwmonIOMock>();
std::string path = "/";
/* Always calls GPIOCHIP and GPIO checks, returning empty string. */
- EXPECT_CALL(eMock, getEnv(StrEq("GPIOCHIP"), Pair(temp, five)))
+ EXPECT_CALL(env::mockEnv, get(StrEq("GPIOCHIP_temp5")))
.WillOnce(Return(""));
- EXPECT_CALL(eMock, getEnv(StrEq("GPIO"), Pair(temp, five)))
- .WillOnce(Return(""));
+ EXPECT_CALL(env::mockEnv, get(StrEq("GPIO_temp5"))).WillOnce(Return(""));
/* Always calls GAIN and OFFSET, can use ON_CALL instead of EXPECT_CALL */
- EXPECT_CALL(eMock, getEnv(StrEq("GAIN"), Pair(temp, five)))
- .WillOnce(Return(""));
- EXPECT_CALL(eMock, getEnv(StrEq("OFFSET"), Pair(temp, five)))
- .WillOnce(Return(""));
- EXPECT_CALL(eMock, getEnv(StrEq("REMOVERCS"), Pair(temp, five)))
+ EXPECT_CALL(env::mockEnv, get(StrEq("GAIN_temp5"))).WillOnce(Return(""));
+ EXPECT_CALL(env::mockEnv, get(StrEq("OFFSET_temp5"))).WillOnce(Return(""));
+ EXPECT_CALL(env::mockEnv, get(StrEq("REMOVERCS_temp5")))
.WillOnce(Return(""));
auto sensor =
@@ -65,9 +70,6 @@
{
/* Constructor test with only the GPIO chip set. */
- StrictMock<EnvMock> eMock;
- envIntf = &eMock;
-
StrictMock<GpioHandleMock> gMock;
gpioIntf = &gMock;
@@ -81,10 +83,9 @@
std::make_unique<hwmonio::HwmonIOMock>();
std::string path = "/";
- EXPECT_CALL(eMock, getEnv(StrEq("GPIOCHIP"), Pair(temp, five)))
+ EXPECT_CALL(env::mockEnv, get(StrEq("GPIOCHIP_temp5")))
.WillOnce(Return("chipA"));
- EXPECT_CALL(eMock, getEnv(StrEq("GPIO"), Pair(temp, five)))
- .WillOnce(Return("5"));
+ EXPECT_CALL(env::mockEnv, get(StrEq("GPIO_temp5"))).WillOnce(Return("5"));
EXPECT_CALL(gMock, build(StrEq("chipA"), StrEq("5")))
.WillOnce(Invoke([&](const std::string& chip, const std::string& line) {
@@ -92,11 +93,9 @@
}));
/* Always calls GAIN and OFFSET, can use ON_CALL instead of EXPECT_CALL */
- EXPECT_CALL(eMock, getEnv(StrEq("GAIN"), Pair(temp, five)))
- .WillOnce(Return(""));
- EXPECT_CALL(eMock, getEnv(StrEq("OFFSET"), Pair(temp, five)))
- .WillOnce(Return(""));
- EXPECT_CALL(eMock, getEnv(StrEq("REMOVERCS"), Pair(temp, five)))
+ EXPECT_CALL(env::mockEnv, get(StrEq("GAIN_temp5"))).WillOnce(Return(""));
+ EXPECT_CALL(env::mockEnv, get(StrEq("OFFSET_temp5"))).WillOnce(Return(""));
+ EXPECT_CALL(env::mockEnv, get(StrEq("REMOVERCS_temp5")))
.WillOnce(Return(""));
auto sensor =
@@ -110,25 +109,20 @@
* when adjusting the value.
*/
- StrictMock<EnvMock> eMock;
- envIntf = &eMock;
-
auto sensorKey = std::make_pair(temp, five);
std::unique_ptr<hwmonio::HwmonIOInterface> hwmonio_mock =
std::make_unique<hwmonio::HwmonIOMock>();
std::string path = "/";
- /* Always calls GPIOCHIP and GPIO checks, returning empty string. */
- EXPECT_CALL(eMock, getEnv(StrEq("GPIOCHIP"), Pair(temp, five)))
+ /* Always calls GPIOCHIP_temp5 and GPIO checks, returning empty string. */
+ EXPECT_CALL(env::mockEnv, get(StrEq("GPIOCHIP_temp5")))
.WillOnce(Return(""));
- EXPECT_CALL(eMock, getEnv(StrEq("GPIO"), Pair(temp, five)))
- .WillOnce(Return(""));
+ EXPECT_CALL(env::mockEnv, get(StrEq("GPIO_temp5"))).WillOnce(Return(""));
- EXPECT_CALL(eMock, getEnv(StrEq("GAIN"), Pair(temp, five)))
- .WillOnce(Return("10"));
- EXPECT_CALL(eMock, getEnv(StrEq("OFFSET"), Pair(temp, five)))
+ EXPECT_CALL(env::mockEnv, get(StrEq("GAIN_temp5"))).WillOnce(Return("10"));
+ EXPECT_CALL(env::mockEnv, get(StrEq("OFFSET_temp5")))
.WillOnce(Return("15"));
- EXPECT_CALL(eMock, getEnv(StrEq("REMOVERCS"), Pair(temp, five)))
+ EXPECT_CALL(env::mockEnv, get(StrEq("REMOVERCS_temp5")))
.WillOnce(Return(""));
auto sensor =