Codecov rules

[mikekryjak]

We recently put Codecov on the repo. As we all know, it has limitations, and this is part of the reason why it was taken off the BOUT++ repo.
We agreed to give it another go and find a balanced, practical way to use it for Hermes-3 because it needs more unit test coverage. It should help us achieve this without being a hindrance to the point where we continuously override it, like what happened last time.

Recently, two Codecov annoyances popped up:

• Test coverage reduced by 0.2% because we disabled some warnings alongside the tests of the system that prepares the warnings: (Suppress permission warnings #468)
• Test coverage reduced because we introduced an integrated MMS test which itself wasn't unit tested: Nonorthogonal with minimal differential operators MMS test. #285

@cmacmackin and I thought that we could write up some rules that would allow us to override codecov where:

• Test coverage is reduced because code is removed
• Test coverage is reduced because another test is added

What do you think? @bendudson @dschwoerer @ZedThree

[mikekryjak]

A third annoyance is that formatting changes are also picked up as changes that need test coverage:
#478

This is not reasonable - Codecov should only request tests for functional code changes, not formatting.

To avoid this one I think we need to set up clang on CI. This is long overdue anyway.
#360

[d7919]

Can one turn coverage decrease from an error (failed action) to a warning? It is then providing useful information to the people doing the code review without being a hard fail. There are many different ways in which coverage can change and it is probably easiest to just expect the reviewers to review the context rather than trying to algorithmically handle these possibilities. Some written reviewer guidance would be very useful in this case though.

[cmacmackin]

I would suggest that we continue to treat it as an error, as it should be viewed as blocking in most PRs. If there is a valid reason why code coverage falls then this can be assessed by a repository owner, who has the permissions to override the error and merge anyway.

[mikekryjak]

In the developer meeting there was consensus that we should keep it as an error, but introduce contribution guidelines that allow Codecov to be overridden in the case of code being removed and code being formatted. The case of adding tests reducing coverage is resolved by excluding the tests directory from Codecov coverage, which is already set up - the failures in neutral_mixed were due to extra functionality added to the component to enable an MMS test, not due to the test code itself.