Reorganize calls for assert_screen to avoid time waste #8757
Conversation
foursixnine
force-pushed
the
oopsitsbrokenagain
branch
from
3bbcd0b
to
982c067
Compare
tests/console/yast2_bootloader.pm
Outdated
| my $timeout = check_var('VIRSH_VMM_FAMILY', 'hyperv') ? 600 : 200; | ||
| assert_screen([qw(yast2_bootloader-missing_package yast2_console-finished)], $timeout); | ||
| my $timeout = (is_aarch64 || is_hyperv)? 600 : 200; | ||
| check_screen([qw(yast2_bootloader-missing_package yast2_console-finished)], $timeout); |
There was a problem hiding this comment.
Note, I'm not checking the tag for yast2_console-finished, because the waitserial will in the end, take care of it too.
There was a problem hiding this comment.
Please do not do this. This will be an annoying time-waste and an annoying red border in case of mismatch. Why not simply create a needle for any accepted situation? We also have soft-fail needles that could e.g. cover any "artifacts" if this is the problem
There was a problem hiding this comment.
@okurz the needle is there, what I could do, is to explicitly return in case yast2_console-finished tag is matched. However again, if it fails, the wait_serial afterwards would take care of it. What would you prefer? :) I'm open to suggestions here
There was a problem hiding this comment.
Yeah I know the "assert_screen+wait_serial" combination is a bit tricky here. First, what you are working on looks related to https://progress.opensuse.org/issues/56651 , please discuss with @asdil12 as well. Second, what I see in https://openqa.suse.de/tests/3506803#step/yast2_bootloader/6 is an ugly screen but functionally correct so a needle "yast2_console-finished" should simply cover this and no changes to the test code should be necessary. If you consider this a non-acceptable product behaviour open a bug and create a soft-fail needle referencing the bug but functionally the behaviour stays the same.
There was a problem hiding this comment.
@okurz indeed there's poo#56651 (I'm just noticing it now, so thanks for catching it up), but what I'm working on, is removing the ugly assert_screen + assert_screen + wait_serial. Although I started by increasing the timeout for aarch64. As for a bug: https://bugzilla.suse.com/show_bug.cgi?id=1154300 ;).
There was a problem hiding this comment.
Thinking again, actually we'd want to check the screen again, if the missing_package tag was found.
There was a problem hiding this comment.
I see. But can we not simply move L41 into the if and keep the assert_screen in place? E.g.:
assert_screen([qw(yast2_bootloader-missing_package yast2_console-finished)], $timeout);
if (match_has_tag('yast2_bootloader-missing_package')) {
send_key 'alt-i';
assert_screen 'yast2_console-finished', $timeout;
}so keep the assert_screen, replace the wait_screen_change by the assert_screen which is only called when necessary
foursixnine
force-pushed
the
oopsitsbrokenagain
branch
from
35fa411
to
e4b38a6
Compare
foursixnine
force-pushed
the
oopsitsbrokenagain
branch
from
e4b38a6
to
92a44d7
Compare
foursixnine
force-pushed
the
oopsitsbrokenagain
branch
from
92a44d7
to
20f4639
Compare
foursixnine
changed the title
|
@okurz can you check again? :) |
|
coming back to this after vacation: LGTM :) |
Bump timeout for aarch64