-
Notifications
You must be signed in to change notification settings - Fork 270
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
[WIP] Move more common test schedule parts into common schedule functions #4619
Conversation
Verified with local schedule validation on various job settings. I am sure I missed something :)
501d876
to
242614f
Compare
@@ -1717,6 +1751,21 @@ sub load_ssh_key_import_tests { | |||
loadtest "x11/ssh_key_verify"; | |||
} | |||
|
|||
sub load_common_opensuse_sle_tests { | |||
load_autoyast_clone_tests if get_var('CLONE_SYSTEM'); |
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 think you changing model here in dangerous way - before we had elsif chain , so if you get into one branch you don't have chance to get into another , with your approach we will be capable to execute wicked and systemd testsuite in single job which not always make sense
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.
other way would be just put as prefix return
:
return load_toolchain_tests if get_var('TCM') || check_var('ADDONS', 'tcm');
Let's wait for more reviews, but I fully support this idea. After merge we can check what we've missed. |
@@ -984,8 +933,10 @@ elsif (get_var('HPC')) { | |||
} | |||
} | |||
} | |||
elsif (get_var('SYSTEMD_TESTSUITE')) { | |||
load_systemd_patches_tests; | |||
elsif (get_var("SES5_DEPLOY")) { |
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.
it's look like something unrelated to this refactoring, right ?
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.
if so , please put it at least in separate commit
I guess I would need to check the schedule evaluation based on more vars.json files locally |
Verified with local schedule validation on various job settings. I am sure I
missed something :)