CI: make `codecov/project` failed when coverage decreased in pull request #45557

wuhuizuo · 2023-07-25T08:11:30Z

What problem does this PR solve?

Issue Number: close #45560

Problem Summary:

What is changed and how it works?

update green range value, current the coverage of trunk branch is greater than 70%.
update .coverage.status.project to make codecov/project github status failed when coverage decreased in pull request.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

ti-chi-bot · 2023-07-25T08:11:32Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

ti-chi-bot · 2023-07-25T08:11:32Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zhaoxinyu for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

tiprow · 2023-07-25T08:11:52Z

Hi @wuhuizuo. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

wuhuizuo · 2023-07-25T08:15:55Z

/test ?

ti-chi-bot · 2023-07-25T08:15:57Z

@wuhuizuo: The following commands are available to trigger required jobs:

/test build
/test canary-scan-security
/test check-dev
/test check-dev2
/test mysql-test
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-br-integration-test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-ddl-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-sqllogic-test
/test pull-tiflash-test
/test unit-test

The following commands are available to trigger optional jobs:

/test pull-notify-when-compatibility-sections-changed

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tiprow · 2023-07-25T08:16:30Z

@wuhuizuo: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

wuhuizuo · 2023-07-25T08:16:53Z

/test unit-test

tiprow · 2023-07-25T08:17:26Z

@wuhuizuo: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test unit-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

codecov · 2023-07-25T08:32:36Z

Codecov Report

Merging #45557 (80593b2) into master (56610b1) will increase coverage by 1.4931%.
Report is 46 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #45557        +/-   ##
================================================
+ Coverage   73.2295%   74.7227%   +1.4931%     
================================================
  Files          1265       1278        +13     
  Lines        389624     403185     +13561     
================================================
+ Hits         285320     301271     +15951     
+ Misses        86005      83945      -2060     
+ Partials      18299      17969       -330

Flag	Coverage Δ
integration	`78.1388% <ø> (?)`
unit	`74.7170% <ø> (+1.4874%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`54.0444% <ø> (ø)`
parser	`87.6802% <ø> (+2.6247%)`	⬆️
br	`52.9554% <ø> (+0.8187%)`	⬆️

hawkingrei · 2023-07-25T09:23:24Z

Why does coverage decrease but you do not change anything?

wuhuizuo · 2023-07-25T09:39:43Z

Why does coverage decrease but you do not change anything?

It's missed:

Will bazel caching will cause it?

wuhuizuo · 2023-07-25T09:49:03Z

Why does coverage decrease but you do not change anything?

It's missed:

Will bazel caching will cause it?

I think the tests are not permanent.

hawkingrei · 2023-07-25T09:59:04Z

.codecov.yml

      default:
-        threshold: 0.2 #Allow the coverage to drop by threshold%, and posting a success status.
+        target: auto
+        threshold: 0% # decline in denial coverage.


So, are you sure about setting this threshold to 0?

Sure, only allow the total coverage to drop by 0%.

If you want to set this threshold to 0%, it's better to provide another command to let one reviewer do the final check and then force merge, since we always have code lines that are hard to cover.

I'm not quite sure whether the zero threshold here is reasonable. The coverage statistics are not always accurate, and the same modification may lead to different results in different environments.

And as chrysan said above, code lines are not always easy to be covered, for example, some insurance codes written for extreme cases may never be generated in the test environment, which means the coverage drop would never reach 0.

By removing some useless code and its test, the coverage will drop, but it's acceptable.

wuhuizuo · 2023-07-25T12:02:45Z

/cc @bb7133

PTAL

wuhuizuo · 2023-07-25T12:04:27Z

/test check?

ti-chi-bot · 2023-07-25T12:04:29Z

@wuhuizuo: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test canary-scan-security
/test check-dev
/test check-dev2
/test mysql-test
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-br-integration-test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-ddl-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-sqllogic-test
/test pull-tiflash-test
/test unit-test

The following commands are available to trigger optional jobs:

/test pull-notify-when-compatibility-sections-changed

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test

In response to this:

/test check?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

wuhuizuo · 2023-07-25T12:04:39Z

/test check-dev2

tiprow · 2023-07-25T12:04:59Z

@wuhuizuo: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test check?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tiprow · 2023-07-25T12:05:10Z

@wuhuizuo: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test check-dev2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ti-chi-bot · 2023-07-31T03:25:14Z

@wuhuizuo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
idc-jenkins-ci-tidb/check_dev	`80593b2`	link	true	`/test check-dev`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Xuanwo · 2023-08-08T09:40:48Z

Wow, TiDB developers must be happy with this change because it's so useful 🙏🙏🙏

Rustin170506 · 2023-08-11T08:58:28Z

Some offline discussions:

hi-rustin: #45557 I'm curious about this PR, it's coverage increased by 1.49%. But its patch coverage was not affected. How should we understand this calculation rule?

hi-rustin: My guess is because it uses a different base ref commit to calculate code coverage. So please give us a clear guide on how to understand these values before enabling the mechanism. And tell us how to make sure the results are not false positives or false negatives. Otherwise it will become very annoying.

hawkingrei: No need to guess, this has already happened.

hi-rustin: Also, not everyone is a CI/CD or codecov expert, so if it's going to affect every R&D, then make sure that this is a manageable cost of learning to understand. Otherwise, we are reducing our development efficiency.

wuhuizuo: CI flow: checkout head + base -> merge head to base -> run tests.

hi-rustin: Also, is there a better way for us to improve test coverage? For example, let's not enforce it, just monitor it. Then set goals within the team to improve it. Instead of making it a CI requirement to force R&D. I have seen very few projects in real life projects that do this kind of requirement.

hi-rustin: ant-design/ant-design#44061 Even for this project, it has 100% code coverage and they don't really enforce it as a required CI status.

hi-rustin: Another example: #45900 I added two tests for one function, but got some really crazy CI status:

wuhuizuo: Rustin Liu just focus on "codecov/project", other statuses fail on target coverage.

wuhuizuo: The 50% patch override for unit is spot on. the base code is also not overridden, but your renaming of the variable on the uncovered line is a hit.
base coverage: https://app.codecov.io/gh/pingcap/tidb/blob/master/statistics%2Fcmsketch.go#L830

hi-rustin: I added threeee tests for this function, but I got some really crazy results again:

hi-rustin: #45900 Yesterday I still can pass some of CI statuses. I didn't change any of my code. Just added some benchmark tests.

wuhuizuo: some CI jobs are not finished, it will update when new data is uploaded.

hi-rustin: This pull request has already been merged, but the CI statuses are still failed. #45900

hawkingrei: #45947

This PR is to clean code. but I do not understand why the coverage of br is decreasing.

wuhuizuo: Stephen Wang, coverage was decreased by Indirect changes.

Stephen Wang, coverage was decreased by Indirect changes.
Ref: https://docs.codecov.com/docs/unexpected-coverage-changes

hawkingrei: This PR can not change coverage. It can not decrease or increase coverage.

wuhuizuo: it's diff with the old base commit, such as:
https://app.codecov.io/gh/pingcap/tidb/pull/45947/blob/plugin/plugin.go
it's coverage data based: b59ce11 (17 hours before)
not 7828409

I think it's the cause.

hi-rustin:

#45988 I only updated the makefile.

wuhuizuo: Jason Mo,It does seem like it could have something to do with the coverage reporting inside the check job you added.
It does seem like it could have something to do with the coverage reporting inside the check job you added.

hi-rustin: Can we turn off the coverage reporting until it's fixed? #46003 This is another pull request to add tests.

At the moment it doesn't do much good except interfere with our ability to see the pr status. Or just put it on the master merged commit.

Rustin170506 · 2023-08-11T09:00:05Z

Based on our discussion, I'm suggesting that we turn off status feedback on PR until we get the codecov statuses fixed.

wuhuizuo · 2023-09-06T08:07:26Z

Pause currently
/cc @bb7133

*: Update .codecov.yml

80593b2

wuhuizuo changed the title *: Update .codecov.yml *: Update .codecov.yml to make codecov/project failed when coverage decreased in pull request. Jul 25, 2023

wuhuizuo changed the title *: Update .codecov.yml to make codecov/project failed when coverage decreased in pull request. CI: make codecov/project failed when coverage decreased in pull request Jul 25, 2023

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Jul 25, 2023

hawkingrei reviewed Jul 25, 2023

View reviewed changes

ti-chi-bot bot requested a review from bb7133 July 25, 2023 12:02

wuhuizuo mentioned this pull request Jul 26, 2023

How to block the pull requests which will decrease the total test coverage. #45560

Open

wuhuizuo marked this pull request as ready for review July 31, 2023 03:12

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 31, 2023

wuhuizuo closed this Sep 6, 2023

wuhuizuo deleted the ci/wuhuizuo-patch-1 branch September 6, 2023 08:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: make `codecov/project` failed when coverage decreased in pull request #45557

CI: make `codecov/project` failed when coverage decreased in pull request #45557

wuhuizuo commented Jul 25, 2023 •

edited

Loading

ti-chi-bot bot commented Jul 25, 2023

ti-chi-bot bot commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023

ti-chi-bot bot commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

codecov bot commented Jul 25, 2023 •

edited

Loading

hawkingrei commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023 •

edited

Loading

wuhuizuo commented Jul 25, 2023

hawkingrei Jul 25, 2023

wuhuizuo Jul 25, 2023

chrysan Aug 1, 2023

JmPotato Aug 8, 2023

JmPotato Aug 8, 2023

you06 Aug 8, 2023

wuhuizuo commented Jul 25, 2023 •

edited

Loading

wuhuizuo commented Jul 25, 2023

ti-chi-bot bot commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

ti-chi-bot bot commented Jul 31, 2023

Xuanwo commented Aug 8, 2023

Rustin170506 commented Aug 11, 2023

Rustin170506 commented Aug 11, 2023

wuhuizuo commented Sep 6, 2023

CI: make codecov/project failed when coverage decreased in pull request #45557

CI: make codecov/project failed when coverage decreased in pull request #45557

Conversation

wuhuizuo commented Jul 25, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot bot commented Jul 25, 2023

ti-chi-bot bot commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023

ti-chi-bot bot commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

codecov bot commented Jul 25, 2023 • edited Loading

Codecov Report

hawkingrei commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023 • edited Loading

wuhuizuo commented Jul 25, 2023

hawkingrei Jul 25, 2023

Choose a reason for hiding this comment

wuhuizuo Jul 25, 2023

Choose a reason for hiding this comment

chrysan Aug 1, 2023

Choose a reason for hiding this comment

JmPotato Aug 8, 2023

Choose a reason for hiding this comment

JmPotato Aug 8, 2023

Choose a reason for hiding this comment

you06 Aug 8, 2023

Choose a reason for hiding this comment

wuhuizuo commented Jul 25, 2023 • edited Loading

wuhuizuo commented Jul 25, 2023

ti-chi-bot bot commented Jul 25, 2023

wuhuizuo commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

tiprow bot commented Jul 25, 2023

ti-chi-bot bot commented Jul 31, 2023

Xuanwo commented Aug 8, 2023

Rustin170506 commented Aug 11, 2023

Rustin170506 commented Aug 11, 2023

wuhuizuo commented Sep 6, 2023

CI: make `codecov/project` failed when coverage decreased in pull request #45557

CI: make `codecov/project` failed when coverage decreased in pull request #45557

wuhuizuo commented Jul 25, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading

wuhuizuo commented Jul 25, 2023 •

edited

Loading

wuhuizuo commented Jul 25, 2023 •

edited

Loading