Bug 171580 - REGRESSION: LayoutTest streams/reference-implementation/readable-stream-templated.html is a flaky failure
Summary: REGRESSION: LayoutTest streams/reference-implementation/readable-stream-templ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-05-02 17:08 PDT by Ryan Haddad
Modified: 2017-05-05 15:37 PDT (History)
8 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (7.52 KB, patch)
2017-05-05 12:03 PDT, Joseph Pecoraro
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2017-05-02 17:08:01 PDT
LayoutTest streams/reference-implementation/readable-stream-templated.html is a flaky failure

https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK1%20(Tests)/r216097%20(935)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=streams%2Freference-implementation%2Freadable-stream-templated.html

--- /Volumes/Data/slave/elcapitan-debug-tests-wk1/build/layout-test-results/streams/reference-implementation/readable-stream-templated-expected.txt
+++ /Volumes/Data/slave/elcapitan-debug-tests-wk1/build/layout-test-results/streams/reference-implementation/readable-stream-templated-actual.txt
@@ -15,7 +15,7 @@
 CONSOLE MESSAGE: Unhandled Promise Rejection: TypeError: cancel() called on a reader owned by no readable stream
 CONSOLE MESSAGE: Unhandled Promise Rejection: TypeError: cancel() called on a reader owned by no readable stream
 
-Harness Error (FAIL), message = cancel() called on a reader owned by no readable stream
+Harness Error (FAIL), message = !!!
 
 PASS Running templatedRSClosed with ReadableStream (closed via call in start) 
 PASS piping to a WritableStream in the writable state should close the writable stream
Comment 1 Ryan Haddad 2017-05-02 17:13:54 PDT
According to the flakiness dashboard, the earliest failure on macOS is 4/28/2017 4:20:02 PM @r215945, but the changes around that time do not appear to be related.
Comment 2 Alexey Proskuryakov 2017-05-04 23:54:24 PDT
The earliest failure I see on internal bots was with r215927, but this failure is not frequent enough to pinpoint the culprit.

Happens on both Mac and iOS.

Looking at the test, I strongly suspect that it became flaky after r215916 (unhandledrejection). Joe, could you please take a look?
Comment 3 Radar WebKit Bug Importer 2017-05-04 23:55:13 PDT
<rdar://problem/32009647>
Comment 4 Joseph Pecoraro 2017-05-05 11:10:07 PDT
I'm unable to reproduce this.

  $ run-webkit-tests --debug --iterations=40 --force streams/reference-implementation
  $ run-webkit-tests --debug --iterations=40 --force streams/reference-implementation -1

So I think the best thing to do is just silence the messages in this test.
Comment 5 Joseph Pecoraro 2017-05-05 11:11:33 PDT
Hmm, the File message is "!!!" instead of the unhandled promise rejection message. Let me try to figure out what could cause that.
Comment 6 Joseph Pecoraro 2017-05-05 12:01:17 PDT
"!!!" is an Error that can be thrown in the tests and is included in promise rejections.
Comment 7 Joseph Pecoraro 2017-05-05 12:03:20 PDT
Created attachment 309194 [details]
[PATCH] Proposed Fix
Comment 8 Alexey Proskuryakov 2017-05-05 12:08:27 PDT
Comment on attachment 309194 [details]
[PATCH] Proposed Fix

rs=me

A couple more things to try when reproducing:

- overload CPUs with many processes (--fully-parallel --child-processes=50 --repeat 1000)
- enable GuardMalloc to change performance characteristics
Comment 9 Joseph Pecoraro 2017-05-05 13:33:00 PDT
<https://trac.webkit.org/changeset/216267/webkit>
Comment 10 youenn fablet 2017-05-05 15:06:02 PDT
The best fix would have been to move from async_test to promise_test.
Landed fix seems good enough to me.
Comment 11 Joseph Pecoraro 2017-05-05 15:25:07 PDT
(In reply to youenn fablet from comment #10)
> The best fix would have been to move from async_test to promise_test.
> Landed fix seems good enough to me.

Yeah, I don't want to modify these tests too much. They are roughly imported, and probably out of date from upstream. I don't know if we should treat these as imported tests and avoid modifications or treat them as our own tests and feel free to modify at will.
Comment 12 youenn fablet 2017-05-05 15:37:34 PDT
Piping is outdated since it ties ReadableStream (up-to-date) to WritableStream (outdated). These tests should probably be removed when improving WritableStream and replaced by WPT existing WritableStream tests