Bug 150332 – WPT server should use its own testharness.js file and generate a warning when it does not match WebKit version

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

[youenn fablet]

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.

[youenn fablet]

Created attachment 263485 [details]
Patch

[youenn fablet]

(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 on attachment 263485 [details]
Patch

Clearing flags on attachment: 263485

Committed r192462: <http://trac.webkit.org/changeset/192462>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 151295

[youenn fablet]

Created attachment 265574 [details]
Fixing python tests

[youenn fablet]

Comment on attachment 265574 [details]
Fixing python tests

Fixing python tests.
Marking it as r? again.

[Ryosuke Niwa]

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]

Created attachment 265852 [details]
Patch for landing

[youenn fablet]

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 on attachment 265852 [details]
Patch for landing

Clearing flags on attachment: 265852

Committed r192617: <http://trac.webkit.org/changeset/192617>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.