New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 643269 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[USS] Add integration test coverage.

Project Member Reported by maxbogue@chromium.org, Sep 1 2016

Issue description

There are currently no tests that leverage USS as an entire system, especially its integration with the sync backend machinery. There should be tests that exercising enabling a USS type and syncing using the fake server.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2824d17cd962de7d41bd00ff0318fe44ff4e2871

commit 2824d17cd962de7d41bd00ff0318fe44ff4e2871
Author: maxbogue <maxbogue@chromium.org>
Date: Tue Sep 06 20:16:59 2016

[Sync] Split fake ModelTypeService impl out of SMTP tests.

This class and logic can be re-used for integration testing, so I'm
spliting it out into its own file. As a happy side-effect, this
massively reduces the size of the SMTP test file.

- Current FakeModelTypeService is renamed StubModelTypeService (does
  nothing; just compiles).
- SMTP test impl is moved into FakeModelTypeService (fully functional
  key/value service using PREFERENCES specifics and an in-memory store.)
- Updated some loops to use const auto &kv.

BUG= 643269 

Review-Url: https://codereview.chromium.org/2306523003
Cr-Commit-Position: refs/heads/master@{#416691}

[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/BUILD.gn
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/api/DEPS
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/api/fake_model_type_service.cc
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/api/fake_model_type_service.h
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/api/model_type_service_unittest.cc
[add] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/api/stub_model_type_service.cc
[add] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/api/stub_model_type_service.h
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/core/shared_model_type_processor_unittest.cc
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/core_impl/model_type_connector_proxy_unittest.cc
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/driver/non_blocking_data_type_controller_unittest.cc
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/driver/non_ui_model_type_controller_unittest.cc
[modify] https://crrev.com/2824d17cd962de7d41bd00ff0318fe44ff4e2871/components/sync/driver/ui_model_type_controller_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1d69879d0c74f5edae8cb1a942ff6538fe398cb

commit a1d69879d0c74f5edae8cb1a942ff6538fe398cb
Author: maxbogue <maxbogue@chromium.org>
Date: Tue Sep 13 23:15:06 2016

[Sync] Add a sanity integration test for USS.

This integration test overwrites the PREFERENCES data type to use a fake
ModelTypeService implementation. This is done using two new static
calls:

- ProfileSyncComponentsFactoryImpl::OverridePrefsForUssTest to force
  PREFERENCES to be registered as a USS type (with a
  ModelTypeController).
- ProfileSyncServiceFactory::SetSyncClientFactoryForTest to override the
  SyncClient that the real ProfileSyncService uses so that the
  ModelTypeService can be injected.

Additionally, this change fixes an issue with deletions under USS that
was uncovered while writing it. The specifics used for a deleted entity
must have ByteSize() == 0 for it to be detected as a deletion, but we
were setting it to the one sent by the server, which has ByteSize() ==
4 so the model type can be extracted from it.

BUG= 643269 

Review-Url: https://codereview.chromium.org/2328393002
Cr-Commit-Position: refs/heads/master@{#418404}

[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/chrome/browser/sync/profile_sync_service_factory.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/chrome/browser/sync/profile_sync_service_factory.h
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/chrome/browser/sync/test/integration/single_client_bookmarks_sync_test.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/chrome/browser/sync/test/integration/single_client_extensions_sync_test.cc
[add] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/chrome/test/BUILD.gn
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/browser_sync/browser/profile_sync_components_factory_impl.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/browser_sync/browser/profile_sync_components_factory_impl.h
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/engine_impl/worker_entity_tracker.h
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/android/fake_server_helper_android.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/bookmark_entity.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/fake_server.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/fake_server_entity.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/fake_server_entity.h
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/permanent_entity.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/tombstone_entity.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/tombstone_entity.h
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/unique_client_entity.cc
[modify] https://crrev.com/a1d69879d0c74f5edae8cb1a942ff6538fe398cb/components/sync/test/fake_server/unique_client_entity.h

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45763c93428ccabb847d00c35f14c5e33e0e0c0e

commit 45763c93428ccabb847d00c35f14c5e33e0e0c0e
Author: maxbogue <maxbogue@chromium.org>
Date: Fri Sep 16 18:28:55 2016

[Sync] Add two more USS integration tests.

- Disabling and enabling the type, which revealed a bug in the DTC.
- Conflict resolution, which revealed a bug in the worker.

BUG= 643269 

Review-Url: https://codereview.chromium.org/2339403004
Cr-Commit-Position: refs/heads/master@{#419222}

[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/components/sync/driver/non_blocking_data_type_controller.cc
[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/components/sync/driver/non_blocking_data_type_controller.h
[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/components/sync/engine_impl/worker_entity_tracker.cc
[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/components/sync/engine_impl/worker_entity_tracker.h
[modify] https://crrev.com/45763c93428ccabb847d00c35f14c5e33e0e0c0e/components/sync/engine_impl/worker_entity_tracker_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dff5a0d6ec28d34867aa238e0b39aa0386004222

commit dff5a0d6ec28d34867aa238e0b39aa0386004222
Author: maxbogue <maxbogue@chromium.org>
Date: Wed Sep 28 23:20:20 2016

[Sync] Add an error integration test for USS.

Caught two bugs:

- The NonBlockingDTC needs to report errors even if the type is running.
- The DataTypeManagerImpl must do ProcessReconfigure asynchronously
  to give the ModelAssociationManager time to finish stopping the type.

BUG= 643269 

Review-Url: https://codereview.chromium.org/2369063003
Cr-Commit-Position: refs/heads/master@{#421676}

[modify] https://crrev.com/dff5a0d6ec28d34867aa238e0b39aa0386004222/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/dff5a0d6ec28d34867aa238e0b39aa0386004222/components/sync/driver/data_type_manager_impl.cc
[modify] https://crrev.com/dff5a0d6ec28d34867aa238e0b39aa0386004222/components/sync/driver/non_blocking_data_type_controller.cc

Status: Fixed (was: Started)

Sign in to add a comment