Open Bug 1435556 Opened 3 years ago Updated 2 months ago

Write changed records back to the mirror and assert the remote tree in tests

Categories

(Firefox :: Sync, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: lina, Unassigned, Mentored, NeedInfo)

References

Details

(Whiteboard: [good next bug])

Attachments

(1 file)

Our mirror tests currently do something like this to make sure we're uploading the correct records: https://searchfox.org/mozilla-central/rev/e06af9c36a73a27864302cd2f829e6200dee8541/toolkit/components/places/tests/sync/test_bookmark_value_changes.js#972-981 (Most of them don't check cleartexts, just IDs).

We already run `assertLocalTree` after merging to make sure Places is consistent. It would also be great to write the changed records that `apply` returns back to the mirror (simulating what the engine does in https://searchfox.org/mozilla-central/rev/e06af9c36a73a27864302cd2f829e6200dee8541/services/sync/modules/engines/bookmarks.js#827-829,832), and add an `assertRemoteTree` helper to ensure the tree in our mirror is consistent.
Priority: -- → P3
Here's an example of how we'd change the tests. Replace `inspectChangeRecords` with `deepEqual(changesToUpload, ...)`, call `writeUploadedChanges`, and then `assertRemoteTree`. It's straightforward, just tedious. :-/ We want to do this for all mirror tests in `toolkit/components/places/tests/sync`.

http://tlrobinson.net/projects/javascript-fun/jsondiff/ helps quite a bit with debugging `deepEqual` assertion failures.

Setting this as a "good next bug" because this is a great bug for a more experienced contributor. `hg pull -r 971db612fc1adf2799e33319bb61e56cbf2f4f10 https://reviewboard-hg.mozilla.org/gecko` (or `git mozreview fetch 971db612fc1adf2799e33319bb61e56cbf2f4f10`) will import my patch into your tree, so you can see how I changed `test_value_structure_conflict`, and get started on the others.
Whiteboard: [good next bug]
No longer blocks: 1433177
hi @kitcambridge. I have already previously contributed to Bugzilla by resolving issues #1446721 and #1428434 and I would like to take on a more interesting issues like this one. Could you please assign this issue to me and give me a little overview of the sync tests system? Thank you.
(In reply to constfilin from comment #3)
> hi @kitcambridge. I have already previously contributed to Bugzilla by
> resolving issues #1446721 and #1428434 and I would like to take on a more
> interesting issues like this one. Could you please assign this issue to me
> and give me a little overview of the sync tests system? Thank you.

Hi constfilin, thanks for contributing! In comment 2, Kit has a patch and other information that should help get you started, and information on running tests can be found at https://developer.mozilla.org/en-US/docs/Mozilla/QA/Running_automated_tests - in this case, Kit has made some changes to "xpcshell" tests.

We'll happily assign this bug to you once you put a patch up asking for feedback - and please feel free to come back here to ask any questions you might have along the way.

Good luck, and thanks for contributing!
See Also: → 1449797

Hi, I'd like to work on this bug.
I have a few questions before I start. I'm currently working on # 1637431 fr the web extensions team. Will I be able to create a bookmark ( like a git branch) to parallely work on this bug as well?
If not, will I have to wait till my other patch is accepted?
Is there a matrix room for FireFox Sync?
Since I am only (almost 1 bug old), is this something I can potentially fix? Or would you suggest that I look somewhere else?
Thanks!

Flags: needinfo?(lina)
You need to log in before you can comment on or make changes to this bug.