This issue has been detected while importing the W3C Test Suite in bug #170898. There's a timing issue on Mac WK1 and the result doesn't match the expected in the following tests: * imported/w3c/web-platform-tests/css/selectors4/focus-within-shadow-001.html * imported/w3c/web-platform-tests/css/selectors4/focus-within-shadow-002.html * imported/w3c/web-platform-tests/css/selectors4/focus-within-shadow-003.html * imported/w3c/web-platform-tests/css/selectors4/focus-within-shadow-004.html * imported/w3c/web-platform-tests/css/selectors4/focus-within-shadow-005.html And probably the same will happen for focus-within-shadow-006.html, but that's skipped for now due to a different reason (see bug #170899). These tests use the "reftest-wait" class (http://web-platform-tests.org/writing-tests/reftests.html) but maybe that's ignored in WK1 or something. Using things like "testRunner.waitUntilDone()" and "testRunner.notifyDone()" make it work as expected.
Actually I've realized that if we remove the "setTimeout()" call on these tests they pass in both WK1 and WK2, so probably this could be directly fixed upstream on WPT repo.
(In reply to Manuel Rego Casasnovas from comment #1) > Actually I've realized that if we remove the "setTimeout()" call > on these tests they pass in both WK1 and WK2, > so probably this could be directly fixed upstream on WPT repo. I've a PR to fix this on WPT (once it's merged, I'll re-import the tests to get this fixed): https://github.com/w3c/web-platform-tests/pull/5594
Created attachment 309033 [details] Patch
(In reply to Manuel Rego Casasnovas from comment #2) > I've a PR to fix this on WPT (once it's merged, I'll re-import the tests to > get this fixed): > https://github.com/w3c/web-platform-tests/pull/5594 This has been merged now, so I'm re-importing the tests to get this issue fixed. Youenn, please could you take a look? Note that 2 new tests are now failing due to bug #47182: * focus-display-none-001.html * focus-within-display-none-001.html I've added the expected results with the "FAIL" messages. What's the way to go with this kind of issues? I can think in different options: 1) Skip the tests in TextExpectations. Probably not good as we won't realize if the tests start to pass due to some other patch. 2) Add the expected results with the "FAIL" messages (what I'm doing here). 3) Add the expected results with "PASS" messages and mark the tests as Failure on TestExpectations. 4) Any other option? Please tell me what's the preferred one. Thanks.
Ping Youenn, could you take a look? Thanks.
(In reply to Manuel Rego Casasnovas from comment #4) > (In reply to Manuel Rego Casasnovas from comment #2) > > I've a PR to fix this on WPT (once it's merged, I'll re-import the tests to > > get this fixed): > > https://github.com/w3c/web-platform-tests/pull/5594 > > This has been merged now, so I'm re-importing the tests to get this issue > fixed. When you are reimporting tests, you could use -t option to get WPT ToT. Something like: Tools/Scripts/import-w3c-tests web-platform-tests/css/selector4 -t The script will create a hidden clone of WPT repository. > Youenn, please could you take a look? Sure > Note that 2 new tests are now failing due to bug #47182: > * focus-display-none-001.html > * focus-within-display-none-001.html > > I've added the expected results with the "FAIL" messages. > What's the way to go with this kind of issues? > I can think in different options: > 1) Skip the tests in TextExpectations. Probably not good as > we won't realize if the tests start to pass due to some other patch. > 2) Add the expected results with the "FAIL" messages (what I'm doing here). > 3) Add the expected results with "PASS" messages and mark the tests as > Failure > on TestExpectations. > 4) Any other option? > > Please tell me what's the preferred one. Thanks. Right, 2 is the preferred option. It is true that WPT are about conformance and regression testing at the same time. I would favor regression testing.
Comment on attachment 309033 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309033&action=review > LayoutTests/imported/w3c/web-platform-tests/css/selectors4/focus-within-display-none-001.html:9 > +<script src="../../../../../resources/testharnessreport.js"></script> This is one small difference here of importing from the hidden clone or from your specific clone. With the hidden clone, testharness links would remain the same. We might want to switch to that type of links but we are not doing so yet. Also, going from the clone created by the script ensures that you only import changes from WPT master.
Created attachment 309714 [details] Patch for landing
Comment on attachment 309033 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309033&action=review Thanks for the information! >> LayoutTests/imported/w3c/web-platform-tests/css/selectors4/focus-within-display-none-001.html:9 >> +<script src="../../../../../resources/testharnessreport.js"></script> > > This is one small difference here of importing from the hidden clone or from your specific clone. > With the hidden clone, testharness links would remain the same. > We might want to switch to that type of links but we are not doing so yet. > > Also, going from the clone created by the script ensures that you only import changes from WPT master. Ok, I've imported using the "-t" command.
Comment on attachment 309714 [details] Patch for landing Attachment 309714 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3718845 New failing tests: media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-buttons-styles.html
Created attachment 309723 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 309714 [details] Patch for landing EWS failures are unrelated to this patch. Let's land this.
Comment on attachment 309714 [details] Patch for landing Clearing flags on attachment: 309714 Committed r216736: <http://trac.webkit.org/changeset/216736>
All reviewed patches have been landed. Closing bug.