Bug 171445 - LayoutTests/js/dom/Promise-static-all/race.html are flakey - Unhandled Promise Rejection messages
Summary: LayoutTests/js/dom/Promise-static-all/race.html are flakey - Unhandled Promis...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-28 12:55 PDT by Joseph Pecoraro
Modified: 2017-04-28 18:19 PDT (History)
6 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (1.76 KB, patch)
2017-04-28 13:07 PDT, Joseph Pecoraro
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2017-04-28 12:55:08 PDT
Summary:
LayoutTests/js/dom/Promise-static-all/race.html are flakey - Unhandled Promise Rejection messages

The layout tests creates two rejected promises p7, and p8 but does nothing with them. They are legitimate unhandled promise rejections.

These promises can get garbage collected during the test run. So the logs depend on whether or not they get GC'd before the RejectionTracker's task happens.

I'm going to silence the unhandledrejection errors in these tests.
Comment 1 Joseph Pecoraro 2017-04-28 13:02:42 PDT
> These promises can get garbage collected during the test run.

Hmm, actually I don't think thats true. They are in window.p7 and window.p8.

Either way, I think disabling the messages is the right thing to do but I'll look into this a bit more.
Comment 2 Joseph Pecoraro 2017-04-28 13:06:06 PDT
Running the test in the browser I always see the unhandled rejection errors, which is the behavior I would expect. The messages are not important for this test so I'm going to silence them.

There may be some subtle issue here regarding the order of operations between microtasks and tasks, but again not important for this test and not particularly important in general for these optional events.
Comment 3 Joseph Pecoraro 2017-04-28 13:07:14 PDT
Created attachment 308585 [details]
[PATCH] Proposed Fix
Comment 4 Joseph Pecoraro 2017-04-28 13:07:40 PDT
For reference:
https://build.webkit.org/results/Apple%20iOS%2010%20Simulator%20Release%20WK2%20(Tests)/r215929%20(1023)/results.html

The diffs look like:
--- /Volumes/Data/slave/ios-simulator-10-release-tests-wk2/build/layout-test-results/js/dom/Promise-static-all-expected.txt
+++ /Volumes/Data/slave/ios-simulator-10-release-tests-wk2/build/layout-test-results/js/dom/Promise-static-all-actual.txt
@@ -1,3 +1,5 @@
+CONSOLE MESSAGE: line 1: Unhandled Promise Rejection: p7
+CONSOLE MESSAGE: line 1: Unhandled Promise Rejection: p8
 Test Promise.all
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Comment 5 Joseph Pecoraro 2017-04-28 13:14:32 PDT
<https://trac.webkit.org/r215938>
Comment 7 Ryan Haddad 2017-04-28 17:47:33 PDT
Reopening this per Matt's comment.
Comment 8 Joseph Pecoraro 2017-04-28 18:14:31 PDT
Comment on attachment 308585 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=308585&action=review

> LayoutTests/js/dom/Promise-static-all.html:13
> +// window.onunhandledrejection = () => false;

Oops, this line was commented out by accident.
Comment 9 Joseph Pecoraro 2017-04-28 18:19:55 PDT
Fixed in: https://trac.webkit.org/r215966

Thanks for pointing it out!