Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-12 Thread Elliot West
Hello devs, I think that at a minimum we should be able to detect coverage regression and generate a visible warning on a PR. Specific statistics in the PR conversation may be less useful, and potentially noisy. However, I'd suggest also including some developer documentation that describes how to

Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-10 Thread PengHui Li
Hi Lin, Thanks for starting this discussion. First, I support having code coverage repo for the PR which will provide the initial impressions of the test coverage to reviewers. Although the code coverage report can produce distorted results, the report is not the supreme indicator, just an auxili

Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-10 Thread Lin Zhao
Hi Xiangying, I totally agree that when new feature or major optimizations are submitted, it should be in the owner's interest to care about code coverage for their new feature. The test cases are protection of their code from unintended side effects from other people's work. In the case the PR i

Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-09 Thread Xiangying Meng
Hi, IMO, there are two scenarios when users need test coverage: 1. When new features or major optimizations are submitted, the owners of the PR may care about the code coverage of these new features. 2. When we need to optimize the original code to increase the overall code test coverage of Pulsar.

Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-09 Thread asn
Hi, IMO, code coverage is not just a number, 50% or 70% makes no sense except to let us feel that we have more confidence. So what is really important? I think it is the *coverage* itself, we need to see where we need to write tests in the future based the result because only if we have this data,

Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-09 Thread tison
> Please provide data point its impact to CI stability. If you take a look at https://github.com/apache/pulsar/pull/17382, it changes pulsar-ci.yml to run commands for generating the codecov report. I'm unsure of the impact of a new step, it may not affect too much since there's already a runner t

Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-09 Thread Lin Zhao
Hi Tison, Thanks for your input. I agree that the community should focus on the priorities regarding CI that you mentioned. At the same time, I'm having a hard time understanding the negative impact that you suggested from this change. 1. To my knowledge code coverage calculation adds little over

Re: [DISSCUSS] Code coverage report for PRs to master

2022-09-09 Thread tison
Hi Lin, Thanks for starting this discussion! As long as it takes a different resource set from current CI tasks, I'm +0 as commented on PR-17382. I hardly read the report. I read the output in your proposal as simply: > The report will serve as additional input for the reviewers. The requester

[DISSCUSS] Code coverage report for PRs to master

2022-09-09 Thread Lin Zhao
Hi, I'd like to start a discussion about turning on CodeCov report for PRs to master to show the PR's impact on unit test coverage. Previous discussion on https://github.com/apache/pulsar/pull/17382. Proposal: 1. Unit test coverage will be added to the CI pipeline and reported to the PR page. Sam