Merge "crash_reporter: Fix unit tests to use ScopedTempDir"

3 files changed, 66 insertions, 55 deletions

diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index b55c324a9..11c8c0d82 100644
--- a/crash_reporter/crash_collector_test.cc
+++ b/crash_reporter/crash_collector_test.cc
@@ -20,6 +20,7 @@
 #include <utility>
 
 #include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
 #include <base/strings/string_util.h>
 #include <base/strings/stringprintf.h>
 #include <brillo/syslog_logging.h>
@@ -52,20 +53,17 @@ class CrashCollectorTest : public ::testing::Test {
     EXPECT_CALL(collector_, SetUpDBus()).WillRepeatedly(Return());
 
     collector_.Initialize(CountCrash, IsMetrics);
-    test_dir_ = FilePath("test");
-    base::CreateDirectory(test_dir_);
+    EXPECT_TRUE(test_dir_.CreateUniqueTempDir());
     brillo::ClearLog();
   }
 
-  void TearDown() {
-    base::DeleteFile(test_dir_, true);
-  }
-
   bool CheckHasCapacity();
 
  protected:
   CrashCollectorMock collector_;
-  FilePath test_dir_;
+
+  // Temporary directory used for tests.
+  base::ScopedTempDir test_dir_;
 };
 
 TEST_F(CrashCollectorTest, Initialize) {
@@ -74,7 +72,7 @@ TEST_F(CrashCollectorTest, Initialize) {
 }
 
 TEST_F(CrashCollectorTest, WriteNewFile) {
-  FilePath test_file = test_dir_.Append("test_new");
+  FilePath test_file = test_dir_.path().Append("test_new");
   const char kBuffer[] = "buffer";
   EXPECT_EQ(strlen(kBuffer),
             collector_.WriteNewFile(test_file,
@@ -122,8 +120,10 @@ TEST_F(CrashCollectorTest, GetCrashPath) {
 
 
 bool CrashCollectorTest::CheckHasCapacity() {
-  static const char kFullMessage[] = "Crash directory test already full";
-  bool has_capacity = collector_.CheckHasCapacity(test_dir_);
+  const char* kFullMessage =
+      StringPrintf("Crash directory %s already full",
+                   test_dir_.path().value().c_str()).c_str();
+  bool has_capacity = collector_.CheckHasCapacity(test_dir_.path());
   bool has_message = FindLog(kFullMessage);
   EXPECT_EQ(has_message, !has_capacity);
   return has_capacity;
@@ -132,19 +132,22 @@ bool CrashCollectorTest::CheckHasCapacity() {
 TEST_F(CrashCollectorTest, CheckHasCapacityUsual) {
   // Test kMaxCrashDirectorySize - 1 non-meta files can be added.
   for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) {
-    base::WriteFile(test_dir_.Append(StringPrintf("file%d.core", i)), "", 0);
+    base::WriteFile(test_dir_.path().Append(StringPrintf("file%d.core", i)),
+                    "", 0);
     EXPECT_TRUE(CheckHasCapacity());
   }
 
   // Test an additional kMaxCrashDirectorySize - 1 meta files fit.
   for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) {
-    base::WriteFile(test_dir_.Append(StringPrintf("file%d.meta", i)), "", 0);
+    base::WriteFile(test_dir_.path().Append(StringPrintf("file%d.meta", i)),
+                    "", 0);
     EXPECT_TRUE(CheckHasCapacity());
   }
 
   // Test an additional kMaxCrashDirectorySize meta files don't fit.
   for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize; ++i) {
-    base::WriteFile(test_dir_.Append(StringPrintf("overage%d.meta", i)), "", 0);
+    base::WriteFile(test_dir_.path().Append(StringPrintf("overage%d.meta", i)),
+                    "", 0);
     EXPECT_FALSE(CheckHasCapacity());
   }
 }
@@ -152,50 +155,52 @@ TEST_F(CrashCollectorTest, CheckHasCapacityUsual) {
 TEST_F(CrashCollectorTest, CheckHasCapacityCorrectBasename) {
   // Test kMaxCrashDirectorySize - 1 files can be added.
   for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) {
-    base::WriteFile(test_dir_.Append(StringPrintf("file.%d.core", i)), "", 0);
+    base::WriteFile(test_dir_.path().Append(StringPrintf("file.%d.core", i)),
+                    "", 0);
     EXPECT_TRUE(CheckHasCapacity());
   }
-  base::WriteFile(test_dir_.Append("file.last.core"), "", 0);
+  base::WriteFile(test_dir_.path().Append("file.last.core"), "", 0);
   EXPECT_FALSE(CheckHasCapacity());
 }
 
 TEST_F(CrashCollectorTest, CheckHasCapacityStrangeNames) {
   // Test many files with different extensions and same base fit.
   for (int i = 0; i < 5 * CrashCollector::kMaxCrashDirectorySize; ++i) {
-    base::WriteFile(test_dir_.Append(StringPrintf("a.%d", i)), "", 0);
+    base::WriteFile(test_dir_.path().Append(StringPrintf("a.%d", i)), "", 0);
     EXPECT_TRUE(CheckHasCapacity());
   }
   // Test dot files are treated as individual files.
   for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 2; ++i) {
-    base::WriteFile(test_dir_.Append(StringPrintf(".file%d", i)), "", 0);
+    base::WriteFile(test_dir_.path().Append(StringPrintf(".file%d", i)), "", 0);
     EXPECT_TRUE(CheckHasCapacity());
   }
-  base::WriteFile(test_dir_.Append("normal.meta"), "", 0);
+  base::WriteFile(test_dir_.path().Append("normal.meta"), "", 0);
   EXPECT_FALSE(CheckHasCapacity());
 }
 
 TEST_F(CrashCollectorTest, MetaData) {
   const char kMetaFileBasename[] = "generated.meta";
-  FilePath meta_file = test_dir_.Append(kMetaFileBasename);
-  FilePath payload_file = test_dir_.Append("payload-file");
+  FilePath meta_file = test_dir_.path().Append(kMetaFileBasename);
+  FilePath payload_file = test_dir_.path().Append("payload-file");
   std::string contents;
   const char kPayload[] = "foo";
   ASSERT_TRUE(base::WriteFile(payload_file, kPayload, strlen(kPayload)));
   collector_.AddCrashMetaData("foo", "bar");
   collector_.WriteCrashMetaData(meta_file, "kernel", payload_file.value());
   EXPECT_TRUE(base::ReadFileToString(meta_file, &contents));
-  const char kExpectedMeta[] =
-      "foo=bar\n"
-      "exec_name=kernel\n"
-      "payload=test/payload-file\n"
-      "payload_size=3\n"
-      "done=1\n";
+  const std::string kExpectedMeta =
+      StringPrintf("foo=bar\n"
+          "exec_name=kernel\n"
+          "payload=%s\n"
+          "payload_size=3\n"
+          "done=1\n",
+          test_dir_.path().Append("payload-file").value().c_str());
   EXPECT_EQ(kExpectedMeta, contents);
 
   // Test target of symlink is not overwritten.
-  payload_file = test_dir_.Append("payload2-file");
+  payload_file = test_dir_.path().Append("payload2-file");
   ASSERT_TRUE(base::WriteFile(payload_file, kPayload, strlen(kPayload)));
-  FilePath meta_symlink_path = test_dir_.Append("symlink.meta");
+  FilePath meta_symlink_path = test_dir_.path().Append("symlink.meta");
   ASSERT_EQ(0,
             symlink(kMetaFileBasename,
                     meta_symlink_path.value().c_str()));
@@ -221,8 +226,8 @@ TEST_F(CrashCollectorTest, MetaData) {
 }
 
 TEST_F(CrashCollectorTest, GetLogContents) {
-  FilePath config_file = test_dir_.Append("crash_config");
-  FilePath output_file = test_dir_.Append("crash_log");
+  FilePath config_file = test_dir_.path().Append("crash_config");
+  FilePath output_file = test_dir_.path().Append("crash_log");
   const char kConfigContents[] =
       "foobar=echo hello there | \\\n  sed -e \"s/there/world/\"";
   ASSERT_TRUE(
diff --git a/crash_reporter/unclean_shutdown_collector_test.cc b/crash_reporter/unclean_shutdown_collector_test.cc
index 3bdeca10c..56d27045e 100644
--- a/crash_reporter/unclean_shutdown_collector_test.cc
+++ b/crash_reporter/unclean_shutdown_collector_test.cc
@@ -19,6 +19,7 @@
 #include <unistd.h>
 
 #include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
 #include <base/strings/string_util.h>
 #include <brillo/syslog_logging.h>
 #include <gmock/gmock.h>
@@ -32,10 +33,6 @@ namespace {
 int s_crashes = 0;
 bool s_metrics = true;
 
-const char kTestDirectory[] = "test";
-const char kTestSuspended[] = "test/suspended";
-const char kTestUnclean[] = "test/unclean";
-
 void CountCrash() {
   ++s_crashes;
 }
@@ -59,12 +56,17 @@ class UncleanShutdownCollectorTest : public ::testing::Test {
 
     collector_.Initialize(CountCrash,
                           IsMetrics);
-    rmdir(kTestDirectory);
-    test_unclean_ = FilePath(kTestUnclean);
-    collector_.unclean_shutdown_file_ = kTestUnclean;
+
+    EXPECT_TRUE(test_dir_.CreateUniqueTempDir());
+
+    test_directory_ = test_dir_.path().Append("test");
+    test_unclean_ = test_dir_.path().Append("test/unclean");
+
+    collector_.unclean_shutdown_file_ = test_unclean_.value().c_str();
     base::DeleteFile(test_unclean_, true);
     // Set up an alternate power manager state file as well
-    collector_.powerd_suspended_file_ = FilePath(kTestSuspended);
+    collector_.powerd_suspended_file_ =
+        test_dir_.path().Append("test/suspended");
     brillo::ClearLog();
   }
 
@@ -75,6 +77,10 @@ class UncleanShutdownCollectorTest : public ::testing::Test {
   }
 
   UncleanShutdownCollectorMock collector_;
+
+  // Temporary directory used for tests.
+  base::ScopedTempDir test_dir_;
+  FilePath test_directory_;
   FilePath test_unclean_;
 };
 
@@ -84,7 +90,7 @@ TEST_F(UncleanShutdownCollectorTest, EnableWithoutParent) {
 }
 
 TEST_F(UncleanShutdownCollectorTest, EnableWithParent) {
-  mkdir(kTestDirectory, 0777);
+  mkdir(test_directory_.value().c_str(), 0777);
   ASSERT_TRUE(collector_.Enable());
   ASSERT_TRUE(base::PathExists(test_unclean_));
 }
@@ -133,15 +139,15 @@ TEST_F(UncleanShutdownCollectorTest, DisableWhenNotEnabled) {
 }
 
 TEST_F(UncleanShutdownCollectorTest, CantDisable) {
-  mkdir(kTestDirectory, 0700);
-  if (mkdir(kTestUnclean, 0700)) {
+  mkdir(test_directory_.value().c_str(), 0700);
+  if (mkdir(test_unclean_.value().c_str(), 0700)) {
     ASSERT_EQ(EEXIST, errno)
-        << "Error while creating directory '" << kTestUnclean
+        << "Error while creating directory '" << test_unclean_.value()
         << "': " << strerror(errno);
   }
   ASSERT_EQ(0, base::WriteFile(test_unclean_.Append("foo"), "", 0))
       << "Error while creating empty file '"
       << test_unclean_.Append("foo").value() << "': " << strerror(errno);
   ASSERT_FALSE(collector_.Disable());
-  rmdir(kTestUnclean);
+  rmdir(test_unclean_.value().c_str());
 }
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index c82271874..d9c9a5b1b 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -65,8 +65,10 @@ class UserCollectorTest : public ::testing::Test {
                           false,
                           false,
                           "");
-    base::DeleteFile(FilePath("test"), true);
-    mkdir("test", 0777);
+
+    EXPECT_TRUE(test_dir_.CreateUniqueTempDir());
+
+    mkdir(test_dir_.path().Append("test").value().c_str(), 0777);
     pid_ = getpid();
     brillo::ClearLog();
   }
@@ -86,6 +88,7 @@ class UserCollectorTest : public ::testing::Test {
 
   UserCollectorMock collector_;
   pid_t pid_;
+  base::ScopedTempDir test_dir_;
 };
 
 TEST_F(UserCollectorTest, ParseCrashAttributes) {
@@ -172,14 +175,15 @@ TEST_F(UserCollectorTest, GetSymlinkTarget) {
                                            &result));
   ASSERT_TRUE(FindLog(
       "Readlink failed on /does_not_exist with 2"));
-  std::string long_link;
+  std::string long_link = test_dir_.path().value();
   for (int i = 0; i < 50; ++i)
     long_link += "0123456789";
   long_link += "/gold";
 
   for (size_t len = 1; len <= long_link.size(); ++len) {
     std::string this_link;
-    static const char kLink[] = "test/this_link";
+    static const char* kLink =
+        test_dir_.path().Append("test/this_link").value().c_str();
     this_link.assign(long_link.c_str(), len);
     ASSERT_EQ(len, this_link.size());
     unlink(kLink);
@@ -340,13 +344,13 @@ TEST_F(UserCollectorTest, CopyOffProcFilesBadPath) {
 }
 
 TEST_F(UserCollectorTest, CopyOffProcFilesBadPid) {
-  FilePath container_path("test/container");
+  FilePath container_path(test_dir_.path().Append("test/container"));
   ASSERT_FALSE(collector_.CopyOffProcFiles(0, container_path));
   EXPECT_TRUE(FindLog("Path /proc/0 does not exist"));
 }
 
 TEST_F(UserCollectorTest, CopyOffProcFilesOK) {
-  FilePath container_path("test/container");
+  FilePath container_path(test_dir_.path().Append("test/container"));
   ASSERT_TRUE(collector_.CopyOffProcFiles(pid_, container_path));
   EXPECT_FALSE(FindLog("Could not copy"));
   static struct {
@@ -370,9 +374,7 @@ TEST_F(UserCollectorTest, CopyOffProcFilesOK) {
 }
 
 TEST_F(UserCollectorTest, ValidateProcFiles) {
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  FilePath container_dir = temp_dir.path();
+  FilePath container_dir = test_dir_.path();
 
   // maps file not exists (i.e. GetFileSize fails)
   EXPECT_FALSE(collector_.ValidateProcFiles(container_dir));
@@ -391,9 +393,7 @@ TEST_F(UserCollectorTest, ValidateProcFiles) {
 }
 
 TEST_F(UserCollectorTest, ValidateCoreFile) {
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  FilePath container_dir = temp_dir.path();
+  FilePath container_dir = test_dir_.path();
   FilePath core_file = container_dir.Append("core");
 
   // Core file does not exist