Closed
Bug 1311739
Opened 4 years ago
Closed 4 years ago
Push H/2 test improvements
Categories
(Core :: DOM: Push Notifications, defect)
Core
DOM: Push Notifications
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•4 years ago
|
||
mozreview-review |
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 5•4 years ago
|
||
mozreview-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 6•4 years ago
|
||
mozreview-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
Comment 8•4 years ago
|
||
bugherder |
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: 4 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•