From 5575efb61318a29b810b841390dce5b80cbeacfb Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Tue, 30 Jan 2024 09:25:03 -0800 Subject: Create TemporaryFileHandle class TemporaryFileHandle class is used to create temp files in the filesystem, and hold a handle to them until the class goes out of scope, at which time they can be removed. It replaces makeFile(), which was not RAII safe if an exception gets thrown, and could potentially leave files in the filesystem if the tests fail. Tested: Unit tests pass Change-Id: I03eb0d342a6cd7b78115a8c42be9175f30c4ccd0 Signed-off-by: Ed Tanous --- test/http/file_test_utilities.hpp | 40 +++++++++++++------ test/http/http_body_test.cpp | 17 ++++---- test/http/http_response_test.cpp | 82 ++++++++++++++++----------------------- 3 files changed, 72 insertions(+), 67 deletions(-) (limited to 'test') diff --git a/test/http/file_test_utilities.hpp b/test/http/file_test_utilities.hpp index 073a0747a4..bd11a90d8d 100644 --- a/test/http/file_test_utilities.hpp +++ b/test/http/file_test_utilities.hpp @@ -5,15 +5,33 @@ #include -inline std::string makeFile(std::string_view sampleData) +struct TemporaryFileHandle { - std::filesystem::path path = std::filesystem::temp_directory_path(); - path /= "bmcweb_http_response_test_XXXXXXXXXXX"; - std::string stringPath = path.string(); - int fd = mkstemp(stringPath.data()); - EXPECT_GT(fd, 0); - EXPECT_EQ(write(fd, sampleData.data(), sampleData.size()), - sampleData.size()); - close(fd); - return stringPath; -} + std::filesystem::path path; + std::string stringPath; + + // Creates a temporary file with the contents provided, removes it on + // destruction. + explicit TemporaryFileHandle(std::string_view sampleData) : + path(std::filesystem::temp_directory_path() / + "bmcweb_http_response_test_XXXXXXXXXXX") + { + stringPath = path.string(); + + int fd = mkstemp(stringPath.data()); + EXPECT_GT(fd, 0); + EXPECT_EQ(write(fd, sampleData.data(), sampleData.size()), + sampleData.size()); + close(fd); + } + + TemporaryFileHandle(const TemporaryFileHandle&) = delete; + TemporaryFileHandle(TemporaryFileHandle&&) = delete; + TemporaryFileHandle& operator=(const TemporaryFileHandle&) = delete; + TemporaryFileHandle& operator=(TemporaryFileHandle&&) = delete; + + ~TemporaryFileHandle() + { + std::filesystem::remove(path); + } +}; diff --git a/test/http/http_body_test.cpp b/test/http/http_body_test.cpp index 0983c1d815..d8c9dd1919 100644 --- a/test/http/http_body_test.cpp +++ b/test/http/http_body_test.cpp @@ -67,9 +67,10 @@ TEST(HttpHttpBodyValueType, CopyOperatorString) TEST(HttpHttpBodyValueType, MoveFile) { HttpBody::value_type value(EncodingType::Base64); - std::string filepath = makeFile("teststring"); + TemporaryFileHandle temporaryFile("teststring"); boost::system::error_code ec; - value.open(filepath.c_str(), boost::beast::file_mode::read, ec); + value.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read, + ec); ASSERT_FALSE(ec); // Move constructor HttpBody::value_type value2(std::move(value)); @@ -90,9 +91,10 @@ TEST(HttpHttpBodyValueType, MoveFile) TEST(HttpHttpBodyValueType, MoveOperatorFile) { HttpBody::value_type value(EncodingType::Base64); - std::string filepath = makeFile("teststring"); + TemporaryFileHandle temporaryFile("teststring"); boost::system::error_code ec; - value.open(filepath.c_str(), boost::beast::file_mode::read, ec); + value.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read, + ec); ASSERT_FALSE(ec); // Move constructor HttpBody::value_type value2 = std::move(value); @@ -109,13 +111,12 @@ TEST(HttpHttpBodyValueType, MoveOperatorFile) EXPECT_EQ(value2.payloadSize(), 16); } -TEST(HttpBodyValueType, SetFd) +TEST(HttpFileBodyValueType, SetFd) { HttpBody::value_type value(EncodingType::Base64); - std::string filepath = makeFile("teststring"); - + TemporaryFileHandle temporaryFile("teststring"); boost::system::error_code ec; - value.setFd(fileno(fopen(filepath.c_str(), "r")), ec); + value.setFd(fileno(fopen(temporaryFile.stringPath.c_str(), "r")), ec); ASSERT_FALSE(ec); std::array buffer{}; diff --git a/test/http/http_response_test.cpp b/test/http/http_response_test.cpp index d0836a2bab..0944e50a1b 100644 --- a/test/http/http_response_test.cpp +++ b/test/http/http_response_test.cpp @@ -36,7 +36,7 @@ std::string getData(boost::beast::http::response& m) boost::beast::http::response_serializer sr{m}; sr.split(true); - + // Reads buffers into ret auto reader = [&sr, &ret](const boost::system::error_code& ec2, const auto& buffer) { EXPECT_FALSE(ec2); @@ -74,63 +74,54 @@ TEST(HttpResponse, StringBody) { crow::Response res; addHeaders(res); - std::string_view bodyvalue = "this is my new body"; - res.write({bodyvalue.data(), bodyvalue.length()}); - EXPECT_EQ(*res.body(), bodyvalue); + std::string_view bodyValue = "this is my new body"; + res.write({bodyValue.data(), bodyValue.length()}); + EXPECT_EQ(*res.body(), bodyValue); verifyHeaders(res); } TEST(HttpResponse, HttpBody) { crow::Response res; addHeaders(res); - std::string path = makeFile("sample text"); - res.openFile(path); + TemporaryFileHandle temporaryFile("sample text"); + res.openFile(temporaryFile.stringPath); verifyHeaders(res); - std::filesystem::remove(path); } TEST(HttpResponse, HttpBodyWithFd) { crow::Response res; addHeaders(res); - std::string path = makeFile("sample text"); - FILE* fd = fopen(path.c_str(), "r+"); + TemporaryFileHandle temporaryFile("sample text"); + FILE* fd = fopen(temporaryFile.stringPath.c_str(), "r+"); res.openFd(fileno(fd)); verifyHeaders(res); fclose(fd); - std::filesystem::remove(path); } TEST(HttpResponse, Base64HttpBodyWithFd) { crow::Response res; addHeaders(res); - std::string path = makeFile("sample text"); - FILE* fd = fopen(path.c_str(), "r+"); + TemporaryFileHandle temporaryFile("sample text"); + FILE* fd = fopen(temporaryFile.stringPath.c_str(), "r"); + ASSERT_NE(fd, nullptr); res.openFd(fileno(fd), bmcweb::EncodingType::Base64); verifyHeaders(res); fclose(fd); - std::filesystem::remove(path); } TEST(HttpResponse, BodyTransitions) { crow::Response res; addHeaders(res); - std::string path = makeFile("sample text"); - res.openFile(path); + TemporaryFileHandle temporaryFile("sample text"); + res.openFile(temporaryFile.stringPath); verifyHeaders(res); res.write("body text"); verifyHeaders(res); - std::filesystem::remove(path); -} - -void testFileData(crow::Response& res, const std::string& data) -{ - auto& fb = res.response; - EXPECT_EQ(getData(fb), data); } std::string generateBigdata() @@ -148,52 +139,47 @@ TEST(HttpResponse, StringBodyWriterLarge) crow::Response res; std::string data = generateBigdata(); res.write(std::string(data)); - testFileData(res, data); + EXPECT_EQ(getData(res.response), data); } TEST(HttpResponse, Base64HttpBodyWriter) { crow::Response res; std::string data = "sample text"; - std::string path = makeFile(data); - FILE* f = fopen(path.c_str(), "r+"); + TemporaryFileHandle temporaryFile(data); + FILE* f = fopen(temporaryFile.stringPath.c_str(), "r+"); res.openFd(fileno(f), bmcweb::EncodingType::Base64); - testFileData(res, crow::utility::base64encode(data)); - fclose(f); - std::filesystem::remove(path); + EXPECT_EQ(getData(res.response), "c2FtcGxlIHRleHQ="); } TEST(HttpResponse, Base64HttpBodyWriterLarge) { crow::Response res; std::string data = generateBigdata(); - std::string path = makeFile(data); - { - boost::beast::file_posix file; - boost::system::error_code ec; - file.open(path.c_str(), boost::beast::file_mode::read, ec); - EXPECT_EQ(ec.value(), 0); - res.openFd(file.native_handle(), bmcweb::EncodingType::Base64); - testFileData(res, crow::utility::base64encode(data)); - } + TemporaryFileHandle temporaryFile(data); - std::filesystem::remove(path); + boost::beast::file_posix file; + boost::system::error_code ec; + file.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read, + ec); + EXPECT_EQ(ec.value(), 0); + res.openFd(file.native_handle(), bmcweb::EncodingType::Base64); + EXPECT_EQ(getData(res.response), crow::utility::base64encode(data)); } TEST(HttpResponse, HttpBodyWriterLarge) { crow::Response res; std::string data = generateBigdata(); - std::string path = makeFile(data); - { - boost::beast::file_posix file; - boost::system::error_code ec; - file.open(path.c_str(), boost::beast::file_mode::read, ec); - EXPECT_EQ(ec.value(), 0); - res.openFd(file.native_handle()); - testFileData(res, data); - } - std::filesystem::remove(path); + TemporaryFileHandle temporaryFile(data); + + boost::beast::file_posix file; + boost::system::error_code ec; + file.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read, + ec); + EXPECT_EQ(ec.value(), 0); + res.openFd(file.native_handle()); + EXPECT_EQ(getData(res.response), data); } } // namespace -- cgit v1.2.3