action #81899
closed
coordination #58184: [saga][epic][use case] full version control awareness within openQA, e.g. user forks and branches, fully versioned test schedules and configuration settings
coordination #108527: [epic] os-autoinst wheels for scalable code reuse of helper functions and segmented test distributions
Move code from isotovideo to a module size:M
0%
Description
Motivation¶
Having code in a module (ideally under lib/) and seperated into functions makes it easier to test and mock.
Suggestions¶
- Move code out of isotovideo, no files/cmake involved here
Updated by livdywan over 2 years ago
Also, as I found during investigations of unit test execution times in #75232, this would speed up trivial operations like --help or --version which should be fast but take just as much time currently as preparing for a test run.
Updated by okurz over 1 year ago
- Tags set to os-autoinst, easy, beginner, entrance level
- Subject changed from Move code from isotovideo a module to [easy][beginner] Move code from isotovideo to a module
- Description updated (diff)
Updated by okurz over 1 year ago
- Status changed from New to In Progress
- Assignee set to okurz
- Priority changed from Normal to Low
- Target version changed from future to Ready
We need that for #108530
Updated by okurz over 1 year ago
Updated by okurz over 1 year ago
https://github.com/os-autoinst/os-autoinst/pull/2031 for signatures (merged), next
https://github.com/os-autoinst/os-autoinst/pull/2032
Updated by okurz over 1 year ago
- Subject changed from [easy][beginner] Move code from isotovideo to a module to [easy][beginner] Move code from isotovideo to a module size:M
Updated by okurz over 1 year ago
- Related to action #108530: os-autoinst wheels: x11_start_program from os-autoinst-distri-openQA dynamically loaded from another git repo size:M added
Updated by okurz over 1 year ago
- Status changed from In Progress to Feedback
Waiting for PR to be merged and switching over to #99663 to not get into my own way.
Updated by okurz 11 months ago
- Assignee set to livdywan
@cdywan I assume you forgot to assign yourself when you set to "In Progress" in https://progress.opensuse.org/issues/81899#note-15
Updated by livdywan 11 months ago
- Assignee deleted (
livdywan)
Here's my proposal to move backend process logic into a new class: https://github.com/os-autoinst/os-autoinst/pull/2194
I was originally thinking to move more into that class but the way global and semi-global variables are used I decided to start smaller.
Another part that might be fine to simply move into Utils is the "debuggers", so I put that in a separate branch: https://github.com/os-autoinst/os-autoinst/pull/2195
Updated by livdywan 11 months ago
- Status changed from Workable to In Progress
- Assignee set to livdywan
Third part, moving the check_asserted_screen logic into the CommandHandler: https://github.com/os-autoinst/os-autoinst/pull/2196/files
Updated by livdywan 11 months ago
One more piece, the loop, could be moved out of isotovideo: https://github.com/os-autoinst/os-autoinst/pull/2200
Updated by livdywan 10 months ago
The final part https://github.com/os-autoinst/os-autoinst/pull/2209
Updated by livdywan 10 months ago
- Related to action #120786: Jobs are now incomplete when postfail hook fails size:S added
Updated by livdywan 10 months ago
Apparently the penultimate change caused a regression. So I proposed an improvement to a unit test to reveal the issue. The other change is in fact fine on its own, and now no longer depends on the broken one.
Updated by livdywan 10 months ago
cdywan wrote:
Apparently the penultimate change caused a regression. So I proposed an improvement to a unit test to reveal the issue. The other change is in fact fine on its own, and now no longer depends on the broken one.
Alas the unit test won't fail regardless of improvements. From Tina's findings it looks like it's affected by tests run before it. TESTS="t/14-debugging-tools.t t/14-isotovideo.t" makes the unit tests pass in CI and container_run_ci. Commenting out the second spawn_debuggers in the test even makes it fail. This may actually be a race condition.
Updated by tinita 10 months ago
I was able to get the 14-isotovideo.t pass locally, and also fail with running 14-debugging-tools.t before.
But running 14-isotovideo.t passed very rarely, and running 14-debugging-tools.t before made it pass much more often.
But all this only with tools/container_run_ci. Running the tests manually with prove on my host, 4-debugging-tools.t always fails, even with code coverage enabled.
So it might be a timing issue, and it's not clear how we could reliably make it fail or pass.
But I'm working on code to establish the previous initialization order, and that at least works locally and in CI in all test runs so far.
Updated by livdywan 10 months ago
Updated by okurz 8 months ago
- Related to action #121045: Performance regression of `os-autoinst` size:M added
Updated by livdywan 8 months ago
- Status changed from Feedback to Resolved
As of now idsotovideo has been simplified significantly, primarily handling arguments and keeping state to ensure a known exit code. OpenQA::Isotovideo::Runner is used to start the backend, setup autotest, run the main loop and handle commands meaning all important code paths are implemented in other modules suitable for mocking. Code is also no longer heavily relyant on global variables.
I think we can consider this resolved.