Ensure full test coverage of test code

* Remove dead code * Avoid skipping tests * Mark lines only executed in case tests are failing or in case tests are executed with verbose logging as uncoverable * Mark lines executed in sub processes where coverage cannot be collected as uncoverable * Fix attempts to mark lines as uncoverable * See https://progress.opensuse.org/issues/165396
os-autoinst · Aug 26, 2024 · 8d7b9a5 · 8d7b9a5
1 parent 78735e1
commit 8d7b9a5
Show file tree

Hide file tree

Showing 19 changed files with 98 additions and 181 deletions.
diff --git a/codecov.yml b/codecov.yml
@@ -13,7 +13,7 @@ coverage:
       default:
         target: 98
       tests:
-        target: 99.6
+        target: 100.0
         paths:
           - t/
     patch:

diff --git a/t/05-scheduler-full.t b/t/05-scheduler-full.t
@@ -30,7 +30,7 @@ use OpenQA::Worker::WebUIConnection;
 use OpenQA::Utils;
 require OpenQA::Test::Database;
 use OpenQA::Test::Utils qw(
-  mock_service_ports setup_mojo_app_with_default_worker_timeout
+  setup_mojo_app_with_default_worker_timeout
   setup_fullstack_temp_dir create_user_for_workers
   create_webapi setup_share_dir create_websocket_server
   stop_service unstable_worker
@@ -55,9 +55,8 @@ my $api_key = $api_credentials->key;
 my $api_secret = $api_credentials->secret;
 
 # create web UI and websocket server
-mock_service_ports;
 my $mojoport = service_port 'webui';
-my $ws = create_websocket_server(undef, 0, 1, 1);
+my $ws = create_websocket_server(undef, 0, 1);
 my $webapi = create_webapi($mojoport, sub { });
 my @workers;
 
@@ -272,7 +271,7 @@ subtest 'Websocket server - close connection test' => sub {
 
     my $log;
     # create unstable ws
-    $ws = create_websocket_server(undef, 1, 0);
+    $ws = create_websocket_server(undef, 1);
     @workers = create_worker(@$worker_settings, 2, \$log);
 
     my $found_connection_closed_in_log = 0;

diff --git a/t/05-scheduler-restart-and-duplicate.t b/t/05-scheduler-restart-and-duplicate.t
@@ -265,9 +265,9 @@ $job3 = job_get($round5->id);
 
 sub _print_job_cluster ($jobs) {
     return undef unless $ENV{HARNESS_IS_VERBOSE};
-    my $cluster_jobs = $jobs->[0]->cluster_jobs;    # uncoverable
-    note 'job ' . $_->TEST . ': ' . $_->id for @$jobs;    # uncoverable
-    diag explain $cluster_jobs;    # uncoverable
+    my $cluster_jobs = $jobs->[0]->cluster_jobs;    # uncoverable statement
+    note 'job ' . $_->TEST . ': ' . $_->id for @$jobs;    # uncoverable statement
+    diag explain $cluster_jobs;    # uncoverable statement
 }
 
 subtest 'restarting one of two independent root jobs (only related indirectly via parallel dependency)' => sub {

diff --git a/t/10-jobs.t b/t/10-jobs.t
@@ -23,7 +23,7 @@ use Test::Mojo;
 use Test::Warnings qw(:report_warnings warning);
 use Mojo::File 'path';
 use Mojo::JSON qw(decode_json encode_json);
-use OpenQA::Test::Utils qw(perform_minion_jobs redirect_output);
+use OpenQA::Test::Utils qw(perform_minion_jobs);
 use OpenQA::Test::TimeLimit '30';
 
 my $schema_name = OpenQA::Test::Database::generate_schema_name;

diff --git a/t/16-utils.t b/t/16-utils.t
@@ -22,7 +22,6 @@ use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib";
 use OpenQA::Utils (qw(:DEFAULT prjdir sharedir resultdir assetdir imagesdir base_host random_string random_hex),
     qw(download_rate download_speed usleep_backoff));
 use OpenQA::Task::SignalGuard;
-use OpenQA::Test::Utils 'redirect_output';
 use OpenQA::Test::TimeLimit '10';
 use Scalar::Util 'reftype';
 use Test::MockObject;

diff --git a/t/20-stale-job-detection.t b/t/20-stale-job-detection.t
@@ -10,12 +10,13 @@ use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib";
 use DateTime;
 use Test::Warnings ':report_warnings';
 use Test::Output qw(combined_like stderr_like);
+use Test::MockModule;
 use OpenQA::App;
 use OpenQA::Constants qw(DEFAULT_WORKER_TIMEOUT DB_TIMESTAMP_ACCURACY);
 use OpenQA::Jobs::Constants;
 use OpenQA::WebSockets;
+use OpenQA::Scheduler;
 require OpenQA::Test::Database;
-use OpenQA::Test::Utils qw(redirect_output);
 use OpenQA::Test::TimeLimit '10';
 
 my $schema = OpenQA::Test::Database->new->create(fixtures_glob => '01-jobs.pl 02-workers.pl 06-job_dependencies.pl');

diff --git a/t/26-controllerrunning.t b/t/26-controllerrunning.t
@@ -280,4 +280,3 @@ sub resultset { FakeSchema::Find->new }
 
 package FakeSchema::Find;
 sub new { bless({}, shift) }
-sub find { undef }
diff --git a/t/33-developer_mode.t b/t/33-developer_mode.t
@@ -76,7 +76,7 @@ ok(Mojolicious::Commands->start_app('OpenQA::WebAPI', 'eval', '1+0'));
 # start Selenium test driver and other daemons
 my $port = service_port 'webui';
 my $driver = call_driver({mojoport => $port});
-$ws = create_websocket_server(undef, 0, 0);
+$ws = create_websocket_server(undef, 0);
 $scheduler = create_scheduler;
 $livehandler = create_live_view_handler;
 

diff --git a/t/43-scheduling-and-worker-scalability.t b/t/43-scheduling-and-worker-scalability.t
@@ -21,7 +21,7 @@ require OpenQA::Test::Database;
 use OpenQA::Jobs::Constants;
 use OpenQA::Log qw(setup_log);
 use OpenQA::Test::Utils qw(
-  mock_service_ports setup_mojo_app_with_default_worker_timeout
+  setup_mojo_app_with_default_worker_timeout
   create_user_for_workers create_webapi create_websocket_server
   stop_service setup_fullstack_temp_dir simulate_load);
 use OpenQA::Test::TimeLimit '20';
@@ -52,16 +52,14 @@ BAIL_OUT 'invalid SCALABILITY_TEST_WORKER_COUNT/SCALABILITY_TEST_JOB_COUNT'
 note("Running scalability test with $worker_count worker(s) and $job_count job(s).");
 note('Set SCALABILITY_TEST_WORKER_COUNT/SCALABILITY_TEST_JOB_COUNT to adjust this.');
 
-mock_service_ports;
-
 # setup basedir, config dir and database
 my $tempdir = setup_fullstack_temp_dir('scalability');
 my $schema = OpenQA::Test::Database->new->create;
 my $workers = $schema->resultset('Workers');
 my $jobs = $schema->resultset('Jobs');
 
 # create web UI and websocket server
-my $web_socket_server = create_websocket_server(undef, 0, 1, 1, 1);
+my $web_socket_server = create_websocket_server(undef, 0, 1, 1);
 my $webui = create_webapi(undef, 1);
 
 # prepare spawning workers

diff --git a/t/api/02-assets.t b/t/api/02-assets.t
@@ -18,11 +18,9 @@ my $t = client(Test::Mojo->new('OpenQA::WebAPI'), apikey => 'ARTHURKEY01', apise
 
 sub la {
     return unless $ENV{HARNESS_IS_VERBOSE};
-    $t->get_ok('/api/v1/assets')->status_is(200);
-    my @assets = @{$t->tx->res->json->{assets}};
-    for my $asset (@assets) {
-        printf "%d %-5s %s\n", $asset->{id}, $asset->{type}, $asset->{name};
-    }
+    $t->get_ok('/api/v1/assets')->status_is(200);    # uncoverable statement
+    my @assets = @{$t->tx->res->json->{assets}};    # uncoverable statement
+    note "asset: $_->{id}: $_->{type}/$_->{name}\n" for @assets;    # uncoverable statement
 }
 
 sub iso_path {

diff --git a/t/api/08-jobtemplates.t b/t/api/08-jobtemplates.t
@@ -955,11 +955,9 @@ subtest 'Create and modify groups with YAML' => sub {
         $t->post_ok("/api/v1/job_templates_scheduling/$job_group_id3", form => \%form);
         $t->status_is(200);
         return diag explain $t->tx->res->body unless $t->success;
-        if (!is($job_templates->search({prio => 16})->count, 2, 'two distinct job templates')) {
-            my $jt = $job_templates->search({prio => 16});
-            while (my $j = $jt->next) {
-                diag explain dump_yaml(string => $j->to_hash);
-            }
+        my $templates = $job_templates->search({prio => 16});
+        if (!is $templates->count, 2, 'two distinct job templates') {
+            diag explain dump_yaml(string => $_->to_hash) for $templates->all;    # uncoverable statement
         }
         my %new_isos_post_params = (
             _GROUP => 'foo',
@@ -1010,11 +1008,9 @@ subtest 'Create and modify groups with YAML' => sub {
                 schema => $schema_filename,
                 template => dump_yaml(string => $yaml)});
         return diag explain $t->tx->res->body unless $t->success;
-        if (!is($job_templates->search({prio => 7})->count, 4, 'four job templates created')) {
-            my $jt = $job_templates->search({prio => 7});
-            while (my $j = $jt->next) {
-                diag explain dump_yaml(string => $j->to_hash);
-            }
+        my $templates = $job_templates->search({prio => 7});
+        if (!is $templates->count, 4, 'four job templates created') {
+            diag explain dump_yaml(string => $_->to_hash) for $templates->all;    # uncoverable statement
         }
     };
 

diff --git a/t/api/14-plugin_obs_rsync_async.t b/t/api/14-plugin_obs_rsync_async.t
@@ -18,11 +18,11 @@ $t->ua(OpenQA::Client->new(apikey => 'ARTHURKEY01', apisecret => 'EXCALIBUR')->i
 $t->app($app);
 
 sub start_gru {
-    start sub {
-        note('starting gru');
-        $0 = 'openqa-gru';
-        $ENV{MOJO_MODE} = 'test';
-        Mojolicious::Commands->start_app('OpenQA::WebAPI', 'gru', 'run', '-m', 'test');
+    start sub {    # uncoverable because we do not track coverage of this sub process
+        note('starting gru');    # uncoverable statement
+        $0 = 'openqa-gru';    # uncoverable statement
+        $ENV{MOJO_MODE} = 'test';    # uncoverable statement
+        Mojolicious::Commands->start_app('OpenQA::WebAPI', 'gru', 'run', '-m', 'test');    # uncoverable statement
     };
 }
 
@@ -52,11 +52,10 @@ sub sleep_until_job_start {
                 && ($other_job->{args}[0]->{project} eq $project)
                 && $other_job->{notes}{project_lock});
         }
-
-        sleep .2;
-        $retries--;
+        sleep .2;    # uncoverable statement
+        $retries--;    # uncoverable statement
     }
-    die 'Timeout reached';
+    die 'Timeout reached';    # uncoverable statement
 }
 
 sub sleep_until_all_jobs_finished {
@@ -66,11 +65,10 @@ sub sleep_until_all_jobs_finished {
     while ($retries > 0) {
         my ($cnt, $jobs) = _jobs('inactive', 'active');
         return 1 unless $cnt;
-
-        sleep .2;
-        $retries--;
+        sleep .2;    # uncoverable statement
+        $retries--;    # uncoverable statement
     }
-    die 'Timeout reached';
+    die 'Timeout reached';    # uncoverable statement
 }
 
 # this function communicates with t/data/openqa-trigger-from-obs/script/rsync.sh

diff --git a/t/full-stack.t b/t/full-stack.t
@@ -48,7 +48,7 @@ use File::Path qw(make_path remove_tree);
 use Module::Load::Conditional 'can_load';
 use OpenQA::Test::Utils
   qw(create_websocket_server create_live_view_handler setup_share_dir),
-  qw(cache_minion_worker cache_worker_service mock_service_ports setup_fullstack_temp_dir),
+  qw(cache_minion_worker cache_worker_service setup_fullstack_temp_dir),
   qw(start_worker stop_service wait_for_or_bail_out);
 use OpenQA::Test::TimeLimit '200';
 use OpenQA::Test::FullstackUtils;
@@ -73,9 +73,8 @@ my $sharedir = setup_share_dir($ENV{OPENQA_BASEDIR});
 # initialize database, start daemons
 my $schema = OpenQA::Test::Database->new->create(schema_name => 'public', drop_schema => 1);
 ok(Mojolicious::Commands->start_app('OpenQA::WebAPI', 'eval', '1+0'), 'assets are prefetched');
-mock_service_ports;
 my $mojoport = service_port 'websocket';
-$ws = create_websocket_server($mojoport, 0, 0);
+$ws = create_websocket_server($mojoport, 0);
 my $driver = call_driver({mojoport => service_port 'webui'});
 $livehandler = create_live_view_handler;
 

diff --git a/t/lib/OpenQA/SeleniumTest.pm b/t/lib/OpenQA/SeleniumTest.pm
@@ -215,7 +215,7 @@ sub javascript_console_has_no_warnings_or_errors ($test_name_suffix = '') {
         # ignore when server replied with 400 response; this may be provoked when testing error cases and if it is
         # not expected tests would fail anyways
         next if ($msg =~ qr/server responded with a status of 400/);    # uncoverable statement
-        push(@errors, $log_entry);
+        push(@errors, $log_entry);    # uncoverable statement
     }
 
     diag "Unexpected Javascript console errors$test_name_suffix: " . pp(\@errors) if @errors;

diff --git a/t/lib/OpenQA/Test/FakeWebSocketTransaction.pm b/t/lib/OpenQA/Test/FakeWebSocketTransaction.pm
@@ -29,8 +29,8 @@ sub send {
     my ($self, $message, $callback) = @_;
 
     if ($self->finish_called) {
-        fail('attempt to send message via finished connection');
-        return undef;
+        fail('attempt to send message via finished connection');    # uncoverable statement
+        return undef;    # uncoverable statement
     }
 
     push @{$self->sent_messages}, $message;