Project

General

Profile

action #114412

Add support for "wait_screen_change" with "no_wait" option to allow to use on cases like "wait for every character to be typed" size:M

Added by okurz 2 months ago. Updated 12 days ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
Feature requests
Target version:
Start date:
Due date:
2022-09-16
% Done:

0%

Estimated time:
Difficulty:

Description

Motivation

As proposed in #109737 there are cases where we could use "wait_screen_change" but by default it waits for 0.5s which is wasteful if done too often in a row, e.g. in cases like "wait for every character to be typed". So same as in "check_screen/assert_screen" we should add support for the "no_wait" option or rethink how much time needs to be waited at all if not continuously checking.

Acceptance criteria

  • AC1: "wait_screen_change" on a repeatedly updated screen waits significantly less than the current 0.5s

Suggestions


Related issues

Copied from openQA Tests - action #109737: [opensuse][sporadic] test fails in chromium due to lost characters when typing in the address bar size:MResolved2022-04-09

History

#1 Updated by okurz 2 months ago

  • Copied from action #109737: [opensuse][sporadic] test fails in chromium due to lost characters when typing in the address bar size:M added

#2 Updated by cdywan 2 months ago

  • Subject changed from Add support for "wait_screen_change" with "no_wait" option to allow to use on cases like "wait for every character to be typed" to Add support for "wait_screen_change" with "no_wait" option to allow to use on cases like "wait for every character to be typed" size:M
  • Description updated (diff)
  • Status changed from New to Workable

#3 Updated by tinita about 1 month ago

  • Priority changed from Normal to High

#4 Updated by mkittler 26 days ago

  • Assignee set to mkittler

there are cases where we could use "wait_screen_change" but by default it waits for 0.5s which is wasteful if done too often in a row

Note that it only waits 0.5 s if there's no immediate change. Simply decreasing this sleep won't help unless we decrease the frequency the backend requests a screen updated and enqueues a new screenshot. So as a first step I need to determine how the backend behaves in that regard and possibly add a way to enforce an immediate update (similar to no_wait for screen checks).

#5 Updated by mkittler 26 days ago

It looks like the isotovideo normally aims for a 1 second update interval. (The function _calc_check_delta in isotovideo checks how much time has passed since the last update and sets the timeout for a 1 second interval.) In case a screen is to be asserted with no_wait option the timeout is reduced to 0.1 seconds. However, all of this is only the interval for calling the backend's check_asserted_screen function. That function will only request a screen update (via VNC) in certain cases, though.

The backend's interval for requesting VNC updates is actually only influenced indirectly via $bmwqemu::vars{SCREENSHOTINTERVAL} which is by default 0.5 seconds. (The update interval is actually half of it, so by default it is 0.25 seconds.)

So assuming defaults are in place, reducing the sleep in wait_screen_change will not make it faster because only when a new screenshot is enqueued the the last image is updated and that's used for this check.

Considering the code in do_capture of baseclass.pm it should be easy to add another variable (that could be set from autotest functions) to either change the screenshot (and possibly VNC update) interval dynamically or to enforce the next VNC update and screenshot immediately.

It would also be possible to decouple wait_screen_change from the screenshot creation and compare with the VNC framebuffer directly. I suppose then we'd also make sure that the VNC socket is drained (as this so far only seems to happen when capturing a screenshot or before an update request).

#6 Updated by mkittler 25 days ago

So far autotest needs to poll the backend in wait_screen_change (with that 0.5 seconds sleep). Maybe it would be nicer if the backend would provide a synchronous call instead. So the backend would simply postpone the reply to the call until the screen changes or a timeout is reached. That would eliminate this one sleep altogether. The call would also be able to lower the screenshot interval to waste less time.

#7 Updated by mkittler 25 days ago

  • Status changed from Workable to In Progress

#8 Updated by openqa_review 24 days ago

  • Due date set to 2022-09-16

Setting due date based on mean cycle time of SUSE QE Tools

#9 Updated by mkittler 20 days ago

This PR shows how removing the sleep-loop in the autotest completely would look like: https://github.com/os-autoinst/os-autoinst/pull/2158

Then I could go ahead and add an option to make the backend reduce the time before enqueuing the next screenshot to waste less time on wait_screen_change calls.

Before that, I'll first give the refactoring a verification run. I'll try the scenario https://openqa.opensuse.org/tests/latest?arch=x86_64&distri=opensuse&flavor=DVD-Updates&machine=64bit-2G&test=gnome&version=15.3 from the original ticket about chromium typing issues (#109737).

#10 Updated by mkittler 19 days ago

PR for actually implementing no_wait (based on the first PR): https://github.com/os-autoinst/os-autoinst/pull/2164

#11 Updated by mkittler 14 days ago

  • Status changed from In Progress to Feedback

The change for moving the loop to the backend was merged last week and has been deployed on o3 since 2022-09-08 (e.g. 2022-09-08 10:52:57|install|os-autoinst|4.6.1662625276.a9cb3fd-lp154.1384.1|aarch64||devel_openQA|9bf1d9106e4e769ae20319bc456ef5657e11051f78e634c6b4f398b10f45e71f|). It was also deployed on OSD today. So far it doesn't look like the refactoring caused any problems.

The change for implmenting no_wait still needs to be merged. I did some performance testing today and it looks like it really makes things faster (see https://github.com/os-autoinst/os-autoinst/pull/2164#issuecomment-1243772718).

#12 Updated by cdywan 12 days ago

  • Status changed from Feedback to Resolved

This ticket is done, and we briefly confirmed our expectations here meaning #109737 is unblocked now and can be verified for the improvements.

Also available in: Atom PDF