[Sync] Add an error integration test for USS.

chrome/browser/sync/test/integration/two_client_uss_sync_test.cc

 // Copyright 2016 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 "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/sync/chrome_sync_client.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
+#include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/status_change_checker.h"
 #include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "components/browser_sync/profile_sync_components_factory_impl.h"
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/sync/api/fake_model_type_service.h"
 
 using browser_sync::ChromeSyncClient;
 using browser_sync::ProfileSyncComponentsFactoryImpl;
 using syncer_v2::ConflictResolution;
 using syncer_v2::FakeModelTypeService;
 using syncer_v2::ModelTypeService;
 using syncer_v2::SharedModelTypeProcessor;
 
 const char kKey1[] = "key1";
+const char kKey2[] = "key2";
 const char kValue1[] = "value1";
 const char kValue2[] = "value2";
 const char kValue3[] = "value3";
 
 // A ChromeSyncClient that provides a ModelTypeService for PREFERENCES.
 class TestSyncClient : public ChromeSyncClient {
  public:
   TestSyncClient(Profile* profile, ModelTypeService* service)
       : ChromeSyncClient(profile), service_(service) {}
 
@@ skipping 138 matching lines @@
 
   bool IsExitConditionSatisfied() override {
     return !service_->db().HasMetadata(key_);
   }
 
   std::string GetDebugMessage() const override {
     return "Waiting for metadata for key '" + key_ + "' to be absent.";
   }
 };
 
+// Wait for PREFERENCES to no longer be running.
+class PrefsNotRunningChecker : public SingleClientStatusChangeChecker {
+ public:
+  explicit PrefsNotRunningChecker(browser_sync::ProfileSyncService* service)
+      : SingleClientStatusChangeChecker(service) {}
+  ~PrefsNotRunningChecker() override {}

[skym, 2016/09/27 20:24:45]

I don't think you need this. I'm going through the other checkers are removing
these. StatusChangeChecker's dtor is not pure.

[maxbogue, 2016/09/28 20:17:08]

Removed them all.

+
+  bool Wait() {
+    SingleClientStatusChangeChecker::Wait();

[skym, 2016/09/27 20:24:44]

This awkwardness should be fixed by my CL.

[maxbogue, 2016/09/28 20:17:09]

Acknowledged.

+    return !TimedOut();
+  }
+
+  bool IsExitConditionSatisfied() override {
+    return !service()->IsDataTypeControllerRunning(syncer::PREFERENCES);
+  }
+
+  std::string GetDebugMessage() const override {
+    return "Waiting for prefs to be not running.";
+  }
+};
+
 class TwoClientUssSyncTest : public SyncTest {
  public:
   TwoClientUssSyncTest() : SyncTest(TWO_CLIENT) {
     DisableVerifier();
     sync_client_factory_ = base::Bind(&TwoClientUssSyncTest::CreateSyncClient,
                                       base::Unretained(this));
     ProfileSyncServiceFactory::SetSyncClientFactoryForTest(
         &sync_client_factory_);
     ProfileSyncComponentsFactoryImpl::OverridePrefsForUssTest(true);
   }
@@ skipping 100 matching lines @@
 
   // Write conflicting entities.
   model1->WriteItem(kKey1, kValue1);
   model2->WriteItem(kKey1, kValue2);
 
   // Wait for them to be resolved to kResolutionValue by the custom conflict
   // resolution logic in TestModelTypeService.
   ASSERT_TRUE(DataChecker(model1, kKey1, kValue3).Wait());
   ASSERT_TRUE(DataChecker(model2, kKey1, kValue3).Wait());
 }
+
+IN_PROC_BROWSER_TEST_F(TwoClientUssSyncTest, Error) {
+  ASSERT_TRUE(SetupSync());
+  TestModelTypeService* model1 = GetModelTypeService(0);
+  TestModelTypeService* model2 = GetModelTypeService(1);
+
+  // Add an entity.
+  model1->WriteItem(kKey1, kValue1);
+  ASSERT_TRUE(DataChecker(model2, kKey1, kValue1).Wait());
+
+  // Queue an error in model 2 and trigger a GetUpdates.
+  model2->SetServiceError(syncer::SyncError::DATATYPE_ERROR);
+  model1->WriteItem(kKey1, kValue2);

[skym, 2016/09/27 20:24:45]

What's the point of this? You don't verify that model2 doesn't have
key1->value2.

[maxbogue, 2016/09/28 20:17:09]

To trigger an ApplySyncChanges (GU) in model2.

+
+  // The type should stop syncing but keep tracking metadata.
+  ASSERT_TRUE(PrefsNotRunningChecker(GetSyncService(1)).Wait());

[skym, 2016/09/27 20:24:44]

Seems odd that you error(2) -> write(1) -> block for shutdown(2). Shouldn't you
error(2) -> block for shutdown(2) -> write(1)?

[maxbogue, 2016/09/28 20:17:09]

SetServiceError stores the error that is returned on the next fallible service
operation. The write is to make sure a service operation happens.

+  ASSERT_EQ(1U, model2->db().metadata_count());
+  model2->WriteItem(kKey2, kValue2);
+  ASSERT_EQ(2U, model2->db().metadata_count());
+}