New issue
Advanced search Search tips

Issue 819349 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

`wpt lint` broken by an upstream change

Project Member Reported by robertma@chromium.org, Mar 6 2018

Issue description

https://github.com/w3c/web-platform-tests/commit/19c79185c1d4978aeb7191494541711ca4f3a582

This upstream change added a preprocess to the paths passed to `wpt lint` which assumes paths given to `wpt lint` are relative to CWD. Yet the invocation in LayoutTests/external/PRESUBMIT.py assumes the paths relative to the (WPT) repo root.

This is a P1 since it renders the lint useless in Blink. I'll change the invocation to pass absolute paths to `wpt lint` instead.
 
CL https://chromium-review.googlesource.com/c/chromium/src/+/943827 has already landed with a lint error. (This is the only one I found so far.)

css/css-tables/width-distribution/td-with-subpixel-padding.html: Testcase file must have a link to a spec (MISSING-LINK)

It needs to be amended to add a spec link.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 7 2018

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

commit eff8f7fe1fa96163fa0ed4a44835f57262890cd5
Author: Robert Ma <robertma@chromium.org>
Date: Wed Mar 07 01:59:44 2018

Pass absolute paths to wpt lint in PRESUBMIT.py

An upstream change in wpt lint interprets relative paths as relative to
CWD instead of the (WPT) repo root, which breaks PRESUBMIT.py.

Use absolute paths instead to avoid any ambiguity.

Bug:  819349 
Change-Id: Idf57e048c30d3143e55ffebfc95df0f774ae4e8f
Reviewed-on: https://chromium-review.googlesource.com/951755
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541280}
[modify] https://crrev.com/eff8f7fe1fa96163fa0ed4a44835f57262890cd5/third_party/WebKit/LayoutTests/external/PRESUBMIT.py

Labels: -Pri-1 Pri-2
Fix has landed.

Investigating how to write a test. Downgrading to P2.
Labels: -Pri-2 Pri-1
Similar problem happened again recently and was fixed by:
https://chromium-review.googlesource.com/c/chromium/src/+/1030109

We really should add a test for this PRESUBMIT.py, as a higher level presubmit check suggests:

** Presubmit Warnings **
Out of 6 files, found none that matched w=['^PRESUBMIT_test\\.py$'], b=None in directory /b/swarming/w/ir/cache/builder/chromium_presubmit/src/third_party/WebKit/LayoutTests/external
Cc: foolip@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2018

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

commit 08006c90a2cc4f347ea7e6e093179b8e0ade0abd
Author: Robert Ma <robertma@chromium.org>
Date: Fri Jun 01 15:35:39 2018

[blinkpy] Add PRESUBMIT tests for `wpt lint`

Write a PRESUBMIT_test.py for LayoutTests/external/PRESUBMIT.py to check
if it can catch WPT lint errors properly. This PRESUBMIT_test.py will
run whenever LayoutTests/external/PRESUBMIT.py is modified.

Besides, we also need to test the basic functionalities of `wpt lint`
when we roll in a new version, so a new PRESUBMIT.py is added to
blinkpy/third_party/wpt which calls the PRESUBMIT_test.py above.
(https://www.chromium.org/developers/how-tos/depottools/presubmit-scripts)

Bug:  819349 
Change-Id: I48766db3dd98f91bf57f86e944f35019d2033ac5
Reviewed-on: https://chromium-review.googlesource.com/1081944
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563643}
[modify] https://crrev.com/08006c90a2cc4f347ea7e6e093179b8e0ade0abd/third_party/WebKit/LayoutTests/external/PRESUBMIT.py
[add] https://crrev.com/08006c90a2cc4f347ea7e6e093179b8e0ade0abd/third_party/WebKit/LayoutTests/external/PRESUBMIT_test.py
[add] https://crrev.com/08006c90a2cc4f347ea7e6e093179b8e0ade0abd/third_party/blink/tools/blinkpy/third_party/wpt/PRESUBMIT.py

Status: Fixed (was: Started)

Sign in to add a comment