Project

General

Profile

action #95170

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 2 months ago. Updated about 1 month ago.

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

0%

Estimated time:
Difficulty:

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

History

#1 Updated by mkittler 2 months ago

  • Assignee set to mkittler

#2 Updated by mkittler 2 months ago

  • Status changed from New to In Progress

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

#3 Updated by openqa_review 2 months ago

  • Due date set to 2021-07-23

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

#4 Updated by mkittler 2 months 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 %.

#5 Updated by okurz 2 months ago

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

#6 Updated by mkittler 2 months 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.

#7 Updated by mkittler 2 months 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).

#8 Updated by mkittler about 2 months 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.

#9 Updated by okurz about 2 months ago

  • Due date deleted (2021-07-23)

#10 Updated by okurz about 1 month ago

  • Tracker changed from coordination to action

Also available in: Atom PDF