1016387 - Uncaught async Promise errors should cause mochi tests to fail

Status: RESOLVED FIXED

(Toolkit :: Async Tooling, defect)

Comment 1

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

Attachment: Causing tests to fail in case of uncaught async error

Comment 2

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

Attachment: Causing tests to fail in case of uncaught async error, v2

Same patch with less line noise in the message.

Comment 3

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

Unfortunately, the blockers for this bug have actually regressed in the past few months. I'll change strategy and whitelist offenders.

Comment 4

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

Attachment: 2. Whitelist mechanism

Comment 5

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

Attachment: 3. Whitelisting offenders from bc1

Whitelisting the offenders from mochitest-bc1. Gijs, you have been picked as reviewer :)

Comment 6

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

Attachment: 4. Whitelisting offenders from bc3

Dao, this one-liner is for you.

Comment 7

[:Gijs (he/him)]

Comment on attachment 8499542 [details] [diff] [review]
3. Whitelisting offenders from bc1

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

Please fix the trailing whitespace.

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8499550 [details] [diff] [review]
4. Whitelisting offenders from bc3

I don't care much since it's a test, but the empty comment lines are weird.

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 8499538 [details] [diff] [review]
2. Whitelist mechanism

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

::: testing/mochitest/browser-test.js
@@ +907,5 @@
>      self.SimpleTest.ignoreAllUncaughtExceptions(aIgnoring);
>    };
>  
> +  this.thisTestLeaksUncaughtRejectionsAndShouldBeFixed = function() {
> +    aTester._treatUncaughtRejectionsAsFailures = false;

I feel like these ought to wind up in the manifest, but that's probably not as straightforward.

Comment 10

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

Attachment: 2. Whitelist mechanism, v2

As discussed over IRC, here is a more explicit v2.

Comment 11

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

Attachment: 3. Whitelisting offenders from bc1, v2

Applied feedback.

Comment 12

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

Attachment: 4. Whitelisting offenders from bc3, v2

Applied feedback.

Comment 13

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

Attachment: 5. Whitelisting more offenders from bc3

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8500053 [details] [diff] [review]
4. Whitelisting offenders from bc3, v2

>+///////////////////
>+//
>+// Whitelisting this test.
>+// As part of bug 1077403, the leaking uncaught rejection should be fixed.
>+//
>+thisTestLeaksUncaughtRejectionsAndShouldBeFixed("");

That's even worse. Should be like this:

// Whitelisting this test.
// As part of bug 1077403, the leaking uncaught rejection should be fixed.
thisTestLeaksUncaughtRejectionsAndShouldBeFixed("");

Though since the verbose function name says it all, "Whitelisting this test" could also be spared...

Comment 15

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

Attachment: 6. Whitelisting offenders from devtools

And here comes the main course: offenders from devtools. Paul, I'd still be grateful if all of these errors could be fixed instead of being whitelisted.

Comment 16

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

Attachment: 4. Whitelisting offenders from bc3, v3

As you wish. I just wanted to make sure that whoever maintains the test cannot miss that single line, because in my experience, most of the uncaught errors represent actual bugs.

Comment 17

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

Apparently, the whitelist is sufficient: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6b7784218b69

Comment 18

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

Let's try and land this.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/746989e4c8d1
https://hg.mozilla.org/integration/fx-team/rev/7fd7d28cd78f
https://hg.mozilla.org/integration/fx-team/rev/039f1a265da2
https://hg.mozilla.org/integration/fx-team/rev/71abd0791f96

Comment 20

[Carsten Book [:Tomcat]]

sorry had to backout this change for test failures like

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=866707&repo=fx-team
and 
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=866722&repo=fx-team

Comment 21

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

Tomcat, according to comment 19, you only landed 4 patches out of 7. This is consistent with the error message in the logs: part 2 is missing.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/301cf058df9c
https://hg.mozilla.org/integration/fx-team/rev/a80dac10afbe
https://hg.mozilla.org/integration/fx-team/rev/10929cc5152d
https://hg.mozilla.org/integration/fx-team/rev/62c89722e983
https://hg.mozilla.org/integration/fx-team/rev/b2857344efc8
https://hg.mozilla.org/integration/fx-team/rev/50f07054fd43

Comment 23

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

Attachment: 7. Whitelisting one more offender from devtools

Ah, apparently, one more test needs to be whitelisted.

Comment 24

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

Attachment: 7. Whitelisting one more offender from devtools, v2

Hopefully, this should take care of all the devtools oranges that have appeared since my last Try.

Comment 25

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

Attachment: 8. Whitelisting offenders from bc1, part 2

And another new one, probably intermittent.

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/9663dfdca203
https://hg.mozilla.org/integration/fx-team/rev/4b851bb56ee5

Comment 27

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

Attachment: 9. Whitelisting offenders from devtools, part 3

Comment 28

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

Attachment: 10. Whitelisting offenders from bc1, part 3

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/cec9e29a0ce9
https://hg.mozilla.org/integration/fx-team/rev/50e51c7922ec

Comment 30

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

Attachment: 11. Whitelisting offenders from bc3, part 3

And one more to go...

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/58b3284c18fe
https://hg.mozilla.org/integration/fx-team/rev/2c4057903a5b

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/301cf058df9c
https://hg.mozilla.org/mozilla-central/rev/a80dac10afbe
https://hg.mozilla.org/mozilla-central/rev/10929cc5152d
https://hg.mozilla.org/mozilla-central/rev/62c89722e983
https://hg.mozilla.org/mozilla-central/rev/b2857344efc8
https://hg.mozilla.org/mozilla-central/rev/50f07054fd43
https://hg.mozilla.org/mozilla-central/rev/9663dfdca203
https://hg.mozilla.org/mozilla-central/rev/4b851bb56ee5
https://hg.mozilla.org/mozilla-central/rev/cec9e29a0ce9
https://hg.mozilla.org/mozilla-central/rev/50e51c7922ec
https://hg.mozilla.org/mozilla-central/rev/58b3284c18fe
https://hg.mozilla.org/mozilla-central/rev/2c4057903a5b