Bug 143778 - streams/reference-implementation/readable-stream.html is flaky
Summary: streams/reference-implementation/readable-stream.html is flaky
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Xabier Rodríguez Calvar
URL:
Keywords:
Depends on: 143774
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-15 09:33 PDT by Alexey Proskuryakov
Modified: 2015-04-17 03:04 PDT (History)
4 users (show)

See Also:


Attachments
Patch (12.99 KB, patch)
2015-04-17 01:03 PDT, Xabier Rodríguez Calvar
no flags Details | Formatted Diff | Diff
Patch (38.35 KB, patch)
2015-04-17 01:55 PDT, Xabier Rodríguez Calvar
no flags Details | Formatted Diff | Diff
Patch for landing (38.61 KB, patch)
2015-04-17 02:16 PDT, Xabier Rodríguez Calvar
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2015-04-15 09:33:55 PDT
streams/reference-implementation/readable-stream.html frequently fails on bots, especially slower ones: https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=streams%2Freference-implementation%2Freadable-stream.html

@@ -10,12 +10,12 @@
 FAIL ReadableStream should be able to enqueue different objects. read is not implemented
 PASS ReadableStream: if start throws an error, it should be re-thrown 
 FAIL ReadableStream: if pull rejects, it should error the stream read is not implemented
-FAIL ReadableStream: should only call pull once upon starting the stream assert_equals: pull should be called once start finishes expected 1 but got 0
+FAIL ReadableStream: should only call pull once upon starting the stream assert_equals: pull should be called exactly once expected 1 but got 0
 FAIL ReadableStream: should only call pull once for a forever-empty stream, even after reading read is not implemented
 FAIL ReadableStream: should only call pull once on a non-empty stream read from before start fulfills read is not implemented
-FAIL ReadableStream: should only call pull twice on a non-empty stream read from after start fulfills assert_equals: pull should be called once start finishes expected 1 but got 0
-FAIL ReadableStream: should call pull in reaction to read()ing the last chunk, if not draining assert_equals: pull should have been called once after read expected 1 but got 0
-FAIL ReadableStream: should not call pull() in reaction to read()ing the last chunk, if draining assert_equals: pull should have been called once after read expected 1 but got 0
+FAIL ReadableStream: should only call pull twice on a non-empty stream read from after start fulfills assert_equals: pull should be called exactly twice expected 2 but got 0
+FAIL ReadableStream: should call pull in reaction to read()ing the last chunk, if not draining assert_equals: pull should be called exactly thrice expected 3 but got 0
+FAIL ReadableStream: should not call pull() in reaction to read()ing the last chunk, if draining assert_equals: pull should be called exactly twice expected 2 but got 0
 FAIL ReadableStream: should not call pull until the previous pull call's promise fulfills read is not implemented
 FAIL ReadableStream: should pull after start, and after every read read is not implemented
 FAIL ReadableStream: should not call pull after start if the stream is now closed read is not implemented
Comment 1 Alexey Proskuryakov 2015-04-15 09:37:41 PDT
Marked as flaky in r182843.
Comment 2 Xabier Rodríguez Calvar 2015-04-17 01:03:44 PDT
Created attachment 251005 [details]
Patch

Comment out flaky tests while working on a more complete solution on bug 143774.
Comment 3 youenn fablet 2015-04-17 01:22:41 PDT
Comment on attachment 251005 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +

Can you add the reason of the flakiness in the changelog: order of execution of setTimeout callbacks and promise resolution callbacks is not deterministic, making first failing assertion changing.
Also, there may be other tests in this file that need to be commented out.
Have you checked all async tests?

> LayoutTests/streams/reference-implementation/readable-stream-expected.txt:13
>  FAIL ReadableStream: should only call pull once for a forever-empty stream, even after reading read is not implemented

Can this test be flakky?
Comment 4 Xabier Rodríguez Calvar 2015-04-17 01:55:01 PDT
Created attachment 251006 [details]
Patch

Comment out more possibly flaky tests
Comment 5 Xabier Rodríguez Calvar 2015-04-17 01:58:28 PDT
(In reply to comment #3)
> Can you add the reason of the flakiness in the changelog: order of execution
> of setTimeout callbacks and promise resolution callbacks is not
> deterministic, making first failing assertion changing.

Done.

> Also, there may be other tests in this file that need to be commented out.
> Have you checked all async tests?

I checked all tests relying on a timeout to call done(), specially the ones that check some condition prior to finish.

> > LayoutTests/streams/reference-implementation/readable-stream-expected.txt:13
> >  FAIL ReadableStream: should only call pull once for a forever-empty stream, even after reading read is not implemented
> 
> Can this test be flakky?

It was subject to, yes.
Comment 6 youenn fablet 2015-04-17 02:07:42 PDT
Comment on attachment 251006 [details]
Patch

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

> LayoutTests/ChangeLog:15
> +

The explanation is a bit hard to understand for me as written.
It seems more related to promise resolution/setTimeout callbacks being run in an undeterministic way.
Additionaly, even if we had all features implemented correctly, we may hit done() before executing other callbacks, hence not executing some important code like checking assertions...

Let's ship the changes and check whether flakiness disappear.
Comment 7 Xabier Rodríguez Calvar 2015-04-17 02:16:21 PDT
Created attachment 251008 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2015-04-17 03:04:39 PDT
Comment on attachment 251008 [details]
Patch for landing

Clearing flags on attachment: 251008

Committed r182942: <http://trac.webkit.org/changeset/182942>
Comment 9 WebKit Commit Bot 2015-04-17 03:04:44 PDT
All reviewed patches have been landed.  Closing bug.