Bug 172523 - Some async tests ignore jsTestIsAsync when ported from js-test-pre.js / js-test-post.js to js-test.js
Summary: Some async tests ignore jsTestIsAsync when ported from js-test-pre.js / js-te...
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: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 172527
  Show dependency treegraph
 
Reported: 2017-05-23 14:12 PDT by Chris Dumez
Modified: 2017-05-23 17:27 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.97 KB, patch)
2017-05-23 14:16 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (1.69 MB, application/zip)
2017-05-23 15:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.76 MB, application/zip)
2017-05-23 15:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-elcapitan (2.51 MB, application/zip)
2017-05-23 15:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (15.07 MB, application/zip)
2017-05-23 15:29 PDT, Build Bot
no flags Details
Patch (6.92 KB, patch)
2017-05-23 15:32 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (6.93 KB, patch)
2017-05-23 15:51 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-05-23 14:12:44 PDT
Some async tests ignore jsTestIsAsync when ported from js-test-pre.js / js-test-post.js to js-test.js.

Impacts LayoutTests/fast/dom/script-charset-update.html.

The onload event is fired too late, the test has already completed.
Comment 1 Chris Dumez 2017-05-23 14:16:16 PDT
Created attachment 311050 [details]
Patch
Comment 2 Build Bot 2017-05-23 15:03:37 PDT
Comment on attachment 311050 [details]
Patch

Attachment 311050 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3802616

New failing tests:
fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events.html
fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html
Comment 3 Build Bot 2017-05-23 15:03:38 PDT
Created attachment 311059 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 4 Chris Dumez 2017-05-23 15:10:55 PDT
(In reply to Build Bot from comment #2)
> Comment on attachment 311050 [details]
> Patch
> 
> Attachment 311050 [details] did not pass mac-ews (mac):
> Output: http://webkit-queues.webkit.org/results/3802616
> 
> New failing tests:
> fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events.
> html
> fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html

Well, it shows bugs in 2 existing tests. Those tests do not have "TEST COMPLETE" in their expected result.
Comment 5 Build Bot 2017-05-23 15:15:11 PDT
Comment on attachment 311050 [details]
Patch

Attachment 311050 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3802679

New failing tests:
fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html
Comment 6 Build Bot 2017-05-23 15:15:13 PDT
Created attachment 311061 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 7 Chris Dumez 2017-05-23 15:20:01 PDT
(In reply to Chris Dumez from comment #4)
> (In reply to Build Bot from comment #2)
> > Comment on attachment 311050 [details]
> > Patch
> > 
> > Attachment 311050 [details] did not pass mac-ews (mac):
> > Output: http://webkit-queues.webkit.org/results/3802616
> > 
> > New failing tests:
> > fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events.
> > html
> > fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html
> 
> Well, it shows bugs in 2 existing tests. Those tests do not have "TEST
> COMPLETE" in their expected result.

diff --git a/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events-expected.txt b/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events-expected.txt
index 72acf40705e..3f61c85285c 100644
--- a/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events-expected.txt
+++ b/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events-expected.txt
@@ -1,3 +1,4 @@
+FAIL: Timed out waiting for notifyDone to be called
 Exercises the requestFrame() call on captureStream(0).
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
@@ -7,4 +8,7 @@ PASS Got a stream from canvas.
 PASS stream.getVideoTracks().length is 1
 PASS track.readyState is "live"
 PASS Plugged stream to video tag.
+PASS Video play callback succeeded.
+PASS Drawing to canvas.
+PASS Calling requestFrame().
  

So this once goes further but times out after calling requestFrame() now that jsTestIsAsync is actually taken into consideration. Youenn, any idea?

diff --git a/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events-expected.txt b/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events-expected.txt
index 6d898fe17dd..a1250b415cb 100644
--- a/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events-expected.txt
+++ b/LayoutTests/fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events-expected.txt
@@ -1,3 +1,4 @@
+CONSOLE MESSAGE: line 33: Turning drawing buffer preservation for the WebGL canvas being captured
 Exercises the potential events on CanvasCaptureMediaStream.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
@@ -7,4 +8,10 @@ PASS Got a stream from canvas.
 PASS stream.getVideoTracks().length is 1
 PASS track.readyState is "live"
 PASS Plugged stream to video tag.
+PASS Video play callback succeeded.
+PASS Drawing to canvas.
+PASS Video canplaythrough callback succeeded.
+PASS successfullyParsed is true
+
+TEST COMPLETE
 
Ok, This test now completes and passes so this is actually a progression.
Comment 8 Chris Dumez 2017-05-23 15:23:08 PDT
Note that these tests are marked as flaky on various platforms already. E.g.
https://bugs.webkit.org/show_bug.cgi?id=169719
Comment 9 Build Bot 2017-05-23 15:25:02 PDT
Comment on attachment 311050 [details]
Patch

Attachment 311050 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3802639

New failing tests:
fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events.html
fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html
Comment 10 Build Bot 2017-05-23 15:25:04 PDT
Created attachment 311063 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 11 Build Bot 2017-05-23 15:29:08 PDT
Comment on attachment 311050 [details]
Patch

Attachment 311050 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3802660

New failing tests:
fast/mediacapturefromelement/CanvasCaptureMediaStream-webgl-events.html
Comment 12 Build Bot 2017-05-23 15:29:10 PDT
Created attachment 311065 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 13 Chris Dumez 2017-05-23 15:32:05 PDT
Created attachment 311066 [details]
Patch
Comment 14 Ryosuke Niwa 2017-05-23 15:33:29 PDT
Comment on attachment 311066 [details]
Patch

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

> LayoutTests/TestExpectations:144
> +# This test times out.
> +webkit.org/b/172527 fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events.html [ Skip ]

Why don't we use Timeout expectation instead?
Comment 15 Alexey Proskuryakov 2017-05-23 15:43:06 PDT
A 100% reproducible timeout takes 30 seconds of time, making tests slower overall.
Comment 16 Chris Dumez 2017-05-23 15:43:41 PDT
(In reply to Ryosuke Niwa from comment #14)
> Comment on attachment 311066 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=311066&action=review
> 
> > LayoutTests/TestExpectations:144
> > +# This test times out.
> > +webkit.org/b/172527 fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events.html [ Skip ]
> 
> Why don't we use Timeout expectation instead?

Doesn't it actually make running the tests slower since we're going to wait for this test to time out?
Comment 17 Alexey Proskuryakov 2017-05-23 15:45:50 PDT
Comment on attachment 311066 [details]
Patch

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

> LayoutTests/ChangeLog:19
> +        Unskip test that is not timing out consistently instead of being flaky.

not -> now?

As you are actually skipping the test, it seems better to say "remove expectation for a test that is now skipped in root TestExpectations file".
Comment 18 Chris Dumez 2017-05-23 15:51:02 PDT
Created attachment 311068 [details]
Patch
Comment 19 WebKit Commit Bot 2017-05-23 17:27:27 PDT
Comment on attachment 311068 [details]
Patch

Clearing flags on attachment: 311068

Committed r217312: <http://trac.webkit.org/changeset/217312>
Comment 20 WebKit Commit Bot 2017-05-23 17:27:29 PDT
All reviewed patches have been landed.  Closing bug.