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
Fix staging and adjust tests for new welcome screen #3522
Conversation
tests/installation/welcome.pm
Outdated
if (!sle_version_at_least('15') || is_staging()) { | ||
# license+lang +product (on sle15) | ||
# On sle 15 license is on different screen, here select the product | ||
if (sle_version_at_least('15') && check_var('STAGING', 'Y')) { |
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 can't get more ugly than this, right? ;-)
How temporary do you consider this depending on the specific staging project?
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.
as temporary as the staging project is
products/sle/main.pm
Outdated
if (my $allmodules = get_var('ALL_MODULES') && sle_version_at_least('15')) { | ||
#default to all modules if set to 1 | ||
my @modules = qw(base sdk desktop legacy script serverapp); | ||
# If ALL_MODULES contains a list fo modules, add only them |
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.
s/fo/of/
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.
Fixed
products/sle/main.pm
Outdated
@@ -287,7 +288,13 @@ if (get_var('DEV_IMAGE')) { | |||
|
|||
# This is workaround setting which will be removed once SCC add repos and allows adding modules | |||
# TODO: remove when not used anymore | |||
if (get_var('ALL_MODULES') && sle_version_at_least('15')) { | |||
if (my $allmodules = get_var('ALL_MODULES') && sle_version_at_least('15')) { |
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 prefer to be more explicit about the type of variables. Better add another specific variable when you want to set the variables and keep the existing one as boolean ALL_MODULES
. I don't think ALL_MODULES=module1,module2 is easy to understand
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.
Idea is that this is temporary, so I didn't want to check multiple variables. But yeah, let's make it cleaner, as we know that temporary changes remain forever =)
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.
Replaced with WORKAROUND_MODULES variable
For staging we don't want to add all modules, as we do in openQA Functional. This change allows to define short names of modules to be added. E.g. ALL_MODULES=base,serverapp In case of value '1', behavior remains same. Another change for staging contains product selection on welcome screen and not on separate screen as it was before. It's relevant for staging Y only at the moment, but we can enable it for production once change is there.
For staging we don't want to add all modules, as we do in
openQA Functional. This change allows to define short names of
modules to be added. E.g. ALL_MODULES=base,serverapp
In case of value '1', behavior remains same.
Another change for staging contains product selection on welcome screen and not
on separate screen as it was before. It's relevant for staging Y only at the moment,
but we can enable it for production once change is there.
After merge we need to update cron job setup to trigger sle15_sp0_staging profile.
Verification run staging Y
Verification run production
Please, merge the NEEDLE.