streams/reference-implementation/readable-stream.html is flaky

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

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

[Alexey Proskuryakov]

Marked as flaky in r182843.

[Xabier Rodríguez Calvar]

Created attachment 251005 [details]
Patch

Comment out flaky tests while working on a more complete solution on bug 143774.

[youenn fablet]

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?

[Xabier Rodríguez Calvar]

Created attachment 251006 [details]
Patch

Comment out more possibly flaky tests

[Xabier Rodríguez Calvar]

(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.

[youenn fablet]

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.

[Xabier Rodríguez Calvar]

Created attachment 251008 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 251008 [details]
Patch for landing

Clearing flags on attachment: 251008

Committed r182942: <http://trac.webkit.org/changeset/182942>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.