New issue
Advanced search Search tips

Issue 837207 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

wpt lint no longer runs as part of presubmit

Project Member Reported by foolip@chromium.org, Apr 26 2018

Issue description

https://chromium-review.googlesource.com/1030192 is passing presubmit, but applied to the commit before https://chromium-review.googlesource.com/1026165 those changes fail like this:

css/css-align/ttwf-reftest-alignContent.html: Support file not in support directory (SUPPORT-WRONG-DIR)
dom/historical.html:9: Tabs used for indentation (INDENT TABS)
dom/historical.html:10: Tabs used for indentation (INDENT TABS)
dom/historical.html:11: Tabs used for indentation (INDENT TABS)

There were 4 errors (INDENT TABS: 3 SUPPORT-WRONG-DIR: 1)
You must fix all errors; for details on how to fix them, see
http://web-platform-tests.org/writing-tests/lint-tool.html

However, instead of fixing a particular error, it's sometimes
OK to add a line to the lint.whitelist file in the root of the
web-platform-tests directory to make the lint tool ignore it.

For example, to make the lint tool ignore all 'INDENT TABS'
errors in the dom/historical.html file,
you could add the following line to the lint.whitelist file.

INDENT TABS: dom/historical.html
 

Comment 1 by foolip@chromium.org, Apr 26 2018

So, it looks like if I add "throw 42" to lint.py, the lint passes. This makes me suspect that the lint is now throwing an exception. Trying to figure out what invokes it...

Comment 3 by foolip@chromium.org, Apr 26 2018

Cc: robertma@chromium.org
Owner: foolip@chromium.org
Fix up for review: https://chromium-review.googlesource.com/c/chromium/src/+/1030109
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 26 2018

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

commit e57cac2093f53775cea49b6f848a402487e4cee0
Author: Philip Jägenstedt <foolip@chromium.org>
Date: Thu Apr 26 14:51:08 2018

Use the wpt lint exit code to determine if lint failed

Upstream changes to lint.py rolled in at
https://chromium-review.googlesource.com/1026165 changed something
about the logging, so there is now no stdout, where previously there
was both stdout and stderr without roughly the same information.

Using stdout+stderr might end up looking strange if stdout once begins
to contain something, but it's hard to predict how to format it.

Example output with https://chromium-review.googlesource.com/1030192:

wpt lint failed:

***************
ERROR:lint:css/css-align/ttwf-reftest-alignContent.html: Support file not in support directory (SUPPORT-WRONG-DIR)
ERROR:lint:dom/historical.html:9: Tabs used for indentation (INDENT TABS)
ERROR:lint:dom/historical.html:10: Tabs used for indentation (INDENT TABS)
ERROR:lint:dom/historical.html:11: Tabs used for indentation (INDENT TABS)
INFO:lint:
INFO:lint:There were 4 errors (INDENT TABS: 3 SUPPORT-WRONG-DIR: 1)
INFO:lint:You must fix all errors; for details on how to fix them, see
INFO:lint:http://web-platform-tests.org/writing-tests/lint-tool.html
INFO:lint:
INFO:lint:However, instead of fixing a particular error, it's sometimes
INFO:lint:OK to add a line to the lint.whitelist file in the root of the
INFO:lint:web-platform-tests directory to make the lint tool ignore it.
INFO:lint:
INFO:lint:For example, to make the lint tool ignore all 'INDENT TABS'
INFO:lint:errors in the dom/historical.html file,
INFO:lint:you could add the following line to the lint.whitelist file.
INFO:lint:
INFO:lint:INDENT TABS: dom/historical.html
***************

Bug:  837207 
Change-Id: Ib439dac32ed852024e63fe2a6345196200366308
Reviewed-on: https://chromium-review.googlesource.com/1030109
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554025}
[modify] https://crrev.com/e57cac2093f53775cea49b6f848a402487e4cee0/third_party/WebKit/LayoutTests/external/PRESUBMIT.py

Comment 5 by foolip@chromium.org, Apr 26 2018

Status: Fixed (was: Assigned)
See  issue 819349  for adding a test for PRESUBMIT.py
Status: Started (was: Fixed)
Reopening this since the affected export PR hasn't been fixed up: https://github.com/w3c/web-platform-tests/pull/10624

foolip@ please close the issue after you fix the lint errors in that PR and land it.
Status: Fixed (was: Started)

Sign in to add a comment