Hello everyone, TL;DR: I would like to change our Sonar integration, so that projects can opt in into having their all of their UTs contribute to code coverage. To do that, we need one change in odlparent and an optional per-project change to JJB.
our current Sonar coverage works on a per-artifact basis, which means that if parts of the codebase are tested outside of the artifact which hosts src/main/java, that coverage is not taken into account by Sonar (or anything else). To illustrate, if I have : foo-api/src/main/ foo-api/src/test/ foo-impl/src/main/ foo-impl/src/test/ Then foo-api/src/main reported coverage is strictly limited to how much foo-api/src/test exercises the code. If foo-impl/src/test/ provides additional coverage, that information does not register with Sonar. I think this is wrong for at least two reasons: - it requires explicit UT coverage of API artifacts, as they now can (and do) include default methods, bloating the code - it discounts "integration test" coverage provided by higher-level functionality, where more complex scenarios are tested This is especially hurtful for projects which provide their functionality through multiple layered artifacts -- like yangtools and mdsal, as our users prefer to express what does not work in terms of higher-level operations. This is evident in YANGTOOLS-892, where the reproducer is provided in terms of a yang-data-codec-xml unit test, whereas the problem lies in yang-data-impl -- the bug is fixed and the bugfix is guarded by a unit test, but that fact is left unreported. As it turns out, this is not a technical limitation on Sonar, maven, or jacoco -- it is the result of how we integrate with Sonar. I have run a prototype on yangtools, and the difference in reported coverage is 60.4% vs. 73.4%. The prototype is quite invasive, as it needs to override things coming from odlparent in each of the artifact and also needs to define (and reference) a global location, which would best be provided by yangtools-sonar. The change I am proposing starts with https://git.opendaylight.org/gerrit/75985, which decouples Sonar and JaCoCo configuration, such JaCoCo configuration does not change when sonar.jacoco.reportPath changes -- otherwise downstreams have to jump through hoops to decouple it themselves -- like majority of https://git.opendaylight.org/gerrit/75996 had to do for yangtools. This has no impact on existing projects and everything continues to work as it did before. The second part is a per-project change to a project's top-level aggregator pom.xml, and is an instruction to run "jacoco:merge" goal, which takes all generated .exec files generated during UT run into a single file, located in aggregator's target/ directory. This implies that the project's top-level pom.xml *must* be a pure aggregator, not a parent pom. This is already our current best practice, but there are projects which are not following it. Those projects cannot opt-in to whole-project reports due to the impact combining aggregator and parent pom roles has on maven build cycle, sorry(*). The final part is to override sonar.jacoco.reportPath either in $project-sonar job or in individual artifacts to point to that combined .exec file. With all that in place, the build will execute as follows: 1) all artifacts are built, running their individual UTs and producing their individual JaCoCo .exec files 2) top-level aggregator will be built, producing the combined JaCoCo .exec file 3) sonar:sonar runs, visits each artifact and for each one of them it uses the combined JaCoCo .exec file This leads to Sonar picking up all coverage provided by all UTs in the project for each artifact. Are there any objections to implementing the change in #75985 and making this the best current practice on how to integrate a project with Sonar? Thanks, Robert (*) top-level POM needs to have instructions that are *not* picked up by any other artifacts, hence it cannot be a parent pom. You can try to make it work, but you are in for a lot of "it almost works" moments with the associated anguish and frustration. Just refactor your project to have a dedicated aggregator pom.xml sitting at the root of your git repository -- you'll never look back, I promise.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ infrastructure mailing list [email protected] https://lists.opendaylight.org/mailman/listinfo/infrastructure
