1425341 - Fail xpcshell tests in case of unexpected TypeError, ReferenceError, SyntaxError

Status: ASSIGNED

(Testing :: XPCShell Harness, defect, P3)

Description

[David Teller [:Yoric] - still alive but not very active]

This will require some whitelisting.

See https://treeherder-manifest.herokuapp.com/?repo=try&revision=7019110dfefb1650815bed25b2f1603ca6906ce5 for the results of an experiment: on Linux x64, I see 166 oranges.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1425341 - Fail xpcshell tests in case of developer-level JS Error;r?ahal

This patch ensures that if we have raised a TypeError, SyntaxError or RangeError, we fail the test, even if the error was accidentally caught. This behavior can be deactivated manually as follows:

• either by manually calling ChromeUtils.clearRecentJSDevError() after having thrown the error (recommended if the test expected the error to be thrown);
• or by calling PromiseUtils.thisTestLeaksUncaughtRejectionsAndShouldBeFixed() to postpone fixing the issue.

Depends on D94989

Comment 2

[David Teller [:Yoric] - still alive but not very active]

ahal, you're now in charge!

Comment 3

[Andrew Halberstadt [:ahal]]

Here's a try push with the patch here and in the other bug landed:
https://treeherder.mozilla.org/jobs?repo=try&revision=5001964eda3c8713c4c63ca63741049f71904813

I'll make sure this is at least on our roadmap and try to come up with a plan to keep it moving forward.

Comment 4

[Florian Quèze [:florian]]

(In reply to Andrew Halberstadt [:ahal] from comment #3)

Here's a try push with the patch here and in the other bug landed:
https://treeherder.mozilla.org/jobs?repo=try&revision=5001964eda3c8713c4c63ca63741049f71904813

I'll make sure this is at least on our roadmap and try to come up with a plan to keep it moving forward.

Almost all of the errors seem to be "SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data", maybe we should ignore that specific error message?

Comment 5

[Andrew Halberstadt [:ahal]]

I was hoping we could set up a key in the manifests to disable this feature per-test. Possibly also per issue per test, but maybe start with just disabling it outright at the test level. Of course I'm not sure when anyone will have time to work on that, so I'm sympathetic to something quick and dirty in the short term.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:Yoric, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit auto_nag documentation.

Comment 8

[Andrew Halberstadt [:ahal]]

This is something that we probably still want, but we can't land it until tests have all been updated or we come up with some sort of exception list system. It's likely also bitrotted by now.