Skip to content

Commit

Permalink
[wptrunner] Dedup wpt_internal/ manifest generation
Browse files Browse the repository at this point in the history
`blinkpy.w3c.wpt_manifest` generated the manifest with the default URL
base of `/` [0], which was mismatched with the URL base specified in
`web_tests/wptrunner.blink.ini` [1]. This change passes the correct URL
base so that [2] does not force a duplicate update that bypasses
`--no-manifest-update`.

Also, log when the update occurs for `run_wpt_tests.py`.

[0]: https://github.com/web-platform-tests/wpt/blob/289e601/tools/manifest/update.py#L71
[1]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/wptrunner.blink.ini;l=5;drc=16567822f77c2a38d71b5df9d993011bb66ac448;bpv=0;bpt=0
[2]: https://github.com/web-platform-tests/wpt/blob/289e601/tools/manifest/manifest.py#L384

Fixed: 1454898
Cq-Include-Trybots: luci.chromium.try:linux-wpt-content-shell-fyi-rel
Change-Id: Ibeb25759aefe494c824206d6c7e8fad627800691
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4623792
Reviewed-by: Weizhong Xia <weizhong@google.com>
Commit-Queue: Jonathan Lee <jonathanjlee@google.com>
Cr-Commit-Position: refs/heads/main@{#1159406}
  • Loading branch information
jonathan-j-lee authored and Chromium LUCI CQ committed Jun 18, 2023
1 parent a70596e commit f457f4a
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 42 deletions.
43 changes: 21 additions & 22 deletions third_party/blink/tools/blinkpy/w3c/test_importer_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# found in the LICENSE file.

import json
import six
import unittest

from blinkpy.common.checkout.git_mock import MockGit
Expand All @@ -27,13 +26,17 @@

MOCK_WEB_TESTS = '/mock-checkout/' + RELATIVE_WEB_TESTS
MANIFEST_INSTALL_CMD = [
'python3', '/mock-checkout/third_party/wpt_tools/wpt/wpt', 'manifest',
'-v', '--no-download', '--tests-root', MOCK_WEB_TESTS + 'external/wpt'
'python3',
'/mock-checkout/third_party/wpt_tools/wpt/wpt',
'manifest',
'-v',
'--no-download',
f'--tests-root={MOCK_WEB_TESTS + "external/wpt"}',
'--url-base=/',
]


class TestImporterTest(LoggingTestCase):

def mock_host(self):
host = MockHost()
host.builders = BuilderList({
Expand Down Expand Up @@ -67,13 +70,14 @@ def mock_host(self):
host.filesystem.write_text_file(ANDROID_DISABLED_TESTS, '')
return host

@staticmethod
def _get_test_importer(host, wpt_github=None):
def _get_test_importer(self, host, wpt_github=None):
port = host.port_factory.get()
return TestImporter(
host,
wpt_github=wpt_github,
wpt_manifests=[port.wpt_manifest('external/wpt')])
manifest = port.wpt_manifest('external/wpt')
# Clear logs from manifest generation.
self.logMessages().clear()
return TestImporter(host,
wpt_github=wpt_github,
wpt_manifests=[manifest])

def test_update_expectations_for_cl_no_results(self):
host = self.mock_host()
Expand Down Expand Up @@ -485,7 +489,6 @@ def test_cl_description_with_empty_environ(self):
'No-Export: true\n'
'Cq-Include-Trybots: luci.chromium.try:linux-wpt-identity-fyi-rel,'
'linux-wpt-input-fyi-rel,linux-blink-rel')
print(host.executive.calls)
self.assertEqual(host.executive.calls,
[MANIFEST_INSTALL_CMD] +
[['git', 'log', '-1', '--format=%B']])
Expand Down Expand Up @@ -519,16 +522,10 @@ def test_sheriff_email_no_response_uses_backup(self):
host = self.mock_host()
importer = self._get_test_importer(host)
self.assertEqual(SHERIFF_EMAIL_FALLBACK, importer.sheriff_email())
if six.PY3:
self.assertLog([
'ERROR: Exception while fetching current sheriff: '
'Expecting value: line 1 column 1 (char 0)\n'
])
else:
self.assertLog([
'ERROR: Exception while fetching current sheriff: '
'No JSON object could be decoded\n'
])
self.assertLog([
'ERROR: Exception while fetching current sheriff: '
'Expecting value: line 1 column 1 (char 0)\n'
])

def test_sheriff_email_no_emails_field(self):
host = self.mock_host()
Expand Down Expand Up @@ -563,7 +560,9 @@ def raise_exception(*_):
host.web.get_binary = raise_exception
importer = self._get_test_importer(host)
self.assertEqual(SHERIFF_EMAIL_FALLBACK, importer.sheriff_email())
self.assertLog(['ERROR: Cannot fetch %s\n' % ROTATIONS_URL])
self.assertLog([
'ERROR: Cannot fetch %s\n' % ROTATIONS_URL,
])

def test_sheriff_email(self):
host = self.mock_host()
Expand Down
28 changes: 20 additions & 8 deletions third_party/blink/tools/blinkpy/w3c/wpt_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,10 @@ def ensure_manifest(port, path=None):
fs.remove(manifest_path)

# TODO(crbug.com/853815): perhaps also cache the manifest for wpt_internal.
if 'external' in path:
#
# `url_base` should match those of `web_tests/wptrunner.blink.ini` (or
# the implicit root `/` URL base).
if path.startswith('external'):
base_manifest_path = fs.join(port.web_tests_dir(), 'external',
BASE_MANIFEST_NAME)
if fs.exists(base_manifest_path):
Expand All @@ -364,26 +367,35 @@ def ensure_manifest(port, path=None):
else:
_log.error('Manifest base not found at "%s".',
base_manifest_path)
url_base = '/'
elif path.startswith('wpt_internal'):
url_base = '/wpt_internal/'

WPTManifest.generate_manifest(port, wpt_path)
WPTManifest.generate_manifest(port, wpt_path, url_base)

if fs.isfile(manifest_path):
_log.debug('Manifest generation completed.')
_log.info(
f'Manifest generation completed for {url_base!r} ({path})')
else:
_log.error(
'Manifest generation failed; creating an empty MANIFEST.json...'
)
f'Manifest generation failed for {url_base!r} ({path}); '
'creating an empty MANIFEST.json...')
fs.write_text_file(manifest_path, '{}')

@staticmethod
def generate_manifest(port, dest_path):
def generate_manifest(port, dest_path, url_base: str = '/'):
"""Generates MANIFEST.json on the specified directory."""
wpt_exec_path = PathFinder(
port.host.filesystem).path_from_chromium_base(
'third_party', 'wpt_tools', 'wpt', 'wpt')
cmd = [
port.python3_command(), wpt_exec_path, 'manifest', '-v',
'--no-download', '--tests-root', dest_path
port.python3_command(),
wpt_exec_path,
'manifest',
'-v',
'--no-download',
f'--tests-root={dest_path}',
f'--url-base={url_base}',
]

# ScriptError will be raised if the command fails.
Expand Down
12 changes: 6 additions & 6 deletions third_party/blink/tools/blinkpy/w3c/wpt_manifest_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ def test_ensure_manifest_copies_new_manifest(self):
'manifest',
'-v',
'--no-download',
'--tests-root',
MOCK_WEB_TESTS + 'external/wpt',
f'--tests-root={MOCK_WEB_TESTS + "external/wpt"}',
'--url-base=/',
]])

def test_ensure_manifest_updates_manifest_if_it_exists(self):
Expand All @@ -53,8 +53,8 @@ def test_ensure_manifest_updates_manifest_if_it_exists(self):
'manifest',
'-v',
'--no-download',
'--tests-root',
MOCK_WEB_TESTS + 'external/wpt',
f'--tests-root={MOCK_WEB_TESTS + "external/wpt"}',
'--url-base=/',
]])

def test_ensure_manifest_raises_exception(self):
Expand All @@ -75,8 +75,8 @@ def test_ensure_manifest_takes_optional_dest(self):
'manifest',
'-v',
'--no-download',
'--tests-root',
MOCK_WEB_TESTS + 'wpt_internal',
f'--tests-root={MOCK_WEB_TESTS + "wpt_internal"}',
'--url-base=/wpt_internal/',
]])

def test_all_test_types_are_identified(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from blinkpy.common.path_finder import PathFinder
from blinkpy.common.system.log_testing import LoggingTestCase
from blinkpy.web_tests.port.factory_mock import MockPortFactory
from blinkpy.w3c.wpt_manifest import BASE_MANIFEST_NAME
from blinkpy.w3c.wpt_results_processor import (
EventProcessingError,
StreamShutdown,
Expand All @@ -23,7 +22,6 @@
class WPTResultsProcessorTest(LoggingTestCase):
def setUp(self):
super().setUp()

self.host = BlinkMockHost()
self.host.port_factory = MockPortFactory(self.host)
self.fs = self.host.filesystem
Expand All @@ -33,7 +31,7 @@ def setUp(self):
# Create a testing manifest containing any test files that we
# might interact with.
self.fs.write_text_file(
self.fs.join(port.web_tests_dir(), 'external', BASE_MANIFEST_NAME),
self.path_finder.path_from_wpt_tests('MANIFEST.json'),
json.dumps({
'items': {
'reftest': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def PresubmitCheckTestExpectations(input_api, output_api):
"to produce output; check by hand. ")
]
errs = errs.decode('utf-8')
if errs.strip() == 'Lint succeeded.':
if errs.rstrip().endswith('Lint succeeded.'):
return []
if errs.rstrip().endswith('Lint succeeded with warnings.'):
return [output_api.PresubmitPromptWarning(errs)]
Expand Down
11 changes: 9 additions & 2 deletions third_party/blink/tools/blinkpy/web_tests/port/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -941,8 +941,15 @@ def reference_files(self, test_name):
path_in_wpt = match.group(2)
for expectation, ref_path_in_wpt in self.wpt_manifest(
wpt_path).extract_reference_list(path_in_wpt):
ref_absolute_path = self._filesystem.join(
self.web_tests_dir(), wpt_path + ref_path_in_wpt)
if 'external/wpt' in wpt_path:
ref_path_in_web_tests = wpt_path + ref_path_in_wpt
else:
# References in this manifest are already generated with
# `/wpt_internal` in the URL. Remove the leading '/' for
# joining.
ref_path_in_web_tests = ref_path_in_wpt[1:]
ref_absolute_path = self._filesystem.join(self.web_tests_dir(),
ref_path_in_web_tests)
reftest_list.append((expectation, ref_absolute_path))
return reftest_list

Expand Down

0 comments on commit f457f4a

Please sign in to comment.