Bug 192515

Summary: webkitpy: Ref tests don't respect platform specific expectations
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, ews-watchlist, glenn, lforschler, mmaxfield, rniwa, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews102 for mac-sierra
none
Archive of layout-test-results from ews106 for mac-sierra-wk2
none
Archive of layout-test-results from ews117 for mac-sierra
none
Archive of layout-test-results from ews205 for win-future
none
Patch none

Description Jonathan Bedard 2018-12-07 14:21:57 PST
Working with webkitpy/port/base.py, it looks like we have two different implementations for finding ref tests, there doesn't seem to be a great reason why.
Comment 1 Radar WebKit Bug Importer 2018-12-07 14:22:27 PST
<rdar://problem/46564839>
Comment 2 Jonathan Bedard 2018-12-07 14:45:10 PST
Actually, it seems to be even worse than this: Ref tests use a parallel scheme when compared with all the other layout tests, this scheme doesn't take into consideration platform test expectations.
Comment 3 Jonathan Bedard 2018-12-07 16:39:47 PST
Ok, digging through this code, there is really only one problem: ref tests don't respect platform specific expected results. Apparently we have also have 2 directories which use 'reftest.list' to define what the ref-test mapping is. Thats where my confusion was coming from.
Comment 4 Jonathan Bedard 2018-12-07 16:44:38 PST
Created attachment 356851 [details]
Patch
Comment 5 Jonathan Bedard 2018-12-07 16:46:07 PST
Comment on attachment 356851 [details]
Patch

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

> Tools/Scripts/webkitpy/port/base.py:568
> +        return result

I still need to add a test for using ref test expectations from platform specific directories. Existing unit tests cover everything else quite well.
Comment 6 Jonathan Bedard 2018-12-07 17:30:23 PST
Created attachment 356856 [details]
Patch
Comment 7 EWS Watchlist 2018-12-07 18:28:04 PST
Comment on attachment 356856 [details]
Patch

Attachment 356856 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10312299

New failing tests:
imported/blink/svg/animations/animateTransform-to-mismatched-types.html
Comment 8 EWS Watchlist 2018-12-07 18:28:05 PST
Created attachment 356857 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 9 EWS Watchlist 2018-12-07 18:44:57 PST
Comment on attachment 356856 [details]
Patch

Attachment 356856 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10312337

New failing tests:
imported/blink/svg/animations/animateTransform-to-mismatched-types.html
Comment 10 EWS Watchlist 2018-12-07 18:44:59 PST
Created attachment 356859 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 11 EWS Watchlist 2018-12-07 19:21:29 PST
Comment on attachment 356856 [details]
Patch

Attachment 356856 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10312388

New failing tests:
imported/blink/svg/animations/animateTransform-to-mismatched-types.html
Comment 12 EWS Watchlist 2018-12-07 19:21:31 PST
Created attachment 356860 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 13 EWS Watchlist 2018-12-07 19:26:51 PST
Comment on attachment 356856 [details]
Patch

Attachment 356856 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10312507

New failing tests:
imported/blink/svg/animations/animateTransform-to-mismatched-types.html
Comment 14 EWS Watchlist 2018-12-07 19:27:03 PST
Created attachment 356861 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 15 Jonathan Bedard 2018-12-07 22:23:30 PST
Created attachment 356864 [details]
Patch
Comment 16 Alexey Proskuryakov 2018-12-10 09:52:35 PST
This behavior almost sounds like it may have been intentional - reftests just don't need platform specific results.

It seems acceptable to make it possible if that simplifies the code, but allowing from complexity that we don't need feels a little wrong.
Comment 17 Jonathan Bedard 2018-12-10 10:14:09 PST
(In reply to Alexey Proskuryakov from comment #16)
> This behavior almost sounds like it may have been intentional - reftests
> just don't need platform specific results.
> 
> It seems acceptable to make it possible if that simplifies the code, but
> allowing from complexity that we don't need feels a little wrong.

This change makes <https://bugs.webkit.org/show_bug.cgi?id=192162> easier (Since device-specific expectations share logic with platform specific expectations). I'm also not convinced this was deliberate, Ryosuke was surprised to hear that ref tests did not respect platform expectations, as were the bot watchers.

Not to mention, while it isn't very common, we actually DO have platform specific ref tests in the tree at the moment <https://trac.webkit.org/changeset/186001/webkit> (just one example, there are a few more).
Comment 18 WebKit Commit Bot 2018-12-10 13:48:23 PST
Comment on attachment 356864 [details]
Patch

Clearing flags on attachment: 356864

Committed r239048: <https://trac.webkit.org/changeset/239048>
Comment 19 WebKit Commit Bot 2018-12-10 13:48:25 PST
All reviewed patches have been landed.  Closing bug.