Project

General

Profile

Actions
Copy link

action #167926

closed

coordination #127031: [saga][epic] openQA for SUSE customers

coordination #130414: [epic] Improved code coverage in os-autoinst

Cover code of os-autoinst path autotest.pm fully (statement coverage) size:S

Added by okurz 4 months ago. Updated 29 days ago.

Status:
Resolved
Priority:
Low
Assignee:
gpathak
Category:
Feature requests
Target version:
Ready
Start date:
2024-10-08
Due date:
% Done:

0%

Estimated time:

Description

Acceptance criteria

  • AC1: the path autotest.pm is listed in codecov.yml under "fully_covered"

Suggestions

Files

coverage.html (8.23 KB) coverage.html gpathak, 2024-12-17 14:47

Related issues 3 (1 open — 2 closed)

Copied from openQA Project (public) - action #167923: Cover code of os-autoinst path script/ fully (statement coverage)Blockedokurz2024-10-08

Actions
Copied to openQA Project (public) - action #167929: Cover code of os-autoinst path testapi.pm fully (statement coverage) size:MResolvedmkittler2024-10-08

Actions
Copied to openQA Project (public) - action #175099: autotest::loadtestdir() creates a warning in find_script() and does not add necessary library path size:SResolvedgpuliti2025-01-08

Actions
Actions
Copy link
 #1

Updated by okurz 4 months ago

  • Copied from action #167923: Cover code of os-autoinst path script/ fully (statement coverage) added
Actions
Copy link
 #2

Updated by okurz 4 months ago

  • Copied to action #167929: Cover code of os-autoinst path testapi.pm fully (statement coverage) size:M added
Actions
Copy link
 #3

Updated by okurz 4 months ago

  • Parent task changed from #167917 to #130414
Actions
Copy link
 #4

Updated by okurz 2 months ago

  • Subject changed from Cover code of os-autoinst path autotest.pm fully (statement coverage) size:M to Cover code of os-autoinst path autotest.pm fully (statement coverage)
Actions
Copy link
 #5

Updated by okurz 2 months ago

  • Target version changed from future to Ready
Actions
Copy link
 #6

Updated by okurz about 2 months ago

  • Subject changed from Cover code of os-autoinst path autotest.pm fully (statement coverage) to Cover code of os-autoinst path autotest.pm fully (statement coverage) size:S
  • Status changed from New to Workable
Actions
Copy link
 #7

Updated by gpathak about 2 months ago

  • Status changed from Workable to In Progress
  • Assignee set to gpathak
Actions
Copy link
 #9

Updated by gpathak about 2 months ago

Attached code coverage generated html file.
It's just, I am unable to understand how to test subrouties calling _exit() at the end.
@okurz @mkittler Please review the attached code coverage. If it looks okay, I will then mark the Pull request ready for merging.

Actions
Copy link
 #10

Updated by gpathak about 2 months ago

  • Status changed from In Progress to Resolved

Pull request merged.

Actions
Copy link
 #11

Updated by okurz about 2 months ago

  • Status changed from Resolved to In Progress

nope. Just because a PR is merged doesn't mean that the goal is reached. Please check again.

Actions
Copy link
 #12

Updated by livdywan about 2 months ago

Indeed. There's uncovered bits visible on Codecov.

Actions
Copy link
 #13

Updated by gpathak about 2 months ago

gpathak wrote in #note-9:

Attached code coverage generated html file.
It's just, I am unable to understand how to test subrouties calling _exit() at the end.
@okurz @mkittler Please review the attached code coverage. If it looks okay, I will then mark the Pull request ready for merging.

I couldn't find a way to catch _exit() in unit tests. I will try to find any existing test implementation.

Actions
Copy link
 #14

Updated by gpathak about 2 months ago

  • Status changed from In Progress to Feedback
  • Assignee changed from gpathak to okurz
Actions
Copy link
 #15

Updated by gpathak about 2 months ago

Assigning it back to myself because AC is not yet achieved.

Actions
Copy link
 #16

Updated by gpathak about 2 months ago

  • Status changed from Feedback to In Progress
  • Assignee changed from okurz to gpathak
Actions
Copy link
 #17

Updated by gpathak about 1 month ago

Created another PR: https://github.com/os-autoinst/os-autoinst/pull/2601 for autotest.pm

Actions
Copy link
 #18

Updated by okurz about 1 month ago

  • Status changed from In Progress to Workable
Actions
Copy link
 #19

Updated by gpathak about 1 month ago

PR merged. Closing the ticket.

Actions
Copy link
 #20

Updated by gpathak about 1 month ago

  • Status changed from Workable to Resolved
Actions
Copy link
 #21

Updated by tinita about 1 month ago

  • Status changed from Resolved to Feedback

I added a comment to https://github.com/os-autoinst/os-autoinst/pull/2601#discussion_r1904029149
loadtestdir doesn't seem to be used anywhere, and the way it is written produces warnings and makes it necessary to use use lib, where loadtest is actually supposed to do that itself.
I suggest to remove loadtestdir or fix it.

Actions
Copy link
 #22

Updated by livdywan 29 days ago

tinita wrote in #note-21:

I added a comment to https://github.com/os-autoinst/os-autoinst/pull/2601#discussion_r1904029149
loadtestdir doesn't seem to be used anywhere, and the way it is written produces warnings and makes it necessary to use use lib, where loadtest is actually supposed to do that itself.
I suggest to remove loadtestdir or fix it.

At least one of the search results looks like a proper use of it, which is pop! os using it, see pop-os/os-autoinst-distri-pop.

Actions
Copy link
 #23

Updated by gpathak 29 days ago

@tinita @livdywan
Modifications to loadtestdir() is confusing and bit cumbersome. I am requesting to create another ticket for improving loadtestdir() and respective unit test code coverage.

Actions
Copy link
 #24

Updated by tinita 29 days ago

  • Copied to action #175099: autotest::loadtestdir() creates a warning in find_script() and does not add necessary library path size:S added
Actions
Copy link
 #25

Updated by tinita 29 days ago

  • Status changed from Feedback to Resolved

Created followup #175099

Actions
Copy link

Also available in: Atom PDF