🌱 Add a make target to generate test coverage report

[fabriziopandini]

What this PR does / why we need it:
This PR adds a make target to generate test coverage reports

[vincepri]

We've had coverage reports in the past and wasn't really helpful in many ways. Is there a related issue we're trying to fix by adding this command? How are we expected to use it?

I'm not opposed to merging this in, just trying to clarify expectations.

[detiber]

It looks like there might be an upstream bot related to producing coverage data here: https://github.com/kubernetes/test-infra/blob/aa3d9843f6fc0443dbdce78dab126687334afade/robots/coverage/docs/design.md

[cpanato]

I spoke with Ben about the coverage bot and he point me the that knative is using that, so i went in their slack and spoke with them to see what we need to do and got this answer below, before was not clear enough to me how to enable the whole thing

High level steps are:

1. Run Prow job that executes coverage tests into combinedcoverage out (looks like this)
2. Filter the coverage file to remove specific paths
3. Use ./gopherage junit command to build xml
4. Configure testgrid test group to utilize the coverage metric in the xml

the 0 step is adding similar command that Fabrizio did here, I'm going to replicate this to capaz as well :D thanks!

[fabriziopandini]

We've had coverage reports in the past and wasn't really helpful in many ways. Is there a related issue we're trying to fix by adding this command? How are we expected to use it?

@vincepri I agree that having coverage reports and not taking actions on them is a waste of resources.
This is why I added a simple make target that can be triggered on demand when we feel it is necessary to deal with this topic.
In this case my work started after the recent changes in KCP, some comments about implementing tests in follow-up PRs, and the issue that derived from this thread #3309

It looks like there might be an upstream bot related to producing coverage data here

I don't see a real value in having those data generate continuously, but I'm. not opposed on having such a bot generating

[benmoss]

I think something like https://codecov.io/ would be really helpful but people insisted on using prow and it is rather convoluted from my brief attempt to make it work

[cpanato]

I tend to agree with @benmoss of using codecov, less work on the configuration side and easy to plug in

[vincepri]

We've discussed this a while ago, it would be great to use what prow offers by default instead of adding additional external tools

[cpanato]

any plans to move forward with this?

[CecileRobertMichon]

@vincepri @fabriziopandini @benmoss are we okay to move forward with this as-is for now?

Personally I also prefer codecov and have used it successfully in the past but I understand the value of standardizing tooling across the k8s projects. Maybe we can reach out to sig-testing for guidance.

I think it's good incremental value to at least be able to calculate the coverage so +1 from me.

[vincepri]

@CecileRobertMichon No problem from my part merging this as-is, it'd only be used in development. One thing I'd like to avoid is to use code coverage percentages as a target we have to achieve or keep up with. Usually, that has the adverse effect and tests are written to cover statements, rather than to test specific behaviors. If we all agree that the intent is just informational, +1 merging and keep iterating

[benmoss]

Yeah I think was all side-track conversation about @vincepri's comment

We've had coverage reports in the past and wasn't really helpful in many ways. Is there a related issue we're trying to fix by adding this command? How are we expected to use it?

I don't have a strong opinion about this PR, I probably won't ever use it 😄

[CecileRobertMichon]

/retest

[CecileRobertMichon]

/lgtm

[benmoss]

/approve

[ncdc]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benmoss, ncdc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ncdc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment