-
Notifications
You must be signed in to change notification settings - Fork 97
TRG Suggestion - Application Testing - Code Coverage #1149
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
base: main
Are you sure you want to change the base?
TRG Suggestion - Application Testing - Code Coverage #1149
Conversation
Suggestion for a Tractus-X Release Guideline (TRG) for Code Coverage
Adapted Markdown for a bare URL external link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing markdown formatting blank lines, unordered list
|
||
1.1. **Code Coverage Threshold** | ||
|
||
- Quality Gate: The minimum threshold for code coverage (line coverage) should be **80.0%**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly advice against this.
In my experience code coverage thresholds ended up in people putting fake tests that covered lines but actually tested nothing.
Test culture is something that needs to be part of the team as good practice that's put in place every day in every code change, and it's up to committers to ensure this through code reviews.
An indicator that show what's the coverage on the changed lines in a PR could help the committer to drive the review in a certain direction, but it shouldn't do the review in their place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note that as I mentioned in the description, we would suggest - at least initially - to only require that a code coverage measurement is implemented. With the value that is measured as an indicator, like you wrote. So not to enforce a certain threshold like 80% to prevent build/deployment processes for applications that fall below it immediately (which I have also experienced in other projects, which is obviously frustrating). We want to have code coverage as a well-established measurement, but not burden the committers / teams with having to bring up their coverage within a short period of time to meet that threshold and thereby risk they are just "faking" it.
the line on which I commented there's the "threshold" word, that to me means that if the actual value is less, the CI should throw an error and block the contribution.
If that's not what was meant, please rephrase or remove the line altogether
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strengthen Andreas comment here. As I already mentioned in the office hours. I would, as a first step, make a guideline in the direction:
- Code Coverage measurements have to be in place
- Code Coverage delta reports for PRs are part of the standard PR feedback
- Committers are responsible to keep an eye on code coverage as part of their PR review
We can give a hint, that there is some statistics that show, that 80% is a turning point and further improvements do typically not pay off, so a general target of 80% is something to think about within a team, but an absolute requirement is counterproductive. I have seen tests in my past that added no value except increasing the coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As just discussed at the committers meeting, I would appreciate it if we could add some examples of how to modify the pipelines to the TRG.
|
||
1.1. **Code Coverage Threshold** | ||
|
||
- Quality Gate: The minimum threshold for code coverage (line coverage) should be **80.0%**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strengthen Andreas comment here. As I already mentioned in the office hours. I would, as a first step, make a guideline in the direction:
- Code Coverage measurements have to be in place
- Code Coverage delta reports for PRs are part of the standard PR feedback
- Committers are responsible to keep an eye on code coverage as part of their PR review
We can give a hint, that there is some statistics that show, that 80% is a turning point and further improvements do typically not pay off, so a general target of 80% is something to think about within a team, but an absolute requirement is counterproductive. I have seen tests in my past that added no value except increasing the coverage.
docs/release/trg-10/trg-10-01.md
Outdated
|
||
- Quality Gate: The minimum threshold for code coverage (line coverage) should be **80.0%**. | ||
- This applies to both unit tests and integration tests. | ||
- Every project not specifically associated to a different Quality Gate will be associated to this one by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not happy with this setup. The current TRGs to my understanding are more about prescribing the use of a certain technical infrastructure to manage a certain quality aspect but leaves the responsibility to achieve the quality goals with the responsible persons of the repository. Here, we leave this path by defining concrete abstract requirements which have no direct impact on the quality. The risk involved with this is always that responsibility is separated. With a construct as this, quality becomes an externally defined construct specified by KPIs. As long as I fulfil the KPIs, I am done and if something goes wrong, how can I be blamed? Keep the responsibility where it belongs, with the committers of a certain repository, they have to deliver quality. And they should get support by standardized tooling, so they do now have to invent everything from scratch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have exposed this PR in the committer meeting from 07.03.2025 and we have considered that there are some missing points where you shall investigate and add to the TRG proposal before it gets approved.
@ds-hzimmer please consider the following points:
-> Test Coverage is not available for everything.
-> Example integration and workflow shall be defined.
-> We do not want to overwhelm the eclipse foundation by creating multiple requests that are not aligned.
-> Template for the eclipse foundation so what is needed for them to set up this TRG and make it work for us, there is one secret at organization level to be created, from the CI integration the ones that created the TRG shall look in to the.
Update with an example integration for Java. Additional changes pending.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adaptations for section 2.1 and 4 regarding code coverage reporting implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adaptation to include both unit tests and integration tests, not each separately.
…porting and code coverage analysis itself, and additional references and clarifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed line on "Every project not specifically associated to a Quality Gate will be associated to this one by default" as this is the default setting in SonarQube Cloud anyway, and thus does not have to be specified in a TRG. Corrected Markdown linting for breaks around headlines.
…lt setting and thus does not need to be icluded in TRG)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just remembered that a couple of years ago the sonar integration via CI pipeline (which AFAIK is the only proper way for test coverage measuring) doesn't (didn't?) work in case of of scanning pull requests coming from forks because the secrets (token) of the upstream repo couldn't get accessed. Could you please find out if that's still the case? If yes, we should recommend to the products a way how to handle this.
|
||
See the following SonarQube Cloud guideline for the importing of test coverage reports with detailed guides for several popular programming languages as [reference](https://docs.sonarsource.com/sonarqube-cloud/enriching/test-coverage/overview/). | ||
|
||
#### 4.2. **GitHub Workflow Integration Code Example** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the TRG is about test coverage and measuring that with sonar, as far as I understood, and the GH workflow example for CI integration is about executing and publishing unit and integration tests, that doesn't make sense to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still the case because the secrets are only available inside the Tractus-X GitHub Organization. For example: vars.SONAR_ORGANIZATION or secrets.SONAR_TOKEN_BACKEND.
So this action would only work if it's executed inside Tractus-X Repositories.
Updated workflow integration example
Suggestion for a Tractus-X Release Guideline (TRG) for Code Coverage
Description
Tractus-X Release Guideline (TRG) for static code analysis code coverage measurements, with a suggested quality gate target threshold of 80.0%.
The goal is to ensure that all released Eclipse Tractus-X software components meet sufficient test coverage to guarantee quality, stability, and reliability. The suggested threshold would at least initially not be technically enforced, i.e. not preventing build pipeline processes if coverage falls below this threshold while committers and teams are still implementing this new TRG for their software components.
The coverage should be measured and monitored using established Open Source tools, for which Eclipse Tractus-X already provides a SonarCloud installation. https://sonarcloud.io/organizations/eclipse-tractusx/projects
Other tools are also possible (two additional suggestions included).
Pull request created by issue:
eclipse-tractusx/sig-release#970
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: