1383709 - Perma failure when 56 merges to beta in browser_notification_open_settings.js | Test timed out -

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Carsten Book [:Tomcat]]

[Tracking Requested - why for this release]:

found via uplift simulations: -> https://treeherder.mozilla.org/logviewer.html#?job_id=116862210&repo=try

 TEST-UNEXPECTED-FAIL | browser/base/content/test/alerts/browser_notification_open_settings.js | Test timed out - 

this is a very frequent failure in the uplift simulations so need to be fixed before 56 merges to beta next week.

Maybe from Bug 1381944 ?

Comment 1

[Carsten Book [:Tomcat]]

Mike, Jared: could you take a look ? Thanks!

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1383709 - Don't wait for the sync-pane-loaded notification in old prefs since it won't be fired when only viewing the Content pane.

Waiting for this notification is only necessary in the 'new prefs', as detailed by the comment that I added in this patch.

Review commit: https://reviewboard.mozilla.org/r/160728/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/160728/

Comment 3

[Gerry Chang [:gchang]]

Track 56+ as this may affect 56 merging to beta.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8889664 [details]
Bug 1383709 - Don't wait for the sync-pane-loaded notification in old prefs since it won't be fired when only viewing the Content pane.

https://reviewboard.mozilla.org/r/160728/#review166780

::: browser/base/content/test/alerts/browser_notification_open_settings.js:20
(Diff revision 1)
> -    // Ensure preferences is loaded before removing the tab.
> -    let syncPaneLoadedPromise = TestUtils.topicObserved("sync-pane-loaded", () => true);
> +    // Ensure preferences is loaded before removing the tab. In the "new prefs", all categories
> +    // get initialized on page load since we need to be able to search them. Sync is *very*
> +    // slow to load and therefore we need to wait for it to load when testing the "new prefs".
> +    // For "old prefs" we only load the actual visited categories so we don't have this problem,
> +    // as well, the "sync-pane-loaded" notification is not sent on "old prefs".
> +    let syncPaneLoadedPromise = useOldPrefs || TestUtils.topicObserved("sync-pane-loaded", () => true);

TIL you can `await` a non-Promise and it'll be automatically coerced into a resolved Promise. Neat!

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Indeed!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35b75b53f9d3
Don't wait for the sync-pane-loaded notification in old prefs since it won't be fired when only viewing the Content pane. r=mconley

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/35b75b53f9d3

Comment 8

[Intermittent Failures Robot]

21 failures in 1008 pushes (0.021 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* try: 21

Platform breakdown:
* linux64: 6
* windows8-64: 4
* windows7-32: 4
* linux32: 3
* windows7-32-vm: 2
* osx-10-10: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1383709&startday=2017-07-24&endday=2017-07-30&tree=all