-
Notifications
You must be signed in to change notification settings - Fork 306
Adds detection and reporting for ISA extensions that are implemented but have no selected tests. #1315
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
base: act4
Are you sure you want to change the base?
Adds detection and reporting for ISA extensions that are implemented but have no selected tests. #1315
Changes from all commits
1123a3f
7e02285
7f49b14
fc11345
4d9eac5
d7ccfcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,8 @@ | |
| # | ||
| # Select tests to run based on UDB config and test list | ||
| ################################## | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import sys | ||
| import re | ||
|
|
||
| from act.parse_test_constraints import TestMetadata | ||
|
|
@@ -67,6 +66,7 @@ def select_tests( | |
| config_params: dict[str, ConfigParamValue], | ||
| *, | ||
| include_priv_tests: bool = True, | ||
| excluded_extensions: set[str] | None = None, | ||
| ) -> dict[str, TestMetadata]: | ||
| """Select tests that match the UDB configuration.""" | ||
| selected_tests: dict[str, TestMetadata] = {} | ||
|
|
@@ -80,4 +80,43 @@ def select_tests( | |
| test_params = test_metadata.params | ||
| if check_test_params(test_params, config_params): | ||
| selected_tests[test_name] = test_metadata | ||
| # Warn about implemented extensions with no tests | ||
| untested_extensions = get_untested_implemented_extensions( | ||
| selected_tests, | ||
| implemented_extensions, | ||
| include_priv_tests=include_priv_tests, | ||
| excluded_extensions=excluded_extensions, | ||
| ) | ||
|
|
||
| if untested_extensions: | ||
| print( | ||
| "Warning: no applicable tests selected for implemented extension(s): " | ||
| + ", ".join(untested_extensions), | ||
| file=sys.stderr, | ||
| ) | ||
|
Comment on lines
+83
to
+96
|
||
|
|
||
| return selected_tests | ||
|
|
||
| def _is_excluded_extension(ext: str, excluded_extensions: set[str]) -> bool: | ||
| return any(ext.startswith(ex) for ex in excluded_extensions) | ||
|
|
||
| def get_untested_implemented_extensions( | ||
| selected_tests: dict[str, TestMetadata], | ||
| implemented_extensions: set[str], | ||
| *, | ||
| include_priv_tests: bool = True, | ||
| excluded_extensions: set[str] | None = None, | ||
| ) -> list[str]: | ||
| """Return implemented extensions that have no selected tests.""" | ||
| covered_extensions = { | ||
| extension for test_metadata in selected_tests.values() for extension in test_metadata.required_extensions | ||
| } | ||
| untested_extensions = implemented_extensions - covered_extensions | ||
| if not include_priv_tests: | ||
| untested_extensions -= PRIV_EXTENSIONS | ||
| if excluded_extensions: | ||
| untested_extensions = { | ||
| ext for ext in untested_extensions | ||
| if not _is_excluded_extension(ext, excluded_extensions) | ||
| } | ||
|
Comment on lines
+117
to
+121
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's drop the excluded extensions check in this function. If an extension is excluded it is valid to print a warning saying that extension is not tested. The EXCLUDED_EXTENSIONS list can be hidden in a Makefile and might not be obvious to users. |
||
| return sorted(untested_extensions) | ||
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.
excluded_extensionsis derived from the CLI--excludestring, whichgenerate_test_dict()applies as a test directory filter (it compares againsttest_file.parent.name, e.g.Sv). Passing this set intoget_untested_implemented_extensions()(which compares against ISA extension tokens likeSv39,Zba, etc.) means excludes likeSvwon’t suppress warnings for implementedSv32/Sv39/..., leading to false-positive warnings. Consider translating excluded directory names into ISA extension names (e.g., treat an excluded prefix likeSvas excluding any implemented extension starting withSv), or only applying this filter when the excluded value is known to be in the same namespace asimplemented_extensions.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.
@copilot apply changes based on this feedback
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 like that didn't work.