WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
167538
[css-grid] LayoutTest fast/css-grid-layout/grid-simplified-layout-positioned.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=167538
Summary
[css-grid] LayoutTest fast/css-grid-layout/grid-simplified-layout-positioned....
Ryan Haddad
Reported
2017-01-27 17:32:10 PST
LayoutTest fast/css-grid-layout/grid-simplified-layout-positioned.html is a flaky failure
https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r211310%20(3060)/results.html
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fcss-grid-layout%2Fgrid-simplified-layout-positioned.html
Attachments
Patch
(3.28 KB, patch)
2017-02-10 03:41 PST
,
Manuel Rego Casasnovas
no flags
Details
Formatted Diff
Diff
Patch for landing
(3.42 KB, patch)
2017-02-13 02:17 PST
,
Manuel Rego Casasnovas
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-elcapitan
(930.15 KB, application/zip)
2017-02-13 03:18 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
(877.43 KB, application/zip)
2017-02-13 03:23 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews117 for mac-elcapitan
(1.67 MB, application/zip)
2017-02-13 03:31 PST
,
Build Bot
no flags
Details
New version with timeout
(3.52 KB, patch)
2017-02-13 06:16 PST
,
Manuel Rego Casasnovas
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-elcapitan
(1.18 MB, application/zip)
2017-02-13 07:05 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
(911.46 KB, application/zip)
2017-02-13 07:21 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews116 for mac-elcapitan
(1.67 MB, application/zip)
2017-02-13 07:27 PST
,
Build Bot
no flags
Details
Patch for landing
(3.77 KB, patch)
2017-04-11 00:57 PDT
,
Manuel Rego Casasnovas
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Ryan Haddad
Comment 1
2017-01-27 17:32:43 PST
The images show that the test is expecting the cursor to be present in the text field, but it is missing.
Ryan Haddad
Comment 2
2017-02-09 15:20:35 PST
Marked test as flaky in
http://trac.webkit.org/projects/webkit/changeset/211994
Ryan Haddad
Comment 3
2017-02-09 15:21:05 PST
This does not appear to be a recent regression. CC'ing test author.
Manuel Rego Casasnovas
Comment 4
2017-02-10 02:30:25 PST
It seems that the only issue is that the caret appears or not, the test is not checking anything regarding the caret. The only important thing was that with "autofocus" we were performing a simplified layout that was crashing before the patch that added the test (
bug #163450
). I'll modify the tests trying to get rid of this issue with the caret.
Manuel Rego Casasnovas
Comment 5
2017-02-10 03:41:43 PST
Created
attachment 301146
[details]
Patch Try to fix the test, let's see what EWS has to say.
Michael Catanzaro
Comment 6
2017-02-11 19:55:18 PST
Hm, I wonder if there is a more idiomatic way to force the cursor to not appear? Is the blur a pattern used by many tests to avoid this? When reading the test, I would be confused as to why the blur is present, since there's no comment. I'm kinda surprised that our test infrastructure doesn't have the ability to add two different expectations and allow the test to match either. Blinky cursors are a very common problem in automated tests and most test environments have a standard mechanism for dealing with them.
Darin Adler
Comment 7
2017-02-12 01:34:47 PST
Comment on
attachment 301146
[details]
Patch I am tempted to mark this as reviewed, but I am wondering: Is this still an effective test? Would the call to blur have side-stepped the crash even before we fixed the bug?
Darin Adler
Comment 8
2017-02-12 01:35:49 PST
Another possibility would be to enhance the test runners so that they understand the flashing caret and always take snapshots with the caret visible.
Darin Adler
Comment 9
2017-02-12 01:36:12 PST
Or always with the caret invisible, or any other sensible rule.
Darin Adler
Comment 10
2017-02-12 01:37:16 PST
Comment on
attachment 301146
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=301146&action=review
> LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html:15 > <input autofocus> > +<script> > + document.querySelector("input").blur(); > +</script>
Here in the expected file, maybe we can just omit autofocus instead of calling blur?
Michael Catanzaro
Comment 11
2017-02-12 05:00:01 PST
Comment on
attachment 301146
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=301146&action=review
> LayoutTests/ChangeLog:13 > + "autofocus" was needed to get the crash, so this change modifies > + the test to blur the element, so the caret is not painted anymore.
autofocus was needed to get the crash.
Darin Adler
Comment 12
2017-02-12 10:49:13 PST
(In reply to
comment #11
)
> autofocus was needed to get the crash.
The expected file doesn’t need to crash. That’s why I suggested omitting it *in the expected file*, not in the test file.
Manuel Rego Casasnovas
Comment 13
2017-02-13 02:17:51 PST
Created
attachment 301338
[details]
Patch for landing
Manuel Rego Casasnovas
Comment 14
2017-02-13 02:19:42 PST
Thanks for the reviews. Uploaded new version for landing adding a comment to explain the "blur()" and also removing "autofocus" on the "-expected" file. (In reply to
comment #7
)
> I am tempted to mark this as reviewed, but I am wondering: Is this still an > effective test? Would the call to blur have side-stepped the crash even > before we fixed the bug?
Yeah I've verified that the test is still effective with the blur() part, as the crash was triggered before.
Build Bot
Comment 15
2017-02-13 03:18:36 PST
Comment on
attachment 301338
[details]
Patch for landing
Attachment 301338
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/3077149
New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 16
2017-02-13 03:18:40 PST
Created
attachment 301341
[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
Build Bot
Comment 17
2017-02-13 03:23:36 PST
Comment on
attachment 301338
[details]
Patch for landing
Attachment 301338
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/3077154
New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 18
2017-02-13 03:23:39 PST
Created
attachment 301342
[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
Build Bot
Comment 19
2017-02-13 03:31:29 PST
Comment on
attachment 301338
[details]
Patch for landing
Attachment 301338
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/3077158
New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 20
2017-02-13 03:31:33 PST
Created
attachment 301343
[details]
Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Manuel Rego Casasnovas
Comment 21
2017-02-13 06:16:13 PST
Created
attachment 301349
[details]
New version with timeout
Manuel Rego Casasnovas
Comment 22
2017-02-13 06:18:52 PST
Oops, so in my previous version the "blur()" was run before "autofocus", so it was not useful at all. Both versions the test and the expected file were showing the caret (so probably it would be still flacky on Mac. New version uses setTimeout() on the test file to ensure that blur() is run after "autofocus". Would you mind to take a new look? Thanks.
Build Bot
Comment 23
2017-02-13 07:05:19 PST
Comment on
attachment 301349
[details]
New version with timeout
Attachment 301349
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/3077880
New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 24
2017-02-13 07:05:23 PST
Created
attachment 301350
[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
Build Bot
Comment 25
2017-02-13 07:21:29 PST
Comment on
attachment 301349
[details]
New version with timeout
Attachment 301349
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/3077922
New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 26
2017-02-13 07:21:34 PST
Created
attachment 301354
[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
Build Bot
Comment 27
2017-02-13 07:27:28 PST
Comment on
attachment 301349
[details]
New version with timeout
Attachment 301349
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/3077921
New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 28
2017-02-13 07:27:32 PST
Created
attachment 301355
[details]
Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Manuel Rego Casasnovas
Comment 29
2017-02-14 08:05:38 PST
Comment on
attachment 301349
[details]
New version with timeout Oops so this is still not working on Mac. I'd need to take a deeper look when I find some time, removing from review queue at this point.
Manuel Rego Casasnovas
Comment 30
2017-04-11 00:57:08 PDT
Created
attachment 306784
[details]
Patch for landing Fix test so it waits until the blur() happens, this makets it passes on Mac.
WebKit Commit Bot
Comment 31
2017-04-11 02:20:20 PDT
Comment on
attachment 306784
[details]
Patch for landing Clearing flags on attachment: 306784 Committed
r215222
: <
http://trac.webkit.org/changeset/215222
>
WebKit Commit Bot
Comment 32
2017-04-11 02:20:22 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.
Top of Page
Format For Printing
XML
Clone This Bug