[windows] Fix parallel runs of installer_util_unittests

Use the RegistryOverrideManager in various test fixtures so that tests can run in parallel and so that their state is properly cleaned up. Fixed: 1475851 Change-Id: Id62bf373cbe961d45f179f50c61e30f609db3438 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4812259 Auto-Submit: Greg Thompson <[email protected]> Reviewed-by: Yann Dago <[email protected]> Commit-Queue: Yann Dago <[email protected]> Cr-Commit-Position: refs/heads/main@{#1189486}
MyoungJinNam · Aug 29, 2023 · 7a29424 · 7a29424
1 parent f77189d
commit 7a29424
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 78 deletions.
diff --git a/chrome/installer/util/create_reg_key_work_item_unittest.cc b/chrome/installer/util/create_reg_key_work_item_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/strings/string_util.h"
+#include "base/test/test_reg_util_win.h"
 #include "base/win/registry.h"
 #include "chrome/installer/util/work_item.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -19,24 +20,21 @@ using base::win::RegKey;
 
 namespace {
 
-wchar_t test_root[] = L"TmpTmp";
-
 class CreateRegKeyWorkItemTest : public testing::Test {
  protected:
   void SetUp() override {
-    // Create a temporary key for testing
-    RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
-    key.DeleteKey(test_root);
-    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
-    ASSERT_EQ(ERROR_SUCCESS,
-              key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_NO_FATAL_FAILURE(
+        registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER));
+    ASSERT_TRUE(RegKey(HKEY_CURRENT_USER, test_root, KEY_SET_VALUE).Valid());
   }
   void TearDown() override {
     logging::CloseLogFile();
-    // Clean up the temporary key
-    RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
-    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(test_root));
   }
+
+  static constexpr wchar_t test_root[] = L"TmpTmp";
+
+ private:
+  registry_util::RegistryOverrideManager registry_override_manager_;
 };
 
 }  // namespace

diff --git a/chrome/installer/util/registry_key_backup_unittest.cc b/chrome/installer/util/registry_key_backup_unittest.cc
@@ -84,8 +84,9 @@ TEST_F(RegistryKeyBackupTest, Swap) {
                                    WorkItem::kWow64Default));
 
   // Now make sure the one we started with is truly empty.
-  EXPECT_EQ(ERROR_SUCCESS, RegKey(test_data_.root_key(), L"", KEY_QUERY_VALUE)
-                               .DeleteKey(destination_path_.c_str()));
+  EXPECT_EQ(ERROR_SUCCESS, RegKey(test_data_.root_key(),
+                                  destination_path_.c_str(), KEY_QUERY_VALUE)
+                               .DeleteKey(L""));
   EXPECT_TRUE(backup.WriteTo(test_data_.root_key(), destination_path_.c_str(),
                              WorkItem::kWow64Default));
   EXPECT_FALSE(

diff --git a/chrome/installer/util/registry_test_data.cc b/chrome/installer/util/registry_test_data.cc
@@ -4,76 +4,55 @@
 
 #include "chrome/installer/util/registry_test_data.h"
 
+#include <windows.h>
+
 #include "base/logging.h"
+#include "base/strings/strcat.h"
 #include "base/win/registry.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::win::RegKey;
 
 RegistryTestData::RegistryTestData() : root_key_(nullptr) {}
 
-RegistryTestData::~RegistryTestData() {
-  Reset();
-}
+RegistryTestData::~RegistryTestData() = default;
 
-// static
-bool RegistryTestData::DeleteKey(HKEY root_key, const wchar_t* path) {
-  LONG result = ERROR_SUCCESS;
-  if (root_key != nullptr && path != nullptr && *path != L'\0') {
-    result = RegKey(root_key, L"", KEY_QUERY_VALUE).DeleteKey(path);
-    if (result == ERROR_FILE_NOT_FOUND) {
-      result = ERROR_SUCCESS;
-    } else {
-      DLOG_IF(DFATAL, result != ERROR_SUCCESS)
-          << "Failed to delete registry key " << path << ", result: " << result;
-    }
+bool RegistryTestData::Initialize(HKEY root_key, const wchar_t* base_path) {
+  if (root_key_) {
+    return false;
   }
-  return result == ERROR_SUCCESS;
-}
 
-void RegistryTestData::Reset() {
-  // Clean up behind a previous use (ignore failures).
-  DeleteKey(root_key_, base_path_.c_str());
+  registry_override_manager_.OverrideRegistry(root_key);
+  if (::testing::Test::HasFatalFailure()) {
+    return false;
+  }
+  LONG result = ERROR_SUCCESS;
 
-  // Forget state.
-  root_key_ = nullptr;
-  base_path_.clear();
-  empty_key_path_.clear();
-  non_empty_key_path_.clear();
-}
+  root_key_ = root_key;
+  base_path_.assign(base_path);
 
-bool RegistryTestData::Initialize(HKEY root_key, const wchar_t* base_path) {
-  LONG result = ERROR_SUCCESS;
+  // Create our data.
+  empty_key_path_ = base::StrCat({base_path_, L"\\EmptyKey"});
+  non_empty_key_path_ = base::StrCat({base_path_, L"\\NonEmptyKey"});
+
+  RegKey key;
 
-  Reset();
-
-  // Take over the new registry location.
-  if (DeleteKey(root_key, base_path)) {
-    // Take on the new values.
-    root_key_ = root_key;
-    base_path_.assign(base_path);
-
-    // Create our data.
-    empty_key_path_.assign(base_path_).append(L"\\EmptyKey");
-    non_empty_key_path_.assign(base_path_).append(L"\\NonEmptyKey");
-
-    RegKey key;
-
-    result = key.Create(root_key_, empty_key_path_.c_str(), KEY_QUERY_VALUE);
-    if (result == ERROR_SUCCESS)
-      result = key.Create(root_key_, non_empty_key_path_.c_str(), KEY_WRITE);
-    if (result == ERROR_SUCCESS)
-      result = key.WriteValue(nullptr, non_empty_key_path_.c_str());
-    if (result == ERROR_SUCCESS)
-      result = key.CreateKey(L"SubKey", KEY_WRITE);
-    if (result == ERROR_SUCCESS)
-      result = key.WriteValue(L"SomeValue", 1UL);
-    DLOG_IF(DFATAL, result != ERROR_SUCCESS)
-        << "Failed to create test registry data based at " << base_path_
-        << ", result: " << result;
-  } else {
-    result = ERROR_INVALID_PARAMETER;
+  result = key.Create(root_key_, empty_key_path_.c_str(), KEY_QUERY_VALUE);
+  if (result == ERROR_SUCCESS) {
+    result = key.Create(root_key_, non_empty_key_path_.c_str(), KEY_WRITE);
+  }
+  if (result == ERROR_SUCCESS) {
+    result = key.WriteValue(nullptr, non_empty_key_path_.c_str());
+  }
+  if (result == ERROR_SUCCESS) {
+    result = key.CreateKey(L"SubKey", KEY_WRITE);
+  }
+  if (result == ERROR_SUCCESS) {
+    result = key.WriteValue(L"SomeValue", 1UL);
   }
+  DLOG_IF(DFATAL, result != ERROR_SUCCESS)
+      << "Failed to create test registry data based at " << base_path_
+      << ", result: " << result;
 
   return result == ERROR_SUCCESS;
 }

diff --git a/chrome/installer/util/registry_test_data.h b/chrome/installer/util/registry_test_data.h
@@ -5,14 +5,12 @@
 #ifndef CHROME_INSTALLER_UTIL_REGISTRY_TEST_DATA_H_
 #define CHROME_INSTALLER_UTIL_REGISTRY_TEST_DATA_H_
 
-#include <windows.h>
-
 #include <string>
 
+#include "base/test/test_reg_util_win.h"
+#include "base/win/windows_types.h"
+
 // A helper class for use by unit tests that need some registry space and data.
-// BEWARE: Instances of this class irrevocably and recursively delete keys and
-// values from the registry.  Carefully read the comments for Initialize and
-// Reset before use.
 class RegistryTestData {
  public:
   RegistryTestData();
@@ -30,9 +28,6 @@ class RegistryTestData {
   // \NonEmptyKey\Subkey ("SomeValue" = DWORD 1)
   bool Initialize(HKEY root_key, const wchar_t* base_path);
 
-  // Deletes the key rooted at base_path and clears all state.
-  void Reset();
-
   // Fires Google Test expectations in the hopes that |path| contains the same
   // data as originally placed in |non_empty_key| by Initialize().
   void ExpectMatchesNonEmptyKey(HKEY root_key, const wchar_t* path);
@@ -47,8 +42,7 @@ class RegistryTestData {
   static void ExpectEmptyKey(HKEY root_key, const wchar_t* path);
 
  private:
-  static bool DeleteKey(HKEY root_key, const wchar_t* path);
-
+  registry_util::RegistryOverrideManager registry_override_manager_;
   HKEY root_key_;
   std::wstring base_path_;
   std::wstring empty_key_path_;