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

grub_test: use 'up' instead of 'esc' to cancel timeout #1047

Merged
merged 1 commit into from
Feb 15, 2016
Merged

grub_test: use 'up' instead of 'esc' to cancel timeout #1047

merged 1 commit into from
Feb 15, 2016

Conversation

DimStar77
Copy link
Contributor

grub2-efi falls to the command prompt when pressing esc, which is not what
we intend to do.

Reported as https://bugzilla.opensuse.org/show_bug.cgi?id=966701 for validation

grub2-efi falls to the command prompt when pressing esc, which is not what
we intend to do.
@MalloZup
Copy link
Contributor

if is a product bug for uefi, i don't get why we should change the "esc" keyboard to "up".

Up in a testing perspective, is more unsafer then "esc".

@DimStar77
Copy link
Contributor Author

a) it's not clear if it's a product bug - that's up to the bug report to identify
b) as a release manager, I cannot accept stalling ALL tumbleweed releases for a 'bug' that must have been there for like 'ever' just because the test is more correct.

@MalloZup
Copy link
Contributor

Dimstar you are a release manager, i'm a tester. we have two different perspectives. if there is a bug, test should fail.

@DimStar77
Copy link
Contributor Author

A test can fail if it discovers a regression - not because somebody came up with a new test that never worked. I can implement hundreds of those and never releaase anything anymore

@MalloZup
Copy link
Contributor

my test work for all scenarios, not uefi. that's different, as what you are saying.
i can understand that you want to release, maybe you use openqa in diffrents way as me, but we should find a compromise. i don't attach a importance to the "esc" keyboard, but i don't want to make a movement in the grub.

Btw why uefi use a different scenario, that's the question.

@DimStar77
Copy link
Contributor Author

it's grub2 vs grub2-efi... it's different code.

I did not say that your test does not work in other cases - I said it breaks uefi, which was not broken prior to the rewrite.

There is no 'cancel the timer' key in grub. if 'up' is too insecure, pressing 'right' might be safer (left moves the cursor in a possibly already typed kernel command, right moves it only more to the right)

@MalloZup
Copy link
Contributor

i would propose that maxlin say his opinion, because he introduced the key "Up".

Additionaly, we can use "up" and "down" so the problem is fixed.

@okurz
Copy link
Member

okurz commented Feb 15, 2016

LGTM

I also thought about 'left' and 'right' but that should be properly investigated as well as "up" and "down", e.g. in efi shell mode, in non-uefi, when booting an already installed system with a grub saved entry, etc. I recommend any of you can open a progress issue to investigate this further. This PR can be merged as is as it restores the previous behaviour.

@DimStar77
Copy link
Contributor Author

'right' is no option in grub2-efi: it behaves like RET and executes the boot
'left' is no option in normal grub2, as it moves the cursor on the visible kernel command line
'esc' bumps to the grub command prompt in grub2-efi (possible bug, to be investigated)

@DimStar77
Copy link
Contributor Author

@nilxam
Copy link
Member

nilxam commented Feb 15, 2016

do not try 'right', it was the same behavior of 'ret' in both grub2 and grub2-efi AFAIK.
I thought 'c' key is the shortcut for CLI mode, somehow 'esc' looks like the same thing on grub2-efi, maybe a bug?!
'left' should be safe as well, it is only usable in edit mode as moving cursor. I usually use 'up' in this case because the boot-to-system entry always are the first one in the entry list.

so 'up' and 'left' is considerable and usable here IMO.

@nilxam
Copy link
Member

nilxam commented Feb 15, 2016

although 'up' and 'left' sounds no difference here, but if the boot entry's order was changed due to a bug or intentional behavior, 'up' can make the test failure as what we expected, ie. hightlight bar is changing to another entry, so we're noticed.

nilxam added a commit that referenced this pull request Feb 15, 2016
grub_test: use 'up' instead of 'esc' to cancel timeout
@nilxam nilxam merged commit 20d2e88 into os-autoinst:master Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants