action #109620
closedcoordination #109668: [saga][epic] Stable and updated non-qemu backends for SLE validation
coordination #109656: [epic] Stable non-qemu backends
os-autoinst: Improve unit-test code coverage for backend::svirt size:M
Description
os-autoinst: Improve unit-test code coverage for backend::svirt
Motivation¶
https://app.codecov.io/gh/os-autoinst/os-autoinst/blob/master/backend/svirt.pm shows that we have at time of writing just 43% statement coverage and multiple methods are not fully covered. Before we go about extending features or fixing bugs we should ensure with better test coverage that we are less likely to introduce regressions and ease development (e.g. for #106685).
Acceptance criteria¶
- AC1: 100% statement coverage reported in https://app.codecov.io/gh/os-autoinst/os-autoinst/blob/master/backend/svirt.pm
Suggestions¶
- Look into t/22-svirt.t, call it locally to get coverage, e.g.
make coverage TESTS=t/22-svirt.t, maybe some other tests on top - Look into the interesting challenge why the above command does not seem to provide any coverage for the file backend/svirt.pm at all (at least for okurz)
- Optionally add a new unit-test level test file, e.g. based on t/29-backend-amt.t
- Add test (with mocking) for existing code in backend/svirt.pm until a sufficiently high statement coverage is reached
Updated by okurz about 3 years ago
- Copied from action #106685: Test using svirt backend incomplete with auto_review:"Error connecting to VNC server.*: IO::Socket::INET: connect: Connection timed out":retry added
Updated by livdywan about 3 years ago
- Subject changed from os-autoinst: Improve unit-test code coverage for backend::svirt to os-autoinst: Improve unit-test code coverage for backend::svirt size:m
- Description updated (diff)
- Status changed from New to Workable
Updated by osukup about 3 years ago
- Assignee set to osukup
I'll try something
@okurz how you get 22-svirt.t not providing any coverage ? from generated html pages I can't get which tests covers covered lines
Updated by okurz about 3 years ago
osukup wrote:
@okurz how you get
22-svirt.tnot providing any coverage ? from generated html pages I can't get which tests covers covered lines
If I call make coverage TESTS=t/22-svirt.t locally then at the end I receive a console output about the coverage of individual files. backend/svirt.pm is not listed in that list
Updated by tinita about 3 years ago
weird. I get
Reading database from /home/tina/openqa-devel/repos/os-autoinst/build/cover_db
------------------------------------- ------ ------
File stmt total
------------------------------------- ------ ------
OpenQA/Benchmark/Stopwatch.pm 7.1 7.1
...
backend/svirt.pm 49.7 49.7
...
Total 23.9 23.9
------------------------------------- ------ ------
HTML output written to /home/tina/openqa-devel/repos/os-autoinst/build/cover_db/coverage.html
What do you get if you do this?
% tools/invoke-tests --coverage t/22-svirt.t
% cover -report html_minimal cover_db
Updated by okurz about 3 years ago
that works fine, same result as you. Also no error about "Devel::Cover: Warning: can't locate structure for statement"
Updated by tinita about 3 years ago
You are working on this with this PR, right? https://github.com/os-autoinst/os-autoinst/pull/2048
Updated by okurz about 3 years ago
- Related to action #108266: grenache: script_run() commands randomly time out since server room move added
Updated by okurz about 3 years ago
- Subject changed from os-autoinst: Improve unit-test code coverage for backend::svirt size:m to os-autoinst: Improve unit-test code coverage for backend::svirt size:M
Updated by livdywan almost 3 years ago
- Related to coordination #109740: [epic] Stable os-autoinst unit tests with good coverage added
Updated by osukup almost 3 years ago
PR has now 100% coverage
bonus -> found problem in can_handle and fixed
Updated by livdywan almost 3 years ago
- Blocks action #111254: Cover code of os-autoinst path backend/ fully (statement coverage) size:M added
Updated by osukup almost 3 years ago
- Status changed from In Progress to Feedback
tests merged, because there are also changes in backend itself --> feedback
Updated by osukup almost 3 years ago
- Status changed from Feedback to Resolved
no problems related to changes in backend observer...