Improve codecov report integration to CI in Certbot (#6934)
So, we observed lately several inconsistencies in how Codecov behave toward the CI pipeline for PRs in Certbot. One example is #6888. The most annoying thing is that the build of PR is **temporary** marked as failed, until all coverage are run.
The correction on the latter is done in two PRs. This is the first part.
TL;DR
This PR separates the Codecov report in two: one for coverage executed on Windows, one for Linux. This is the correct way to do regarding our current CI pipeline. Actions are required by a GitHub administrator of Certbot once this PR is merged.
Complete explanation
So the failure stated in the introduction is essentially due to several things interacting together:
* AppVeyor generates a coverage report for Windows, that have a coverage value a little lower than on Linux (96%)
* Travis generates a coverage report for Linux. Its coverage is higher, and slowly decrease as more specific Windows code is added to Certbot, that cannot be tested on Travis
* Since AppVeyor saw its capacity increasing, it finishes its coverage job before the one from Travis
* Certbot GitHub repo is configured to require the coverage pipeline to succeed (in whatever that means) to success the overall PR build
So here the suite of events:
1) PR is issued. GitHub expect three pipeline to succeed: AppVeyor CI, Travis CI and Codecov (displayed in the PR page)
2) Codecov receive first the report of AppVeyor coverage. It is 96%. It is a failure for now, because coverage in master (AppVeyor+Travis) is 98.6%.
3) GitHub is reported of the failure on Codecov, so fail the PR build
4) Codecov receive then the report of Travis coverage. It is 98%. It merges it with the report from AppVeyor, leading to the 98.6%. The failure becomes a success.
5) GitHub is reported of the success on Codecov, so, nevermind, the PR build is a success finally!
So we have a CI flow that change its mind. Great. This is because of 2) and 4), and we could expect that Codecov should handle that. This is not the case: it is somewhat misleading, because Codecov adverts a lot about its capability to merge reports, including from different CI. But it is about the final state, not about the transient state, while reports are progressively received.
Two things to things that a transient state is existing, with a result that can change:
* first, from Codecov doc itself, explaining that reports should not be trusted during the CI pipeline execution: https://docs.codecov.io/docs/ci-service-relationship#section-checking-ci-status
* second, is an example of transient state of `cryptography` project, this is advert by Codecov to be a reference of the implementation:
As you can see above, build state of `cryptography` is failing after the first report is received, and until all coverage reports from Travis are received.
So, what can we do about it? Thing is, we are aggregating coverage from very two unrelated sources (two different OS systems), and Codecov has something for that. This is flags: https://docs.codecov.io/docs/flags
Flags allow to flag coverage material depending on any logic you apply to the command that uploaded the coverage report (eg. `codecov -F a_flag`). Then, several logics can be applied on it, for instance having in Codecov UI the capability to filter the coverage other a flag, having status of build for each flag and ... having a report for a specific flag.
So:
1) I modified Travis and AppVeyor to send their report under a specific flag: `linux` or `windows`
2) I created a project specific `.codecov.yml` configuration in Certbot repository, to instruct Codecov to push two separate reports on GitHub build: one for Linux, one for Windows. Each report can be validated against its specific coverage from the `master` branch (more on this just after)
With all of this, now the GitHub is succeeding, because each coverage is validated independently.
I think it is the good approach, because it solves the specific issue here, and because it reflects the logic behind: merging coverage from different OS architectures does not make much sense. It would be a long-term problem, because as I said at the beginning, coverages will slowly decrease as more platform specific code is added in Certbot.
Now, it is not finished. Two things need to be done: an administrator action, and a second PR
Administrator action
Certbot GitHub as a a branch protection rule (Settings > Branches > Branch protection rules). It needs to be changed.
Indeed this rule is expecting the full coverage report (named `codecov/project`) to be valid on a PR. It needs to be changed to expect two coverage reports: `codecov/project/linux` and `codecov/project/windows`. The `codecov/project` needs to be removed.
This can be done once this PR is merged, and the specific coverage reports have been generated on master.
Second PR
Once this PR is merged and administrative actions have been done. I will make a new PR modifying `.codecov.yml` with two things:
* disable the faulty full coverage report, that is not required anymore by GitHub branch protection rules
* modify the `linux` and `windows` reports to validate against the relevant coverage calculated from `master` (indeed, in this PR it is a fixed ratio rule, since the coverage to compare on master is the full coverage one, significantly higher)
* Tag reports
* Set per-project codecov configuration
2019-04-09 14:43:26 -04:00
|
|
|
coverage:
|
|
|
|
|
status:
|
|
|
|
|
project:
|
2019-04-09 15:47:53 -04:00
|
|
|
default: off
|
Improve codecov report integration to CI in Certbot (#6934)
So, we observed lately several inconsistencies in how Codecov behave toward the CI pipeline for PRs in Certbot. One example is #6888. The most annoying thing is that the build of PR is **temporary** marked as failed, until all coverage are run.
The correction on the latter is done in two PRs. This is the first part.
TL;DR
This PR separates the Codecov report in two: one for coverage executed on Windows, one for Linux. This is the correct way to do regarding our current CI pipeline. Actions are required by a GitHub administrator of Certbot once this PR is merged.
Complete explanation
So the failure stated in the introduction is essentially due to several things interacting together:
* AppVeyor generates a coverage report for Windows, that have a coverage value a little lower than on Linux (96%)
* Travis generates a coverage report for Linux. Its coverage is higher, and slowly decrease as more specific Windows code is added to Certbot, that cannot be tested on Travis
* Since AppVeyor saw its capacity increasing, it finishes its coverage job before the one from Travis
* Certbot GitHub repo is configured to require the coverage pipeline to succeed (in whatever that means) to success the overall PR build
So here the suite of events:
1) PR is issued. GitHub expect three pipeline to succeed: AppVeyor CI, Travis CI and Codecov (displayed in the PR page)
2) Codecov receive first the report of AppVeyor coverage. It is 96%. It is a failure for now, because coverage in master (AppVeyor+Travis) is 98.6%.
3) GitHub is reported of the failure on Codecov, so fail the PR build
4) Codecov receive then the report of Travis coverage. It is 98%. It merges it with the report from AppVeyor, leading to the 98.6%. The failure becomes a success.
5) GitHub is reported of the success on Codecov, so, nevermind, the PR build is a success finally!
So we have a CI flow that change its mind. Great. This is because of 2) and 4), and we could expect that Codecov should handle that. This is not the case: it is somewhat misleading, because Codecov adverts a lot about its capability to merge reports, including from different CI. But it is about the final state, not about the transient state, while reports are progressively received.
Two things to things that a transient state is existing, with a result that can change:
* first, from Codecov doc itself, explaining that reports should not be trusted during the CI pipeline execution: https://docs.codecov.io/docs/ci-service-relationship#section-checking-ci-status
* second, is an example of transient state of `cryptography` project, this is advert by Codecov to be a reference of the implementation:
As you can see above, build state of `cryptography` is failing after the first report is received, and until all coverage reports from Travis are received.
So, what can we do about it? Thing is, we are aggregating coverage from very two unrelated sources (two different OS systems), and Codecov has something for that. This is flags: https://docs.codecov.io/docs/flags
Flags allow to flag coverage material depending on any logic you apply to the command that uploaded the coverage report (eg. `codecov -F a_flag`). Then, several logics can be applied on it, for instance having in Codecov UI the capability to filter the coverage other a flag, having status of build for each flag and ... having a report for a specific flag.
So:
1) I modified Travis and AppVeyor to send their report under a specific flag: `linux` or `windows`
2) I created a project specific `.codecov.yml` configuration in Certbot repository, to instruct Codecov to push two separate reports on GitHub build: one for Linux, one for Windows. Each report can be validated against its specific coverage from the `master` branch (more on this just after)
With all of this, now the GitHub is succeeding, because each coverage is validated independently.
I think it is the good approach, because it solves the specific issue here, and because it reflects the logic behind: merging coverage from different OS architectures does not make much sense. It would be a long-term problem, because as I said at the beginning, coverages will slowly decrease as more platform specific code is added in Certbot.
Now, it is not finished. Two things need to be done: an administrator action, and a second PR
Administrator action
Certbot GitHub as a a branch protection rule (Settings > Branches > Branch protection rules). It needs to be changed.
Indeed this rule is expecting the full coverage report (named `codecov/project`) to be valid on a PR. It needs to be changed to expect two coverage reports: `codecov/project/linux` and `codecov/project/windows`. The `codecov/project` needs to be removed.
This can be done once this PR is merged, and the specific coverage reports have been generated on master.
Second PR
Once this PR is merged and administrative actions have been done. I will make a new PR modifying `.codecov.yml` with two things:
* disable the faulty full coverage report, that is not required anymore by GitHub branch protection rules
* modify the `linux` and `windows` reports to validate against the relevant coverage calculated from `master` (indeed, in this PR it is a fixed ratio rule, since the coverage to compare on master is the full coverage one, significantly higher)
* Tag reports
* Set per-project codecov configuration
2019-04-09 14:43:26 -04:00
|
|
|
linux:
|
|
|
|
|
flags: linux
|
Fix codecov quality gate since flags have been removed (#7173)
Because some users were complaining about staled workflow when flags (https://docs.codecov.io/docs/flags) are enabled, Codecov decided to remove them when calculating the coverage on branches until they improved this functionality.
See: https://docs.codecov.io/docs/flags#section-flags-in-the-codecov-ui
The flags are still taken into account on PR builds, but not on based branch.
This is a problem for us, because we use the flags to compare specifically the coverage of a PR against its base branch for Windows on one side, and Linux on the other side. Without flags taken into account on the base branch, the CI fails because the coverage on Windows is too low.
As a temporary fix until the situation is clarified on Codecov side, this PR replaces the validation condition, that was a comparison against the base branch, to a fixed coverage registered in the local .codecov.yml file in Certbot repository.
This way, the coverage on PR builds, that takes into account the flags, is validated against an appropriate value.
This is a temporary solution, that will require an explicit update of .codecov.yml in the mean time if the coverage significantly increases, or decreases on some developments. But until the situation is fixed, this will allow to have a functional quality gate.
2019-06-21 15:00:03 -04:00
|
|
|
# Fixed target instead of auto set by #7173, can
|
|
|
|
|
# be removed when flags in Codecov are added back.
|
2019-07-25 18:25:36 -04:00
|
|
|
target: 97.5
|
Improve codecov report integration to CI in Certbot (#6934)
So, we observed lately several inconsistencies in how Codecov behave toward the CI pipeline for PRs in Certbot. One example is #6888. The most annoying thing is that the build of PR is **temporary** marked as failed, until all coverage are run.
The correction on the latter is done in two PRs. This is the first part.
TL;DR
This PR separates the Codecov report in two: one for coverage executed on Windows, one for Linux. This is the correct way to do regarding our current CI pipeline. Actions are required by a GitHub administrator of Certbot once this PR is merged.
Complete explanation
So the failure stated in the introduction is essentially due to several things interacting together:
* AppVeyor generates a coverage report for Windows, that have a coverage value a little lower than on Linux (96%)
* Travis generates a coverage report for Linux. Its coverage is higher, and slowly decrease as more specific Windows code is added to Certbot, that cannot be tested on Travis
* Since AppVeyor saw its capacity increasing, it finishes its coverage job before the one from Travis
* Certbot GitHub repo is configured to require the coverage pipeline to succeed (in whatever that means) to success the overall PR build
So here the suite of events:
1) PR is issued. GitHub expect three pipeline to succeed: AppVeyor CI, Travis CI and Codecov (displayed in the PR page)
2) Codecov receive first the report of AppVeyor coverage. It is 96%. It is a failure for now, because coverage in master (AppVeyor+Travis) is 98.6%.
3) GitHub is reported of the failure on Codecov, so fail the PR build
4) Codecov receive then the report of Travis coverage. It is 98%. It merges it with the report from AppVeyor, leading to the 98.6%. The failure becomes a success.
5) GitHub is reported of the success on Codecov, so, nevermind, the PR build is a success finally!
So we have a CI flow that change its mind. Great. This is because of 2) and 4), and we could expect that Codecov should handle that. This is not the case: it is somewhat misleading, because Codecov adverts a lot about its capability to merge reports, including from different CI. But it is about the final state, not about the transient state, while reports are progressively received.
Two things to things that a transient state is existing, with a result that can change:
* first, from Codecov doc itself, explaining that reports should not be trusted during the CI pipeline execution: https://docs.codecov.io/docs/ci-service-relationship#section-checking-ci-status
* second, is an example of transient state of `cryptography` project, this is advert by Codecov to be a reference of the implementation:
As you can see above, build state of `cryptography` is failing after the first report is received, and until all coverage reports from Travis are received.
So, what can we do about it? Thing is, we are aggregating coverage from very two unrelated sources (two different OS systems), and Codecov has something for that. This is flags: https://docs.codecov.io/docs/flags
Flags allow to flag coverage material depending on any logic you apply to the command that uploaded the coverage report (eg. `codecov -F a_flag`). Then, several logics can be applied on it, for instance having in Codecov UI the capability to filter the coverage other a flag, having status of build for each flag and ... having a report for a specific flag.
So:
1) I modified Travis and AppVeyor to send their report under a specific flag: `linux` or `windows`
2) I created a project specific `.codecov.yml` configuration in Certbot repository, to instruct Codecov to push two separate reports on GitHub build: one for Linux, one for Windows. Each report can be validated against its specific coverage from the `master` branch (more on this just after)
With all of this, now the GitHub is succeeding, because each coverage is validated independently.
I think it is the good approach, because it solves the specific issue here, and because it reflects the logic behind: merging coverage from different OS architectures does not make much sense. It would be a long-term problem, because as I said at the beginning, coverages will slowly decrease as more platform specific code is added in Certbot.
Now, it is not finished. Two things need to be done: an administrator action, and a second PR
Administrator action
Certbot GitHub as a a branch protection rule (Settings > Branches > Branch protection rules). It needs to be changed.
Indeed this rule is expecting the full coverage report (named `codecov/project`) to be valid on a PR. It needs to be changed to expect two coverage reports: `codecov/project/linux` and `codecov/project/windows`. The `codecov/project` needs to be removed.
This can be done once this PR is merged, and the specific coverage reports have been generated on master.
Second PR
Once this PR is merged and administrative actions have been done. I will make a new PR modifying `.codecov.yml` with two things:
* disable the faulty full coverage report, that is not required anymore by GitHub branch protection rules
* modify the `linux` and `windows` reports to validate against the relevant coverage calculated from `master` (indeed, in this PR it is a fixed ratio rule, since the coverage to compare on master is the full coverage one, significantly higher)
* Tag reports
* Set per-project codecov configuration
2019-04-09 14:43:26 -04:00
|
|
|
threshold: 0.1
|
|
|
|
|
base: auto
|
|
|
|
|
windows:
|
|
|
|
|
flags: windows
|
Fix codecov quality gate since flags have been removed (#7173)
Because some users were complaining about staled workflow when flags (https://docs.codecov.io/docs/flags) are enabled, Codecov decided to remove them when calculating the coverage on branches until they improved this functionality.
See: https://docs.codecov.io/docs/flags#section-flags-in-the-codecov-ui
The flags are still taken into account on PR builds, but not on based branch.
This is a problem for us, because we use the flags to compare specifically the coverage of a PR against its base branch for Windows on one side, and Linux on the other side. Without flags taken into account on the base branch, the CI fails because the coverage on Windows is too low.
As a temporary fix until the situation is clarified on Codecov side, this PR replaces the validation condition, that was a comparison against the base branch, to a fixed coverage registered in the local .codecov.yml file in Certbot repository.
This way, the coverage on PR builds, that takes into account the flags, is validated against an appropriate value.
This is a temporary solution, that will require an explicit update of .codecov.yml in the mean time if the coverage significantly increases, or decreases on some developments. But until the situation is fixed, this will allow to have a functional quality gate.
2019-06-21 15:00:03 -04:00
|
|
|
# Fixed target instead of auto set by #7173, can
|
|
|
|
|
# be removed when flags in Codecov are added back.
|
2019-08-01 13:39:46 -04:00
|
|
|
target: 97.6
|
Improve codecov report integration to CI in Certbot (#6934)
So, we observed lately several inconsistencies in how Codecov behave toward the CI pipeline for PRs in Certbot. One example is #6888. The most annoying thing is that the build of PR is **temporary** marked as failed, until all coverage are run.
The correction on the latter is done in two PRs. This is the first part.
TL;DR
This PR separates the Codecov report in two: one for coverage executed on Windows, one for Linux. This is the correct way to do regarding our current CI pipeline. Actions are required by a GitHub administrator of Certbot once this PR is merged.
Complete explanation
So the failure stated in the introduction is essentially due to several things interacting together:
* AppVeyor generates a coverage report for Windows, that have a coverage value a little lower than on Linux (96%)
* Travis generates a coverage report for Linux. Its coverage is higher, and slowly decrease as more specific Windows code is added to Certbot, that cannot be tested on Travis
* Since AppVeyor saw its capacity increasing, it finishes its coverage job before the one from Travis
* Certbot GitHub repo is configured to require the coverage pipeline to succeed (in whatever that means) to success the overall PR build
So here the suite of events:
1) PR is issued. GitHub expect three pipeline to succeed: AppVeyor CI, Travis CI and Codecov (displayed in the PR page)
2) Codecov receive first the report of AppVeyor coverage. It is 96%. It is a failure for now, because coverage in master (AppVeyor+Travis) is 98.6%.
3) GitHub is reported of the failure on Codecov, so fail the PR build
4) Codecov receive then the report of Travis coverage. It is 98%. It merges it with the report from AppVeyor, leading to the 98.6%. The failure becomes a success.
5) GitHub is reported of the success on Codecov, so, nevermind, the PR build is a success finally!
So we have a CI flow that change its mind. Great. This is because of 2) and 4), and we could expect that Codecov should handle that. This is not the case: it is somewhat misleading, because Codecov adverts a lot about its capability to merge reports, including from different CI. But it is about the final state, not about the transient state, while reports are progressively received.
Two things to things that a transient state is existing, with a result that can change:
* first, from Codecov doc itself, explaining that reports should not be trusted during the CI pipeline execution: https://docs.codecov.io/docs/ci-service-relationship#section-checking-ci-status
* second, is an example of transient state of `cryptography` project, this is advert by Codecov to be a reference of the implementation:
As you can see above, build state of `cryptography` is failing after the first report is received, and until all coverage reports from Travis are received.
So, what can we do about it? Thing is, we are aggregating coverage from very two unrelated sources (two different OS systems), and Codecov has something for that. This is flags: https://docs.codecov.io/docs/flags
Flags allow to flag coverage material depending on any logic you apply to the command that uploaded the coverage report (eg. `codecov -F a_flag`). Then, several logics can be applied on it, for instance having in Codecov UI the capability to filter the coverage other a flag, having status of build for each flag and ... having a report for a specific flag.
So:
1) I modified Travis and AppVeyor to send their report under a specific flag: `linux` or `windows`
2) I created a project specific `.codecov.yml` configuration in Certbot repository, to instruct Codecov to push two separate reports on GitHub build: one for Linux, one for Windows. Each report can be validated against its specific coverage from the `master` branch (more on this just after)
With all of this, now the GitHub is succeeding, because each coverage is validated independently.
I think it is the good approach, because it solves the specific issue here, and because it reflects the logic behind: merging coverage from different OS architectures does not make much sense. It would be a long-term problem, because as I said at the beginning, coverages will slowly decrease as more platform specific code is added in Certbot.
Now, it is not finished. Two things need to be done: an administrator action, and a second PR
Administrator action
Certbot GitHub as a a branch protection rule (Settings > Branches > Branch protection rules). It needs to be changed.
Indeed this rule is expecting the full coverage report (named `codecov/project`) to be valid on a PR. It needs to be changed to expect two coverage reports: `codecov/project/linux` and `codecov/project/windows`. The `codecov/project` needs to be removed.
This can be done once this PR is merged, and the specific coverage reports have been generated on master.
Second PR
Once this PR is merged and administrative actions have been done. I will make a new PR modifying `.codecov.yml` with two things:
* disable the faulty full coverage report, that is not required anymore by GitHub branch protection rules
* modify the `linux` and `windows` reports to validate against the relevant coverage calculated from `master` (indeed, in this PR it is a fixed ratio rule, since the coverage to compare on master is the full coverage one, significantly higher)
* Tag reports
* Set per-project codecov configuration
2019-04-09 14:43:26 -04:00
|
|
|
threshold: 0.1
|
|
|
|
|
base: auto
|