Project

General

Profile

Actions

action #95170

closed

coordination #93883: [epic] Speedup openQA coverage tests with running minion jobs synchronously using new upstream "perform_jobs_in_foreground" mojo function

Increase code coverage of critical component OpenQA::Worker::Job without introducing slow-down due to subprocess coverage collection

Added by okurz almost 3 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Feature requests
Target version:
Start date:
2021-07-07
Due date:
% Done:

0%

Estimated time:

Description

Motivation

https://codecov.io/gh/os-autoinst/openQA/src/master/lib/OpenQA/Worker/Job.pm currently reports a statement coverage of 77% which is below our average. Similar as in the parent #93883 we are using subprocesses with Mojo::IOLoop suffering from the same problem as minion jobs: Collecting coverage in system-level tests is hard and if we ask Devel::Cover to collect coverage then it can be quite slow. Better if we have explicit unit-level testing for code intended to run in subprocesses and/or run subprocesses as synchronous foreground code.

Acceptance criteria

Suggestions

  • Run make coverage KEEP_DB=1 TESTS=t/24-worker-jobs.t locally
  • Extend the current test coverage before applying any changes
  • Refactor to simplify OpenQA::Worker::Job
  • Review the design regarding the use of subprocesses
Actions #1

Updated by mkittler almost 3 years ago

  • Assignee set to mkittler
Actions #2

Updated by mkittler almost 3 years ago

  • Status changed from New to In Progress

PR for collecting coverage of image upload code: https://github.com/os-autoinst/openQA/pull/4026

Actions #3

Updated by openqa_review almost 3 years ago

  • Due date set to 2021-07-23

Setting due date based on mean cycle time of SUSE QE Tools

Actions #4

Updated by mkittler almost 3 years ago

  • Status changed from In Progress to Feedback

The PR has been merged and has increased the coverage by 8.66 % (absolute). The file's coverage is now at 85.92 %.

Actions #5

Updated by okurz almost 3 years ago

good improvement. Do you want to use the opportunity and try to reach 100% statement coverage including considering the use of # uncoverable statement?

Actions #6

Updated by mkittler almost 3 years ago

I can try. Here's another PR for a slight improvement: https://github.com/os-autoinst/openQA/pull/4040 - This leaves only the functions _upload_asset and _upload_log_file (and _log_upload_error which is used by them) completely uncovered.

EDIT: _upload_asset is now covered as well.

Actions #7

Updated by mkittler almost 3 years ago

PR for _upload_log_file: https://github.com/os-autoinst/openQA/pull/4046

With that the file's coverage is now at 99.55 % which is actually quite good. The few remaining lines would be possible to cover as well but of course the effort/benefit ratio is much worse (as they're spread over different functions/branches).

Actions #8

Updated by mkittler over 2 years ago

  • Status changed from Feedback to Resolved

The PR has been merged. I'll leave it at 99.55 %. I wouldn't mark the remaining lines as uncoverable because they should all be coverable. It is just some effort.

Actions #9

Updated by okurz over 2 years ago

  • Due date deleted (2021-07-23)
Actions #10

Updated by okurz over 2 years ago

  • Tracker changed from coordination to action
Actions

Also available in: Atom PDF