-
Notifications
You must be signed in to change notification settings - Fork 313
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
[circt-test] Allow tests to filter according to test runners #8084
Conversation
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.
One nit regarding filter types but otherwise it looks good to me.
tools/circt-test/circt-test.cpp
Outdated
auto attr = attrs.get(attrName); | ||
if (!attr) | ||
return; | ||
if (auto arrayAttr = dyn_cast<ArrayAttr>(attr)) { | ||
for (auto elementAttr : arrayAttr.getValue()) | ||
if (auto stringAttr = dyn_cast<StringAttr>(elementAttr)) | ||
names.insert(stringAttr); | ||
return; | ||
} | ||
if (auto stringAttr = dyn_cast<StringAttr>(attr)) | ||
names.insert(stringAttr); |
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.
Allowing null/array/string looks a bit too untyped to me. Can we at least reject StringAttr case? Also it would be nice to have a document for parameters.
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.
Yeah that's a good point. I'll make this fallible and add some error reporting. I can add the attributes that have some specific meaning to the op definition, and then later on we can maybe move them to the docs where they're easier for users to find?
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.
Sounds great, thanks!
c6358ab
to
3088e8d
Compare
Interpret the `require_runners` and `exclude_runners` attributes on formal tests as a filter for the potential runners that can execute a test. If the set of required runners is not empty, a runner from this set is picked. Tests for which no runner is available report as "unsupported", for example if none of the available runners match the filter.
3088e8d
to
beb4d6c
Compare
Interpret the
require_runners
andexclude_runners
attributes on formal tests as a filter for the potential runners that can execute a test. If the set of required runners is not empty, a runner from this set is picked. Tests for which no runner is available report as "unsupported", for example if none of the available runners match the filter.