Introduce a SetRegValueWorkItem overload that accepts a callback instead (...)

chrome/installer/util/set_reg_value_work_item_unittest.cc

Index: chrome/installer/util/set_reg_value_work_item_unittest.cc
diff --git a/chrome/installer/util/set_reg_value_work_item_unittest.cc b/chrome/installer/util/set_reg_value_work_item_unittest.cc
index 163f8882c3769bc7398f76415057def89f50d7c7..0bea858be9d81ed5ee79741fe8a0e76d67e936cf 100644
--- a/chrome/installer/util/set_reg_value_work_item_unittest.cc
+++ b/chrome/installer/util/set_reg_value_work_item_unittest.cc
@@ -4,74 +4,70 @@
 
 #include <windows.h>
 
+#include <string>
+
+#include "base/bind.h"
 #include "base/files/file_path.h"
+#include "base/macros.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 "chrome/installer/util/set_reg_value_work_item.h"
 #include "chrome/installer/util/work_item.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using base::win::RegKey;
-
 namespace {
 
-const wchar_t kTestRoot[] = L"TempTemp";
+const wchar_t kTestKey[] = L"TempTemp";
 const wchar_t kDataStr1[] = L"data_111";
 const wchar_t kDataStr2[] = L"data_222";
 const wchar_t kNameStr[] = L"name_str";
 const wchar_t kNameDword[] = L"name_dword";
 
-const DWORD dword1 = 0;
-const DWORD dword2 = 1;
+const DWORD kDword1 = 12345;
+const DWORD kDword2 = 6789;
 
 class SetRegValueWorkItemTest : public testing::Test {
  protected:
+  SetRegValueWorkItemTest() {}
+
   void SetUp() override {
-    // Create a temporary key for testing
-    RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
-    key.DeleteKey(kTestRoot);
-    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, kTestRoot, KEY_READ));
-    ASSERT_EQ(ERROR_SUCCESS,
-        key.Create(HKEY_CURRENT_USER, kTestRoot, KEY_READ));
-  }
-  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(kTestRoot));
+    registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER);
+
+    // Create a temporary key for testing.
+    ASSERT_NE(ERROR_SUCCESS,
+              test_key_.Open(HKEY_CURRENT_USER, kTestKey, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS, test_key_.Create(HKEY_CURRENT_USER, kTestKey,
+                                              KEY_READ | KEY_SET_VALUE));
   }
+
+  base::win::RegKey test_key_;
+
+ private:
+  registry_util::RegistryOverrideManager registry_override_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(SetRegValueWorkItemTest);
 };
 
 }  // namespace
 
 // Write a new value without overwrite flag. The value should be set.
 TEST_F(SetRegValueWorkItemTest, WriteNewNonOverwrite) {
-  RegKey key;
-
-  std::wstring parent_key(kTestRoot);
-  parent_key.append(&base::FilePath::kSeparators[0], 1);
-  parent_key.append(L"WriteNewNonOverwrite");
-  ASSERT_EQ(ERROR_SUCCESS,
-      key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ));
-
-  std::wstring name_str(kNameStr);
-  std::wstring data_str(kDataStr1);
   scoped_ptr<SetRegValueWorkItem> work_item1(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name_str,
-                                          data_str,
+                                          kNameStr,
+                                          kDataStr1,
                                           false));
 
-  std::wstring name_dword(kNameDword);
   scoped_ptr<SetRegValueWorkItem> work_item2(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name_dword,
-                                          dword1,
+                                          kNameDword,
+                                          kDword1,
                                           false));
 
   EXPECT_TRUE(work_item1->Do());
@@ -79,46 +75,35 @@ TEST_F(SetRegValueWorkItemTest, WriteNewNonOverwrite) {
 
   std::wstring read_out;
   DWORD read_dword;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_str.c_str(), &read_out));
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name_dword.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword));
   EXPECT_EQ(read_out, kDataStr1);
-  EXPECT_EQ(read_dword, dword1);
+  EXPECT_EQ(read_dword, kDword1);
 
   work_item1->Rollback();
   work_item2->Rollback();
 
   // Rollback should delete the value.
-  EXPECT_FALSE(key.HasValue(name_str.c_str()));
-  EXPECT_FALSE(key.HasValue(name_dword.c_str()));
+  EXPECT_FALSE(test_key_.HasValue(kNameStr));
+  EXPECT_FALSE(test_key_.HasValue(kNameDword));
 }
 
 // Write a new value with overwrite flag. The value should be set.
 TEST_F(SetRegValueWorkItemTest, WriteNewOverwrite) {
-  RegKey key;
-
-  std::wstring parent_key(kTestRoot);
-  parent_key.append(&base::FilePath::kSeparators[0], 1);
-  parent_key.append(L"WriteNewOverwrite");
-  ASSERT_EQ(ERROR_SUCCESS,
-      key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ));
-
-  std::wstring name_str(kNameStr);
-  std::wstring data_str(kDataStr1);
   scoped_ptr<SetRegValueWorkItem> work_item1(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name_str,
-                                          data_str,
+                                          kNameStr,
+                                          kDataStr1,
                                           true));
 
-  std::wstring name_dword(kNameDword);
   scoped_ptr<SetRegValueWorkItem> work_item2(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name_dword,
-                                          dword1,
+                                          kNameDword,
+                                          kDword1,
                                           true));
 
   EXPECT_TRUE(work_item1->Do());
@@ -126,185 +111,241 @@ TEST_F(SetRegValueWorkItemTest, WriteNewOverwrite) {
 
   std::wstring read_out;
   DWORD read_dword;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_str.c_str(), &read_out));
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name_dword.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword));
   EXPECT_EQ(read_out, kDataStr1);
-  EXPECT_EQ(read_dword, dword1);
+  EXPECT_EQ(read_dword, kDword1);
 
   work_item1->Rollback();
   work_item2->Rollback();
 
   // Rollback should delete the value.
-  EXPECT_FALSE(key.HasValue(name_str.c_str()));
-  EXPECT_FALSE(key.HasValue(name_dword.c_str()));
+  EXPECT_FALSE(test_key_.HasValue(kNameStr));
+  EXPECT_FALSE(test_key_.HasValue(kNameDword));
 }
 
 // Write to an existing value without overwrite flag. There should be
 // no change.
 TEST_F(SetRegValueWorkItemTest, WriteExistingNonOverwrite) {
-  RegKey key;
-
-  std::wstring parent_key(kTestRoot);
-  parent_key.append(&base::FilePath::kSeparators[0], 1);
-  parent_key.append(L"WriteExistingNonOverwrite");
-  ASSERT_EQ(ERROR_SUCCESS,
-      key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
-                 KEY_READ | KEY_SET_VALUE));
-
   // First test REG_SZ value.
   // Write data to the value we are going to set.
-  std::wstring name(kNameStr);
-  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), kDataStr1));
+  ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDataStr1));
 
-  std::wstring data(kDataStr2);
   scoped_ptr<SetRegValueWorkItem> work_item(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name,
-                                          data,
+                                          kNameStr,
+                                          kDataStr2,
                                           false));
   EXPECT_TRUE(work_item->Do());
 
   std::wstring read_out;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
-  EXPECT_EQ(0, read_out.compare(kDataStr1));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
+  EXPECT_EQ(read_out, kDataStr1);
 
   work_item->Rollback();
-  EXPECT_TRUE(key.HasValue(name.c_str()));
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
+  EXPECT_TRUE(test_key_.HasValue(kNameStr));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
   EXPECT_EQ(read_out, kDataStr1);
 
   // Now test REG_DWORD value.
   // Write data to the value we are going to set.
-  name.assign(kNameDword);
-  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), dword1));
+  ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameDword, kDword1));
   work_item.reset(WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                                      parent_key,
+                                                      kTestKey,
                                                       WorkItem::kWow64Default,
-                                                      name,
-                                                      dword2,
+                                                      kNameDword,
+                                                      kDword2,
                                                       false));
   EXPECT_TRUE(work_item->Do());
 
   DWORD read_dword;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
-  EXPECT_EQ(read_dword, dword1);
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword));
+  EXPECT_EQ(read_dword, kDword1);
 
   work_item->Rollback();
-  EXPECT_TRUE(key.HasValue(name.c_str()));
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
-  EXPECT_EQ(read_dword, dword1);
+  EXPECT_TRUE(test_key_.HasValue(kNameDword));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword));
+  EXPECT_EQ(read_dword, kDword1);
 }
 
 // Write to an existing value with overwrite flag. The value should be
 // overwritten.
 TEST_F(SetRegValueWorkItemTest, WriteExistingOverwrite) {
-  RegKey key;
-
-  std::wstring parent_key(kTestRoot);
-  parent_key.append(&base::FilePath::kSeparators[0], 1);
-  parent_key.append(L"WriteExistingOverwrite");
-  ASSERT_EQ(ERROR_SUCCESS,
-      key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
-                 KEY_READ | KEY_SET_VALUE));
-
   // First test REG_SZ value.
   // Write data to the value we are going to set.
-  std::wstring name(kNameStr);
-  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), kDataStr1));
+  ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDataStr1));
 
-  std::wstring name_empty(L"name_empty");
-  ASSERT_EQ(ERROR_SUCCESS, RegSetValueEx(key.Handle(), name_empty.c_str(), NULL,
+  const wchar_t kNameEmpty[] = L"name_empty";
+  ASSERT_EQ(ERROR_SUCCESS, RegSetValueEx(test_key_.Handle(), kNameEmpty, NULL,
                                          REG_SZ, NULL, 0));
 
-  std::wstring data(kDataStr2);
   scoped_ptr<SetRegValueWorkItem> work_item1(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name,
-                                          data,
+                                          kNameStr,
+                                          kDataStr2,
                                           true));
   scoped_ptr<SetRegValueWorkItem> work_item2(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name_empty,
-                                          data,
+                                          kNameEmpty,
+                                          kDataStr2,
                                           true));
 
   EXPECT_TRUE(work_item1->Do());
   EXPECT_TRUE(work_item2->Do());
 
   std::wstring read_out;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
-  EXPECT_EQ(0, read_out.compare(kDataStr2));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
+  EXPECT_EQ(read_out, kDataStr2);
 
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_empty.c_str(), &read_out));
-  EXPECT_EQ(0, read_out.compare(kDataStr2));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameEmpty, &read_out));
+  EXPECT_EQ(read_out, kDataStr2);
 
   work_item1->Rollback();
   work_item2->Rollback();
 
-  EXPECT_TRUE(key.HasValue(name.c_str()));
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
+  EXPECT_TRUE(test_key_.HasValue(kNameStr));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
   EXPECT_EQ(read_out, kDataStr1);
 
   DWORD type = 0;
   DWORD size = 0;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_empty.c_str(), NULL, &size,
-                                         &type));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameEmpty, NULL, &size, &type));
   EXPECT_EQ(REG_SZ, type);
   EXPECT_EQ(0, size);
 
   // Now test REG_DWORD value.
   // Write data to the value we are going to set.
-  name.assign(kNameDword);
-  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), dword1));
+  ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameDword, kDword1));
   scoped_ptr<SetRegValueWorkItem> work_item3(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          kTestKey,
                                           WorkItem::kWow64Default,
-                                          name,
-                                          dword2,
+                                          kNameDword,
+                                          kDword2,
                                           true));
   EXPECT_TRUE(work_item3->Do());
 
   DWORD read_dword;
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
-  EXPECT_EQ(read_dword, dword2);
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword));
+  EXPECT_EQ(read_dword, kDword2);
 
   work_item3->Rollback();
-  EXPECT_TRUE(key.HasValue(name.c_str()));
-  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
-  EXPECT_EQ(read_dword, dword1);
+  EXPECT_TRUE(test_key_.HasValue(kNameDword));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword));
+  EXPECT_EQ(read_dword, kDword1);
 }
 
 // Write a value to a non-existing key. This should fail.
 TEST_F(SetRegValueWorkItemTest, WriteNonExistingKey) {
-  RegKey key;
-
-  std::wstring parent_key(kTestRoot);
-  parent_key.append(&base::FilePath::kSeparators[0], 1);
-  parent_key.append(L"WriteNonExistingKey");
+  std::wstring non_existing(kTestKey);
+  non_existing.append(&base::FilePath::kSeparators[0], 1);
+  non_existing.append(L"NonExistingKey");
 
-  std::wstring name(L"name");
-  std::wstring data(kDataStr1);
   scoped_ptr<SetRegValueWorkItem> work_item(
       WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                          parent_key,
+                                          non_existing.c_str(),
                                           WorkItem::kWow64Default,
-                                          name,
-                                          data,
+                                          kNameStr,
+                                          kDataStr1,
                                           false));
   EXPECT_FALSE(work_item->Do());
 
   work_item.reset(WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
-                                                      parent_key,
+                                                      non_existing.c_str(),
                                                       WorkItem::kWow64Default,
-                                                      name,
-                                                      dword1,
+                                                      kNameStr,
+                                                      kDword1,
                                                       false));
   EXPECT_FALSE(work_item->Do());
 }
+
+// Verifies that |actual_previous_value| is |expected_previous_value| and
+// returns |desired_new_value| to replace it.
+std::wstring VerifyPreviousValueAndReplace(
+    const std::wstring& expected_previous_value,
+    const std::wstring& desired_new_value,
+    const std::wstring& actual_previous_value) {
+  EXPECT_EQ(expected_previous_value, actual_previous_value);
+  return desired_new_value;
+}
+
+// Modify an existing value with the callback API.
+TEST_F(SetRegValueWorkItemTest, ModifyExistingWithCallback) {
+  // Write |kDataStr1| to the value we are going to modify.
+  ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDataStr1));
+
+  scoped_ptr<SetRegValueWorkItem> work_item(
+      WorkItem::CreateSetRegValueWorkItem(
+          HKEY_CURRENT_USER,
+          kTestKey,
+          WorkItem::kWow64Default,
+          kNameStr,
+          base::Bind(&VerifyPreviousValueAndReplace, kDataStr1, kDataStr2)));

[grt (UTC plus 2), 2015/06/29 15:37:48]

could you make the test ASSERT that the callback is run? one way to do so would
be to pass a ptr to a bool to the function that the fn sets to true (i.e.
"was_run") and then ASSERT that it was after the call to Do. you could also
reset the bool and ASSERT that it isn't run during rollback.

[gab, 2015/06/29 20:11:01]

I feel this is tangling into how the implementation works a bit too much. The
API of this call is "I will set what this callback tells me", not "I will call
this callback" (which is of course related, but not what we're testing).

The only place kDataStr2 is mentioned before the EXPECT_EQ on 295 is in this
Bind, so I don't see how it could have arrived there without the callback having
ran and I think this does verify that it did.

As for testing it's not called in Rollback(), we are also testing that the
result of the rollback is not kDataStr2, so I guess the only point of checking
the callback was *not* called is performance which I don't think is very
relevant here (i.e. in the limit, hooking everything and ensuring only the
strict minimum was called by the impl in every unittest is probably overkill).

[grt (UTC plus 2), 2015/06/30 20:26:42]

My reason for testing that the callback is called: if the test asserted that the
callback was called and this assertion failed, you would know exactly where to
look to diagnose/fix the error. On the other hand, if only the expectation of
the data value fails, then you have more places where you need to poke around to
see why it failed. Since the callback is part of the API contract, I don't think
testing that it was called falls in the category of white-box testing (where the
test is too coupled to the implementation).
Now you're justifying that the test is properly written and that it can't fail.
This is a weaker argument that having a test that will simply fail if the right
thing doesn't happen.
I disagree. Rolling back should restore the previous value, which is not a
function of the callback. A bug in the code that invoked the callback at the
wrong time is a bug.

[gab, 2015/07/01 02:55:19]

Done because I don't care so much, but I feel this clutters up the test for no
added value (other than mild bonus if the test starts failing at which point
whoever is debugging it could add a few logs to see whether the issue is that
the callback isn't being called or the callee is misbehaving).

+
+  EXPECT_TRUE(work_item->Do());

[grt (UTC plus 2), 2015/06/29 15:37:48]

all of these should be ASSERT_TRUE rather than EXPECT_TRUE since there's no
point in evaluating anything else if the work items fails.

[gab, 2015/06/29 20:11:01]

Good point, replaced for every test in this file (since I'm already touching
them a bunch).

+
+  std::wstring read_out;
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
+  EXPECT_EQ(read_out, kDataStr2);
+
+  work_item->Rollback();
+
+  EXPECT_TRUE(test_key_.HasValue(kNameStr));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
+  EXPECT_EQ(read_out, kDataStr1);
+}
+
+// Modify a non-existing value with the callback API.
+TEST_F(SetRegValueWorkItemTest, ModifyNonExistingWithCallback) {
+  scoped_ptr<SetRegValueWorkItem> work_item(
+      WorkItem::CreateSetRegValueWorkItem(
+          HKEY_CURRENT_USER,
+          kTestKey,
+          WorkItem::kWow64Default,
+          kNameStr,
+          base::Bind(&VerifyPreviousValueAndReplace, L"", kDataStr1)));
+
+  EXPECT_TRUE(work_item->Do());
+
+  std::wstring read_out;
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out));
+  EXPECT_EQ(read_out, kDataStr1);
+
+  work_item->Rollback();
+
+  EXPECT_FALSE(test_key_.HasValue(kNameStr));
+}
+
+// Modify an existing value which is not a string (REG_SZ) with the string
+// callback API.
+TEST_F(SetRegValueWorkItemTest, ModifyExistingNonStringWithStringCallback) {
+  // Write |kDword1| to the value we are going to modify.
+  ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDword1));
+
+  scoped_ptr<SetRegValueWorkItem> work_item(
+      WorkItem::CreateSetRegValueWorkItem(
+          HKEY_CURRENT_USER,
+          kTestKey,
+          WorkItem::kWow64Default,
+          kNameStr,
+          base::Bind(&VerifyPreviousValueAndReplace, L"", kDataStr1)));
+
+  EXPECT_TRUE(work_item->Do());
+
+  std::wstring read_str_out;
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_str_out));
+  EXPECT_EQ(read_str_out, kDataStr1);
+
+  work_item->Rollback();
+
+  DWORD read_dword_out = 0;
+  EXPECT_TRUE(test_key_.HasValue(kNameStr));
+  EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameStr, &read_dword_out));
+  EXPECT_EQ(read_dword_out, kDword1);
+}