crash_reporter: Fix unit tests to use ScopedTempDir
Some of the unit tests manually create their own temp
directories from a relative path. This creates a temp folder
in the folder that the test is run from. This is a problem
when using test_droid or a non-interactive ADB shell command,
since the root directory is not writable. Instead use a
ScopedTempDir to handle temporary folders.
Bug: 26671657
Change-Id: Iaf71d345565dcb4393e80426784a864f7f8b5929
Test: Unit tests pass when running brillo_WhitelistedGtests
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index b55c324..11c8c0d 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 @@
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, 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 @@
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 @@
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, 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, 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 3bdeca1..56d2704 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 @@
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 @@
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 @@
}
UncleanShutdownCollectorMock collector_;
+
+ // Temporary directory used for tests.
+ base::ScopedTempDir test_dir_;
+ FilePath test_directory_;
FilePath test_unclean_;
};
@@ -84,7 +90,7 @@
}
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, 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 638ea34..1a62aac 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -65,8 +65,10 @@
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();
}
@@ -87,6 +89,7 @@
UserCollectorMock collector_;
pid_t pid_;
+ base::ScopedTempDir test_dir_;
};
TEST_F(UserCollectorTest, ParseCrashAttributes) {
@@ -173,14 +176,15 @@
&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);
@@ -341,13 +345,13 @@
}
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 {
@@ -371,9 +375,7 @@
}
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));
@@ -392,9 +394,7 @@
}
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