Simplify WorkItemListTest.

chrome/installer/util/work_item_list_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <windows.h>
+#include "chrome/installer/util/work_item_list.h"
 
-#include "base/base_paths.h"
-#include "base/files/file_util.h"
+#include <memory>
+
+#include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/strings/string_util.h"
-#include "base/test/test_reg_util_win.h"
-#include "base/win/registry.h"
+#include "base/macros.h"
 #include "chrome/installer/util/conditional_work_item_list.h"
 #include "chrome/installer/util/work_item.h"
-#include "chrome/installer/util/work_item_list.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using base::win::RegKey;
-
 namespace {
 
-const wchar_t kTestRoot[] = L"ListList";
-const wchar_t kDataStr[] = L"data_111";
-const wchar_t kName[] = L"name";
+class MockWorkItem : public WorkItem {
+ public:
+  MockWorkItem() = default;
 
-class WorkItemListTest : public testing::Test {
- protected:
-  void SetUp() override {
-    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER);
-  }
+  MOCK_METHOD0(Do, bool());
+  MOCK_METHOD0(Rollback, void());
 
-  void TearDown() override { logging::CloseLogFile(); }
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockWorkItem);
+};
 
-  base::ScopedTempDir temp_dir_;
-  registry_util::RegistryOverrideManager registry_override_manager_;
-};
+using StrictMockWorkItem = testing::StrictMock<MockWorkItem>;
+using testing::InSequence;
+using testing::Return;
 
 }  // namespace
 
-// Execute a WorkItem list successfully and then rollback.
-TEST_F(WorkItemListTest, ExecutionSuccess) {
-  scoped_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
-  scoped_ptr<WorkItem> work_item;
+// Execute a WorkItemList successfully and then rollback.
+TEST(WorkItemListTest, ExecutionSuccess) {
+  std::unique_ptr<WorkItemList> list(WorkItem::CreateWorkItemList());
 
-  base::FilePath top_dir_to_create(temp_dir_.path());
-  top_dir_to_create = top_dir_to_create.AppendASCII("a");
-  base::FilePath dir_to_create(top_dir_to_create);
-  dir_to_create = dir_to_create.AppendASCII("b");
-  ASSERT_FALSE(base::PathExists(dir_to_create));
+  // Create the mock work items.
+  std::unique_ptr<StrictMockWorkItem> item1(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item2(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item3(new StrictMockWorkItem);
 
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateCreateDirWorkItem(dir_to_create)));
-  work_item_list->AddWorkItem(work_item.release());
+  {
+    // Expect all three items to be done in order then undone.
+    InSequence s;
 
-  std::wstring key_to_create(kTestRoot);
-  key_to_create.push_back(base::FilePath::kSeparators[0]);
-  key_to_create.append(L"ExecutionSuccess");
+    EXPECT_CALL(*item1, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item2, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item3, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item3, Rollback());
+    EXPECT_CALL(*item2, Rollback());
+    EXPECT_CALL(*item1, Rollback());
+  }
 
-  work_item.reset(
-      reinterpret_cast<WorkItem*>(WorkItem::CreateCreateRegKeyWorkItem(
-          HKEY_CURRENT_USER, key_to_create, WorkItem::kWow64Default)));
-  work_item_list->AddWorkItem(work_item.release());
+  // Add the items to the list.
+  list->AddWorkItem(item1.release());
+  list->AddWorkItem(item2.release());
+  list->AddWorkItem(item3.release());
 
-  std::wstring name(kName);
-  std::wstring data(kDataStr);
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          key_to_create,
-                                          WorkItem::kWow64Default,
-                                          name,
-                                          data,
-                                          false)));
-  work_item_list->AddWorkItem(work_item.release());
-
-  EXPECT_TRUE(work_item_list->Do());
-
-  // Verify all WorkItems have been executed.
-  RegKey key;
-  EXPECT_EQ(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  std::wstring read_out;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
-  EXPECT_EQ(0, read_out.compare(kDataStr));
-  key.Close();
-  EXPECT_TRUE(base::PathExists(dir_to_create));
-
-  work_item_list->Rollback();
-
-  // Verify everything is rolled back.
-  // The value must have been deleted first in roll back otherwise the key
-  // can not be deleted.
-  EXPECT_NE(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  EXPECT_FALSE(base::PathExists(top_dir_to_create));
+  // Do and rollback the list.
+  EXPECT_TRUE(list->Do());
+  list->Rollback();
 }
 
-// Execute a WorkItem list. Fail in the middle. Rollback what has been done.
-TEST_F(WorkItemListTest, ExecutionFailAndRollback) {
-  scoped_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
-  scoped_ptr<WorkItem> work_item;
+// Execute a WorkItemList. Fail in the middle. Rollback what has been done.
+TEST(WorkItemListTest, ExecutionFailAndRollback) {
+  std::unique_ptr<WorkItemList> list(WorkItem::CreateWorkItemList());
 
-  base::FilePath top_dir_to_create(temp_dir_.path());
-  top_dir_to_create = top_dir_to_create.AppendASCII("a");
-  base::FilePath dir_to_create(top_dir_to_create);
-  dir_to_create = dir_to_create.AppendASCII("b");
-  ASSERT_FALSE(base::PathExists(dir_to_create));
+  // Create the mock work items.
+  std::unique_ptr<StrictMockWorkItem> item1(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item2(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item3(new StrictMockWorkItem);
 
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateCreateDirWorkItem(dir_to_create)));
-  work_item_list->AddWorkItem(work_item.release());
+  {
+    // Expect the two first work items to be done in order then undone.
+    InSequence s;
 
-  std::wstring key_to_create(kTestRoot);
-  key_to_create.push_back(base::FilePath::kSeparators[0]);
-  key_to_create.append(L"ExecutionFail");
+    EXPECT_CALL(*item1, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item2, Do()).WillOnce(Return(false));
+    EXPECT_CALL(*item2, Rollback());
+    EXPECT_CALL(*item1, Rollback());
+  }
 
-  work_item.reset(
-      reinterpret_cast<WorkItem*>(WorkItem::CreateCreateRegKeyWorkItem(
-          HKEY_CURRENT_USER, key_to_create, WorkItem::kWow64Default)));
-  work_item_list->AddWorkItem(work_item.release());
+  // Add the items to the list.
+  list->AddWorkItem(item1.release());
+  list->AddWorkItem(item2.release());
+  list->AddWorkItem(item3.release());
 
-  std::wstring not_created_key(kTestRoot);
-  not_created_key.push_back(base::FilePath::kSeparators[0]);
-  not_created_key.append(L"NotCreated");
-  std::wstring name(kName);
-  std::wstring data(kDataStr);
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          not_created_key,
-                                          WorkItem::kWow64Default,
-                                          name,
-                                          data,
-                                          false)));
-  work_item_list->AddWorkItem(work_item.release());
-
-  // This one will not be executed because we will fail early.
-  work_item.reset(
-      reinterpret_cast<WorkItem*>(WorkItem::CreateCreateRegKeyWorkItem(
-          HKEY_CURRENT_USER, not_created_key, WorkItem::kWow64Default)));
-  work_item_list->AddWorkItem(work_item.release());
-
-  EXPECT_FALSE(work_item_list->Do());
-
-  // Verify the first 2 WorkItems have been executed.
-  RegKey key;
-  EXPECT_EQ(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  key.Close();
-  EXPECT_TRUE(base::PathExists(dir_to_create));
-  // The last one should not be there.
-  EXPECT_NE(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, not_created_key.c_str(), KEY_READ));
-
-  work_item_list->Rollback();
-
-  // Verify everything is rolled back.
-  EXPECT_NE(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  EXPECT_FALSE(base::PathExists(top_dir_to_create));
+  // Do and rollback the list.
+  EXPECT_FALSE(list->Do());
+  list->Rollback();
 }
 
-TEST_F(WorkItemListTest, ConditionalExecutionSuccess) {
-  scoped_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
-  scoped_ptr<WorkItem> work_item;
+// Execute a ConditionalWorkItemList whose condition is met and then rollback.
+TEST(WorkItemListTest, ConditionalExecutionSuccess) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  base::FilePath top_dir_to_create(temp_dir_.path());
-  top_dir_to_create = top_dir_to_create.AppendASCII("a");
-  base::FilePath dir_to_create(top_dir_to_create);
-  dir_to_create = dir_to_create.AppendASCII("b");
-  ASSERT_FALSE(base::PathExists(dir_to_create));
+  std::unique_ptr<WorkItemList> list(WorkItem::CreateConditionalWorkItemList(
+      new ConditionRunIfFileExists(temp_dir.path())));

[grt (UTC plus 2), 2016/04/13 15:37:07]

this is testing both CWIL and CRIFFE. could the latter be in its own test, and
this test simplified to using a mock Condition?

[fdoray, 2016/04/13 16:46:45]

Done.

[grt (UTC plus 2), 2016/04/13 18:08:51]

Sorry, forgot to mention before that these belong in
conditional_work_item_list_unittest.cc, no?

[fdoray, 2016/04/13 18:57:16]

Done.

 
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateCreateDirWorkItem(dir_to_create)));
-  work_item_list->AddWorkItem(work_item.release());
+  // Create the mock work items.
+  std::unique_ptr<StrictMockWorkItem> item1(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item2(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item3(new StrictMockWorkItem);
 
-  scoped_ptr<WorkItemList> conditional_work_item_list(
-      WorkItem::CreateConditionalWorkItemList(
-          new ConditionRunIfFileExists(dir_to_create)));
+  {
+    // Expect all three items to be done in order then undone.
+    InSequence s;
 
-  std::wstring key_to_create(kTestRoot);
-  key_to_create.push_back(base::FilePath::kSeparators[0]);
-  key_to_create.append(L"ExecutionSuccess");
-  work_item.reset(
-      reinterpret_cast<WorkItem*>(WorkItem::CreateCreateRegKeyWorkItem(
-          HKEY_CURRENT_USER, key_to_create, WorkItem::kWow64Default)));
-  conditional_work_item_list->AddWorkItem(work_item.release());
+    EXPECT_CALL(*item1, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item2, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item3, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item3, Rollback());
+    EXPECT_CALL(*item2, Rollback());
+    EXPECT_CALL(*item1, Rollback());
+  }
 
-  std::wstring name(kName);
-  std::wstring data(kDataStr);
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          key_to_create,
-                                          WorkItem::kWow64Default,
-                                          name,
-                                          data,
-                                          false)));
-  conditional_work_item_list->AddWorkItem(work_item.release());
+  // Add the items to the list.
+  list->AddWorkItem(item1.release());
+  list->AddWorkItem(item2.release());
+  list->AddWorkItem(item3.release());
 
-  work_item_list->AddWorkItem(conditional_work_item_list.release());
-
-  EXPECT_TRUE(work_item_list->Do());
-
-  // Verify all WorkItems have been executed.
-  RegKey key;
-  EXPECT_EQ(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  std::wstring read_out;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
-  EXPECT_EQ(0, read_out.compare(kDataStr));
-  key.Close();
-  EXPECT_TRUE(base::PathExists(dir_to_create));
-
-  work_item_list->Rollback();
-
-  // Verify everything is rolled back.
-  // The value must have been deleted first in roll back otherwise the key
-  // can not be deleted.
-  EXPECT_NE(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  EXPECT_FALSE(base::PathExists(top_dir_to_create));
+  // Do and rollback the list.
+  EXPECT_TRUE(list->Do());
+  list->Rollback();
 }
 
-TEST_F(WorkItemListTest, ConditionalExecutionConditionFailure) {
-  scoped_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
-  scoped_ptr<WorkItem> work_item;
+// Execute a ConditionalWorkItemList whose condition is met. Fail in the middle.
+// Rollback what has been done.
+TEST(WorkItemListTest, ConditionalExecutionFailAndRollback) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  base::FilePath top_dir_to_create(temp_dir_.path());
-  top_dir_to_create = top_dir_to_create.AppendASCII("a");
-  base::FilePath dir_to_create(top_dir_to_create);
-  dir_to_create = dir_to_create.AppendASCII("b");
-  ASSERT_FALSE(base::PathExists(dir_to_create));
+  std::unique_ptr<WorkItemList> list(WorkItem::CreateConditionalWorkItemList(
+      new ConditionRunIfFileExists(temp_dir.path())));
 
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateCreateDirWorkItem(dir_to_create)));
-  work_item_list->AddWorkItem(work_item.release());
+  // Create the mock work items.
+  std::unique_ptr<StrictMockWorkItem> item1(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item2(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item3(new StrictMockWorkItem);
 
-  scoped_ptr<WorkItemList> conditional_work_item_list(
-      WorkItem::CreateConditionalWorkItemList(
-          new ConditionRunIfFileExists(dir_to_create.AppendASCII("c"))));
+  {
+    // Expect the two first work items to be done in order then undone.
+    InSequence s;
 
-  std::wstring key_to_create(kTestRoot);
-  key_to_create.push_back(base::FilePath::kSeparators[0]);
-  key_to_create.append(L"ExecutionSuccess");
-  work_item.reset(
-      reinterpret_cast<WorkItem*>(WorkItem::CreateCreateRegKeyWorkItem(
-          HKEY_CURRENT_USER, key_to_create, WorkItem::kWow64Default)));
-  conditional_work_item_list->AddWorkItem(work_item.release());
+    EXPECT_CALL(*item1, Do()).WillOnce(Return(true));
+    EXPECT_CALL(*item2, Do()).WillOnce(Return(false));
+    EXPECT_CALL(*item2, Rollback());
+    EXPECT_CALL(*item1, Rollback());
+  }
 
-  std::wstring name(kName);
-  std::wstring data(kDataStr);
-  work_item.reset(reinterpret_cast<WorkItem*>(
-      WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          key_to_create,
-                                          WorkItem::kWow64Default,
-                                          name,
-                                          data,
-                                          false)));
-  conditional_work_item_list->AddWorkItem(work_item.release());
+  // Add the items to the list.
+  list->AddWorkItem(item1.release());
+  list->AddWorkItem(item2.release());
+  list->AddWorkItem(item3.release());
 
-  work_item_list->AddWorkItem(conditional_work_item_list.release());
+  // Do and rollback the list.
+  EXPECT_FALSE(list->Do());
+  list->Rollback();
+}
 
-  EXPECT_TRUE(work_item_list->Do());
+// Execute a ConditionalWorkItemList whose condition isn't met.
+TEST(WorkItemListTest, ConditionalExecutionConditionFailure) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  // Verify that the WorkItems added as part of the conditional list have NOT
-  // been executed.
-  RegKey key;
-  EXPECT_NE(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  std::wstring read_out;
-  EXPECT_NE(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
-  key.Close();
+  std::unique_ptr<WorkItemList> list(
+      WorkItem::CreateConditionalWorkItemList(new ConditionRunIfFileExists(
+          temp_dir.path().Append(FILE_PATH_LITERAL("DoesNotExist")))));
 
-  // Verify that the other work item was executed.
-  EXPECT_TRUE(base::PathExists(dir_to_create));
+  // Create the mock work items.
+  std::unique_ptr<StrictMockWorkItem> item1(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item2(new StrictMockWorkItem);
+  std::unique_ptr<StrictMockWorkItem> item3(new StrictMockWorkItem);
 
-  work_item_list->Rollback();
+  // Don't expect any call to the methods of the work items.
 
-  // Verify everything is rolled back.
-  // The value must have been deleted first in roll back otherwise the key
-  // can not be deleted.
-  EXPECT_NE(ERROR_SUCCESS,
-      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
-  EXPECT_FALSE(base::PathExists(top_dir_to_create));
+  // Add the items to the list.
+  list->AddWorkItem(item1.release());
+  list->AddWorkItem(item2.release());
+  list->AddWorkItem(item3.release());
+
+  // Do and rollback the list.
+  EXPECT_TRUE(list->Do());
+  list->Rollback();
 }