Closed Bug 1311739 Opened 3 years ago Closed 3 years ago

Push H/2 test improvements

Categories

(Core :: DOM: Push Notifications, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: Lina, Assigned: Lina)

Details

Attachments

(3 files)

:hurley has a Try run where several of the Push H/2 tests time out. A few things stand out from the log at https://archive.mozilla.org/pub/firefox/try-builds/hurley@todesschaf.org-b66f2859f3a291570367cffeb08f47af94cf0ce9/try-macosx64/try_yosemite_r7_test-xpcshell-bm136-tests1-macosx-build678.txt.gz:

* It looks like the MOZHTTP2_PORT env var is empty, causing the tests to try to connect to "https://localhost:/". That looks like a different bug, but I think we should fail noisily instead of retrying and eventually timing out.

* `PushDB.update` returns undefined instead of throwing if a record doesn't exist, or the new record is invalid. I can't remember why it did that, but it's broken because callers don't null-check the result. This causes us to swallow rejections and print errors like '[JavaScript Error: "TypeError: record is undefined" {file: "resource://gre/modules/PushServiceHttp2.jsm" line: 667}]'.

* We don't observe 'serverURL' in tests, so there's lots of 'JavaScript Error: "Attempt to stop observing a preference "serverURL" that's not being observed" {file: "resource://gre/modules/Preferences.jsm" line: 323}' logspam.
Comment on attachment 8802967 [details]
Bug 1311739 - Fail push tests with an informative error if the H/2 server port is missing.

https://reviewboard.mozilla.org/r/87220/#review87034
Attachment #8802967 - Flags: review?(dd.mozilla) → review+
Comment on attachment 8802968 [details]
Bug 1311739 - `PushDB.update` should reject instead of returning `undefined` for invalid records.

https://reviewboard.mozilla.org/r/87222/#review87036
Attachment #8802968 - Flags: review?(dd.mozilla) → review+
Comment on attachment 8802969 [details]
Bug 1311739 - Silence `serverURL` pref warnings in push tests.

https://reviewboard.mozilla.org/r/87224/#review87050
Attachment #8802969 - Flags: review?(dd.mozilla) → review+
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/357a4a25901a
Fail push tests with an informative error if the H/2 server port is missing. r=dragana
https://hg.mozilla.org/integration/autoland/rev/c0c3b590372c
`PushDB.update` should reject instead of returning `undefined` for invalid records. r=dragana
https://hg.mozilla.org/integration/autoland/rev/8fd3c7221c70
Silence `serverURL` pref warnings in push tests. r=dragana
https://hg.mozilla.org/mozilla-central/rev/357a4a25901a
https://hg.mozilla.org/mozilla-central/rev/c0c3b590372c
https://hg.mozilla.org/mozilla-central/rev/8fd3c7221c70
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.