ci: run the sonar scanner so coverage and analysis actually work - #145
Merged
Conversation
The sonar { } block in build.gradle is never executed -- CI runs jar and test, never
the Gradle sonar task -- so its cpd.exclusions setting has had no effect and the two
per-version YamlWrapperImpl copies were reported as 66.7% duplicated. Automatic
Analysis reads sonar-project.properties, so the exclusion goes there instead.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Nothing in CI invokes the Gradle sonar task, so the sonar { } block and the
org.sonarqube plugin never ran. Analysis comes from SonarCloud Automatic Analysis,
which is configured by sonar-project.properties instead. Removing the plugin also
stops renovate bumping a dependency nothing uses.
svaningelgem
changed the title
Keeps sonar-project.properties as the single source of truth for settings both
analysers understand, and restores the sonar { } block to read it -- the Gradle
scanner does not read that file on its own. Scanner-only settings (project identity,
build outputs, jacoco report paths) stay in build.gradle, since Automatic Analysis
never builds the project and cannot act on them.
Nothing invoked the Gradle sonar task, so build.gradle's sonar block never ran -- which is why the CPD exclusion was ignored and coverage never appeared. Automatic Analysis cannot report coverage at all, since it never builds or tests the project. Adds the scanner step after the four test runs so all four jacoco reports exist, and records why the YamlWrapperImpl duplication is deliberate. Requires disabling Automatic Analysis and adding a SONAR_TOKEN secret.
svaningelgem
changed the title
fetch-depth: 0 is already set so blame data is available, but sonar.scm.disabled threw it away. Every quality gate condition is a new_* metric, and 'new code' is derived from SCM data -- including the coverage now being enabled.
main has no sources of its own, so the plugin infers an empty compile classpath and warns about a missing sonar.java.libraries -- leaving the Java analyser unable to resolve SnakeYAML or annotation types. Adds common plus the SnakeYAML matching the current build, the same way jacoco already selects one version's source set.
|
svaningelgem
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SonarCloud was running Automatic Analysis, which scans source directly and never invokes Gradle. Every setting in
build.gradle'ssonarblock was therefore inert. This runs the scanner instead, which turns that existing config on and fixes three separate symptoms at once.+22/-2 lines, two files, no production code touched.
What was broken
gradle-build.ymlcached~/.sonar/cache, then ran onlyclean jarand fourtestinvocations — never./gradlew sonar. Consequences:v1/andv2/YamlWrapperImplreported 66.7% duplicatedsonar.cpd.exclusionswas declared but never readsonar.coverage.jacoco.xmlReportPathsnever read — and Automatic Analysis cannot report coverage at all, since it doesn't build or testThe block was already correct. It just needed to run.
Changes
gradle-build.yml— scanner step after all four test runs, so everyreport-*.xmlexists when it reads them.build.gradlesonar.scm.disabled = true.fetch-depth: 0is set specifically so Sonar can read git blame, and this was discarding it. Every gate condition is anew_*metric, and "new code" is derived from SCM data — including the coverage being enabled here. It dated from Implement automated switching between snakeyaml 1.14 & 2.2 #30, where it was boilerplate in a bulk commit and never mattered because nothing ran.sonar.java.libraries/sonar.java.test.libraries, fixing "Missing 'sonar.java.libraries' property. You might end up with less precise analysis results." —main.java.srcDirs = [], so the plugin infers an empty classpath and the Java analyser cannot resolve SnakeYAML or annotation types. Only the SnakeYAML matching the current build is added: putting 1.x and 2.x on one classpath would make the twoToStringRepresentervariants resolve against the wrong API. This mirrors how jacoco already selects one version's source set viaisV1.YamlWrapperImplduplication is deliberate.Verified on the PR analysis
All CI steps green, including the new scanner step.
Why the duplication itself stays
The two files are byte-for-byte identical apart from
package. The entire v1/v2 API divergence is two lines, both inToStringRepresenter—super()vssuper(new DumperOptions()), andScalarStyle.PLAIN.getChar()vsScalarStyle.PLAIN.YamlWrapperImplcould move tocommontaking aRepresenter, since every API it touches is signature-identical across 1.14 and 2.5. Butcommondeliberately has no SnakeYAML dependency — that is what makes dual-version support trustworthy. Sharing it would pincommonto one major version and run it against the other, trading a compile-time guarantee for a runtime assumption to save 18 lines.Prerequisites (already done)
Automatic Analysis disabled in the SonarCloud UI, and a
SONAR_TOKENrepo secret added. Both were required — the two analysis methods are mutually exclusive.