Closed Bug 1433827 Opened 7 years ago Closed 6 years ago

Work out best way to have TPS exercise structured bookmark application

Categories

(Firefox :: Sync, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: markh, Unassigned)

References

(Blocks 1 open bug)

Details

In a perfect world, we'd run all bookmark related tests twice - once with structured application enabled and once without. If that's not practical, we should arrange to do *something* to help us track issues with structured application.
Hi Richard! I'm wondering if the Dockerized TPS setup that you've been working on might make this easier. We could run TPS with two configs, one with `services.sync.engine.bookmarks.buffer` set and one without...it'll take longer to run all the tests, but this should be temporary; we expect to eventually remove the original engine.
Flags: needinfo?(rpappalardo)
(In reply to Kit Cambridge (they/them) [:kitcambridge] from comment #1) > Hi Richard! I'm wondering if the Dockerized TPS setup that you've been > working on might make this easier. We could run TPS with two configs, one > with `services.sync.engine.bookmarks.buffer` set and one without...it'll > take longer to run all the tests, but this should be temporary; we expect to > eventually remove the original engine. :kitcambridge, absolutely we can. each job is defined by the config (1 for STAGE, 1 for PROD). so in theory, if there were alternate configs, we'd simply add a separate job for each. if you'd rather, we could still put them in 1 job, but we might need to see if it would run beyond the default job timeout. Once you have the new config(s), can you send them to me and I'll set that up?
Flags: needinfo?(rpappalardo) → needinfo?(kit)
(In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #2) > Once you have the new config(s), can you send them to me and I'll set that > up? Awesome, thanks, Richard! Here's the config I use to run TPS locally with the buffered engine: https://gist.github.com/kitcambridge/86fc62d0018327a1833bdcb743a724aa `"services.sync.engine.bookmarks.buffer"` is the only meaningful change.
Flags: needinfo?(kit)
(In reply to Kit Cambridge (they/them) [:kitcambridge] from comment #3) > (In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #2) > > Once you have the new config(s), can you send them to me and I'll set that > > up? > > Awesome, thanks, Richard! Here's the config I use to run TPS locally with > the buffered engine: > https://gist.github.com/kitcambridge/86fc62d0018327a1833bdcb743a724aa > `"services.sync.engine.bookmarks.buffer"` is the only meaningful change. thank you! should this fix the failing test or is it part of the fix? either way, I can go ahead and apply it.
Flags: needinfo?(kit)
(In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #4) > thank you! should this fix the failing test or is it part of the fix? > either way, I can go ahead and apply it. Oh, apologies, I wasn't clear. The config I posted is one we'd like to add, not replace the existing one. So we'll want to run TPS twice, once with `services.sync.engine.bookmarks.buffer` set, and once without. I think we could also restrict the test files for the buffer config to just the bookmark tests, but it's probably easier to just run everything twice.
Flags: needinfo?(kit)
Component: TPS → Sync
Product: Testing → Firefox
Version: Version 3 → unspecified
(In reply to Kit Cambridge (they/them) [:kitcambridge] from comment #5) > (In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #4) > > thank you! should this fix the failing test or is it part of the fix? > > either way, I can go ahead and apply it. > > Oh, apologies, I wasn't clear. The config I posted is one we'd like to add, > not replace the existing one. So we'll want to run TPS twice, once with > `services.sync.engine.bookmarks.buffer` set, and once without. I think we > could also restrict the test files for the buffer config to just the > bookmark tests, but it's probably easier to just run everything twice. Gotcha. and judging from the config, we would only run this additionally on STAGE, true?
Flags: needinfo?(kit)
I think stage-only is fine, yes. Thanks!
Flags: needinfo?(kit)
Priority: -- → P2
(In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #4) > > Awesome, thanks, Richard! Here's the config I use to run TPS locally with > > the buffered engine: > > https://gist.github.com/kitcambridge/86fc62d0018327a1833bdcb743a724aa > > `"services.sync.engine.bookmarks.buffer"` is the only meaningful change. howdy :kitcambridge in your config file you have: "extensiondir": "/Users/k/Projects/mozilla-unified/services/sync/tps/extensions", "testdir": "/Users/k/Projects/mozilla-unified/services/sync/tests/tps", I'm unzipping gecko-dev-master/services/sync into the docker container. obviously, "/Users/k/Projects/mozilla-unified" is local to your machine, but since I see there's a sync/tests/tps dir, shall I adjust the path to that? There is, however, no sync/tps/extensions directory in gecko-dev-master Should i be using the sync/tps/addons directory instead? or do i need to create a new dir for "extensions?" thank you!
Flags: needinfo?(kit)
per :kitcambridge on slack, can use same STAGE config as original but just add the buffer pref
Flags: needinfo?(kit)
:kitcambridge, :markh I've added a second STAGE job w/ the buffer pref which should start running regularly tomorrow. So going forward, you should get three emails on sync-core w/ daily test results: 1 for PROD, 1 for STAGE, 1 for STAGE-buffer
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.