New issue
Advanced search Search tips

Issue 787829 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

WPT import can cause failures in LayoutTests/http, which then blocks import

Project Member Reported by foolip@chromium.org, Nov 22 2017

Issue description

https://chromium-review.googlesource.com/c/chromium/src/+/784893 was a big import+changes CL to get around the fact that import was failing. The cause and failed import is linked in that review.

It's a bug that changes in LayoutTests/external/wpt can affect those tests.
 
Root cause: https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/servers/apache_http.py?l=83&rcl=dc0d5bedfc2bee965938fb7bbbf68da1947b6373

Non-WPT layout tests have one more dependency on WPT resources: webidl2.js.

I think we should also check this file into LayoutTests/resources, and include it in  issue 685854 , so that we completely cut the dependency and make the infra cleaner overall (less alias magic).


P.S. foolip@, I confused myself just now during our chat. No, we don't skip testharness.js or testdriver.js; there are two copies, one for WPT, the other for other layout tests.
Cc: -robertma@chromium.org
Owner: robertma@chromium.org
Status: Started (was: Available)
https://chromium-review.googlesource.com/c/chromium/src/+/785992
foolip@ pointed out there also exists `LayoutTests/resources/WebIDLParser.js` (thanks!).

This is a historic artifact. See upstream issue: https://github.com/w3c/web-platform-tests/issues/5608

The current situation is:
* There is no WebIDLParser.js in WPT, but a lot of tests still refer to it and the requests are redirected to webidl2.js by wptserve, which is also true when running WPT in Blink via r-w-t.
* Many (historic) non-WPT layout tests request `/w3c/resources/WebIDLParser.js`, which are redirected to WPT's webidl2.js by our Apache (which is what I was trying to fix in https://chromium-review.googlesource.com/c/chromium/src/+/785992/1).
* However, there are three non-WPT layout tests requesting `LayoutTests/resources/WebIDLParser.js` directly:
(https://cs.chromium.org/search/?q=%22resources/WebIDLParser.js%22+f:LayoutTests/%5B%5Ee%5D&type=cs)
LayoutTests/webauth/idl.html
LayoutTests/installedapp/idl.html
LayoutTests/webaudio/audio-worklet/audio-worklet-node-idl.html

Let me try to modify them and get rid of `LayoutTests/resources/WebIDLParser.js`.
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0578baafc77e7360e0027975b9436a4d9385fa03

commit 0578baafc77e7360e0027975b9436a4d9385fa03
Author: Robert Ma <robertma@chromium.org>
Date: Tue Nov 28 21:08:27 2017

Create a copy of webidl2.js for non-WPT layout tests

With this change, non-WPT layout tests are completely independent of
WPT files and hence won't be affected by imports. The change also makes
the behaviour more consistent as we already have copies of
testharness.js, testdriver.js and idlharness.js, with webidl2.js being
the only left. An apache alias rule is changed accordingly.

See  crbug.com/685854  for maintaining these Chromium copies.

Besides, WebIDLParser.js (which is an old version of webidl2.js) is
removed from resources/ to avoid confusion. The only three files using
it are modified to use webidl2.js instead.

Bug:  787829 ,  685854 
Change-Id: I2d9966646cc7db557d87ec403cd0c833ee262279
Reviewed-on: https://chromium-review.googlesource.com/785992
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519834}
[modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/installedapp/idl.html
[modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/resources/README.txt
[rename] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/resources/webidl2.js
[modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/webaudio/audio-worklet/audio-worklet-node-idl.html
[modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/webauth/idl.html
[modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/servers/apache_http.py

Status: Fixed (was: Started)

Sign in to add a comment