Modernize, unify and clean up sync unit tests |
|||
Issue descriptionIt seems like there are many test doubles that can be improved by: - Moving them all to components/sync/test - Unifying naming, e.g. RecordingModelTypeChangeProcessor vs MockModelTypeProcessor. - Removing some classes, if a replacement is close enough (e.g. FakeModelTypeProcessor seems almost unused). - Possibly adopting more gmock-idiomatic approaches, e.g. matchers and actual mocks. This might also be a nice exercise for our team to become more used to the APIs and overall design.
,
Jan 17 2018
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dfea344184dcb5a6ca0b55f100e3c7856ae4b851 commit dfea344184dcb5a6ca0b55f100e3c7856ae4b851 Author: Mikel Astiz <mastiz@chromium.org> Date: Thu Jun 21 12:20:37 2018 Simplify ModelTypeController tests In particular, remove any notion of 'bridge' and hence stop using ModelTypeChangeProcessor altogether, since it's irrelevant for testing ModelTypeController. Bug: 791939 Change-Id: I76abc5e3ff48fb73ecb943e17fd114b11ec5aed6 Reviewed-on: https://chromium-review.googlesource.com/1109822 Commit-Queue: Mikel Astiz <mastiz@chromium.org> Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org> Cr-Commit-Position: refs/heads/master@{#569227} [modify] https://crrev.com/dfea344184dcb5a6ca0b55f100e3c7856ae4b851/components/sync/driver/model_type_controller_unittest.cc
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7832bbec561257dfa3c6b99ef53ca31396f648e7 commit 7832bbec561257dfa3c6b99ef53ca31396f648e7 Author: Mikel Astiz <mastiz@chromium.org> Date: Thu Jun 21 17:59:31 2018 Migrate various users of FakeModelTypeChangeProcessor We'd like to get rid of the fake, in favor of the mock, since most tests are anyway using the fake for mocking purposes (counting function calls and verifying arguments). Bug: 791939 Change-Id: Ie48fba806604af4aa622a56dc0e76e36c943f90e Reviewed-on: https://chromium-review.googlesource.com/1109511 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#569317} [modify] https://crrev.com/7832bbec561257dfa3c6b99ef53ca31396f648e7/components/reading_list/core/reading_list_store_unittest.cc [modify] https://crrev.com/7832bbec561257dfa3c6b99ef53ca31396f648e7/components/sync/device_info/device_info_sync_bridge_unittest.cc [modify] https://crrev.com/7832bbec561257dfa3c6b99ef53ca31396f648e7/components/sync/user_events/user_event_service_impl_unittest.cc
,
Jan 7
Some possible next steps here: - Remove RecordingModelTypeChangeProcessor (required migrating TypedURLSyncBridgeTest). - Remove FakeModelTypeChangeProcessor. |
|||
►
Sign in to add a comment |
|||
Comment 1 by gangwu@chromium.org
, Dec 18 2017