Project

General

Profile

action #109620

coordination #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

Added by okurz about 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Low
Assignee:
osukup
Category:
Feature requests
Target version:
Ready
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:

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

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

Related issues

Related to openQA Infrastructure - action #108266: grenache: script_run() commands randomly time out since server room moveNew2022-03-14

Related to openQA Project - coordination #109740: [epic] Stable os-autoinst unit tests with good coverageResolved2021-06-30

Blocks openQA Project - action #111254: Cover code of os-autoinst path backend/ fully (statement coverage) size:MResolved2022-05-18

Copied from openQA Project - action #106685: Test using svirt backend incomplete with auto_review:"Error connecting to VNC server.*: IO::Socket::INET: connect: Connection timed out":retryNew

History

#1 Updated by okurz about 1 year 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

#2 Updated by okurz about 1 year ago

  • Parent task set to #109656

#3 Updated by cdywan about 1 year 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

#4 Updated by osukup about 1 year 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

#5 Updated by okurz about 1 year ago

osukup wrote:

okurz how you get 22-svirt.t not 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

#6 Updated by tinita about 1 year 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

#7 Updated by okurz about 1 year ago

that works fine, same result as you. Also no error about "Devel::Cover: Warning: can't locate structure for statement"

#8 Updated by osukup about 1 year ago

  • Status changed from Workable to In Progress

#9 Updated by tinita about 1 year ago

You are working on this with this PR, right? https://github.com/os-autoinst/os-autoinst/pull/2048

#10 Updated by okurz about 1 year ago

  • Related to action #108266: grenache: script_run() commands randomly time out since server room move added

#11 Updated by okurz about 1 year 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

#12 Updated by cdywan about 1 year ago

#13 Updated by osukup about 1 year ago

PR has now 100% coverage

bonus -> found problem in can_handle and fixed

#14 Updated by cdywan about 1 year ago

  • Blocks action #111254: Cover code of os-autoinst path backend/ fully (statement coverage) size:M added

#15 Updated by osukup about 1 year ago

  • Status changed from In Progress to Feedback

tests merged, because there are also changes in backend itself --> feedback

#16 Updated by osukup about 1 year ago

  • Status changed from Feedback to Resolved

no problems related to changes in backend observer...

Also available in: Atom PDF