-
Notifications
You must be signed in to change notification settings - Fork 119
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
Override test attribute from plan #2865
base: main
Are you sure you want to change the base?
Conversation
New option for discover -h fmf to adjust discovered tests Fix: #2430
Note that you need teemtee/fmf#230 to try this PR |
@@ -155,6 +155,12 @@ class DiscoverFmfStepData(tmt.steps.discover.DiscoverStepData): | |||
show_default=True, | |||
help="Copy only immediate directories of executed tests and their required files.") | |||
|
|||
# Edit discovered tests | |||
adjust_tests: list[Any] = field( |
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.
Would it make sense for this field to be pretty much the same as adjust
, https://github.com/teemtee/tmt/blob/main/tmt/base.py#L681 ?
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.
I suppose it could, it would be re-using the adjust
concept, i.e. the same set of keys? We would use _RawAdjustRule
type instead of Any
(it's almost the same, but still better than Any
, it's easier to follow in code).
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.
thanks, was blind to see that we already have adjust rules
@@ -562,7 +568,8 @@ def do_the_discovery(self, path: Optional[Path] = None) -> None: | |||
tree = tmt.Tree( | |||
logger=self._logger, | |||
path=tree_path, | |||
fmf_context=self.step.plan._fmf_context) | |||
fmf_context=self.step.plan._fmf_context, | |||
additional_rules=self.get('adjust-tests')) |
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.
self.data.adjust
should be preferred over self.get()
as self.get()
can't handle the type annotations.
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.
so like this?
additional_rules=self.get('adjust-tests')) | |
additional_rules=self.data.adjust_tests) |
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.
Yes, that should work, type checkers should grab the right type (because self.data
is a dataclass), and match it against the additional_rules
signature.
fmf released today, but it will take some time it reaches stable repos, thus aiming for next tmt version with this. |
New option for discover -h fmf to adjust discovered tests
I went with 'adjust-tests' keyword proposed in #2430 (reply in thread)
Still to decide:
Extend test coverage
require: [$FORCE_REQUIRE]
Pull Request Checklist