-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[run-webkit-tests] Support user specifying variant directly #5149
[run-webkit-tests] Support user specifying variant directly #5149
Conversation
EWS run on previous version of this PR (hash ff2ff7e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable. I tried refactoring a bit to make it easier for me to follow. If you're in a hurry, no need to take these suggestions.
paths_to_walk = itertools.chain(*(sort_by_directory_key(_filter_test_variant(path, filesystem.glob)) for path in paths)) | ||
all_files = itertools.chain(*(sort_by_directory_key( | ||
_filter_test_variant(path, lambda p: filesystem.files_under(p, skipped_directories, file_filter)) | ||
) for path in paths_to_walk)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested refactor:
def sorted_paths_generator(fs_selector):
for test_selector in paths_to_walk:
path, separator, variant = test_selector.partition('?')
matching_tests = (matching_path + separator + variant for matching_path in fs_selector(path))
if directory_sort_key:
yield from sorted(matching_tests, key=directory_sort_key)
else:
yield from matching_tests
paths_to_walk = sorted_paths_generator(filesystem.glob)
all_files = sorted_paths_generator(lambda p: filesystem.files_under(p, skipped_directories, file_filter))
This would replace _filter_test_variant
and the local sort_by_directory_key
function altogether. Using str.partition
instead of str.split
removes a bunch of the conditional logic.
Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy.py
Outdated
Show resolved
Hide resolved
if not passed_variant or variant.startswith(passed_variant): | ||
expanded.append(f + variant) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the above suggestion:
if not passed_variant or variant.startswith(passed_variant): | |
expanded.append(f + variant) | |
if not passed_variant or variant.startswith(variant_separator + passed_variant): | |
expanded.append(f + variant) |
split = f.split('?') | ||
passed_variant = None | ||
if len(split) == 2: | ||
f = split[0] | ||
passed_variant = '?{}'.format(split[1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
split = f.split('?') | |
passed_variant = None | |
if len(split) == 2: | |
f = split[0] | |
passed_variant = '?{}'.format(split[1]) | |
f, variant_separator, passed_variant = f.partition('?') |
ff2ff7e
to
eb0d3f4
Compare
EWS run on current version of this PR (hash eb0d3f4) |
https://bugs.webkit.org/show_bug.cgi?id=246228 rdar://100907445 Reviewed by Elliott Williams. * Tools/Scripts/webkitpy/common/find_files.py: (_normalized_find): Consider variants of a single test file. (_normalized_find.sorted_paths_generator): * Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy.py: (LayoutTestFinder._expand_variants): Handle specific variant as argument. * Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy_unittest.py: (LayoutTestFinderTests.test_find_glob_query): Canonical link: https://commits.webkit.org/255305@main
eb0d3f4
to
f8b4d27
Compare
Committed 255305@main (f8b4d27): https://commits.webkit.org/255305@main Reviewed commits have been landed. Closing PR #5149 and removing active labels. |
f8b4d27
eb0d3f4
π macπ wpeπ π§ͺ winπ mac-debugπ gtkπ wincairoπ§ͺ ios-wk2π§ͺ gtk-wk2π§ͺ api-iosπ§ͺ api-macπ§ͺ api-gtkπ tvπ§ͺ mac-wk1π§ͺ mac-wk2π§ͺ mac-AS-debug-wk2π§ͺ mac-wk2-stress