Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send total coverage back to GitLab instead of new coverage #225

Merged
merged 1 commit into from
Mar 6, 2021

Conversation

teake
Copy link
Contributor

@teake teake commented Aug 6, 2020

Reworked version of #157 (and #145). Third time's the charm, right? 🤞

The coverage send back via the GitLab API should be the total coverage
of the project, not the coverage of the new code in the PR. The GitLab
docs explicitly mention this:
https://docs.gitlab.com/ee/api/commits.html#post-the-build-status-to-a-commit

The coverage reported by decorating the PR via a comment is not affected;
this still shows both the total and new coverage.

Note that there is an open GitLab issue for natively displaying new coverage
for just the diff of a PR (https://gitlab.com/gitlab-org/gitlab/-/issues/20895),
but this is not yet implemented.

In addition, reporting coverage back is optional. So if there is no coverage
information, we do not report it back.

The coverage send back via the GitLab API should be the total coverage
of the project, not the coverage of the new code in the PR. The GitLab
docs explicitly mention this:
https://docs.gitlab.com/ee/api/commits.html#post-the-build-status-to-a-commit

The coverage reported by decorating the PR via a comment is not affected;
this still shows both the total and new coverage.

Note that there is an open GitLab issue for natively displaying new coverage
for just the diff of a PR (https://gitlab.com/gitlab-org/gitlab/-/issues/20895),
but this is not yet implemented.

In addition, reporting coverage back is optional. So if there is no coverage
information, we do not report it back.
@teake teake closed this Aug 7, 2020
@teake teake deleted the gitlab-correct-coverage branch August 7, 2020 07:24
@teake teake restored the gitlab-correct-coverage branch August 7, 2020 07:25
@teake
Copy link
Contributor Author

teake commented Aug 7, 2020

Whoops, I was a bit overzealous in the housekeeping of my fork of this project -- I didn't mean to close the PR. Reopening now.

@teake teake reopened this Aug 7, 2020
@LaSylv
Copy link

LaSylv commented Dec 8, 2020

Any news on this ?

@tisoft
Copy link
Contributor

tisoft commented Feb 19, 2021

@mc1arke Could this be merged? Thanks. 😄

@sonarcloud
Copy link

sonarcloud bot commented Mar 6, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

85.7% 85.7% Coverage
0.0% 0.0% Duplication

@mc1arke mc1arke merged commit 3d2bd86 into mc1arke:master Mar 6, 2021
@mc1arke
Copy link
Owner

mc1arke commented Mar 6, 2021

Merged. Thanks for your contribution!

@lucasoares
Copy link

lucasoares commented Aug 11, 2022

@mc1arke just ran an analysis with sonarqube 9.5 and with plugin version 1.12.0 and the reported coverage in the SonarQube external job is the new code coverage and not the total coverage.

image

image

Maybe should revisit this?

Karql referenced this pull request Mar 21, 2023
The access to metrics from a Pull Request analysis is exposed through an
`AnalysisDetails` instance, which also provides the ability to extract a
formatted report. As a number of the metrics used in the summary report
need to be retrieved through various additional DAOs, and as the
resolution of URLs for links and images requiring access to core
Sonarqube configuration, `AnalysisDetails` holds references to a high
number of classes from Sonarqube's core. Some of those core Sonarqube
classes are also referenced directly in some decorators which don't make
use of the summary report but need equivalent metrics to those shown in
the summary which means some searching logic is duplicated across the
plugin.

This change pulls the report generation into a `ReportGenerator` class,
with the report being an interim set of collected metrics that each
decorator can extract required information, or generate a formatted
report from.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants