-
Notifications
You must be signed in to change notification settings - Fork 269
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
Give more time to match needles on exit_firefox #15381
Give more time to match needles on exit_firefox #15381
Conversation
Great PR! Please pay attention to the following items before merging: Files matching
This is an automatically generated QA checklist based on modified files |
lib/x11test.pm
Outdated
wait_still_screen 1, 2; | ||
send_key_until_needlematch([qw(firefox-save-and-quit xterm-left-open xterm-without-focus)], "alt-f4", 3, 30); | ||
send_key_until_needlematch([qw(firefox-save-and-quit xterm-left-open xterm-without-focus)], "alt-f4", 6, 30); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michaelgrifalconi
Seems a timing or performance issue here, I am wondering if we can add some timeout here
wait_still_screen 1, 2;
BTW, I would suggest do more VRs on other SLE versions which hit the same issue as well :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, started more validation runs.
I don't think a wait_still_screen at the beginning can help much, the issue is between multiple runs of send_key_until_needlematch.
It will send a alt-f4 key initially, wait for it to close firefox and if firefox is not fast enough to close, send another alt-f4 and then both firefox and xterm close and it fails. Increasing the time between sending these alt-f4 keys is the only relevant factor IMHO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the reason why I add the comments is that, I try to do some manual tests on my local setup.
ctrl-q
key can close the firfox page, then I don't need to send alt-f4
to force closing it.
Anyway, I agree with you, your changes make more sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! the ctrl-q is the one supposed to close it, I missed that sorry! Maybe giving more time could help! I will try to increase it as well.
Issue is still happening in some tries, looking more into it |
Tried some statistical investigation with the fix, since the issue is sporadic: https://openqa.suse.de/tests/overview?build=mgrifalconi-firefoxdev Does not look too bad. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Give more time to match needles on exit_firefox, to avoid sending too many alt-f4 and then failing because we can't find the open terminal