Bug 150332

Summary: WPT server should use its own testharness.js file and generate a warning when it does not match WebKit version
Product: WebKit Reporter: youenn fablet <youennf>
Component: Tools / TestsAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, glenn, lforschler, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 151262, 151295    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Fixing python tests
none
Patch for landing none

youenn fablet
Reported 2015-10-19 09:10:44 PDT
As discussed with rniwa and bem in the past, wpt server uses WebKit testharness.js file to run the tests. This is ok in most cases, except when syncing WPT tests. When syncing WPT tests, one needs first to sync WebKit testharness.js to ensure that nothing wrong happens. Then syncing WPT tests can be done. This adds unnecessary burden to the task of syncing WPT tests, which is already complex enough. It might be more appropriate to: - Always use WPT version for WPT tests. - Add a warning when WK testharness.js is not matching WPT version so that WK testharness.js gets updated.
Attachments
Patch (3.54 KB, patch)
2015-10-19 09:13 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Fixing python tests (7.10 KB, patch)
2015-11-16 01:13 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (7.15 KB, patch)
2015-11-18 23:57 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2015-10-19 09:13:39 PDT
Created attachment 263485 [details] Patch
youenn fablet
Comment 2 2015-10-21 00:18:41 PDT
(In reply to comment #1) > Created attachment 263485 [details] > Patch Need to wait to land this patch as WebKit testharness timeouts are different from WPT default values.
WebKit Commit Bot
Comment 3 2015-11-15 10:17:32 PST
Comment on attachment 263485 [details] Patch Clearing flags on attachment: 263485 Committed r192462: <http://trac.webkit.org/changeset/192462>
WebKit Commit Bot
Comment 4 2015-11-15 10:17:37 PST
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 5 2015-11-15 10:48:19 PST
Re-opened since this is blocked by bug 151295
youenn fablet
Comment 6 2015-11-16 01:13:36 PST
Created attachment 265574 [details] Fixing python tests
youenn fablet
Comment 7 2015-11-16 01:14:08 PST
Comment on attachment 265574 [details] Fixing python tests Fixing python tests. Marking it as r? again.
Ryosuke Niwa
Comment 8 2015-11-16 17:04:47 PST
Comment on attachment 265574 [details] Fixing python tests View in context: https://bugs.webkit.org/attachment.cgi?id=265574&action=review > Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:103 > + wk_testharnessjs_file = self._filesystem.join(self._layout_root, "resources", "testharness.js") Since everything is in WebKit, it's probably better to call it layout_test_testharnessjs_file instead. > Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:104 > + # Next two lines are a temp hack for this patch to land smoothly on bots. They should be removed once patch landed and each bot runs these lines once. Nit: an extra space at the beginning. Also, this should probably be a FIXME. > Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:107 > + # Let's check whether WPT testharness.js is the same as WK version I don't think this comment is necessary. Please remove it. > Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:109 > + _log.warning("\n//////////\nWPT tests are not using the same testharness.js file as other WebKit Layout tests.\nWebKit testharness.js might need to be updated according WPT testharness.js.\n//////////\n") Nit: according WPT testharness.js.
youenn fablet
Comment 9 2015-11-18 23:57:16 PST
Created attachment 265852 [details] Patch for landing
youenn fablet
Comment 10 2015-11-19 00:00:27 PST
Thanks for the review. I updated according your comments... > > Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:109 > > + _log.warning("\n//////////\nWPT tests are not using the same testharness.js file as other WebKit Layout tests.\nWebKit testharness.js might need to be updated according WPT testharness.js.\n//////////\n") > > Nit: according WPT testharness.js. Except for this one. I am not sure what you are suggesting. Let me know and I'll be able to fix that as a follow-up patch.
WebKit Commit Bot
Comment 11 2015-11-19 00:17:50 PST
Comment on attachment 265852 [details] Patch for landing Clearing flags on attachment: 265852 Committed r192617: <http://trac.webkit.org/changeset/192617>
WebKit Commit Bot
Comment 12 2015-11-19 00:17:55 PST
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.