fix(ci): stop pull request scans overwriting the dashboard's picture of main (#197)
SonarQube Community has no branch analysis. Every scan published under a project key replaces that project's single analysis, whatever revision it came from — so each pull request, and each push to one, overwrote the dashboard's analysis of main with the branch. The new-code period, the gate result, the coverage percentages and the hotspot list then all described whatever was scanned last, with nothing on the dashboard saying which revision that was. A gate that went green on a feature branch read exactly like a gate that went green on main. It was caught only by luck: #180's hotspots reported line numbers that landed on a comment and a blank line in main, which is the kind of nonsense a person notices. Everything else it misreported would have looked fine. scripts/scan-local.sh has always refused to do this, defaulting to a scratch key, and its header says why in as many words. CI walked into the hazard that script guards against. Now the two tell the same story. The suites still run on pull requests, which is where their value is — only publishing is restricted. The measures report is skipped alongside the scan, because with nothing published it would print main's numbers into a pull request's log, which is noise at best and misread as the branch's own at worst. A test asserts both steps carry the restriction and that the three suites do not, because the failure leaves no trace and the `if:` is one line for somebody to drop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -200,8 +200,25 @@ jobs:
|
||||
# sonar.qualitygate.wait, so a degraded run marks the dashboard and is
|
||||
# overwritten by the next good one rather than blocking anything; and the
|
||||
# job fails regardless, so no run in this state reads as clean.
|
||||
# Not on pull requests. SonarQube Community has no branch analysis: every
|
||||
# scan published under a project key replaces that project's single
|
||||
# analysis, whatever revision it came from. So a pull request scan
|
||||
# overwrote the dashboard's picture of main with the branch, silently, and
|
||||
# the new-code period, gate result, coverage and hotspot list then all
|
||||
# described whatever was scanned last with nothing saying which revision
|
||||
# that was. #197 caught it in the act — the dashboard describing a feature
|
||||
# branch while reporting hotspot line numbers that landed on a blank line
|
||||
# in main.
|
||||
#
|
||||
# scripts/scan-local.sh already refuses to do this, defaulting to a
|
||||
# scratch key for exactly this reason. CI walked into the hazard that
|
||||
# script guards against; now the two tell the same story.
|
||||
#
|
||||
# The suites above still run on pull requests, which is where their value
|
||||
# is. Only publishing is restricted.
|
||||
- name: SonarQube Scan
|
||||
id: scan
|
||||
if: github.event_name != 'pull_request'
|
||||
continue-on-error: true
|
||||
uses: sonarsource/sonarqube-scan-action@v4
|
||||
env:
|
||||
@@ -238,8 +255,11 @@ jobs:
|
||||
# every path, so it cannot fail the job anyway, and guarding it would
|
||||
# oblige it to appear in the gate below — which exists to fail the job,
|
||||
# the opposite of what a report should do. See #261.
|
||||
# Skipped alongside the scan on pull requests. With nothing published, this
|
||||
# would report main's numbers under a pull request's log, which is noise
|
||||
# at best and misread as the branch's own at worst.
|
||||
- name: Report SonarQube measures
|
||||
if: always()
|
||||
if: always() && github.event_name != 'pull_request'
|
||||
env:
|
||||
SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }}
|
||||
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
|
||||
|
||||
@@ -29,6 +29,7 @@ interface Step {
|
||||
name: string;
|
||||
id: string | null;
|
||||
guarded: boolean;
|
||||
condition: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -51,7 +52,7 @@ function parseSteps(source: string): Step[] {
|
||||
for (const line of source.split(/\r?\n/)) {
|
||||
const named = /^ {6}- name: (.+?)\s*$/.exec(line);
|
||||
if (named?.[1] !== undefined) {
|
||||
current = { name: named[1], id: null, guarded: false };
|
||||
current = { name: named[1], id: null, guarded: false, condition: null };
|
||||
steps.push(current);
|
||||
continue;
|
||||
}
|
||||
@@ -61,6 +62,9 @@ function parseSteps(source: string): Step[] {
|
||||
if (id?.[1] !== undefined) current.id = id[1];
|
||||
|
||||
if (/^ {8}continue-on-error: true\s*$/.test(line)) current.guarded = true;
|
||||
|
||||
const condition = /^ {8}if: (.+?)\s*$/.exec(line);
|
||||
if (condition?.[1] !== undefined) current.condition = condition[1];
|
||||
}
|
||||
|
||||
return steps;
|
||||
@@ -181,3 +185,35 @@ describe('sonarqube.yml fails at the end rather than part way through', () => {
|
||||
expect(unguarded.map((step) => step.name)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The guard for #197.
|
||||
*
|
||||
* SonarQube Community has no branch analysis: every scan published under a
|
||||
* project key replaces that project's single analysis, whatever revision it came
|
||||
* from. Publishing from a pull request therefore overwrites the dashboard's
|
||||
* picture of main with the branch — silently, because nothing on the dashboard
|
||||
* says which revision it describes. It was caught only because hotspot line
|
||||
* numbers landed on a blank line.
|
||||
*
|
||||
* scripts/scan-local.sh has always refused to do this, defaulting to a scratch
|
||||
* key. This asserts CI refuses too, because the failure leaves no trace and the
|
||||
* `if:` is one line for somebody to drop.
|
||||
*/
|
||||
describe('publishing is restricted to non-pull-request runs', () => {
|
||||
it.each(['SonarQube Scan', 'Report SonarQube measures'])('%s does not run on a pull request', (name) => {
|
||||
const step = steps.find((candidate) => candidate.name === name);
|
||||
|
||||
expect(step).toBeDefined();
|
||||
expect(step?.condition).toContain("github.event_name != 'pull_request'");
|
||||
});
|
||||
|
||||
// The suites are the reason pull requests run this workflow at all. Gating
|
||||
// them would turn a fix for a reporting problem into a loss of every check.
|
||||
it.each(['unit', 'integration', 'e2e'])('the %s suite still runs on pull requests', (id) => {
|
||||
const step = steps.find((candidate) => candidate.id === id);
|
||||
|
||||
expect(step).toBeDefined();
|
||||
expect(step?.condition ?? '').not.toContain('pull_request');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user