Closed
Bug 1261181
Opened 8 years ago
Closed 8 years ago
Various Sync xpcshell tests are going to permafail when Gecko is bumped to version 49
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: RyanVM, Assigned: markh)
Details
Attachments
(1 file)
27.11 KB,
patch
|
lina
:
review+
RyanVM
:
feedback+
|
Details | Diff | Splinter Review |
Merge day is about two weeks away, so this is pretty critical. https://treeherder.mozilla.org/logviewer.html#?job_id=18720292&repo=try
Flags: needinfo?(markh)
Assignee | ||
Comment 1•8 years ago
|
||
As seems customary these days, most of these errors are due to telemetry probes expiring.
Flags: firefox-backlog+
Assignee | ||
Comment 2•8 years ago
|
||
Lots of probes are set to expire in 48, so this removes them completely (ie, from Histograms.json, from the code and from the tests). Ryan, can you please verify this solves the test failures? The all still pass locally for me, but I'm not sure how to push a try run that simulates a world post-merge.
Assignee: nobody → markh
Status: NEW → ASSIGNED
Flags: needinfo?(markh) → needinfo?(ryanvm)
Attachment #8737044 -
Flags: review?(kcambridge)
Reporter | ||
Comment 3•8 years ago
|
||
https://hg.mozilla.org/try/rev/75ce8e268e62 is all you need to apply locally.
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6b60d777e9e
Comment 5•8 years ago
|
||
Comment on attachment 8737044 [details] [diff] [review] 0001-Bug-1261181-remove-telemetry-probes-set-to-expire-in.patch Review of attachment 8737044 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! <3 deleting superfluous code. ::: services/sync/modules/engines.js @@ +1228,5 @@ > Observers.notify("weave:engine:sync:applied", count, this.name); > }, > > _noteApplyFailure: function () { > + // here would be a good place to record telemetry... The probes we had weren't actionable, I assume.
Attachment #8737044 -
Flags: review?(kcambridge) → review+
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Kit Cambridge [:kitcambridge] from comment #5) > The probes we had weren't actionable, I assume. Probably closer to saying "they weren't being actioned" (but yeah, last I looked we actioned all we could). Note that even without this change, the probes were being ignored starting 49, so there's no value in keeping them without bumping the expires version. 48 will be in release until August this year, so the data is still available, and I figured we should just re-add new probes as we find them necessary.
Comment 7•8 years ago
|
||
Cool, sounds like a plan!
Reporter | ||
Comment 8•8 years ago
|
||
Comment on attachment 8737044 [details] [diff] [review] 0001-Bug-1261181-remove-telemetry-probes-set-to-expire-in.patch Looks good on Try, thanks!
Attachment #8737044 -
Flags: feedback+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/eaab4a335361
Updated•6 years ago
|
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•