995198 - Uncaught async Promise errors should appear as soon as possible during xpcshell tests

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

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

Subset of bug 976205 that can land without blockers.

Comment 1

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

Attachment: 1. Promise debugging API

This patch has been reviewed by Paolo as part of bug 976205.

Comment 2

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

Attachment: 2. Print uncaught async errors at each run_next_test / add_task.

Patch reviewed by ted as part of bug 976205 + trivial string change.

Comment 3

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

Try: https://tbpl.mozilla.org/?tree=Try&rev=4cd1792c32c9

Comment 4

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

Attachment: 2. Print uncaught async errors at each run_next_test / add_task., v2

I have tweaked xpcshell's head.js a little bit more.
Summary of changes:
* since we always use Promise, we now import it eagerly as global _Promise;
* instead of a TEST-UNEXPECTED-FAIL, we now display a TEST-KNOWN-FAIL and a scary warning.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8405758 [details] [diff] [review]
2. Print uncaught async errors at each run_next_test / add_task., v2

Review of attachment 8405758 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/xpcshell/head.js
@@ +18,5 @@
>  var _cleanupFunctions = [];
>  var _pendingTimers = [];
>  var _profileInitialized = false;
>  
> +let _Promise = Components.utils.import("resource://gre/modules/Promise.jsm", this).Promise;

We should really scope everything in this file to avoid it leaking out to tests. (But you don't have to fix that.)

@@ +1438,5 @@
>      throw new Error("run_next_test() called from an add_task() test function. " +
>                      "run_next_test() should not be called from inside add_task() " +
>                      "under any circumstances!");
>    }
> +    

nit: whitespace

::: testing/xpcshell/xpcshell_b2g.ini
@@ +6,5 @@
>  [include:dom/mobilemessage/tests/xpcshell.ini]
>  [include:dom/network/tests/unit_stats/xpcshell.ini]
>  [include:dom/system/gonk/tests/xpcshell.ini]
>  [include:dom/wappush/tests/xpcshell.ini]
> +[include:toolkit/components/osfile/tests/xpcshell/xpcshell.ini]

This seems unrelated?

Comment 6

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

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> Comment on attachment 8405758 [details] [diff] [review]
> 2. Print uncaught async errors at each run_next_test / add_task., v2
> 
> Review of attachment 8405758 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/xpcshell/head.js
> @@ +18,5 @@
> >  var _cleanupFunctions = [];
> >  var _pendingTimers = [];
> >  var _profileInitialized = false;
> >  
> > +let _Promise = Components.utils.import("resource://gre/modules/Promise.jsm", this).Promise;
> 
> We should really scope everything in this file to avoid it leaking out to
> tests. (But you don't have to fix that.)

That's what I was thinking. Filed as bug 996652.

Comment 7

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

Attachment: 2. Print uncaught async errors at each run_next_test / add_task, v3

Applied feedback.
Try: https://tbpl.mozilla.org/?tree=Try&rev=61af13329e8c

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/963eaafdb70f
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dbade92ce0a

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/963eaafdb70f
https://hg.mozilla.org/mozilla-central/rev/1dbade92ce0a