Skip to content

Commit

Permalink
Do not convert testharness paths in import-w3c-tests
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=185876

Reviewed by Jonathan Bedard.

Since the csswg-tests repo was merged into WPT, there is only one test repository now in
imported/w3c/resources/TestRepositories, which does not specify the option to convert
testharness paths from relative paths to explicit paths. Additionally, wptserve has been
used for some time, so run-webkit-tests should have no problem with relative paths to
the testharness paths. Thus, there is no reason to continue converting testharness paths,
so this patch removes all code related to that conversion.

* Scripts/webkitpy/w3c/test_converter.py:
(convert_for_webkit):
(_W3CTestConverter.__init__):
(_W3CTestConverter.convert_attributes_if_needed):
* Scripts/webkitpy/w3c/test_converter_unittest.py:
(verify_test_harness_paths):
* Scripts/webkitpy/w3c/test_importer.py:
(TestImporter.find_importable_tests):
(TestImporter.import_tests):

Canonical link: https://commits.webkit.org/265842@main
  • Loading branch information
csnardi authored and gsnedders committed Jul 7, 2023
1 parent 90b3991 commit 1866a4e
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 51 deletions.
21 changes: 3 additions & 18 deletions Tools/Scripts/webkitpy/w3c/test_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@
_log = logging.getLogger(__name__)


def convert_for_webkit(new_path, filename, reference_support_info, host=Host(), convert_test_harness_links=True, webkit_test_runner_options=''):
def convert_for_webkit(new_path, filename, reference_support_info, host=Host(), webkit_test_runner_options=''):
""" Converts a file's |contents| so it will function correctly in its |new_path| in Webkit.
Returns the list of modified properties and the modified text if the file was modifed, None otherwise."""
contents = host.filesystem.read_text_file(filename)

converter = _W3CTestConverter(new_path, filename, reference_support_info, host, convert_test_harness_links, webkit_test_runner_options)
converter = _W3CTestConverter(new_path, filename, reference_support_info, host, webkit_test_runner_options)
if filename.endswith('.css'):
return converter.add_webkit_prefix_to_unprefixed_properties_and_values(contents)
elif filename.endswith('.js'):
Expand All @@ -60,7 +60,7 @@ def convert_for_webkit(new_path, filename, reference_support_info, host=Host(),


class _W3CTestConverter(HTMLParser):
def __init__(self, new_path, filename, reference_support_info, host=Host(), convert_test_harness_links=True, webkit_test_runner_options=''):
def __init__(self, new_path, filename, reference_support_info, host=Host(), webkit_test_runner_options=''):
if sys.version_info > (3, 0):
HTMLParser.__init__(self, convert_charrefs=False)
else:
Expand All @@ -80,17 +80,10 @@ def __init__(self, new_path, filename, reference_support_info, host=Host(), conv
self.webkit_test_runner_options = webkit_test_runner_options
self.has_started = False

resources_path = self.path_from_webkit_root('LayoutTests', 'resources')
resources_relpath = self._filesystem.relpath(resources_path, new_path)
self.new_test_harness_path = resources_relpath.replace(os.sep, '/')
self.convert_test_harness_links = convert_test_harness_links

# These settings might vary between WebKit and Blink
css_property_file = self.path_from_webkit_root('Source', 'WebCore', 'css', 'CSSProperties.json')
css_property_value_file = self.path_from_webkit_root('Source', 'WebCore', 'css', 'CSSValueKeywords.in')

self.test_harness_re = re.compile('/resources/testharness')

self.prefixed_properties = self.read_webkit_prefixed_css_property_list(css_property_file)
prop_regex = r'([\s{]|^)(' + "|".join(prop.replace('-webkit-', '') for prop in self.prefixed_properties) + r')(\s+:|:)'
self.prop_re = re.compile(prop_regex)
Expand Down Expand Up @@ -204,14 +197,6 @@ def convert_style_data(self, data):

def convert_attributes_if_needed(self, tag, attrs):
converted = self.get_starttag_text()
if self.convert_test_harness_links and tag in ('script', 'link'):
attr_name = 'src'
if tag != 'script':
attr_name = 'href'
for attr in attrs:
if attr[0] == attr_name:
new_path = re.sub(self.test_harness_re, self.new_test_harness_path + '/testharness', attr[1])
converted = re.sub(re.escape(attr[1]), new_path, converted)

for attr in attrs:
if attr[0] == 'style':
Expand Down
24 changes: 8 additions & 16 deletions Tools/Scripts/webkitpy/w3c/test_converter_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def test_convert_for_webkit_harness_only(self):
converted = converter.output()

self.verify_conversion_happened(converted)
self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 1, 1)
self.verify_test_harness_paths(converted[2], 1, 1)
self.verify_prefixed_properties(converted, [])
self.verify_prefixed_property_values(converted, [])

Expand Down Expand Up @@ -140,7 +140,7 @@ def test_convert_for_webkit_properties_only(self):
converted = converter.output()

self.verify_conversion_happened(converted)
self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 1, 1)
self.verify_test_harness_paths(converted[2], 1, 1)
self.verify_prefixed_properties(converted, test_content[0])
self.verify_prefixed_property_values(converted, test_content[1])

Expand Down Expand Up @@ -174,7 +174,7 @@ def test_convert_for_webkit_harness_and_properties(self):
converted = converter.output()

self.verify_conversion_happened(converted)
self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 1, 1)
self.verify_test_harness_paths(converted[2], 1, 1)
self.verify_prefixed_properties(converted, test_content[0])
self.verify_prefixed_property_values(converted, test_content[1])

Expand All @@ -196,7 +196,7 @@ def test_convert_test_harness_paths(self):
converted = converter.output()

self.verify_conversion_happened(converted)
self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 2, 1)
self.verify_test_harness_paths(converted[2], 2, 1)

def test_convert_prefixed_properties(self):
""" Tests convert_prefixed_properties() file that has 20 properties requiring the -webkit- prefix:
Expand Down Expand Up @@ -360,21 +360,13 @@ def verify_conversion_happened(self, converted):
def verify_no_conversion_happened(self, converted, original):
self.assertEqual(converted[2], original, 'test should not have been converted')

def verify_test_harness_paths(self, converter, converted, test_path, num_src_paths, num_href_paths):
def verify_test_harness_paths(self, converted, num_src_paths, num_href_paths):
if isinstance(converted, str) or isinstance(converted, unicode):
converted = BeautifulSoup(converted)

resources_dir = converter.path_from_webkit_root("LayoutTests", "resources")

# Verify the original paths are gone, and the new paths are present.
orig_path_pattern = re.compile('\"/resources/testharness')
self.assertEqual(len(converted.findAll(src=orig_path_pattern)), 0, 'testharness src path was not converted')
self.assertEqual(len(converted.findAll(href=orig_path_pattern)), 0, 'testharness href path was not converted')

new_relpath = os.path.relpath(resources_dir, test_path).replace(os.sep, '/')
relpath_pattern = re.compile(new_relpath)
self.assertEqual(len(converted.findAll(src=relpath_pattern)), num_src_paths, 'testharness src relative path not correct')
self.assertEqual(len(converted.findAll(href=relpath_pattern)), num_href_paths, 'testharness href relative path not correct')
orig_path_pattern = re.compile('^/resources/testharness')
self.assertEquals(len(converted.findAll(src=orig_path_pattern)), num_src_paths, 'testharness src path should not have been converted')
self.assertEquals(len(converted.findAll(href=orig_path_pattern)), num_href_paths, 'testharness href path should not have been converted')

def verify_prefixed_properties(self, converted, test_properties):
self.assertEqual(len(set(converted[0])), len(set(test_properties)), 'Incorrect number of properties converted')
Expand Down
21 changes: 4 additions & 17 deletions Tools/Scripts/webkitpy/w3c/test_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,11 @@
2. LayoutTests/imported/w3c/resources/ImportExpectations list the test suites or tests to NOT import.
- All files are converted to work in WebKit:
1. Paths to testharness.js files are modified to point to Webkit's copy of them in
LayoutTests/resources, using the correct relative path from the new location.
This is applied to CSS tests but not to WPT tests.
2. All CSS properties requiring the -webkit-vendor prefix are prefixed - this current
1. All CSS properties requiring the -webkit-vendor prefix are prefixed - this current
list of what needs prefixes is read from Source/WebCore/CSS/CSSProperties.in
3. Each reftest has its own copy of its reference file following the naming conventions
2. Each reftest has its own copy of its reference file following the naming conventions
new-run-webkit-tests expects
4. If a reference files lives outside the directory of the test that uses it, it is checked
3. If a reference files lives outside the directory of the test that uses it, it is checked
for paths to support files as it will be imported into a different relative position to the
test file (in the same directory)
Expand Down Expand Up @@ -121,8 +118,6 @@ def parse_args(args):

parser.add_argument('-n', '--no-overwrite', dest='overwrite', action='store_false', default=True,
help='Flag to prevent duplicate test files from overwriting existing tests. By default, they will be overwritten')
parser.add_argument('-l', '--no-links-conversion', dest='convert_test_harness_links', action='store_false', default=True,
help='Do not change links (testharness js or css e.g.). This option only applies when providing a source directory, in which case by default, links are converted to point to WebKit testharness files. When tests are downloaded from W3C repository, links are converted for CSS tests and remain unchanged for WPT tests')

parser.add_argument('-t', '--tip-of-tree', dest='use_tip_of_tree', action='store_true', default=False,
help='Import all tests using the latest repository revision')
Expand Down Expand Up @@ -380,14 +375,6 @@ def should_keep_subdir(filesystem, path):
self.import_list.append({'dirname': root, 'copy_list': copy_list,
'reftests': reftests, 'jstests': jstests, 'crashtests': crashtests, 'total_tests': total_tests})

def should_convert_test_harness_links(self, test):
if self._importing_downloaded_tests:
for test_repository in self.test_downloader().test_repositories:
if test.startswith(test_repository['name']):
return 'convert_test_harness_links' in test_repository['import_options']
return True
return self.options.convert_test_harness_links

def _webkit_test_runner_options(self, path):
if not(self.filesystem.isfile(path)):
return ''
Expand Down Expand Up @@ -546,7 +533,7 @@ def import_tests(self):
and ('html' in str(mimetype[0]) or 'xml' in str(mimetype[0]) or 'css' in str(mimetype[0]) or 'javascript' in str(mimetype[0])):
_log.info("Rewriting: %s" % new_filepath)
try:
converted_file = convert_for_webkit(new_path, filename=orig_filepath, reference_support_info=reference_support_info, host=self.host, convert_test_harness_links=self.should_convert_test_harness_links(subpath), webkit_test_runner_options=self._webkit_test_runner_options(new_filepath))
converted_file = convert_for_webkit(new_path, filename=orig_filepath, reference_support_info=reference_support_info, host=self.host, webkit_test_runner_options=self._webkit_test_runner_options(new_filepath))
except:
_log.warn('Failed converting %s', orig_filepath)
failed_conversion_files.append(orig_filepath)
Expand Down

0 comments on commit 1866a4e

Please sign in to comment.