Bug 143778

Summary: streams/reference-implementation/readable-stream.html is flaky
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Xabier Rodríguez Calvar <calvaris>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, calvaris, commit-queue, youennf
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 143774    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Alexey Proskuryakov
Reported 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
Attachments
Patch (12.99 KB, patch)
2015-04-17 01:03 PDT, Xabier Rodríguez Calvar 		no flags
DetailsFormatted DiffDiff
Patch (38.35 KB, patch)
2015-04-17 01:55 PDT, Xabier Rodríguez Calvar 		no flags
DetailsFormatted DiffDiff
Patch for landing (38.61 KB, patch)
2015-04-17 02:16 PDT, Xabier Rodríguez Calvar 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2015-04-15 09:37:41 PDT
Marked as flaky in r182843.
Xabier Rodríguez Calvar
Comment 2 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.
youenn fablet
Comment 3 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?
Xabier Rodríguez Calvar
Comment 4 2015-04-17 01:55:01 PDT
Created attachment 251006 [details] Patch Comment out more possibly flaky tests
Xabier Rodríguez Calvar
Comment 5 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.
youenn fablet
Comment 6 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.
Xabier Rodríguez Calvar
Comment 7 2015-04-17 02:16:21 PDT
Created attachment 251008 [details] Patch for landing
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2015-04-17 03:04:44 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.