[Sync] Introduce common test suite for implementations of USS ModelTypeSyncBridge |
||
Issue description(Here's a thought, motivated by crbug.com/736006 ) USS moves quite some responsibility to the datatype, specifically the subclasses of ModelTypeSyncBridge. These implementations must comply with a strict contract that is testable, leading to (in the best case) duplicates tests or sometimes lack of test coverage for advanced features. It doesn't seem to scale well for all sync datatypes. I'd propose introducing a common test suite via typed tests (https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#typed-tests) Taking a look at existing tests, TypedURLSyncBridgeTest seems one that is quite extensive and generalizable. One way to do this is by introducing a test-only class per datatype that allows local model reads and manipulations (e.g. create a local test entity, mutate a local entity). Hence, the test suite with be template with two arguments: a) the specific bridge type and b) the local model tester. pavely@, skym@: does this make sense to you?
,
Jan 4 2018
I really like this idea, I think it would add a lot of value. I tried to cobble something together when writing the unit tests for DeviceInfoSyncBridge that was more agnostic, and failed miserably. There are some pieces of the bridge implementations that vary which could cause you problems. Some bridges have async vs sync access to data. Init is often awkward as well. DeviceInfo deletes all data upon disabling sync. Sessions doesn't typically delete tab data. Bookmarks is bookmarks.
,
Jan 6 2018
The idea seems interesting. You would create helper classes that implement the same interface. Test would interact with them through the interface and classes would interact with bridge implementations in some datatype specific way. On the other end you can connect to SMTP as a fake worker and observe that the right interactions with server happen in response to local changes. I'm not sure how hard would it be to implement something like this. I would approach it by taking two existing unittests for different datatypes (leveldb vs. sqlite) and trying to replicate their test cases in common test suite.
,
Jan 8 2018
I started prototyping this idea and will share some code soon. Please let me know if you have preferences wrt which two/three datatypes to begin with, otherwise I'll use typed URLs and two others arbitrarily.
,
Jan 9 2018
I put together a draft which is not ready for actual review, but gives an idea of what could be achieved (i.e. which tests can be generalized). pavely@ and/or skym@: would you mind taking a quick look? https://chromium-review.googlesource.com/#/c/chromium/src/+/857473 My first impression is that the fraction of generic tests is smaller than I was hoping for, so I'm no longer fully convinced we want to go this way. However, the list would hopefully keep growing, and it does seem like something is better than nothing (we already have some datatypes with little test coverage). WDYT
,
Jan 11 2018
pavely@, skym@: friendly ping, it'd be great if either of you could take a look at the draft CL linked above, in particular to the tests in model_type_sync_bridge_test_template.h, and comment on whether this is a desirable direction to head to.
,
Jan 11 2018
I will look now, sorry for the delay.
,
Jan 17 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by mastiz@chromium.org
, Jan 4 2018