Fixes #1058: Track system-tests coverage in Codecov.io

[jiridanek]

https://app.codecov.io/github/skupperproject/skupper-router/pull/1048

Doing code coverage on system_tests is cheating. It runs a lot of code without asserting anything about the vast majority of it. So, counting all the code that was run as "covered by tests" is untrue.

The better way to measure system tests coverage is not by counting source code lines, but by counting covered user-facing features and scenarios.

@ganeshmurthy lets see what this does; I have great expectations, but maybe I'm imagining it wrong, what this is going to do

[codecov]

Codecov Report

Merging #1048 (84fb44b) into main (0f4e165) will increase coverage by 47.23%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1048       +/-   ##
===========================================
+ Coverage   30.85%   78.09%   +47.23%     
===========================================
  Files         174      236       +62     
  Lines       38052    60498    +22446     
  Branches     5331     5632      +301     
===========================================
+ Hits        11741    47244    +35503     
+ Misses      25155    10649    -14506     
- Partials     1156     2605     +1449     

Flag	Coverage Δ
pysystemtests	87.21% <ø> (?)
pyunittests	54.50% <ø> (ø)
systemtests	72.03% <ø> (?)
unittests	27.37% <ø> (-3.49%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
calculator	30.97% <ø> (+0.11%)	⬆️
systemtests	78.68% <ø> (∅)

[jiridanek]

Looks like we can't collect python coverage while running tests in parallel, https://github.com/jiridanek/skupper-router/actions/runs/4609453728/jobs/8146605198#step:34:498

• Race conditions when using 'tox --parallel' nedbat/coveragepy#1514

Also, we need to decide what to do with regards to coverage if tests fail, since CentOS tests currently rarely pass. There is some setting in codecov about this

codecov:
  require_ci_to_pass: true

and also the uploading github action step gets skipped if the previous step failed, so that codecov setting does not even get to play a role, currently

what you see in https://app.codecov.io/github/skupperproject/skupper-router/commit/b2095422f08b6d4e5b564e662a819a97a3a69210/tree is coverage from the even-numbered systemtests, because only shard 2 passed.

@ganeshmurthy thoughts?

[jiridanek]

I expected some way in the codecov.io web ui to be able to show lines covered by tests uploaded under a particular flag

but, that does not seem to be the case, and all are lumped under https://app.codecov.io/github/skupperproject/skupper-router/commit/b2095422f08b6d4e5b564e662a819a97a3a69210/tree

It appears that I can filter out the flags in yaml, https://docs.codecov.com/docs/flags#hide-builds-eg-nightly-builds

flags:
  nightly:
    joined: false

[jiridanek]

Actually the coverage.py problem is something different, it is nedbat/coveragepy#883 (comment) and it is self-inflicted through the ctest -j.

[jiridanek]

Hmm, looks like I won't be able to guess it. Let me try locally, hopefully I can put together something that works to report python coverage reliably.

Still, I don't want to indiscriminately squash systemtests and unittest coverage when showing the line-per-line coverage in codecov.io web ui...

[jiridanek]

I wrote to Codecov forum about the displaying of covered lines per flag, https://community.codecov.com/t/can-i-see-line-by-line-coverage-in-file-explorer-uploaded-from-a-specific-flag-or-a-subset-of-flags/4251

Edit: and again, reported it to Feedback page https://app.codecov.io/gh/feedback/p/feature-request-allow-selecting-subset-of-flags-to-display-coverage-from-in-the

[jiridanek]

@ganeshmurthy ok, this should be it. first ci run failed on network issue uploading to codecov.io, so I'm rerunning it again now, so that we see the results

[jiridanek]

Now the only coverage missing is what the embedded python interpreter runs from inside skrouterd. I don't consider resolving that in scope of this PR.

[ganeshmurthy]

Let's get this merged. Thanks for doing this.

[ganeshmurthy]

Can you please add an issue for this and change the commit message to point to the new issue ? Thx

[jiridanek]

Can you please add an issue for this and change the commit message to point to the new issue ? Thx

I'll do that, but in general, it seems fairly superfluous thing to do on GitHub, where PRs and Issues work very similarly and you can link PRs to releases, same as (or instead of) the issues.

For example, what's the point of having both #1053 and #1054 when you can link the PR to release and be done with it.

(On the other hand, issue like #1055 is helpful because it explains the background for the change. But, that can be just as well explained right in the PR comment #1056)

What does make sense, IMO, is to create issue when you don't want to immediately create the PR. But otherwise, just the PR suffices.