feat(sonar): let a caller hand over coverage it already measured - #31
Merged
Conversation
CI-based analysis, because Automatic Analysis does not import coverage — its own documentation says "code coverage information is not supported". The nine `theokit*` projects were analysing today and reporting no coverage at all, which leaves the quality gate blind to the metric the suites are actually enforcing. One workflow rather than a step copied into eight repositories, and the reason is measured. The one inline version that existed — `theokit-tui`, added by B-126 — carried `if: matrix.node-version == '22.x'` against a matrix of `['22.12', '22']`. That value never occurs, so the step reported `skipped` on every run since it was written, and a skipped step renders exactly like a passing one. Copying it eight times would have copied that. Three decisions worth stating: - `fetch-depth: 0`. On a shallow clone every line looks like it arrived in the last commit, so the scanner attributes all issues to it and the new-code period — which is what the quality gate measures — describes the wrong change. - Coverage runs with `continue-on-error`. The caller's own test job gates the suite; here a broken coverage run should cost the coverage number, not take the security findings down with it. - `sonar-project.properties` is required, not defaulted. Without it the scanner analyses build output and reports a file as a duplicate of its own bundle — measured on theokit-tui. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
… does not replace it
The header claimed SonarCloud "disables Automatic Analysis for a project as soon as a CI-based
analysis reports". It does not. The scanner fails:
[ERROR] ScannerEngine: You are running CI analysis while Automatic Analysis is enabled.
Please consider disabling one or the other.
Measured on theokit-tui: with the step finally executing (the `22.x` condition fixed) and the
token present, the run failed on exactly this and nothing else. Turning the toggle off by hand
under Administration > Analysis Method is a prerequisite, not something the workflow handles.
Written down here because a caller reading the old sentence would adopt the workflow and get a
red check with no idea it needed a manual step.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
…everything `secrets: inherit` hands the called workflow every secret the caller holds. zizmor flags it (`secrets-inherit`), and it is right to: this workflow needs one token to publish an analysis, and inheriting gives it npm credentials, app private keys and anything else the repository carries. Declared as `required: false`, so a repository without the secret still runs the job — the existing step warns and skips rather than failing, which is what lets a repository adopt the workflow before the secret exists. Caught by running zizmor locally against the four callers before pushing them, rather than by watching four CI runs go red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
…rough dist/
theokit-sdk's first analysis passed both checks and reported no coverage at all. The
scanner said only:
No coverage information will be saved because all LCOV files cannot be found.
The coverage step above it had already failed, and the log carried the reason:
Failed to resolve entry for package "@theokit/sdk"
sdk-cache: missing dist/index.js — run 'pnpm build'
Its packages import each other through their published entry points, so nine suites
cannot even load before `pnpm build` has run. This workflow installed and measured,
never built — and `continue-on-error` on the coverage step turned that into a green
check with an empty number, which is the failure mode this whole workflow exists to
remove.
`build-command` is a separate step rather than a prefix inside `coverage-command`:
the two fail for different reasons, and a build failure that reads as a coverage
failure is how the last one hid for a day.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
# Conflicts: # .github/workflows/sonar.yml
`build-command` fixed the repositories whose suite needs `dist/`. It does not fix theokit, whose coverage job first checks out a sibling repository and builds it, rebuilds native bindings, installs bubblewrap, and only then runs the suite — 89 suites fail to load without that, and the scan reported no coverage while passing. Re-deriving that recipe inside this workflow would be a second copy of it, free to drift from the one that gates the repository. `coverage-artifact` reads the artifact the caller's own coverage job already uploaded, which also makes the number Sonar publishes literally the number the gate measured rather than a second run entitled to disagree. Setting both coverage inputs is refused rather than resolved: a caller that sets both holds a belief about which coverage the report describes, and half of it would be wrong.
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.
Por quê
build-command(#30) conserta quem só precisa dedist/. Não conserta otheokit.O job de cobertura dele faz, antes de rodar um único teste:
theokit-sdk(dependência de workspace)pnpm rebuild better-sqlite3(guarda de ABI nativa)pnpm build:packagesSem isso:
Failed Suites 89,Cannot find module .../create-theokit/dist/cli.js— e o scan passou reportando cobertura nenhuma.Reproduzir essa receita aqui seria uma segunda cópia dela, livre para divergir da que efetivamente porteia o repositório.
O que muda
coverage-artifact: <nome>baixa paracoverage/o artefato que um job anterior do mesmo run já subiu. Otheokitjá publicacoverage-reportdesde sempre.Efeito colateral que é o argumento principal: o número que o Sonar publica passa a ser literalmente o número que o gate do repositório mediu — não uma segunda execução com direito a discordar.
Exclusividade
coverage-command+coverage-artifactjuntos → o job falha com mensagem explícita. Deixar um vencer em silêncio significaria que metade da crença do caller sobre qual cobertura o relatório descreve está errada.--min-severity low: sem achadosdownload-artifactpinado no mesmo SHA já usado no ecossistema (37930b1v7.0.0)