Skip to content
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

Remove Workaround for bsc#1070233 #4973

Merged
merged 1 commit into from
May 8, 2018
Merged

Conversation

@Jiawei517 Jiawei517 changed the title Remove Workaround for bsc#1070233 [WIP] Remove Workaround for bsc#1070233 May 4, 2018
@mitiao mitiao added the WIP Work in progress label May 4, 2018
@Jiawei517 Jiawei517 changed the title [WIP] Remove Workaround for bsc#1070233 Remove Workaround for bsc#1070233 May 4, 2018
@Jiawei517
Copy link
Contributor Author

@mitiao Thanks for help, I have rebase this commit, please help remove [WIP] label

@mitiao mitiao removed the WIP Work in progress label May 4, 2018
@@ -50,6 +45,13 @@ sub run {
assert_screen 'booting-section-selected';
send_key 'ret';
}

# Config bootloader is not be supported durning an upgrade
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/durning/during/

@okurz
Copy link
Member

okurz commented May 4, 2018

Please add a corresponding openSUSE needle. See latest failure in https://openqa.opensuse.org/tests/668866

And for the future: Please don't forget about openSUSE :)

@Jiawei517
Copy link
Contributor Author

@okurz needle for openSUSE os-autoinst/os-autoinst-needles-opensuse#364

@@ -50,6 +45,13 @@ sub run {
assert_screen 'booting-section-selected';
send_key 'ret';
}

# Config bootloader is not be supported during an upgrade
if (get_var('UPGRADE') && (!is_sle('<15') || !is_leap('<15.0'))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's about using is_sle feature for version comparison instead of negation? E.g.

is_sle('>=15') || is_leap('>=15.0') || is_tumbleweed

I guess we will need is_tumbleweed, as current expression will return true in case UPGRADE is set and distri is TW. WDYT?

Copy link
Contributor Author

@Jiawei517 Jiawei517 May 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I have rebased the code.

  • OpenSUSE verify run: openqa-apac1.suse.de/tests/897#step/disable_grub_timeout/13

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, overall I prefer to avoid is_tumbleweed as that is not really a good version to check for. Rather we should accept the TW behaviour always as the default and handle exceptions differently, e.g. in this case handle the is_sle('<15') || is_leap('<15') case as exception and accept everything else as default.

If you agree, please change that. If you do not want to change this in this PR then please ping me or anyone else to merge directly as is.

@Jiawei517 Jiawei517 force-pushed the ipsec branch 2 times, most recently from 43b7e21 to 06d96de Compare May 8, 2018 05:42
Bug 1070233 has been fixed, remove workaround
@okurz okurz merged commit de24dc8 into os-autoinst:master May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants