[release] Add option to filter test by name prefix #55670
Conversation
khluu
commented
Aug 15, 2025
khluu
commented
- This filter test func can now use either or both of name prefix filter or regex filter. If both are used at the same time, test matches either of the filter is accepted
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new option to filter tests by a name prefix, complementing the existing regex-based attribute filtering. The implementation correctly applies an 'OR' logic when both filters are used. The added tests cover the new functionality well. I've suggested a small refactoring to improve the readability of the filtering logic.
| # If no filter, select all tests | ||
| selected = not (test_name_prefix_filter or test_attr_regex_filters) | ||
|
|
||
| # Check if test name matches prefix filter | ||
| if test_name_prefix_filter and test.get_name().startswith( | ||
| test_name_prefix_filter | ||
| ): | ||
| selected = True | ||
|
|
||
| # Check if any test attributes match regex filters | ||
| if test_attr_regex_filters: | ||
| attr_mismatch = False | ||
| for attr, regex in test_attr_regex_filters.items(): | ||
| attr_value = _unflattened_lookup(test, attr) or "" | ||
| if not re.fullmatch(regex, attr_value): | ||
| attr_mismatch = True | ||
| break | ||
| if not attr_mismatch: | ||
| selected = True | ||
|
|
||
| if not selected: | ||
| continue |
There was a problem hiding this comment.
The current filtering logic using the selected flag is a bit complex to follow. It can be refactored to be more explicit and readable by checking for each filter condition separately and combining them with a clear or.
# If filters are provided, a test is selected if it matches at least one of them.
# If no filters are provided, all tests are selected.
if test_name_prefix_filter or test_attr_regex_filters:
prefix_match = test_name_prefix_filter and test.get_name().startswith(
test_name_prefix_filter
)
regex_match = False
if test_attr_regex_filters:
regex_match = all(
re.fullmatch(regex, _unflattened_lookup(test, attr) or "")
for attr, regex in test_attr_regex_filters.items()
)
if not (prefix_match or regex_match):
continue
ray-gardener
bot
added
core
| test_collection: List[Test], | ||
| frequency: Frequency, | ||
| test_attr_regex_filters: Optional[Dict[str, str]] = None, | ||
| test_name_prefix_filter: Optional[str] = None, |
There was a problem hiding this comment.
how does one set this? this seems to be unused yet? is there a followup PR?
| # If no filter, select all tests | ||
| selected = not (test_name_prefix_filter or test_attr_regex_filters) |
There was a problem hiding this comment.
this feels a bit twisted..
could you refactor the selection logic out into a util function? basically:
def _match_test_selection(test, name_prefix, attr_regex) -> bool:
if match_prefix():
return True
if match_regex():
return True
return False
then for the test filtering for loop:
select_all = not (test_name_prefix_filter or test_attr_regex_filters)
for test in Tests:
if not select_all and not _match_test_selection(test, ...):
continue
# do the extra smoke test / frequency / filtering here.
| ] | ||
|
|
||
| # Test filter by prefix alone | ||
| filtered = self._filter_names_smoke( |
There was a problem hiding this comment.
nit: can we rename _filter_names_smoke to just _filter_names?
|
could you manually test it for one last time before merging?
|
and post the link here? |
|
@aslonnie I made some changes to rename |
|
I ran a test with prefix filter |
|
@aslonnie It's ready now.. https://buildkite.com/ray-project/release/builds/55314 as proof that it works with prefix |
- This filter test func can now use either or both of name prefix filter or regex filter. If both are used at the same time, test matches either of the filter is accepted --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
- This filter test func can now use either or both of name prefix filter or regex filter. If both are used at the same time, test matches either of the filter is accepted --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
- This filter test func can now use either or both of name prefix filter or regex filter. If both are used at the same time, test matches either of the filter is accepted --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Gang Zhao <gang@gang-JQ62HD2C37.local>
- This filter test func can now use either or both of name prefix filter or regex filter. If both are used at the same time, test matches either of the filter is accepted --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: sampan <sampan@anyscale.com>
- This filter test func can now use either or both of name prefix filter or regex filter. If both are used at the same time, test matches either of the filter is accepted --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>
