action #10132
closed[functional][u] "wait_screen_change" return value should not be silently ignored
0%
Description
Observation¶
https://openqa.suse.de/tests/175675 fails because the machine was heavily loaded causing "wait_screen_change" to timeout after 10 seconds where the return value was silently ignored, see:
https://openqa.suse.de/tests/175675/file/autoinst-log.txt
acceptance criteria¶
- wait_screen_change makes test die if screen does not change during timeout
problem¶
https://github.com/os-autoinst/os-autoinst-distri-opensuse/search?utf8=%E2%9C%93&q=wait_screen_change shows that wait_screen_change has a return value which is sometimes ignored, sometimes properly parsed. "wait_screen_change" should probably die on timeout without the need for explicit return value checking
suggestion¶
- move return value check into wait_screen_change. If code relies on wait_screen_change timing out the code should be reworked accordingly, e.g. using another method.
- check if we can disallow ignoring return value everywhere in "os-autoinst" space, e.g. by a proper style check
Updated by RBrownSUSE almost 9 years ago
- Checklist item changed from to [ ] SLE, [ ] Leap, [ ] TW
Updated by okurz over 7 years ago
- Status changed from New to In Progress
https://github.com/os-autoinst/os-autoinst/pull/689 added "assert_screen_change" but I guess it is not working as intended, see https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/2313
Updated by okurz almost 7 years ago
- Subject changed from "wait_screen_change" return value should not be silently ignored to [functional]"wait_screen_change" return value should not be silently ignored
- Target version set to Milestone 18
Updated by okurz over 6 years ago
- Target version changed from Milestone 18 to Milestone 18
Updated by okurz over 6 years ago
- Target version changed from Milestone 18 to future
Updated by okurz about 6 years ago
- Subject changed from [functional]"wait_screen_change" return value should not be silently ignored to [functional][u] "wait_screen_change" return value should not be silently ignored
- Status changed from In Progress to Workable
Updated by okurz almost 6 years ago
- Description updated (diff)
- Status changed from Workable to In Progress
- Assignee set to okurz
I think the ticket in the current state is more confusing then helpful as we have http://open.qa/api/testapi/#_assert_screen_change since some time. I will create followup tickets with more detailed steps what needs to be done now.
Updated by okurz almost 6 years ago
- Copied to action #48752: [easy][beginner] Add proper unit-test for `assert_screen_change` added
Updated by okurz almost 6 years ago
- Checklist item changed from [ ] SLE, [ ] Leap, [ ] TW to
- Status changed from In Progress to Resolved
See followup tickets.