tools: Include all touched packages in code coverage

[shahzadlone]

Relevant issue(s)

Resolves #672

Description

Ensures the empty packages aren't left out (because previously packages with 0 tests won't be included in the coverage). This should help avoid seeing the random coverage drops we see in PRs that are actually introducing tests.

Moreover:

• Removes the make rule make test:coverage-quick.
• Change make rule make test:coverage-full to make test:coverage
• Adds the ability to also provide path=... to make test:coverage like we had for make test:coverage-html.
• Account for integration test coverage for the rule make test:coverage-html.

Limitation

Using -coverpkg does not actually help if the package is never called in any test at all. -coverpkg is a cover-up that allows packages from a different package to provide coverage for this package.
So, say package a has no tests, and thus no coverage. But package b has tests, and those tests call into a. As a result, if you start using a -coverpkg that covers both packages, the tests from package b will report its coverage of package a as coverage for a.
However, if we have a package c that is never called by either a or b then that package will still be left out in the dry, even if it would match the -coverpkg given.
Reference: golang/go#24570

Tasks

• I made sure the code is well commented, particularly hard-to-understand areas.
• I made sure the repository-held documentation is changed accordingly.
• I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
• I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Individually tested all changed / updated make rules. CI will also need to pass (specially codecov action).

Specify the platform(s) on which this was tested:

• Manjaro

[orpheuslummis]

Code coverage is a metric we should optimize (to some extent). It’s good to improve our measurement of it.

[shahzadlone]

request: If someone with iOS can test that these two commands for me, it will be bless...
make test:coverage-html path="./api/..."
and
make test:coverage-html

@orpheuslummis @fredcarle

[orpheuslummis]

http Go Coverage Report.html.zip

[orpheuslummis]

first command opens this webpage directly in browser (zipped to be included as attachment here)

[orpheuslummis]

http Go Coverage Report full.html.zip

here is for second command

[fredcarle]

@shahzadlone I'm not sure I can run this on my iPhone 😜

[orpheuslummis]

first command opens this webpage directly in browser (zipped to be included as attachment here)

was this default behavior pre-this PR on MacOS ? On Linux it doesn't do that for me.

Yes it was doing it pre-PR as well

[shahzadlone]

Wow interesting! was not the default behavior on linux before, nor is now. Actually I do like that approach better, will see if can make that the default behavior for linux and mac in both cases.

Open a web browser displaying annotated source code:
        go tool cover -html=c.out

Write out an HTML file instead of launching a web browser:
        go tool cover -html=c.out -o coverage.html

[orpheuslummis]

In Linux, there is the freedesktop standards giving xdg-open that can open a default/configured browser. It is well established, but can't be assumed to be the case. On macOS it can be assumed that open is present.

[shahzadlone]

In Linux, there is the freedesktop standards giving xdg-open that can open a default/configured browser. It is well established, but can't be assumed to be the case. On macOS it can be assumed that open is present.

Yes that is a good point! I see this to be OK the way it is as it is preserving the previous behavior.

[shahzadlone]

@fredcarle can you confirm the error below (from discord) isn't happening in this PR anymore?

make test:coverage-html
gotestsum -- ./... -v -race -shuffle=on -coverprofile=coverage.out
make: gotestsum: No such file or directory

[orpheuslummis]

This PR will affect the codecov bot report positively?

[orpheuslummis]

By itself, the PR LGTM.

[shahzadlone]

This PR will affect the codecov bot report positively?

Not sure what you mean by positively. What this will do is not ignore any packages that don't have tests, which would help avoid the situation of not knowing the "true" coverage. This helps eliminate the random cases where someone who writes a test for a package with no tests, but ends up with an overall lower coverage. Moreover, there is an edge case where a package might still not be included, you can read the Limitation section I added just now in PR description for more info.

[orpheuslummis]

Oops sorry for my unclear wording. I was unsure about whether it would affect the codecov, but I just re-read the Upload coverage to Codecov and that settles my question.

In light of reading your comment and the limitation section, it may be more accurate to name the commit Include all touched packages in code coverage or so? I might be misunderstanding.

[shahzadlone]

Oops sorry for my unclear wording. I was unsure about whether it would affect the codecov, but I just re-read the Upload coverage to Codecov and that settles my question.

In light of reading your comment and the limitation section, it may be more accurate to name the commit Include all touched packages in code coverage or so? I might be misunderstanding.

Done.

[fredcarle]

LGTM 👍