-
Notifications
You must be signed in to change notification settings - Fork 144
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
Execute must not use discover phases disabled by their when
#3493
Conversation
27fe736
to
29fc1e5
Compare
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.
Should we add tests?
I verified it fixes the problem
Yep, it's there now. It took me some time to modify and extend the existing one. |
854da66
to
73cdd3b
Compare
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 for the quick fix!
/packit retest-failed |
This seems to be an omission from when `when` was added: it is evaluated by tmt, not by fmf, disabled phases are still present in the tree of steps and phases. Most places already ignore them, but `execute` did not. `execute` runs its single phase with all `discover` phases in sequence, and must make sure it does not try to run the disabled ones. Fixes #3492.
73cdd3b
to
8af4e4d
Compare
This seems to be an omission from when
when
was added: it is evaluated by tmt, not by fmf, disabled phases are still present in the tree of steps and phases. Most places already ignore them, butexecute
did not.execute
runs its single phase with alldiscover
phases in sequence, and must make sure it does not try to run the disabled ones.Fixes #3492.
Pull Request Checklist