From 57efb038bb4537b17bcbd9a7d1324752849c5432 Mon Sep 17 00:00:00 2001 From: nisse Date: Thu, 18 May 2017 03:55:59 -0700 Subject: [PATCH] Reland of reduce dependencies on rtc::FileSystem in FileRotatingStream tests... (patchset #1 id:1 of https://codereview.webrtc.org/2885393002/ ) Reason for revert: Downstream project now fixed. Original issue's description: > Revert of Reduce dependencies on rtc::FileSystem in FileRotatingStream tests, adding helpers in webrtc::test:: (patchset #7 id:120001 of https://codereview.webrtc.org/2872283002/ ) > > Reason for revert: > Fails to compile successfully. > > > Original issue's description: > > 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} > > Committed: https://chromium.googlesource.com/external/webrtc/+/dd7b5f32b59d9def668b9e9487589a572f20f6e0 > > TBR=pthatcher@webrtc.org,kjellander@webrtc.org,tommi@webrtc.org,nisse@webrtc.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=webrtc:7345 > > Review-Url: https://codereview.webrtc.org/2885393002 > Cr-Commit-Position: refs/heads/master@{#18180} > Committed: https://chromium.googlesource.com/external/webrtc/+/deaa33d2f59783e849ddc01428be807d92431983 TBR=pthatcher@webrtc.org,kjellander@webrtc.org,tommi@webrtc.org,ehmaldonado@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:7345 Review-Url: https://codereview.webrtc.org/2885413002 Cr-Commit-Position: refs/heads/master@{#18193} --- 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);