Switch to using a filesystem class for better testing

This commit is contained in:
pmb
2025-06-25 11:59:07 -07:00
parent 3d35345eb9
commit e880f6ba1d
5 changed files with 200 additions and 39 deletions
+2 -2
View File
@@ -37,7 +37,7 @@ jobs:
if: matrix.os == 'ubuntu-latest' if: matrix.os == 'ubuntu-latest'
run: | run: |
sudo apt-get update sudo apt-get update
sudo apt-get install -y libboost-system-dev libgtest-dev build-essential sudo apt-get install -y libboost-system-dev libgtest-dev libgmock-dev build-essential
- name: Setup build directory - name: Setup build directory
run: | run: |
@@ -83,7 +83,7 @@ jobs:
- name: Install dependencies and coverage tools - name: Install dependencies and coverage tools
run: | run: |
sudo apt-get update sudo apt-get update
sudo apt-get install -y libboost-system-dev libgtest-dev build-essential lcov sudo apt-get install -y libboost-system-dev libgtest-dev libgmock-dev build-essential lcov
- name: Setup build directory with coverage - name: Setup build directory with coverage
run: | run: |
+13 -28
View File
@@ -6,6 +6,11 @@
const std::filesystem::path kPATH{"/var/finger/users/"}; const std::filesystem::path kPATH{"/var/finger/users/"};
std::string process(const std::string &username) { std::string process(const std::string &username) {
RealFilesystemWrapper fs;
return process(username, fs);
}
std::string process(const std::string &username, const IFilesystemWrapper &fs) {
try { try {
// Check for directory traversal patterns // Check for directory traversal patterns
if (username.find("../") != std::string::npos || if (username.find("../") != std::string::npos ||
@@ -31,39 +36,19 @@ std::string process(const std::string &username) {
// Attempt to open the plan file (if any) and return the contents as a string // Attempt to open the plan file (if any) and return the contents as a string
std::filesystem::path planPath = kPATH / username; std::filesystem::path planPath = kPATH / username;
// Check if the plan file exists // Check if the plan file exists using the filesystem wrapper
if (!std::filesystem::exists(planPath)) { if (!fs.exists(planPath)) {
// If no plan file exists, return just the username // If no plan file exists, return just the username
return username; return username;
} }
// Try to read the plan file // Try to read the plan file using the filesystem wrapper
try { std::string content = fs.read_file(planPath);
std::ifstream planFile(planPath);
if (!planFile.is_open()) {
return username;
}
std::string content; if (content.empty()) {
std::string line; // If file exists but is empty or couldn't be read, return just the username
while (std::getline(planFile, line)) {
content += line + "\n";
}
// Return the plan content with proper line endings
if (!content.empty() && content.back() == '\n') {
content.pop_back(); // Remove the last newline
content += "\r\n";
return content;
} else if (!content.empty()) {
content += "\r\n";
return content;
} else {
// If file exists but is empty, return just the username
return username;
}
} catch (...) {
// If there's any error reading the file, just return the username
return username; return username;
} }
return content;
} }
+44 -5
View File
@@ -1,15 +1,54 @@
#pragma once #pragma once
#include <string>
#include <exception> #include <exception>
#include <filesystem>
#include <fstream>
#include <stdexcept> #include <stdexcept>
#include <string>
class InvalidInput : public std::runtime_error class IFilesystemWrapper {
{
public: public:
InvalidInput(const std::string& what = "") : std::runtime_error(what) {} virtual ~IFilesystemWrapper() = default;
virtual bool exists(const std::filesystem::path &path) const = 0;
virtual std::string read_file(const std::filesystem::path &path) const = 0;
}; };
class RealFilesystemWrapper : public IFilesystemWrapper {
public:
bool exists(const std::filesystem::path &path) const override {
return std::filesystem::exists(path);
}
std::string read_file(const std::filesystem::path &path) const override {
std::ifstream file(path);
if (!file.is_open()) {
return "";
}
std::string content;
std::string line;
while (std::getline(file, line)) {
content += line + "\n";
}
// Return the content with proper line endings
if (!content.empty() && content.back() == '\n') {
content.pop_back(); // Remove the last newline
content += "\r\n";
return content;
} else if (!content.empty()) {
content += "\r\n";
return content;
}
return "";
}
};
class InvalidInput : public std::runtime_error {
public:
InvalidInput(const std::string &what = "") : std::runtime_error(what) {}
};
std::string process(const std::string &username); std::string process(const std::string &username);
std::string process(const std::string &username, const IFilesystemWrapper &fs);
+10 -3
View File
@@ -6,8 +6,9 @@ project('finger', 'cpp',
# Find Boost dependencies - prefer static libraries # Find Boost dependencies - prefer static libraries
boost_dep = dependency('boost', modules : ['system'], static : true) boost_dep = dependency('boost', modules : ['system'], static : true)
# Find Google Test dependency # Find Google Test and Google Mock dependencies
gtest_dep = dependency('gtest', main : true, required : true) gtest_dep = dependency('gtest', main : true, required : true)
gmock_dep = dependency('gmock', main : true, required : true)
executable('finger', executable('finger',
'main.cpp','handler.cpp', 'main.cpp','handler.cpp',
@@ -18,7 +19,13 @@ executable('finger',
# Test executable # Test executable
test_exe = executable('test_handler', test_exe = executable('test_handler',
'test_handler.cpp', 'handler.cpp', 'test_handler.cpp', 'handler.cpp',
dependencies : [boost_dep, gtest_dep]) dependencies : [boost_dep, gtest_dep, gmock_dep])
# Register the test # Mock test executable
test_mock_exe = executable('test_handler_mock',
'test_handler_mock.cpp', 'handler.cpp',
dependencies : [boost_dep, gtest_dep, gmock_dep])
# Register the tests
test('handler_tests', test_exe) test('handler_tests', test_exe)
test('handler_mock_tests', test_mock_exe)
+130
View File
@@ -0,0 +1,130 @@
#include "handler.hpp"
#include <filesystem>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <memory>
// Note: This is a demonstration of how to set up Google Mock for filesystem
// operations. To properly mock std::filesystem::exists, you would need to
// refactor handler.cpp to use dependency injection or create a filesystem
// wrapper interface.
// Mock interface for filesystem operations
/*
class IFilesystemWrapper {
public:
virtual ~IFilesystemWrapper() = default;
virtual bool exists(const std::filesystem::path& path) const = 0;
virtual std::string read_file(const std::filesystem::path& path) const = 0;
};
*/
#include "handler.hpp"
// Mock implementation
class MockFilesystemWrapper : public IFilesystemWrapper {
public:
MOCK_METHOD(bool, exists, (const std::filesystem::path &path),
(const, override));
MOCK_METHOD(std::string, read_file, (const std::filesystem::path &path),
(const, override));
};
// Test fixture for demonstrating Google Mock setup
class ProcessMockTest : public ::testing::Test {
protected:
void SetUp() override {
mock_filesystem = std::make_unique<MockFilesystemWrapper>();
}
void TearDown() override { mock_filesystem.reset(); }
std::unique_ptr<MockFilesystemWrapper> mock_filesystem;
};
// Test using the actual process function with mocked filesystem
TEST_F(ProcessMockTest, ProcessWithFileExists) {
// Setup mock expectations
EXPECT_CALL(*mock_filesystem, exists(::testing::_))
.WillOnce(::testing::Return(true));
EXPECT_CALL(*mock_filesystem, read_file(::testing::_))
.WillOnce(::testing::Return("Mock file content\r\n"));
// Test the actual process function with mocked filesystem
std::string result = process("testuser", *mock_filesystem);
EXPECT_EQ(result, "Mock file content\r\n");
}
// Test process function when file doesn't exist
TEST_F(ProcessMockTest, ProcessWithFileNotFound) {
EXPECT_CALL(*mock_filesystem, exists(::testing::_))
.WillOnce(::testing::Return(false));
// Test the actual process function - should return username when file doesn't
// exist
std::string result = process("nonexistentuser", *mock_filesystem);
EXPECT_EQ(result, "nonexistentuser");
}
// Test process function when file exists but is empty
TEST_F(ProcessMockTest, ProcessWithEmptyFile) {
EXPECT_CALL(*mock_filesystem, exists(::testing::_))
.WillOnce(::testing::Return(true));
EXPECT_CALL(*mock_filesystem, read_file(::testing::_))
.WillOnce(::testing::Return(""));
// Test the actual process function - should return username when file is
// empty
std::string result = process("emptyfileuser", *mock_filesystem);
EXPECT_EQ(result, "emptyfileuser");
}
// Test showing multiple expectations
TEST_F(ProcessMockTest, MultipleFileOperations) {
using ::testing::_;
using ::testing::Return;
EXPECT_CALL(*mock_filesystem, exists(_))
.Times(2)
.WillOnce(Return(true))
.WillOnce(Return(false));
EXPECT_CALL(*mock_filesystem, read_file(_))
.WillOnce(Return("First file content\r\n"));
// Test multiple calls
EXPECT_TRUE(mock_filesystem->exists("/path1"));
EXPECT_EQ(mock_filesystem->read_file("/path1"), "First file content\r\n");
EXPECT_FALSE(mock_filesystem->exists("/path2"));
}
/*
* REFACTORING SUGGESTION:
*
* To properly mock std::filesystem::exists in your handler.cpp, consider:
*
* 1. Create a filesystem wrapper interface:
* class IFilesystemWrapper {
* public:
* virtual bool exists(const std::filesystem::path& path) const = 0;
* virtual std::string read_file(const std::filesystem::path& path) const
* = 0;
* };
*
* 2. Modify process() function to accept the wrapper:
* std::string process(const std::string& username,
* const IFilesystemWrapper& fs =
* RealFilesystemWrapper{});
*
* 3. Use dependency injection in tests:
* MockFilesystemWrapper mock_fs;
* EXPECT_CALL(mock_fs, exists(_)).WillOnce(Return(true));
* std::string result = process("testuser", mock_fs);
*/
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}