okurz wrote:
Trying out with make check && (cd t && cover -report html_basic)
the error about tidy seems to have vanished and the coverage seems to be a bit "more complete" as more files are mentioned but I could see two issues:
- Files outside the project are covered as well, e.g. /usr/share/automake-1.15/Automake/ChannelDefs.pm and such
This seems to have been a problem on my side as there was still an old directory t/cover_db/ around where probably I played around with cover manually without any exclude/include rules. Deleting the directory and running tests again collecting coverage the result is reliable and including only files within the os-autoinst working copy.
- Not all test files are included (or none)
That we can change, e.g. with https://github.com/os-autoinst/os-autoinst/pull/1289
So what I tried in https://github.com/os-autoinst/os-autoinst/pull/1289#issuecomment-559862318 is to remove the include use Devel::Cover
from one test and it reduced coverage. I still do not understand why. What I tried then is to add use Devel::Cover
in simply all test files and this further increases coverage so I created
https://github.com/os-autoinst/os-autoinst/pull/1291
EDIT: 2019-11-30: coolo prefers to have test files themselves to be not recorded in code coverage, fine.
So I conducted another two runs locally with tests not included, one with use Devel::Cover
in each file and one without:
rm -rf cover_db/ t/cover_db/ && ./configure && make && make VERBOSE=1 coverage && mv t/cover_db{/,_all_devel_cover_no_tests_included/} && firefox t/cover_db_all_devel_cover_no_tests_included/coverage.html && git co origin/master && rm -rf cover_db/ t/cover_db/ && ./configure && make && make VERBOSE=1 coverage && mv t/cover_db{/,_master_no_tests_included/} && firefox t/cover_db_master_no_tests_included/coverage.html
There are differences in reports but they do not seem to correlate with the change but rather a timing based non-deterministic execution of certain code paths. E.g. in one case I observed that "autotest.pm" was imported 58 times, in the other case 59 times.
I can see that some lines are only covered sometimes, e.g.
autotest.pm:
128 8 50 0 10 unless (blessed($args{run_args}) && $args{run_args}->isa('OpenQA::Test::RunArgs')) {
129 8 16 die 'The run_args must be a sub-class of OpenQA::Test::RunArgs';
130 }
131 0 0 $test->{run_args} = $args{run_args};
132 0 0 delete $args{run_args};
133 }
134
135 429 576 my $nr = '';
136 421 896 while (exists $tests{$fullname . $nr}) {
137 # to all perl hardcore hackers: fuck off!
138 8 50 11 $nr = $nr eq '' ? 1 : $nr + 1;
139 8 23 $test->{name} = join("#", $name, $nr);
The lines 128-129 and 138-139 were not executed in the other case. Probably we can look for the test that should check the loading of the same test module multiple times as it seems to be not fully deterministic.
Another test again where I have use Devel::Cover
removed from 07-commands.t:
sudo rm -rf cover_db/ t/cover_db/ && ./configure && make && make VERBOSE=1 coverage && mv t/cover_db{/,_devel_cover_use_removed/} && firefox t/cover_db_devel_cover_use_removed/coverage.html
The overall coverage reduced from 55.0+-0.1 to 53.3. Could it be that some subprocesses do not get covered by Devel::Cover unless use Devel::Cover
is explicitly stated somewhere? Especially in 07-commands.t where we spawn a separate server instance. By the way, the original PR explained all these problems already in https://github.com/os-autoinst/os-autoinst/pull/987 , seems mkittler has forgot that ;) Seems also https://stackoverflow.com/questions/56856646/how-do-i-collect-coverage-from-child-processes-when-running-cover-test-and-n tries to answer that question and the best they could come up with it seems is:
my $is_covering = !!(eval 'Devel::Cover::get_coverage()');
my @perl = ($^X, $is_covering ? ('-MDevel::Cover=-silent,1') : ());
diag $is_covering ? 'Devel::Cover running' : 'Devel::Cover not covering';
# Pass the Devel::Cover option, if any, to the child process
system(@perl, 'prt', ...);
to start processes with coverage. Now also perlpunk started some experiments in https://github.com/os-autoinst/os-autoinst/pull/1295 so I will wait for this
EDIT: 2019-12-01: https://github.com/os-autoinst/os-autoinst/pull/1297 created to explain better why we have "use Devel::Cover" in t/07-commands.t but nowhere else. I closed https://github.com/os-autoinst/os-autoinst/pull/1292