Bug 192515 – webkitpy: Ref tests don't respect platform specific expectations

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

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.

[Radar WebKit Bug Importer]

<rdar://problem/46564839>

[Jonathan Bedard]

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.

[Jonathan Bedard]

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.

[Jonathan Bedard]

Created attachment 356851 [details]
Patch

[Jonathan Bedard]

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.

[Jonathan Bedard]

Created attachment 356856 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[Jonathan Bedard]

Created attachment 356864 [details]
Patch

[Alexey Proskuryakov]

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.

[Jonathan Bedard]

(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).

[WebKit Commit Bot]

Comment on attachment 356864 [details]
Patch

Clearing flags on attachment: 356864

Committed r239048: <https://trac.webkit.org/changeset/239048>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.