AU/unittest: postinstall tests use uniquely named stateful + working directories

This should allow these tests to be run in parallel. Also includes an
extension to PostinstallRunnerAction and two global powerwash marker
utility functions to allow testing with uniquely named marker file paths
(necessary to avoid race conditions when multiple tests are run in
parallel).

BUG=chromium:236465
TEST=Uniquely named directories created, used and removed.

Change-Id: I5dde0c0732c51e9e3bb2240cf7e0cac03bcde529
Reviewed-on: https://gerrit.chromium.org/gerrit/60864
Commit-Queue: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
diff --git a/postinstall_runner_action.cc b/postinstall_runner_action.cc
index 1143447..0f8059e 100644
--- a/postinstall_runner_action.cc
+++ b/postinstall_runner_action.cc
@@ -58,7 +58,7 @@
   completer.set_should_complete(false);
 
   if (install_plan_.powerwash_required) {
-    if (utils::CreatePowerwashMarkerFile()) {
+    if (utils::CreatePowerwashMarkerFile(powerwash_marker_file_)) {
       powerwash_marker_created_ = true;
     } else {
       completer.set_code(kErrorCodePostinstallPowerwashError);
@@ -84,7 +84,7 @@
 
     // Undo any changes done to trigger Powerwash using clobber-state.
     if (powerwash_marker_created_)
-      utils::DeletePowerwashMarkerFile();
+      utils::DeletePowerwashMarkerFile(powerwash_marker_file_);
 
     if (return_code == 3) {
       // This special return code means that we tried to update firmware,
diff --git a/postinstall_runner_action.h b/postinstall_runner_action.h
index 2308a0f..0136ee2 100644
--- a/postinstall_runner_action.h
+++ b/postinstall_runner_action.h
@@ -17,7 +17,9 @@
 
 class PostinstallRunnerAction : public InstallPlanAction {
  public:
-  PostinstallRunnerAction(): powerwash_marker_created_(false) {}
+  PostinstallRunnerAction()
+      : powerwash_marker_created_(false),
+        powerwash_marker_file_(NULL) {}
 
   void PerformAction();
 
@@ -42,6 +44,17 @@
   // False otherwise. Used for cleaning up if post-install fails.
   bool powerwash_marker_created_;
 
+  // Non-NULL value will cause post-install to override the default marker file
+  // name; used for testing.
+  const char* powerwash_marker_file_;
+
+  // Special ctor + friend declaration for testing purposes.
+  PostinstallRunnerAction(const char* powerwash_marker_file)
+      : powerwash_marker_created_(false),
+        powerwash_marker_file_(powerwash_marker_file) {}
+
+  friend class PostinstallRunnerActionTest;
+
   DISALLOW_COPY_AND_ASSIGN(PostinstallRunnerAction);
 };
 
diff --git a/postinstall_runner_action_unittest.cc b/postinstall_runner_action_unittest.cc
index fdde180..0deb859 100644
--- a/postinstall_runner_action_unittest.cc
+++ b/postinstall_runner_action_unittest.cc
@@ -38,6 +38,9 @@
   // DoTest with various combinations of do_losetup, err_code and
   // powerwash_required.
   void DoTest(bool do_losetup, int err_code, bool powerwash_required);
+
+ private:
+  static const char* kImageMountPointTemplate;
 };
 
 class PostinstActionProcessorDelegate : public ActionProcessorDelegate {
@@ -89,6 +92,9 @@
   DoTest(true, 3, false);
 }
 
+const char* PostinstallRunnerActionTest::kImageMountPointTemplate =
+    "au_destination-XXXXXX";
+
 void PostinstallRunnerActionTest::DoTest(
     bool do_losetup,
     int err_code,
@@ -98,46 +104,50 @@
   // True if the post-install action is expected to succeed.
   bool should_succeed = do_losetup && !err_code;
 
-  const string mountpoint(string(kStatefulPartition) +
-                          "/au_destination");
-
-  string cwd;
+  string orig_cwd;
   {
     vector<char> buf(1000);
     ASSERT_EQ(&buf[0], getcwd(&buf[0], buf.size()));
-    cwd = string(&buf[0], strlen(&buf[0]));
+    orig_cwd = string(&buf[0], strlen(&buf[0]));
   }
 
-  // create the au destination, if it doesn't exist
-  ASSERT_EQ(0, System(string("mkdir -p ") + mountpoint));
+  // Create a unique named working directory and chdir into it.
+  string cwd;
+  ASSERT_TRUE(utils::MakeTempDirectory(
+          orig_cwd + "/postinstall_runner_action_unittest-XXXXXX",
+          &cwd));
+  ASSERT_EQ(0, Chdir(cwd));
 
-  // create 10MiB sparse file
-  ASSERT_EQ(0, system("dd if=/dev/zero of=image.dat seek=10485759 bs=1 "
-                      "count=1"));
-
-  // format it as ext2
+  // Create a 10MiB sparse file to be used as image; format it as ext2.
+  ASSERT_EQ(0, system(
+          "dd if=/dev/zero of=image.dat seek=10485759 bs=1 count=1"));
   ASSERT_EQ(0, system("mkfs.ext2 -F image.dat"));
 
-  // mount it
+  // Create a uniquely named image mount point, mount the image.
+  ASSERT_EQ(0, System(string("mkdir -p ") + kStatefulPartition));
+  string mountpoint;
+  ASSERT_TRUE(utils::MakeTempDirectory(
+          string(kStatefulPartition) + "/" + kImageMountPointTemplate,
+          &mountpoint));
   ASSERT_EQ(0, System(string("mount -o loop image.dat ") + mountpoint));
 
-  // put a postinst script in
-  string script = StringPrintf("#!/bin/bash\n"
-                               "mount | grep au_postint_mount | grep ext2\n"
-                               "if [ $? -eq 0 ]; then\n"
-                               "  touch %s/postinst_called\n"
-                               "fi\n",
-                               cwd.c_str());
-  if (err_code) {
-    script = StringPrintf("#!/bin/bash\nexit %d", err_code);
-  }
-  ASSERT_TRUE(WriteFileString(mountpoint + "/postinst", script));
-  ASSERT_EQ(0, System(string("chmod a+x ") + mountpoint + "/postinst"));
+  // Generate a fake postinst script inside the image.
+  string script = (err_code ?
+                   StringPrintf("#!/bin/bash\nexit %d", err_code) :
+                   StringPrintf("#!/bin/bash\n"
+                                "mount | grep au_postint_mount | grep ext2\n"
+                                "if [ $? -eq 0 ]; then\n"
+                                "  touch %s/postinst_called\n"
+                                "fi\n",
+                                cwd.c_str()));
+  const string script_file_name = mountpoint + "/postinst";
+  ASSERT_TRUE(WriteFileString(script_file_name, script));
+  ASSERT_EQ(0, System(string("chmod a+x ") + script_file_name));
 
+  // Unmount image; do not remove the uniquely named directory as it will be
+  // reused during the test.
   ASSERT_TRUE(utils::UnmountFilesystem(mountpoint));
 
-  ASSERT_EQ(0, System(string("rm -f ") + cwd + "/postinst_called"));
-
   // get a loop device we can use for the install device
   string dev = "/dev/null";
 
@@ -147,13 +157,17 @@
                                                        &dev));
   }
 
+  // We use a test-specific powerwash marker file, to avoid race conditions.
+  string powerwash_marker_file = mountpoint + "/factory_install_reset";
+  LOG(INFO) << ">>> powerwash_marker_file=" << powerwash_marker_file;
+
   ActionProcessor processor;
   ObjectFeederAction<InstallPlan> feeder_action;
   InstallPlan install_plan;
   install_plan.install_path = dev;
   install_plan.powerwash_required = powerwash_required;
   feeder_action.set_obj(install_plan);
-  PostinstallRunnerAction runner_action;
+  PostinstallRunnerAction runner_action(powerwash_marker_file.c_str());
   BondActions(&feeder_action, &runner_action);
   ObjectCollectorAction<InstallPlan> collector_action;
   BondActions(&runner_action, &collector_action);
@@ -176,7 +190,7 @@
   if (should_succeed)
     EXPECT_TRUE(install_plan == collector_action.object());
 
-  const FilePath kPowerwashMarkerPath(kPowerwashMarkerFile);
+  const FilePath kPowerwashMarkerPath(powerwash_marker_file);
   string actual_cmd;
   if (should_succeed && powerwash_required) {
     EXPECT_TRUE(file_util::ReadFileToString(kPowerwashMarkerPath, &actual_cmd));
@@ -199,9 +213,13 @@
   if (do_losetup) {
     loop_releaser.reset(NULL);
   }
-  ASSERT_EQ(0, System(string("rm -f ") + cwd + "/postinst_called"));
-  ASSERT_EQ(0, System(string("rm -f ") + cwd + "/image.dat"));
-  utils::DeletePowerwashMarkerFile();
+
+  // Remove unique stateful directory.
+  ASSERT_EQ(0, System(string("rm -fr ") + mountpoint));
+
+  // Remove the temporary work directory.
+  ASSERT_EQ(0, Chdir(orig_cwd));
+  ASSERT_EQ(0, System(string("rm -fr ") + cwd));
 }
 
 // Death tests don't seem to be working on Hardy
diff --git a/test_utils.h b/test_utils.h
index 4285c4d..6d3c5bd 100644
--- a/test_utils.h
+++ b/test_utils.h
@@ -62,6 +62,10 @@
   return mkdir(path.c_str(), mode);
 }
 
+inline int Chdir(const std::string& path) {
+  return chdir(path.c_str());
+}
+
 void FillWithData(std::vector<char>* buffer);
 
 namespace {
diff --git a/utils.cc b/utils.cc
index 9d3bd26..74e0627 100644
--- a/utils.cc
+++ b/utils.cc
@@ -1006,30 +1006,31 @@
   return "Unknown error: " + base::UintToString(static_cast<unsigned>(code));
 }
 
-bool CreatePowerwashMarkerFile() {
-  bool result = utils::WriteFile(kPowerwashMarkerFile,
+bool CreatePowerwashMarkerFile(const char* file_path) {
+  const char* marker_file = file_path ? file_path : kPowerwashMarkerFile;
+  bool result = utils::WriteFile(marker_file,
                                  kPowerwashCommand,
                                  strlen(kPowerwashCommand));
-  if (result)
-    LOG(INFO) << "Created " << kPowerwashMarkerFile
-              << " to powerwash on next reboot";
-  else
-    PLOG(ERROR) << "Error in creating powerwash marker file: "
-                << kPowerwashMarkerFile;
+  if (result) {
+    LOG(INFO) << "Created " << marker_file << " to powerwash on next reboot";
+  } else {
+    PLOG(ERROR) << "Error in creating powerwash marker file: " << marker_file;
+  }
 
   return result;
 }
 
-bool DeletePowerwashMarkerFile() {
-  const FilePath kPowerwashMarkerPath(kPowerwashMarkerFile);
+bool DeletePowerwashMarkerFile(const char* file_path) {
+  const char* marker_file = file_path ? file_path : kPowerwashMarkerFile;
+  const FilePath kPowerwashMarkerPath(marker_file);
   bool result = file_util::Delete(kPowerwashMarkerPath, false);
 
   if (result)
     LOG(INFO) << "Successfully deleted the powerwash marker file : "
-              << kPowerwashMarkerFile;
+              << marker_file;
   else
     PLOG(ERROR) << "Could not delete the powerwash marker file : "
-                << kPowerwashMarkerFile;
+                << marker_file;
 
   return result;
 }
diff --git a/utils.h b/utils.h
index bd4d2d6..9b93134 100644
--- a/utils.h
+++ b/utils.h
@@ -318,13 +318,15 @@
 // error codes or the bit flags) for logging purposes.
 std::string CodeToString(ErrorCode code);
 
-// Creates the powerwash marker file with the appropriate commands in it.
-// Returns true if successfully created. False otherwise.
-bool CreatePowerwashMarkerFile();
+// Creates the powerwash marker file with the appropriate commands in it.  Uses
+// |file_path| as the path to the marker file if non-NULL, otherwise uses the
+// global default. Returns true if successfully created.  False otherwise.
+bool CreatePowerwashMarkerFile(const char* file_path);
 
-// Deletes the marker file used to trigger Powerwash using clobber-state.
-// Returns true if successfully deleted. False otherwise.
-bool DeletePowerwashMarkerFile();
+// Deletes the marker file used to trigger Powerwash using clobber-state.  Uses
+// |file_path| as the path to the marker file if non-NULL, otherwise uses the
+// global default. Returns true if successfully deleted. False otherwise.
+bool DeletePowerwashMarkerFile(const char* file_path);
 
 // Assumes you want to install on the "other" device, where the other
 // device is what you get if you swap 1 for 2 or 3 for 4 or vice versa