Fix workaround bsc#1161421

[jknphy]

Adjust #14860

• Related ticket: https://progress.opensuse.org/issues/107815
• Verification run: sle

[github-actions]

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

• Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files

[jknphy]

hi @czerw I remember why I had to put that code line outside the workaround, which looked a bit weird, but do you know why is different behavior between this two:
https://openqa.suse.de/tests/8737686#step/kdump_and_crash/38
https://openqa.suse.de/tests/8743111#step/yast2_kdump/16

both are enabled at the beginning but only one has the popup to reboot, perhaps I'm missing something obvious here? :)
I can see the worker is different and there are some previous package installed, crash, kernel-* etc that we don't have in our test ...
bug?

[czerw]

I don't think it is a bug, test should detect either need to confirm reboot or just continue without it. yast could detect different memory and it could lead for new memory value (both tests have different memory setup).

[jknphy]

I don't think it is a bug, test should detect either need to confirm reboot or just continue without it. yast could detect different memory and it could lead for new memory value (both tests have different memory setup).

ahm ok, it is that, right, thanks for the hint Petr, 4GB vs 1GB.

Nevertheless the tests should be predictable to save us time in their maintenance, selecting behavior based on needles is very fragile. This is old code we try to do better since a time ago, I remember we inherited this test suite a time ago but didn't have the chance to improve it yet.

Current code is very deceiving because types 2 'ret' one for enter the value and another for accept the screen, one line for two very different things, and then there is a alt-ok at the end that in these two archs serves for the ok button of the popup but in other cases just for the ok of the main screen. Definitely there is some gap for improvement here.

I will create a ticked for us to take a look, perhaps this module could be a good candidate for libyui REST API and a better design.

In the meantime I added another 'hack' that might work for all the cases, it is ugly, but after many tries and not finding what is going on due to lack of video, I came up with that, it looks like sometimes some ret is lost and sometimes does what is supposed to do depending on there is typing or not... I will check verification on Monday (openqa is slow at this time of the day). Please, feel free to merge it if it works or revert previous code if needed, apparently wasn't so easy to remove the workaround.

[czerw]

https://openqa.suse.de/tests/8750817 still fails, but there is no rush for solution

[czerw]

Hmm looks like that some scenarios still needs memory workaround https://openqa.suse.de/tests/8763993#step/kdump_and_crash/69 i will double check it more.

[b10n1k]

if i am not wrong the $expect_restart_info doesnt need to used twice. In additional, add a proper commit message explaining why you do what you do, please

[jknphy]

Hmm looks like that some scenarios still needs memory workaround https://openqa.suse.de/tests/8763993#step/kdump_and_crash/69 i will double check it more.

let me know, because from the point of view of YaST scope, given that we actually don't try to crash the system for real and just focus on the configuration, I could make it conditional to the memory size the upper block instead of using the product, in other words for yast2_kdump might not make sense to display the workaround in any product.

[czerw]

let me know, because from the point of view of YaST scope, given that we actually don't try to crash the system for real and just focus on the configuration, I could make it conditional to the memory size the upper block instead of using the product, in other words for yast2_kdump might not make sense to display the workaround in any product.

I think you are right, just please remove the QEMURAM condition,we can merge it. I will migrate all our tests to cli kdump version (including maintenance jobs).

[jknphy]

let me know, because from the point of view of YaST scope, given that we actually don't try to crash the system for real and just focus on the configuration, I could make it conditional to the memory size the upper block instead of using the product, in other words for yast2_kdump might not make sense to display the workaround in any product.

I think you are right, just please remove the QEMURAM condition,we can merge it. I will migrate all our tests to cli kdump version (including maintenance jobs).

This is the cleanest solution I could find for this approach, basically the workaround will be done always except when we signal it from the test module which does not do any crash yast2_kdump. Verifications updated in 1st comment.