Project

General

Profile

Actions

action #109620

closed

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 almost 2 years ago. Updated almost 2 years ago.

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

0%

Estimated time:

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 4 (2 open2 closed)

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

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

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

Actions
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

Actions
Actions #1

Updated by okurz almost 2 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
Actions #2

Updated by okurz almost 2 years ago

  • Parent task set to #109656
Actions #3

Updated by livdywan almost 2 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
Actions #4

Updated by osukup almost 2 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

Actions #5

Updated by okurz almost 2 years 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

Actions #6

Updated by tinita almost 2 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
Actions #7

Updated by okurz almost 2 years ago

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

Actions #8

Updated by osukup almost 2 years ago

  • Status changed from Workable to In Progress
Actions #9

Updated by tinita almost 2 years ago

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

Actions #10

Updated by okurz almost 2 years ago

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

Updated by okurz almost 2 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
Actions #12

Updated by livdywan almost 2 years ago

Actions #13

Updated by osukup almost 2 years ago

PR has now 100% coverage

bonus -> found problem in can_handle and fixed

Actions #14

Updated by livdywan almost 2 years ago

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

Updated by osukup almost 2 years ago

  • Status changed from In Progress to Feedback

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

Actions #16

Updated by osukup almost 2 years ago

  • Status changed from Feedback to Resolved

no problems related to changes in backend observer...

Actions

Also available in: Atom PDF