Basic code coverage feedback

[MaximilianAlgehed]

I was playing around with HPC to get some idea of the state of the tests and I thought it might be worth a shot to get the feedback in PRs mostly as an exercise. More importantly, we should do something about some of the low numbers here.

[TimSheard]

This PR sets up the creation of code coverage measurements.
Not being an expert, I cannot comment on whether this will have any adverse consequences, unless one figures increased build times?

[MaximilianAlgehed]

@TimSheard we aren't changing any requirements, so we aren't forcing coverage metrics on anyone (which is probably wise). What we are trying to achieve here is mostly getting a grip on "how good are our tests at exercising the code" and the main take-away is that we are probably missing some stuff (big or small?) and we should look into it.

The next thing to do would be to compare coverage on master and the PR branch in the comment, that would give us a better idea of how a PR may or may not change coverage.

[github-actions]

Coverage report

• 73% expressions used (14892/20176)
• 52% boolean coverage (179/340)
  • 50% guards (140/277), 98 always True, 19 always False, 20 unevaluated
  • 53% 'if' conditions (23/43), 6 always True, 4 always False, 10 unevaluated
  • 80% qualifiers (16/20), 2 always True, 2 unevaluated
• 76% alternatives used (1360/1783)
• 91% local declarations used (225/246)
• 58% top-level declarations used (1214/2092)

On master

Coverage report

• 73% expressions used (14853/20176)
• 53% boolean coverage (181/340)
  • 50% guards (141/277), 96 always True, 16 always False, 24 unevaluated
  • 55% 'if' conditions (24/43), 5 always True, 4 always False, 10 unevaluated
  • 80% qualifiers (16/20), 2 always True, 2 unevaluated
• 76% alternatives used (1356/1783)
• 90% local declarations used (223/246)
• 57% top-level declarations used (1213/2092)

[Soupstraw]

LGTM

[neilmayhew]

I love having the coverage in a PR comment!

[neilmayhew]

FYI, in ledger we prefer to have all merges in master be "simple", ie they could have been an ff-only merge, and we have a CI action that ensures this. That means we rebase PR branches rather than merge master into them. However, I don't know whether we want to follow that convention here, and of course it's fine to do whatever's convenient during development.

[MaximilianAlgehed]

@neilmayhew would you mind doing another review here?

[MaximilianAlgehed]

@neilmayhew if you'd like to have a look now and hopefully press the green button that'd be great.

I think I've addressed all your concerns. I've also taken the libery to remove the cabal haddock part of this job as doing haddocks for every ghc version in every build is not just wasteful and fails to catch any actual problems, it's also messing up the caching.

There is an open issue for proper haddoc CI that I will get to eventually.

[neilmayhew]

Looks great now!

Hopefully this can be used as a reference for adding coverage to other repos' CI

[neilmayhew]

Unfortunately, there are 5 checks that are never going to complete because GitHub has become confused by the change of job name. I've enabled auto-merge, but I think it's going to need someone with admin rights (@lehins) to force the merge.

[MaximilianAlgehed]

Unfortunately, there are 5 checks that are never going to complete because GitHub has become confused by the change of job name. I've enabled auto-merge, but I think it's going to need someone with admin rights (@lehins) to force the merge.

I can open a new PR instead.

[MaximilianAlgehed]

Unfortunately, there are 5 checks that are never going to complete because GitHub has become confused by the change of job name. I've enabled auto-merge, but I think it's going to need someone with admin rights (@lehins) to force the merge.

I can open a new PR instead.

[lehins]

Sorry @MaximilianAlgehed
But there are much better ways to deal with coverage on Haskell projects.
This is an overly complex solution with an outcome that is far inferior to other solutions.

Here is an example of coverage report for a Haskell project named cuddle that we use in ledger as well: https://coveralls.io/github/input-output-hk/cuddle

If you'd like to set it up in the same way, I'd be in favor.
Or I can ask one of our team mates to do this instead if you'd prefer. Here is how it is configured: https://github.com/input-output-hk/cuddle/blob/7fe3abb67a4cf28368d45b40576572ca667dd66a/.github/workflows/ci.yml#L129-L141

[MaximilianAlgehed]

@lehins I'd be super grateful if someone could set up something like this! Maybe better to ask someone whose done it before and has the correct privileges. It will help in doing exactly what I was hoping for here.

I'll admit that this PR spun out of me just using hpc locally to get an idea of the state of things - it should have crossed my mind that IOG has an existing solution for this.

I was planning to take the output here and make it much prettier, but better to use an existing solution for that. Not much work lost and a bunch saved!