From dd7b5f32b59d9def668b9e9487589a572f20f6e0 Mon Sep 17 00:00:00 2001 From: nisse Date: Wed, 17 May 2017 01:08:35 -0700 Subject: [PATCH] Reduce dependencies on rtc::FileSystem in FileRotatingStream tests. Use webrtc::test::OutputPath instead of Filesystem::GetAppTempFolder. Added functions RemoveFile and RemoveDir in the webrtc::test namespace, to replace use of Filesystem::DeleteFolderAndContents. This makes Filesystem::DeleteFolderAndContents unused, to be deleted together with related code in a followup cl. BUG=webrtc:7345 Review-Url: https://codereview.webrtc.org/2872283002 Cr-Commit-Position: refs/heads/master@{#18173} --- webrtc/base/filerotatingstream.h | 2 +- webrtc/base/filerotatingstream_unittest.cc | 52 +++++++++++++--------- webrtc/test/testsupport/fileutils.cc | 16 +++++++ webrtc/test/testsupport/fileutils.h | 6 +++ 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/webrtc/base/filerotatingstream.h b/webrtc/base/filerotatingstream.h index a8522ff0c5..a3e808cda5 100644 --- a/webrtc/base/filerotatingstream.h +++ b/webrtc/base/filerotatingstream.h @@ -68,7 +68,7 @@ class FileRotatingStream : public StreamInterface { std::string GetFilePath(size_t index) const; // Returns the number of files that will used by this stream. - size_t GetNumFiles() { return file_names_.size(); } + size_t GetNumFiles() const { return file_names_.size(); } protected: size_t GetMaxFileSize() const { return max_file_size_; } diff --git a/webrtc/base/filerotatingstream_unittest.cc b/webrtc/base/filerotatingstream_unittest.cc index 2a0e8589f4..7acb174bb3 100644 --- a/webrtc/base/filerotatingstream_unittest.cc +++ b/webrtc/base/filerotatingstream_unittest.cc @@ -16,9 +16,21 @@ #include "webrtc/base/fileutils.h" #include "webrtc/base/gunit.h" #include "webrtc/base/pathutils.h" +#include "webrtc/test/testsupport/fileutils.h" namespace rtc { +namespace { + +void CleanupLogDirectory(const FileRotatingStream& stream) { + for (size_t i = 0; i < stream.GetNumFiles(); ++i) { + // Ignore return value, not all files are expected to exist. + webrtc::test::RemoveFile(stream.GetFilePath(i)); + } +} + +} // namespace + #if defined (WEBRTC_ANDROID) // Fails on Android: https://bugs.chromium.org/p/webrtc/issues/detail?id=4364. #define MAYBE_FileRotatingStreamTest DISABLED_FileRotatingStreamTest @@ -35,23 +47,23 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test { const std::string& file_prefix, size_t max_file_size, size_t num_log_files) { - Pathname test_path; - ASSERT_TRUE(Filesystem::GetAppTempFolder(&test_path)); + dir_path_ = webrtc::test::OutputPath(); + // Append per-test output path in order to run within gtest parallel. - test_path.AppendFolder(dir_name); - ASSERT_TRUE(Filesystem::CreateFolder(test_path)); - dir_path_ = test_path.pathname(); - ASSERT_TRUE(dir_path_.size()); + dir_path_.append(dir_name); + dir_path_.push_back(Pathname::DefaultFolderDelimiter()); + ASSERT_TRUE(webrtc::test::CreateDir(dir_path_)); stream_.reset(new FileRotatingStream(dir_path_, file_prefix, max_file_size, num_log_files)); } void TearDown() override { + // On windows, open files can't be removed. + stream_->Close(); + CleanupLogDirectory(*stream_); + EXPECT_TRUE(webrtc::test::RemoveDir(dir_path_)); + stream_.reset(); - if (dir_path_.size() && Filesystem::IsFolder(dir_path_) && - Filesystem::IsTemporaryPath(dir_path_)) { - Filesystem::DeleteFolderAndContents(dir_path_); - } } // Writes the data to the stream and flushes it. @@ -204,23 +216,23 @@ TEST_F(MAYBE_FileRotatingStreamTest, GetFilePath) { class MAYBE_CallSessionFileRotatingStreamTest : public ::testing::Test { protected: void Init(const std::string& dir_name, size_t max_total_log_size) { - Pathname test_path; - ASSERT_TRUE(Filesystem::GetAppTempFolder(&test_path)); + dir_path_ = webrtc::test::OutputPath(); + // Append per-test output path in order to run within gtest parallel. - test_path.AppendFolder(dir_name); - ASSERT_TRUE(Filesystem::CreateFolder(test_path)); - dir_path_ = test_path.pathname(); - ASSERT_TRUE(dir_path_.size()); + dir_path_.append(dir_name); + dir_path_.push_back(Pathname::DefaultFolderDelimiter()); + ASSERT_TRUE(webrtc::test::CreateDir(dir_path_)); stream_.reset( new CallSessionFileRotatingStream(dir_path_, max_total_log_size)); } virtual void TearDown() { + // On windows, open files can't be removed. + stream_->Close(); + CleanupLogDirectory(*stream_); + EXPECT_TRUE(webrtc::test::RemoveDir(dir_path_)); + stream_.reset(); - if (dir_path_.size() && Filesystem::IsFolder(dir_path_) && - Filesystem::IsTemporaryPath(dir_path_)) { - Filesystem::DeleteFolderAndContents(dir_path_); - } } // Writes the data to the stream and flushes it. diff --git a/webrtc/test/testsupport/fileutils.cc b/webrtc/test/testsupport/fileutils.cc index 899bf31f90..02ef06580b 100644 --- a/webrtc/test/testsupport/fileutils.cc +++ b/webrtc/test/testsupport/fileutils.cc @@ -232,6 +232,22 @@ bool CreateDir(const std::string& directory_name) { return true; } +bool RemoveDir(const std::string& directory_name) { +#ifdef WIN32 + return RemoveDirectoryA(directory_name.c_str()) != FALSE; +#else + return rmdir(directory_name.c_str()) == 0; +#endif +} + +bool RemoveFile(const std::string& file_name) { +#ifdef WIN32 + return DeleteFileA(file_name.c_str()) != FALSE; +#else + return unlink(file_name.c_str()) == 0; +#endif +} + std::string ResourcePath(const std::string& name, const std::string& extension) { #if defined(WEBRTC_IOS) diff --git a/webrtc/test/testsupport/fileutils.h b/webrtc/test/testsupport/fileutils.h index 549f96b7e7..2135681380 100644 --- a/webrtc/test/testsupport/fileutils.h +++ b/webrtc/test/testsupport/fileutils.h @@ -71,6 +71,12 @@ std::string WorkingDir(); // false if a file with the same name already exists. bool CreateDir(const std::string& directory_name); +// Removes a directory, which must already be empty. +bool RemoveDir(const std::string& directory_name); + +// Removes a file. +bool RemoveFile(const std::string& file_name); + // Checks if a file exists. bool FileExists(const std::string& file_name);