SyncFS: Fix LocalSyncOperationResolverTest to cover all test cases

chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc

 // Copyright (c) 2013 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 <string>
 #include <vector>
 
 #include "chrome/browser/sync_file_system/local_sync_operation_resolver.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/fileapi/syncable/file_change.h"
 #include "webkit/fileapi/syncable/sync_file_type.h"
 
 namespace sync_file_system {
 
 namespace {
 
 struct Input {
   bool has_remote_change;
   FileChange remote_file_change;

[kinuko, 2013/05/07 04:26:10]

Maybe this could be scoped_ptr<FileChange> remote_file_change and can be NULL
when no remote change's there?

Similarly we may pass a ptr to remote_file_change to
LocalSyncOperationResolver::Resolve only when has_remote_change is true? (As we
do for drive_metadata)

[nhiroki, 2013/05/07 11:13:17]

Done.

+  SyncFileType remote_file_type_in_metadata;
 
-  std::string DebugString() {
+  std::string DebugString() const {
     std::ostringstream ss;
     ss << "has_remote_change: " << (has_remote_change ? "true" : "false")
-       << ", RemoteFileChange: " << remote_file_change.DebugString();
+       << ", RemoteFileChange: " << remote_file_change.DebugString()
+       << ", RemoteFileTypeInMetadata: " << remote_file_type_in_metadata;
     return ss.str();
   }
 };
 
 template <typename type, size_t array_size>
 std::vector<type> CreateList(const type (&inputs)[array_size]) {
   return std::vector<type>(inputs, inputs + array_size);
 }
 
-FileChange CreateDummyFileChange() {
-  return FileChange(FileChange::FILE_CHANGE_ADD_OR_UPDATE,
-                    SYNC_FILE_TYPE_UNKNOWN);
+std::vector<Input> CreateInput() {
+  FileChange dummy_change(FileChange::FILE_CHANGE_ADD_OR_UPDATE,
+                          SYNC_FILE_TYPE_UNKNOWN);
+  SyncFileType dummy_file_type = SYNC_FILE_TYPE_UNKNOWN;
+
+  const Input inputs[] = {
+    // When has_remote_change==false, the resolver does not take care of
+    // remote_file_change.
+    { false, dummy_change, SYNC_FILE_TYPE_UNKNOWN },
+    { false, dummy_change, SYNC_FILE_TYPE_FILE },
+    { false, dummy_change, SYNC_FILE_TYPE_DIRECTORY },
+
+    // When has_remote_change==true, the resolver does not take care of
+    // remote_file_type_in_metadata.
+    { true, FileChange(FileChange::FILE_CHANGE_ADD_OR_UPDATE,
+                       SYNC_FILE_TYPE_FILE), dummy_file_type },
+    { true, FileChange(FileChange::FILE_CHANGE_ADD_OR_UPDATE,
+                       SYNC_FILE_TYPE_DIRECTORY), dummy_file_type },
+    { true, FileChange(FileChange::FILE_CHANGE_DELETE,
+                       SYNC_FILE_TYPE_FILE), dummy_file_type },
+    { true, FileChange(FileChange::FILE_CHANGE_DELETE,
+                       SYNC_FILE_TYPE_DIRECTORY), dummy_file_type },
+  };
+  return CreateList(inputs);
 }
 
-std::vector<Input> CreateInput() {
-  const Input inputs[] = {
-    { false, CreateDummyFileChange() },
-    { true, FileChange(FileChange::FILE_CHANGE_ADD_OR_UPDATE,
-                       SYNC_FILE_TYPE_FILE) },
-    { true, FileChange(FileChange::FILE_CHANGE_ADD_OR_UPDATE,
-                       SYNC_FILE_TYPE_DIRECTORY) },
-    { true, FileChange(FileChange::FILE_CHANGE_DELETE,
-                       SYNC_FILE_TYPE_FILE) },
-    { true, FileChange(FileChange::FILE_CHANGE_DELETE,
-                       SYNC_FILE_TYPE_DIRECTORY) },
-  };
-  return CreateList(inputs);
+std::string DebugString(const std::vector<Input>& inputs, int number) {
+  std::ostringstream ss;
+  ss << "Case " << number << ": (" << inputs[number].DebugString() << ")";
+  return ss.str();
 }
 
 }  // namespace
 
 class LocalSyncOperationResolverTest : public testing::Test {
  public:
   LocalSyncOperationResolverTest() {}
 
  protected:
   typedef LocalSyncOperationResolver Resolver;
   typedef std::vector<LocalSyncOperationType> ExpectedTypes;
 
   DISALLOW_COPY_AND_ASSIGN(LocalSyncOperationResolverTest);
 };
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForAddOrUpdateFile) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_ADD_FILE,
+    LOCAL_SYNC_OPERATION_UPDATE_FILE,
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
+
     LOCAL_SYNC_OPERATION_CONFLICT,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
-  // TODO(nhiroki): Fix inputs so that these tests can cover all cases.
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)

[kinuko, 2013/05/07 04:26:10]

nit: please put { } for multi-line body (here and in all tests)

[nhiroki, 2013/05/07 11:13:17]

Done.

     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForAddOrUpdateFile(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change,
-                  SYNC_FILE_TYPE_UNKNOWN))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change,
+                  inputs[i].remote_file_type_in_metadata))
+        << DebugString(inputs, i);
 }
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForAddOrUpdateFileInConflict) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_CONFLICT,
     LOCAL_SYNC_OPERATION_CONFLICT,
+    LOCAL_SYNC_OPERATION_CONFLICT,
+
+    LOCAL_SYNC_OPERATION_CONFLICT,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)
     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForAddOrUpdateFileInConflict(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change))
+        << DebugString(inputs, i);
 }
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForAddDirectory) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_ADD_DIRECTORY,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
     LOCAL_SYNC_OPERATION_NONE,
+
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
+    LOCAL_SYNC_OPERATION_NONE,
     LOCAL_SYNC_OPERATION_ADD_DIRECTORY,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
-  // TODO(nhiroki): Fix inputs so that these tests can cover all cases.
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)
     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForAddDirectory(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change,
-                  SYNC_FILE_TYPE_UNKNOWN))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change,
+                  inputs[i].remote_file_type_in_metadata))
+        << DebugString(inputs, i);
 }
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForAddDirectoryInConflict) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
+
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_LOCAL,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)
     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForAddDirectoryInConflict(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change))
+        << DebugString(inputs, i);
 }
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForDeleteFile) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_NONE,
+    LOCAL_SYNC_OPERATION_DELETE_FILE,
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
+
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
-  // TODO(nhiroki): Fix inputs so that these tests can cover all cases.
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)
     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForDeleteFile(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change,
-                  SYNC_FILE_TYPE_UNKNOWN))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change,
+                  inputs[i].remote_file_type_in_metadata))
+        << DebugString(inputs, i);
 }
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForDeleteFileInConflict) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
+
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)
     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForDeleteFileInConflict(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change))
+        << DebugString(inputs, i);
 }
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForDeleteDirectory) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_NONE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
+    LOCAL_SYNC_OPERATION_DELETE_DIRECTORY,
+
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
-  // TODO(nhiroki): Fix inputs so that these tests can cover all cases.
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)
     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForDeleteDirectory(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change,
-                  SYNC_FILE_TYPE_UNKNOWN))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change,
+                  inputs[i].remote_file_type_in_metadata))
+        << DebugString(inputs, i);
 }
 
 TEST_F(LocalSyncOperationResolverTest, ResolveForDeleteDirectoryInConflict) {
   const LocalSyncOperationType kExpectedTypes[] = {
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
+
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
+    LOCAL_SYNC_OPERATION_RESOLVE_TO_REMOTE,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
     LOCAL_SYNC_OPERATION_DELETE_METADATA,
   };
 
   ExpectedTypes expected_types = CreateList(kExpectedTypes);
   std::vector<Input> inputs = CreateInput();
 
   ASSERT_EQ(expected_types.size(), inputs.size());
   for (ExpectedTypes::size_type i = 0; i < expected_types.size(); ++i)
     EXPECT_EQ(expected_types[i],
               Resolver::ResolveForDeleteDirectoryInConflict(
-                  inputs[i].has_remote_change, inputs[i].remote_file_change))
-        << "Case " << i << ": (" << inputs[i].DebugString() << ")";
+                  inputs[i].has_remote_change,
+                  inputs[i].remote_file_change))
+        << DebugString(inputs, i);
 }
 
 }  // namespace sync_file_system