Skip to content

Commit

Permalink
Revert "[web-tests] Clean up obsolete WPT metadata checks"
Browse files Browse the repository at this point in the history
This reverts commit 63e5dea.

Reason for revert: May have caused wpt lint failure on win-presubmit bot.
https://ci.chromium.org/ui/p/chromium/builders/ci/win-presubmit/3185/overview

Original change's description:
> [web-tests] Clean up obsolete WPT metadata checks
>
> These aren't necessary anymore since we plan to abandon the `.ini`
> format. Two of the new checks regressed performance significantly:
> * `lint-wpt`: Unnecessarily regenerates the manifest.
> * `_check_no_wpt_lines(...)` in `lint_test_expectations.py`: Needs to
>   iterate the entire manifest to map URLs to files; this is a lazy
>   operation that was not called before.
>
> With this change, [0] now takes 40s, not 65s.
>
> [0]: git cl presubmit --upload -v
>   --files='third_party/blink/web_tests/external/wpt/badging*'
>
> Bug: 1492264, 1492238
> Change-Id: Ifd8e11718af9314f54d335b483034a63c619a9cb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4935624
> Reviewed-by: Weizhong Xia <weizhong@google.com>
> Commit-Queue: Jonathan Lee <jonathanjlee@google.com>
> Cr-Commit-Position: refs/heads/main@{#1209171}

Bug: 1492264, 1492238
Change-Id: I5b48616aab0540299ed66565e8477b912f6bc975
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4937935
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Auto-Submit: Keishi Hattori <keishi@chromium.org>
Owners-Override: Keishi Hattori <keishi@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209265}
  • Loading branch information
Keishi Hattori authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent eb55a3b commit 95c89e0
Show file tree
Hide file tree
Showing 8 changed files with 1,770 additions and 14 deletions.
34 changes: 20 additions & 14 deletions third_party/blink/tools/blinkpy/presubmit/common_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,29 @@


def lint_wpt_root(input_api, output_api, repo_root: Optional[str] = None):
"""Run `wpt lint` against the specified directory."""
"""Run `blink_tool.py lint-wpt` against the specified directory."""
repo_root = repo_root or input_api.PresubmitLocalPath()
wpt_executable = input_api.os_path.join(input_api.change.RepositoryRoot(),
'third_party', 'wpt_tools', 'wpt',
'wpt')

# TODO(crbug.com/1406669): Changing a test file should also lint its
# corresponding reference/*-expected.txt file, if any, because the
# test-side change may invalidate the other files' contents. For example,
# removing a test variant will orphan its expectations.
tool_path = input_api.os_path.join(input_api.os_path.dirname(__file__),
input_api.os_path.pardir,
input_api.os_path.pardir,
'blink_tool.py')

# TODO(crbug.com/1406669): After switching to wptrunner, changing a test
# file should also lint its corresponding metadata file, if any, because the
# test-side change may invalidate the metadata file's contents. For example,
# removing a test variant will orphan its expectations, which the linter
# should flag for cleanup.
paths = []
for abs_path in input_api.AbsoluteLocalPaths():
# For now, skip checking metadata files in `presubmit --{files,all}` for
# the invalidation reason mentioned above.
if input_api.no_diffs and abs_path.endswith('.ini'):
continue
if abs_path.endswith(input_api.os_path.relpath(abs_path, repo_root)):
paths.append(abs_path)

# Without an explicit file list, `wpt lint` will lint all files in the
# root, which is slow.
# Without an explicit file list, `lint-wpt` will lint all files in the root,
# which is slow.
if not paths:
return []

Expand All @@ -36,8 +42,8 @@ def lint_wpt_root(input_api, output_api, repo_root: Optional[str] = None):
paths_name = f.name
args = [
input_api.python3_executable,
wpt_executable,
'lint',
tool_path,
'lint-wpt',
'--repo-root=%s' % repo_root,
# To avoid false positives, do not lint files not upstreamed from
# Chromium.
Expand All @@ -56,7 +62,7 @@ def lint_wpt_root(input_api, output_api, repo_root: Optional[str] = None):

if proc.returncode != 0:
return [
output_api.PresubmitError('`wpt lint` failed:',
output_api.PresubmitError('`blink_tool.py lint-wpt` failed:',
long_text=(stdout + stderr).decode())
]
return []
2 changes: 2 additions & 0 deletions third_party/blink/tools/blinkpy/tool/blink_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
from blinkpy.tool.commands.rebaseline import Rebaseline
from blinkpy.tool.commands.rebaseline_cl import RebaselineCL
from blinkpy.tool.commands.update_metadata import UpdateMetadata
from blinkpy.tool.commands.lint_wpt import LintWPT

_log = logging.getLogger(__name__)

Expand Down Expand Up @@ -84,6 +85,7 @@ def __init__(self, path):
Rebaseline(),
RebaselineCL(self, io_pool),
UpdateMetadata(self, io_pool),
LintWPT(self),
]
self.help_command = HelpCommand(tool=self)
self.commands.append(self.help_command)
Expand Down

0 comments on commit 95c89e0

Please sign in to comment.