File System: Dependency Injection and Mock
1. Add an abstract FileSystemInterface for Dependency Injection
2. Add FileSystemWrapper for real file system operations, which is
called in produciton
3. Add FileSystemMock for unit test mock only
4. Add bm_mode_transition_unittest.cpp to cover file system based bm
mode transition
Tested:
1/14 eth OK 0.22s
2/14 flash OK 0.21s
3/14 machine OK 0.12s
4/14 bmc_mode OK 0.05s
5/14 cable OK 0.27s
6/14 cpld OK 0.25s
7/14 entity OK 0.24s
8/14 google_accel_oob OK 0.20s
9/14 pcie OK 0.11s
10/14 poweroff OK 0.09s
11/14 psu OK 0.08s
12/14 pcie_bifurcation OK 0.07s
13/14 bm_mode_transition OK 0.29s
14/14 handler OK 0.44s
Ok: 14
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Change-Id: I8d0d9096fbeee7af1946d7e73f548e4a249d3bee
Signed-off-by: Hao Zhou <haoooamazing@google.com>
diff --git a/bmc_mode_enum.hpp b/bmc_mode_enum.hpp
index dbad89b..b099b5b 100644
--- a/bmc_mode_enum.hpp
+++ b/bmc_mode_enum.hpp
@@ -20,6 +20,11 @@
{
namespace ipmi
{
+inline constexpr auto bmDriveCleaningFlagPath = "/run/bm-drive-cleaning.flag";
+inline constexpr auto bmDriveCleaningDoneFlagPath =
+ "/run/bm-drive-cleaning-done.flag";
+inline constexpr auto bmDriveCleaningDoneAckFlagPath =
+ "/run/bm-drive-cleaning-done-ack.flag";
enum class BmcMode : uint8_t
{
diff --git a/file_system_wrapper.cpp b/file_system_wrapper.cpp
new file mode 100644
index 0000000..c9a8b79
--- /dev/null
+++ b/file_system_wrapper.cpp
@@ -0,0 +1,44 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "file_system_wrapper_impl.hpp"
+
+#include <fstream>
+#include <system_error>
+
+namespace google
+{
+namespace ipmi
+{
+namespace fs = std::filesystem;
+
+bool FileSystemWrapper::exists(const fs::path& path, std::error_code& ec) const
+{
+ return fs::exists(path, ec);
+}
+
+void FileSystemWrapper::rename(const fs::path& oldPath, const fs::path& newPath,
+ std::error_code& ec) const
+{
+ fs::rename(oldPath, newPath, ec);
+}
+
+void FileSystemWrapper::create(const char* path) const
+{
+ std::ofstream ofs;
+ ofs.open(path, std::ofstream::out);
+ ofs.close();
+}
+} // namespace ipmi
+} // namespace google
diff --git a/file_system_wrapper.hpp b/file_system_wrapper.hpp
new file mode 100644
index 0000000..701451c
--- /dev/null
+++ b/file_system_wrapper.hpp
@@ -0,0 +1,64 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include <filesystem>
+#include <system_error>
+
+namespace google
+{
+namespace ipmi
+{
+namespace fs = std::filesystem;
+
+class FileSystemInterface
+{
+ public:
+ virtual ~FileSystemInterface() = default;
+
+ /**
+ * Checks if the given file status or path corresponds to an existing file
+ * or directory
+ *
+ * @param[in] path - path to examine.
+ * @param[in] ec - out-parameter for error reporting in the non-throwing
+ * overload
+ * @return the name of the device.
+ */
+ virtual bool exists(const fs::path& path, std::error_code& ec) const = 0;
+
+ /**
+ * Moves or renames the filesystem object identified by oldPath to newPath
+ *
+ * @param[in] oldPath - path to move or rename.
+ * @param[in] newPath - target path for the move/rename operation.
+ * @param[in] ec - out-parameter for error reporting in the non-throwing
+ * overload
+ * @return the name of the device.
+ */
+ virtual void rename(const fs::path& oldPath, const fs::path& newPath,
+ std::error_code& ec) const = 0;
+
+ /**
+ * Create an empty file at path specified
+ *
+ * @param[in] path - path to create the empty file.
+ * @return the name of the device.
+ */
+ virtual void create(const char* path) const = 0;
+};
+
+} // namespace ipmi
+} // namespace google
diff --git a/file_system_wrapper_impl.hpp b/file_system_wrapper_impl.hpp
new file mode 100644
index 0000000..9c4dfab
--- /dev/null
+++ b/file_system_wrapper_impl.hpp
@@ -0,0 +1,35 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include "file_system_wrapper.hpp"
+
+namespace google
+{
+namespace ipmi
+{
+namespace fs = std::filesystem;
+
+class FileSystemWrapper : public FileSystemInterface
+{
+ public:
+ bool exists(const fs::path& path, std::error_code& ec) const;
+ void rename(const fs::path& oldPath, const fs::path& newPath,
+ std::error_code& ec) const;
+ void create(const char* path) const;
+};
+
+} // namespace ipmi
+} // namespace google
diff --git a/handler.cpp b/handler.cpp
index 9032055..9a6ac04 100644
--- a/handler.cpp
+++ b/handler.cpp
@@ -62,26 +62,19 @@
{
namespace ipmi
{
-namespace fs = std::filesystem;
using Json = nlohmann::json;
using namespace phosphor::logging;
using InternalFailure =
sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
-static constexpr auto bmDriveCleaningFlagPath = "/run/bm-drive-cleaning.flag";
-static constexpr auto bmDriveCleaningDoneFlagPath =
- "/run/bm-drive-cleaning-done.flag";
-static constexpr auto bmDriveCleaningDoneAckFlagPath =
- "/run/bm-drive-cleaning-done-ack.flag";
-
-uint8_t isBmcInBareMetalMode()
+uint8_t isBmcInBareMetalMode(const std::unique_ptr<FileSystemInterface>& fs)
{
#if BARE_METAL
return static_cast<uint8_t>(BmcMode::BM_MODE);
#else
std::error_code ec;
- if (fs::exists(bmDriveCleaningDoneAckFlagPath, ec))
+ if (fs->exists(bmDriveCleaningDoneAckFlagPath, ec))
{
std::fprintf(
stderr,
@@ -90,9 +83,9 @@
return static_cast<uint8_t>(BmcMode::BM_MODE);
}
- if (fs::exists(bmDriveCleaningDoneFlagPath, ec))
+ if (fs->exists(bmDriveCleaningDoneFlagPath, ec))
{
- fs::rename(bmDriveCleaningDoneFlagPath, bmDriveCleaningDoneAckFlagPath,
+ fs->rename(bmDriveCleaningDoneFlagPath, bmDriveCleaningDoneAckFlagPath,
ec);
std::fprintf(
stderr,
@@ -101,13 +94,11 @@
return static_cast<uint8_t>(BmcMode::BM_MODE);
}
- if (fs::exists(BM_SIGNAL_PATH, ec))
+ if (fs->exists(BM_SIGNAL_PATH, ec))
{
- if (!fs::exists(bmDriveCleaningFlagPath, ec))
+ if (!fs->exists(bmDriveCleaningFlagPath, ec))
{
- std::ofstream ofs;
- ofs.open(bmDriveCleaningFlagPath, std::ofstream::out);
- ofs.close();
+ fs->create(bmDriveCleaningFlagPath);
}
std::fprintf(
@@ -127,7 +118,7 @@
uint8_t Handler::getBmcMode()
{
// BM_CLEANING_MODE is not implemented yet
- return isBmcInBareMetalMode();
+ return isBmcInBareMetalMode(this->getFs());
}
std::tuple<std::uint8_t, std::string>
@@ -157,7 +148,7 @@
}
std::error_code ec;
- if (!fs::exists(path, ec))
+ if (!this->getFs()->exists(path, ec))
{
std::fprintf(stderr, "Path: '%s' doesn't exist.\n", path.c_str());
throw IpmiException(::ipmi::ccInvalidFieldRequest);
@@ -187,7 +178,7 @@
// Check for file
std::error_code ec;
- if (!fs::exists(opath.str(), ec))
+ if (!this->getFs()->exists(opath.str(), ec))
{
std::fprintf(stderr, "Path: '%s' doesn't exist.\n",
opath.str().c_str());
@@ -480,6 +471,11 @@
return sdbusplus::bus::new_default();
}
+const std::unique_ptr<FileSystemInterface>& Handler::getFs() const
+{
+ return this->fsPtr;
+}
+
uint32_t Handler::accelOobDeviceCount() const
{
ArrayOfObjectPathsAndTieredAnyTypeLists data;
@@ -674,7 +670,8 @@
void Handler::linuxBootDone() const
{
- if (isBmcInBareMetalMode() != static_cast<uint8_t>(BmcMode::BM_MODE))
+ if (isBmcInBareMetalMode(this->fsPtr) !=
+ static_cast<uint8_t>(BmcMode::BM_MODE))
{
return;
}
diff --git a/handler_impl.hpp b/handler_impl.hpp
index 5d74435..7b58523 100644
--- a/handler_impl.hpp
+++ b/handler_impl.hpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Google LLC
+// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@
#pragma once
#include "bifurcation.hpp"
+#include "file_system_wrapper_impl.hpp"
#include "handler.hpp"
#include <nlohmann/json.hpp>
@@ -22,6 +23,7 @@
#include <cstdint>
#include <map>
+#include <memory>
#include <string>
#include <string_view>
#include <tuple>
@@ -39,12 +41,13 @@
{
public:
explicit Handler(const std::string& entityConfigPath = defaultConfigFile) :
+ fsPtr(std::make_unique<FileSystemWrapper>()),
_configFile(entityConfigPath),
bifurcationHelper(BifurcationStatic::createBifurcation()){};
Handler(std::reference_wrapper<BifurcationInterface> bifurcationHelper,
const std::string& entityConfigPath = defaultConfigFile) :
- _configFile(entityConfigPath),
- bifurcationHelper(bifurcationHelper){};
+ fsPtr(std::make_unique<FileSystemWrapper>()),
+ _configFile(entityConfigPath), bifurcationHelper(bifurcationHelper){};
~Handler() = default;
uint8_t getBmcMode() override;
@@ -75,8 +78,11 @@
protected:
// Exposed for dependency injection
virtual sdbusplus::bus_t getDbus() const;
+ virtual const std::unique_ptr<FileSystemInterface>& getFs() const;
private:
+ std::unique_ptr<FileSystemInterface> fsPtr;
+
std::string _configFile;
bool _entityConfigParsed = false;
diff --git a/meson.build b/meson.build
index ea0312c..f68c882 100644
--- a/meson.build
+++ b/meson.build
@@ -76,6 +76,7 @@
'pcie_i2c.cpp',
'google_accel_oob.cpp',
'pcie_bifurcation.cpp',
+ 'file_system_wrapper.cpp',
'psu.cpp',
'util.cpp',
implicit_include_directories: false,
diff --git a/test/bm_mode_transition_unittest.cpp b/test/bm_mode_transition_unittest.cpp
new file mode 100644
index 0000000..b39d56c
--- /dev/null
+++ b/test/bm_mode_transition_unittest.cpp
@@ -0,0 +1,120 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+#include "bm_config.h"
+
+#include "bmc_mode_enum.hpp"
+#include "errors.hpp"
+#include "file_system_mock.hpp"
+#include "handler.hpp"
+#include "handler_impl.hpp"
+
+#include <filesystem>
+#include <fstream>
+#include <functional>
+#include <memory>
+#include <string>
+
+#include <gtest/gtest.h>
+
+namespace google
+{
+namespace ipmi
+{
+using ::testing::_;
+using ::testing::Eq;
+using ::testing::IsNull;
+using ::testing::NotNull;
+using ::testing::Pointee;
+using ::testing::Return;
+using ::testing::StrEq;
+
+namespace fs = std::filesystem;
+
+class MockFsHandler : public Handler
+{
+ public:
+ MockFsHandler(std::unique_ptr<FileSystemMock> mock,
+ const std::string& config = "") :
+ Handler(config)
+ {
+ fsPtr_ = move(mock);
+ }
+
+ protected:
+ const std::unique_ptr<FileSystemInterface>& getFs() const override
+ {
+ return fsPtr_;
+ }
+
+ private:
+ std::unique_ptr<FileSystemInterface> fsPtr_;
+};
+
+TEST(BmcModeTransitionTest, DriveCleaningDoneAckFlag)
+{
+ auto fsMockPtr = std::make_unique<FileSystemMock>();
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningDoneAckFlagPath), _))
+ .WillOnce(Return(true));
+
+ MockFsHandler h(move(fsMockPtr));
+ EXPECT_EQ(h.getBmcMode(), static_cast<uint8_t>(BmcMode::BM_MODE));
+}
+
+TEST(BmcModeTransitionTest, DriveCleaningDoneFlag)
+{
+ auto fsMockPtr = std::make_unique<FileSystemMock>();
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningDoneAckFlagPath), _))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningDoneFlagPath), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*fsMockPtr, rename(fs::path(bmDriveCleaningDoneFlagPath),
+ fs::path(bmDriveCleaningDoneAckFlagPath), _))
+ .Times(1);
+
+ MockFsHandler h(move(fsMockPtr));
+ EXPECT_EQ(h.getBmcMode(), static_cast<uint8_t>(BmcMode::BM_MODE));
+}
+
+TEST(BmcModeTransitionTest, FirstCleaningFlag)
+{
+ auto fsMockPtr = std::make_unique<FileSystemMock>();
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningDoneAckFlagPath), _))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningDoneFlagPath), _))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(BM_SIGNAL_PATH), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningFlagPath), _))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*fsMockPtr, create(StrEq(bmDriveCleaningFlagPath))).Times(1);
+
+ MockFsHandler h(move(fsMockPtr));
+ EXPECT_EQ(h.getBmcMode(), static_cast<uint8_t>(BmcMode::BM_CLEANING_MODE));
+}
+
+TEST(BmcModeTransitionTest, NonBmMode)
+{
+ auto fsMockPtr = std::make_unique<FileSystemMock>();
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningDoneAckFlagPath), _))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(bmDriveCleaningDoneFlagPath), _))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*fsMockPtr, exists(fs::path(BM_SIGNAL_PATH), _))
+ .WillOnce(Return(false));
+ MockFsHandler h(move(fsMockPtr));
+ EXPECT_EQ(h.getBmcMode(), static_cast<uint8_t>(BmcMode::NON_BM_MODE));
+}
+
+} // namespace ipmi
+} // namespace google
diff --git a/test/file_system_mock.hpp b/test/file_system_mock.hpp
new file mode 100644
index 0000000..de4f38a
--- /dev/null
+++ b/test/file_system_mock.hpp
@@ -0,0 +1,41 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include "file_system_wrapper.hpp"
+
+#include <gmock/gmock.h>
+
+namespace google
+{
+namespace ipmi
+{
+namespace fs = std::filesystem;
+
+class FileSystemMock : public FileSystemInterface
+{
+ public:
+ ~FileSystemMock() = default;
+
+ MOCK_METHOD(bool, exists, (const fs::path&, std::error_code&),
+ (const, override));
+ MOCK_METHOD(void, rename,
+ (const fs::path&, const fs::path&, std::error_code&),
+ (const, override));
+ MOCK_METHOD(void, create, (const char*), (const, override));
+};
+
+} // namespace ipmi
+} // namespace google
diff --git a/test/meson.build b/test/meson.build
index 7fa06c4..c3387dd 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -30,6 +30,7 @@
'pcie_bifurcation',
'bmc_mode',
'linux_boot_done',
+ 'bm_mode_transition',
]
foreach t : tests