Bug 150332 - WPT server should use its own testharness.js file and generate a warning when it does not match WebKit version
Summary: WPT server should use its own testharness.js file and generate a warning when...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on: 151262 151295
Blocks:
  Show dependency treegraph
 
Reported: 2015-10-19 09:10 PDT by youenn fablet
Modified: 2015-11-19 00:17 PST (History)
6 users (show)

See Also:


Attachments
Patch (3.54 KB, patch)
2015-10-19 09:13 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Fixing python tests (7.10 KB, patch)
2015-11-16 01:13 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (7.15 KB, patch)
2015-11-18 23:57 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 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.
Comment 1 youenn fablet 2015-10-19 09:13:39 PDT
Created attachment 263485 [details]
Patch
Comment 2 youenn fablet 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.
Comment 3 WebKit Commit Bot 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>
Comment 4 WebKit Commit Bot 2015-11-15 10:17:37 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 WebKit Commit Bot 2015-11-15 10:48:19 PST
Re-opened since this is blocked by bug 151295
Comment 6 youenn fablet 2015-11-16 01:13:36 PST
Created attachment 265574 [details]
Fixing python tests
Comment 7 youenn fablet 2015-11-16 01:14:08 PST
Comment on attachment 265574 [details]
Fixing python tests

Fixing python tests.
Marking it as r? again.
Comment 8 Ryosuke Niwa 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.
Comment 9 youenn fablet 2015-11-18 23:57:16 PST
Created attachment 265852 [details]
Patch for landing
Comment 10 youenn fablet 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.
Comment 11 WebKit Commit Bot 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>
Comment 12 WebKit Commit Bot 2015-11-19 00:17:55 PST
All reviewed patches have been landed.  Closing bug.