Shared support for failing tests on TypeError, SyntaxError, ReferenceError
Categories
(Testing :: General, enhancement, P3)
Tracking
(firefox84 fixed)
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: Yoric, Assigned: Yoric)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
No description provided.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8937826 [details] Bug 1426219 - Assert.throws now cleans up recentJSDevError; https://reviewboard.mozilla.org/r/208526/#review214506 Did you mean to also request review on this part? ::: testing/modules/Assert.jsm:371 (Diff revision 1) > + // that we probably need to clean it up. > + let cleanupRecentJSDevError = false; > + if ("recentJSDevError" in ChromeUtils) { > + // Check that we're in a build of Firefox that supports > + // the `recentJSDevError` mechanism (i.e. Nightly build). > + if (ChromeUtils.recentJSDevError == undefined) { Was this meant to be an === check? ::: testing/modules/Assert.jsm:407 (Diff revision 1) > this.report(false, expected, expected, message); > + > + // Make sure that we don't cause failures for JS Dev Errors that > + // were expected, typically for tests that attempt to check > + // that we react properly to TypeError, ReferenceError, SyntaxError. > + if (cleanupRecentJSDevError && "clearRecentJSDevError" in ChromeUtils) { I would put braces around the 'in' check here... but I actually I think we can just remove it completely as cleanupRecentJSDevError will never be true in a build that doesn't support this.
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8937825 [details] Bug 1426219 - Extend PromiseTestUtils to uncaught JavaScript Developer Errors; https://reviewboard.mozilla.org/r/208524/#review214508 ::: toolkit/modules/tests/modules/PromiseTestUtils.jsm:107 (Diff revision 1) > + return; > + } > + let recentJSDevError = ChromeUtils.recentJSDevError; > + if (recentJSDevError) { > + ChromeUtils.clearRecentJSDevError(); > + let message = recentJSDevError.message + "\n" + recentJSDevError.stack + "\n----\n Detected at\n"; nit: this may look nicer with a template string. I would be curious to see what the test log looks like. Should we add ':' after 'Detected at'? Is the the abuse of promise rejections not causing confusing output?
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 5•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8937825 [details] Bug 1426219 - Extend PromiseTestUtils to uncaught JavaScript Developer Errors; https://reviewboard.mozilla.org/r/208524/#review214508 > nit: this may look nicer with a template string. > > I would be curious to see what the test log looks like. Should we add ':' after 'Detected at'? Is the the abuse of promise rejections not causing confusing output? I'll admit that it's a bit confusing. Sigh. I probably should create a new module for that purpose.
Updated•6 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 6•4 years ago
|
||
This patch uses (and somewhat abuses) the existing PromiseTestUtils
mechanism to also be able to whitelist uncaught errors that are not
actual Promise rejections.
This uses ChromeUtils.recentJSDevError
, which lets us find out
whether there is a recent ReferenceError/SyntaxError/TypeError in
chrome code, even if that error was caught.
MozReview-Commit-ID: 5z1pffURNYm
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
Depends on D94972
Assignee | ||
Comment 8•4 years ago
|
||
We are throwing TypeError
in many cases that are not type errors. Since we have no manner of distinguishing which of these cases are actual (developer) errors and which are runtime exceptions, this patch removes TypeError
from the list of developer errors.
Depends on D94972
Updated•4 years ago
|
Pushed by dteller@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/38a6d65eb220 TypeError is not a developer error anymore;r=yulia https://hg.mozilla.org/integration/autoland/rev/8fba1860ba2b Extend PromiseTestUtils to uncaught JavaScript Developer Errors;r=florian https://hg.mozilla.org/integration/autoland/rev/371e4b2a0fef Assert.throws now cleans up recentJSDevError;r=ahal
Comment 10•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/38a6d65eb220
https://hg.mozilla.org/mozilla-central/rev/8fba1860ba2b
https://hg.mozilla.org/mozilla-central/rev/371e4b2a0fef
Description
•