Check runner results after some settling time #7679
Conversation
In case the runner does not show up properly after 10 attempts, fail the test as the error is hard to spot otherwise. Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
StefanBruens
force-pushed
the
recheck_screen
branch
from
30bd1dc
to
cb7cb55
Compare
Currently, when the runner results does not show the results immediately (default timeout is 0), the check fails, the runner is closed with 'esc', and the system waits for 30 seconds. Instead, give the runner a reasonable chance to present its results, and only repeat the sequence if the results does not show up after the default timeout of 30 seconds. See poo#53045 Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
|
Is it a replacement of #7540 ? |
|
Could we have some tests run on x86_64 and aarch64, please? |
| if ($retries > 1) { | ||
| if ($retries == 1) { | ||
| assert_screen('desktop-runner-plasma-suggestions', $timeout); | ||
| } elsif (!check_screen('desktop-runner-plasma-suggestions', $timeout)) { |
There was a problem hiding this comment.
I consider this approach racy as well. Can you evaluate if you can use a multi-tag assert_screen with match_has_tag instead of check_screen with non-zero timeout to prevent introducing any timing dependant behaviour, to save test execution time as well as state more explicitly from the testers point of view what are the expected alternatives. For example:
assert_screen([qw(yast2_console-finished yast2_missing_package)]);
if (match_has_tag('yast2_missing_package')) {
send_key 'alt-o'; # confirm package installation
assert_screen 'yast2_console-finished';
}I agree that the previous approach by @ggardet might also not be the perfect solution in the end. Please see https://progress.opensuse.org/issues/35589 with my – rather lengthy – investigation. And any try to fix this proberly demands good statistics for a proof :) If you like you can actually use ressources on openqa.opensuse.org yourself for testing, @ggardet does the same with good success already. http://open.qa/docs/#_triggering_tests_based_on_an_any_remote_git_refspec_or_open_github_pull_request describes how that can be achieved. Feel free to ping "okurz" on irc://chat.freenode.net/opensuse-factory for help and getting API access to the instance.
There was a problem hiding this comment.
I consider this approach racy as well. Can you evaluate if you can use a multi-tag
assert_screenwithmatch_has_taginstead ofcheck_screenwith non-zero timeout to prevent introducing any timing dependant behaviour, to save test execution time as well as state more explicitly from the testers point of view what are the expected alternatives.
Can you please elaborate which events would lead to a race here? Obviously, krunner needs some time to show the results, so checking immediately after the last character typed has a very low success rate.
The check here is only for the krunner suggestions, so there is exactly one tag involved.
There was a problem hiding this comment.
ok, sorry. in this case I don't mean "racy" but wasting time in case of no-match. In this specific case I actually think check_screen with a non-zero timeout would work ok but unfortunately I had been repeatedly fighting against many check_screen calls which are wasting time a lot and nobody realizes. I would appreciate if you can try to fix the same problem with the multi-tag assert_screen I suggested above. If you want to keep the check_screen call still I will accept that and merge the PR though :)
There was a problem hiding this comment.
The "nobody realizes" can not happen here, as the final repetition uses an "assert_screen", see https://openqa.opensuse.org/tests/962535#step/dolphin/46 (that one was still missing a matching needle).
Anything but check_screen would likely waste more time:
- with "0" timeout:
- the command is typed, krunner starts processing
- screen is grabbed, check_screen fails
- wait some time?
- check again?
- with short timeout
- the command is typed, krunner starts processing
- screen is grabbed, check_screen does not match, but no timeout
- the suggestions appear
- screen is grabbed, check_screen succeeds immediately
An assert_screen [ 'desktop-runner-plasma-suggestions', 'desktop-runner' ] would immeditaly succeed even without the suggestions, so we would have to repeat it - but immediately, or after waiting, and how often? Imagine the suggestions appear only after 5 seconds (which does not seem to be the case here, but after 1 frame at most) ...
There was a problem hiding this comment.
Well assert_screen ['desktop-runner-plasma-suggestions', 'desktop-runner' ] is obviously not the right call on plasma but we can agree that it simply has to show the suggestions before we should try to press return. IMHO 9e0534a was already wrong because it introduced a check for the suggestions when the task of "init_desktop_runner" should just be to type, nothing more. We are looking again for the suggestions window in x11_start_program where we also try to handle retries. Your PR here seems to make things better but I actually tend towards reverting 9e0534a as well. WDYT? @ggardet as well?
There was a problem hiding this comment.
@okurz The thing is 9e0534a make things to work. Without 9e0534a there are a lots of failure on aarch64 (also seen on x86_64, with less occurrences). So, I would prefer to merge this current PR which improve things a lot. We still can find another working solution afterwards. But please do not revert PR which make things to work without another PR which fixes previously fixed/workarounded problems.
|
All cases where a sufficient needle was available passed on the first try (especially oowriter, oocalc, oomath): I scheduled two more runs, this time including the updated/added needle. |
|
The other two tests have also finished successfully without any hickups or running into timeouts. |
|
Some background information why this notoriously failed for oowriter etc, but not e.g. echo, xterm, firefox etc:
|
|
LGTM. |
…gram" again This is a followup to os-autoinst#7442 as well as os-autoinst#7679 and os-autoinst#7540 which introduced further retries. This commit tries to move the retries into the place where we already handle the plasma suggestions list. Related progress issue: https://progress.opensuse.org/issues/35589
Currently, when the runner results does not show the results immediately,
the check fails, the runner is closed with 'esc', and the system waits
for 30 seconds.
Instead, give the runner a reasonable chance to present its results,
and only repeat the sequence if the results does not show up after 25
seconds.