ci: analyse with SonarCloud from the workflow instead of Automatic Analysis - #95
Closed
insatomcat wants to merge 4 commits into
Closed
ci: analyse with SonarCloud from the workflow instead of Automatic Analysis#95insatomcat wants to merge 4 commits into
insatomcat wants to merge 4 commits into
Conversation
vm_manager selects its backend when it is first imported: if `rados` and `rbd` are importable it exposes the cluster API, otherwise the libvirt-only one. Those bindings ship with Ceph and are not installable from PyPI, so on the CI runner `cluster_mode` is False. The consequence was that tests/test_vm_manager_cmd.py, whose docstring says it has no cluster dependencies, was skipped in its entirety by its own `skipif(not vm_manager.cluster_mode)` guard. The only pure unit tests in the repository never actually ran. Register stub `rados` and `rbd` modules from tests/conftest.py, before vm_manager is imported, when the real bindings are absent. The stubs only satisfy the import: instantiating one raises, so a test that reaches real Ceph code fails loudly rather than passing against a fake. On a machine with Ceph the genuine bindings are found and nothing is stubbed. CI now collects 36 tests instead of 32 plus 4 skipped, and cluster-side code that needs no cluster (argparse, XML building, crm command construction) becomes reachable by future unit tests. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
The OpenSSF gold criteria test_statement_coverage90 and test_branch_coverage80 both require a measured number, and nothing in this repository measured one: no pytest-cov, no coverage configuration, no CI step. Add pytest-cov and coverage[toml] to the `test` extra, enable branch coverage over the vm_manager package (excluding the hand-driven integration scripts under vm_manager/helpers/tests/), and have CI print a summary table in the job summary and publish coverage.xml plus the HTML report as an artifact. No exclusions and no fail_under for now: the point of this change is to get an honest baseline before deciding where to set thresholds. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
Cover the argparse layer and the main() dispatch table with every
vm_manager entry point replaced by a recorder, so nothing here needs
libvirt, Ceph or Pacemaker: the ParseMetaData action, the registered
subcommands and their required arguments, and the function plus
arguments main() forwards each command to.
Statement coverage of vm_manager_cmd.py goes from 39% to 92%. What is
left uncovered is the standalone-mode half of get_parser(), which cannot
be reached while the tests run in cluster mode.
Three tests are marked xfail(strict) because they document two real
bugs found while writing them:
- `create --disable` and `clone --disable` do not disable anything.
main() only assigns args.enable under `if "enable" in args`, but
`enable` is never an argparse dest for those subcommands, so the key
never reaches the backend, and _configure_vm() treats a missing
'enable' key as True.
- `create --enable-live-migration` is a no-op. main() guards the
rename with `if "live_migration" in args`, and `live_migration` is
likewise never a dest, so _configure_vm() never writes the
_live_migration metadata.
add-to-cluster assigns both unconditionally and behaves correctly; the
tests covering it are the reference for what create and clone should do.
Coverage confirms the diagnosis: lines 542, 547 and 550 of
vm_manager_cmd.py are unreachable.
The markers are strict, so whoever fixes the guards will get an XPASS
failure telling them to drop the marker.
Signed-off-by: Florent Carli <florent.carli@rte-france.com>
…alysis The project runs SonarCloud Automatic Analysis (autoscanEnabled=true, ciName=Autoscan): the SonarCloud GitHub App analyses every push server-side, with no scanner in the repository. That mode cannot work for what this branch is about, for two reasons. Automatic Analysis never runs the build or the tests, so it cannot import a coverage report. SonarCloud has therefore never had coverage data for this project: the `coverage` metric is absent on main and `new_lines_to_cover` is 0 on pull requests. The coverage.xml produced by CI has nowhere to go. It also has no git history to blame lines with, so on a pull request every line of a touched file counts as new code. On PR #94 that is visible on .github/workflows/ci.yml line 54: the line is identical to main, at the same line number, and SonarCloud dates the issue on it 2026-02-19, yet it lands in the new-code period and fails the new_security_rating gate. main itself is green with the same four findings. Run the scanner from the test job instead, after the tests, with fetch-depth: 0 so blame works, and declare the coverage report path in sonar-project.properties. Scope stays the whole repository as it was under Automatic Analysis, so the workflow, Dockerfile and XML analysers keep reporting. Requires two manual steps before this can work: turn Automatic Analysis off in the SonarCloud project settings, and add the SONAR_TOKEN repository secret. SonarCloud rejects a CI analysis while Automatic Analysis is enabled, so the order matters. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
|
Member
Author
|
Closing: this change has moved into #94. This branch was opened while Automatic Analysis was still enabled, to show Its three leading commits were older copies of #94's, which is why this Nothing here is lost, and nothing needs to be merged from this branch. |
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
1 participant
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.
Draft, opened to test the freshly rotated
SONAR_TOKEN. Not for review yet.Stacked on #94: the first three commits are that pull request, only
ci: analyse with SonarCloud from the workflow instead of Automatic Analysisis new here.
Automatic Analysis never runs the build or the tests, so it can never import
a coverage report, and it has no git blame data, so every line of a touched
file counts as new code. This replaces it with an analysis launched from the
workflow after the tests, with
fetch-depth: 0and asonar-project.propertiesdeclaringcoverage.xml.What this run is meant to tell us, from the
SonarCloud analysisstep:403 Forbiddenonapi.sonarcloud.io/analysis/jresmeans the token isstill refused, so the account behind it lacks Execute Analysis.
token works and only the project setting is left.
Merging requires Automatic Analysis to be turned off first, otherwise
SonarCloud rejects the CI analysis.