Skip to content

Commit 6a5e509

Browse files
joaodinissfclaude
andcommitted
ci: fast static analysis with early-fail lint + inline SARIF
Run lint and SpotBugs in independent lanes with compilation and the shared Maven dependency cache. Keep maven-verify responsible for the build and tests without repeating static analysis. Upload one merged SARIF run per analyzer for source annotations, with upload failures remaining nonblocking. Require a single usable SARIF document from every expected source-bearing module before merging. Reject absent, empty, malformed, or incomplete reports, unsuccessful invocations, and error-level execution/configuration notifications. Apply the same checks to SpotBugs, PMD, and Checkstyle so Maven --fail-never cannot turn a reported analysis failure into a clean lane. Count findings only after validating per-module and merged reports; parsing failures are fatal. Preserve originalUriBaseIds and prefix each input run's keys and references. This retains chained URI resolution and module source roots without rewriting URI paths or depending on runner checkout locations. Union driver rule descriptors by id and discard result indexes tied to the original rule arrays. Use the expected-module list for merging and CPD counting, excluding aggregator XML and unrelated reports. Count CPD duplications directly with awk so a single report is handled correctly and input errors fail the gate. Validate extracted workflow blocks with 58 local cases covering clean output, findings, missing/corrupt reports, independent analysis-failure signals, multiple runs, corrupted merged output, module scope, URI-base collisions, rule retention, and CPD controls. Include real configured SpotBugs/PMD error reports and a real SpotBugs 4.10.4 finding report. Check YAML and shell syntax separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 697eaf3 commit 6a5e509

4 files changed

Lines changed: 412 additions & 9 deletions

File tree

‎.github/scripts/sarif.sh‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/usr/bin/env bash
2+
3+
sarif_source_modules() {
4+
grep -oE '<module>\.\./[^<]+</module>' ddk-parent/pom.xml \
5+
| sed -E 's#.*\.\./([^<]+)</module>#\1#' \
6+
| while IFS= read -r module; do
7+
if [ -f "$module/META-INF/MANIFEST.MF" ] && [ -d "$module/src" ]; then
8+
echo "$module"
9+
fi
10+
done
11+
}
12+
13+
validate_sarif() {
14+
local report=$1
15+
if ! jq -se '
16+
def optional_array($key): (has($key) | not) or (.[$key] | type == "array");
17+
length == 1 and (.[0] |
18+
type == "object" and .version == "2.1.0"
19+
and (.runs | type == "array" and length > 0)
20+
and all(.runs[];
21+
(.tool.driver.name | type == "string" and length > 0)
22+
and (.results | type == "array")
23+
and optional_array("invocations")
24+
and all(.results[]?;
25+
type == "object" and (.message | type == "object"))
26+
and all(.invocations[]?;
27+
type == "object"
28+
and .executionSuccessful == true
29+
and optional_array("toolExecutionNotifications")
30+
and optional_array("toolConfigurationNotifications")
31+
and all(.toolExecutionNotifications[]?, .toolConfigurationNotifications[]?;
32+
type == "object" and .level != "error"))))
33+
' "$report" >/dev/null; then
34+
echo "::error::${report} is missing, invalid SARIF, or reports unsuccessful analysis."
35+
return 1
36+
fi
37+
}
38+
39+
merge_sarif() {
40+
local report=$1 output=$2 modules=$3 module
41+
local inputs=()
42+
for module in $modules; do
43+
validate_sarif "${module}/target/${report}" || return 1
44+
inputs+=("${module}/target/${report}")
45+
done
46+
if [ ${#inputs[@]} -eq 0 ]; then
47+
echo "::error::No expected modules supplied for ${report}."
48+
return 1
49+
fi
50+
mkdir -p "$(dirname "$output")"
51+
jq -s '
52+
def namespace_bases($prefix):
53+
walk(if type == "object" and has("uriBaseId")
54+
then .uriBaseId = ($prefix + .uriBaseId) else . end)
55+
| .originalUriBaseIds = ((.originalUriBaseIds // {})
56+
| with_entries(.key = ($prefix + .key)));
57+
[.[].runs[]] | to_entries
58+
| map(.key as $index | .value | namespace_bases("run\($index)_")) as $runs
59+
| {
60+
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
61+
version: "2.1.0",
62+
runs: [{
63+
tool: ($runs[0].tool | .driver.rules = ([$runs[].tool.driver.rules[]?] | unique_by(.id))),
64+
results: [$runs[].results[]? | del(.ruleIndex)],
65+
invocations: [$runs[].invocations[]?],
66+
originalUriBaseIds: (reduce $runs[] as $run ({}; . + $run.originalUriBaseIds))
67+
}]
68+
}
69+
' "${inputs[@]}" > "$output"
70+
}

‎.github/workflows/verify.yml‎

Lines changed: 169 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,29 @@
11
name: verify
22
on:
33
pull_request:
4+
5+
# Code Scanning needs write access to upload SARIF results for inline annotations.
6+
permissions:
7+
contents: read
8+
security-events: write
9+
410
jobs:
5-
pmd:
11+
# Fast, early-fail lint lane: PMD + Checkstyle (+ CPD). Turns red in a few
12+
# minutes on any violation, independent of the long build below, so a stray
13+
# PMD/Checkstyle issue is reported immediately rather than after `verify`.
14+
#
15+
# `compile` is in the same invocation as the analysis goals: PMD's
16+
# type-resolving rules (e.g. InvalidLogMessageFormat on the SLF4J
17+
# trailing-Throwable idiom) need Tycho's aux-classpath, which a fresh `mvn`
18+
# does not inherit from a prior step's target/classes.
19+
#
20+
# `--fail-never` lets every module produce its report (no Maven cascade-skip),
21+
# so the uploaded SARIF — and therefore the inline annotations — are complete.
22+
# The trade-off: --fail-never suppresses even compile and target-resolution
23+
# failures (mvn exits 0 on a broken build), so each expected SARIF report is
24+
# validated for successful analysis before findings are counted.
25+
# Compilation itself is independently gated by maven-verify.
26+
lint:
627
runs-on: ubuntu-24.04
728
steps:
829
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
@@ -12,10 +33,96 @@ jobs:
1233
java-version: '21'
1334
- name: Set up Workspace Environment Variable
1435
run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV
15-
- name: PMD Check
16-
run: mvn pmd:pmd pmd:cpd pmd:check pmd:cpd-check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
17-
checkstyle:
36+
- name: Restore Maven dependency cache
37+
# Restore-only, mirroring snapshot.yml's producer cache exactly (path and
38+
# key are hashed into the cache version — see the maven-verify step).
39+
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
40+
with:
41+
path: ~/.m2/repository
42+
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
43+
restore-keys: ${{ runner.os }}-maven-publish-
44+
45+
- name: PMD + Checkstyle reports (SARIF)
46+
# PMD: SarifRenderer FQCN — emits pmd.sarif.json AND keeps pmd.xml.
47+
# Checkstyle: output.format=sarif — SARIF content in checkstyle-result.xml.
48+
# CPD is excluded here: the global -Dformat flag uses PMD's Renderer
49+
# hierarchy and would ClassCastException CPD's CPDReportRenderer.
50+
run: |
51+
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
52+
compile \
53+
pmd:pmd checkstyle:checkstyle \
54+
-Dformat=net.sourceforge.pmd.renderers.SarifRenderer \
55+
-Dcheckstyle.output.format=sarif
56+
57+
- name: CPD report (separate invocation — no SARIF support)
58+
# CPD has no SARIF renderer; emits cpd.xml only. Run standalone so the
59+
# PMD -Dformat flag isn't in scope.
60+
# NOTE: the CPD token threshold is governed by pmd.cpd.min in
61+
# ddk-parent/pom.xml.
62+
run: |
63+
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
64+
compile \
65+
pmd:cpd-check
66+
67+
- name: Merge per-module SARIFs (PMD + Checkstyle)
68+
if: always()
69+
# Merge only expected module reports into one run per analyzer.
70+
run: |
71+
set -euo pipefail
72+
source .github/scripts/sarif.sh
73+
source_modules=$(sarif_source_modules)
74+
merge_sarif pmd.sarif.json .sarif-merged/pmd.sarif "${LINT_EXPECT_REPORTS:-$source_modules}"
75+
merge_sarif checkstyle-result.xml .sarif-merged/checkstyle.sarif "${LINT_EXPECT_REPORTS:-$source_modules}"
76+
77+
- name: Gate on PMD / CPD / Checkstyle violations
78+
# Require successful reports from every expected module before counting findings.
79+
run: |
80+
set -euo pipefail
81+
source .github/scripts/sarif.sh
82+
source_modules=$(sarif_source_modules)
83+
cpd_reports=()
84+
for mod in ${LINT_EXPECT_REPORTS:-$source_modules}; do
85+
validate_sarif "${mod}/target/pmd.sarif.json"
86+
validate_sarif "${mod}/target/checkstyle-result.xml"
87+
if [ ! -s "${mod}/target/cpd.xml" ]; then
88+
echo "::error::${mod} produced no cpd.xml — a build failure was swallowed by --fail-never."
89+
exit 1
90+
fi
91+
cpd_reports+=("${mod}/target/cpd.xml")
92+
done
93+
for f in .sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif; do
94+
validate_sarif "$f"
95+
done
96+
if [ ${#cpd_reports[@]} -eq 0 ]; then
97+
echo "::error::No expected CPD reports — the analysis silently failed."
98+
exit 1
99+
fi
100+
sarif_total=$(jq -s '[.[].runs[].results[]] | length' \
101+
.sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif)
102+
cpd_total=$(awk '/<duplication / {total++} END {print total+0}' "${cpd_reports[@]}")
103+
echo "PMD/Checkstyle SARIF violations: $sarif_total"
104+
echo "CPD duplications: $cpd_total"
105+
if [ "$sarif_total" != "0" ] || [ "$cpd_total" != "0" ]; then
106+
echo "::error::Static analysis found violations (PMD/CPD/Checkstyle)."
107+
exit 1
108+
fi
109+
110+
- name: Upload PMD/Checkstyle SARIF to Code Scanning
111+
if: always()
112+
# Annotation-only, never the gate: a fork PR gets a read-only token and
113+
# upload-sarif 403s, which must not red an otherwise-clean lane.
114+
continue-on-error: true
115+
uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4
116+
with:
117+
sarif_file: .sarif-merged
118+
category: lint
119+
120+
# SpotBugs is the slow critical-path analysis (the experiments' durable
121+
# finding), so it runs in its own parallel lane and never delays `lint`.
122+
spotbugs:
18123
runs-on: ubuntu-24.04
124+
env:
125+
MAVEN_OPTS: -Xmx4g
19126
steps:
20127
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
21128
- uses: actions/setup-java@de7274f081f381c8f8158605e0321c36c376e2e6 # v6.0.1
@@ -24,14 +131,69 @@ jobs:
24131
java-version: '21'
25132
- name: Set up Workspace Environment Variable
26133
run: echo "WORKSPACE=${{ github.workspace }}" >> $GITHUB_ENV
27-
- name: Checkstyle Check
28-
run: mvn checkstyle:checkstyle checkstyle:check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
134+
- name: Restore Maven dependency cache
135+
# Restore-only, mirroring snapshot.yml's producer cache exactly (path and
136+
# key are hashed into the cache version — see the maven-verify step).
137+
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
138+
with:
139+
path: ~/.m2/repository
140+
key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
141+
restore-keys: ${{ runner.os }}-maven-publish-
142+
143+
- name: SpotBugs report (SARIF)
144+
# sarifOutput=true emits spotbugsSarif.json (also writes spotbugsXml.xml).
145+
run: |
146+
mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
147+
compile \
148+
spotbugs:spotbugs \
149+
-Dspotbugs.sarifOutput=true
150+
151+
- name: Merge per-module SpotBugs SARIFs
152+
if: always()
153+
run: |
154+
set -euo pipefail
155+
source .github/scripts/sarif.sh
156+
source_modules=$(sarif_source_modules)
157+
merge_sarif spotbugsSarif.json .sarif-merged/spotbugs.sarif "${SPOTBUGS_EXPECT_REPORTS:-$source_modules}"
158+
159+
- name: Gate on SpotBugs violations
160+
# Require successful reports from every expected module before counting findings.
161+
run: |
162+
set -euo pipefail
163+
source .github/scripts/sarif.sh
164+
source_modules=$(sarif_source_modules)
165+
for mod in ${SPOTBUGS_EXPECT_REPORTS:-$source_modules}; do
166+
validate_sarif "${mod}/target/spotbugsSarif.json"
167+
done
168+
validate_sarif .sarif-merged/spotbugs.sarif
169+
sb_total=$(jq '[.runs[].results[]] | length' .sarif-merged/spotbugs.sarif)
170+
echo "SpotBugs SARIF violations: $sb_total"
171+
if [ "$sb_total" != "0" ]; then
172+
echo "::error::SpotBugs found violations."
173+
exit 1
174+
fi
175+
176+
- name: Upload SpotBugs SARIF to Code Scanning
177+
if: always()
178+
# Annotation-only, never the gate: a fork PR gets a read-only token and
179+
# upload-sarif 403s, which must not red an otherwise-clean lane.
180+
continue-on-error: true
181+
uses: github/codeql-action/upload-sarif@7fd177fa680c9881b53cdab4d346d32574c9f7f4 # v3.35.4
182+
with:
183+
sarif_file: .sarif-merged
184+
category: spotbugs
185+
29186
line-endings:
30187
runs-on: ubuntu-24.04
31188
steps:
32189
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
33190
- name: Check LF line endings
34191
run: bash .github/scripts/check-line-endings.sh
192+
193+
# Build + tests only. Static analysis now lives in the `lint` and `spotbugs`
194+
# jobs, so the redundant checkstyle/pmd/spotbugs goals are dropped from here —
195+
# this is the wall-clock long pole and no longer re-runs analysis.
196+
# No `-T 2C`: tests are not known to pass reliably under reactor parallelism.
35197
maven-verify:
36198
runs-on: ubuntu-24.04
37199
steps:
@@ -74,9 +236,7 @@ jobs:
74236
# all downloaded artifacts cached.
75237
run: rm -rf ~/.m2/repository/.cache/tycho/https/dsldevkit.github.io ~/.m2/repository/.cache/tycho/https/ddk.tools.avaloq.com
76238
- name: Build with Maven within a virtual X Server Environment
77-
# Run pmd:pmd and pmd:cpd first to generate reports for all modules, then run pmd:check and pmd:cpd-check
78-
# This ensures all violations are collected and reported before the build fails
79-
run: xvfb-run mvn clean verify checkstyle:check pmd:pmd pmd:cpd pmd:check pmd:cpd-check spotbugs:check -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
239+
run: xvfb-run mvn clean verify -f ./ddk-parent/pom.xml --batch-mode --fail-at-end
80240
- name: Fail on missing surefire reports
81241
if: always()
82242
run: bash .github/scripts/check-surefire-reports.sh

‎docs/ci-measurement-protocol.md‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# CI timing-measurement protocol
2+
3+
How to get **trustworthy** numbers for a CI-shape change. Written because the
4+
earlier experiment program (2026-05) produced numbers that can't be trusted:
5+
single samples, taken during an Eclipse p2 mirror-flaky window, against a CI
6+
shape that has since changed. Don't repeat that.
7+
8+
## What we're measuring
9+
10+
- **Wall-clock** per run = the slowest job (what a developer waits for).
11+
- **Per-job duration** = the analysis bottleneck (settles e.g. spotbugs-vs-maven-verify).
12+
- **Failure latency** = time-to-red on a *planted* lint violation — the metric the
13+
early-fail goal actually cares about. Measure it separately.
14+
15+
Wall-clock is the headline, but per-job + failure-latency are what tell you *why*.
16+
17+
## Noise floor (observed on this repo's `verify` runs)
18+
19+
Per-job spread across recent runs — any change must beat this to be real signal:
20+
21+
| Job | median | spread (±) |
22+
|---|---|---|
23+
| line-endings | ~5s | ±3s |
24+
| checkstyle (old shape) | ~115s | ±~104s (≈90%!) |
25+
| pmd (old shape) | ~191s | ±63s |
26+
| maven-verify | ~869s (~14.5m) | ±~93s (≈11%) |
27+
28+
**Decision rule:** accept candidate B as faster than A only if
29+
`median(B) < median(A) − 2 × IQR(A)`. For maven-verify a real win must exceed ~100s;
30+
for checkstyle, ~200s. Anything smaller is noise.
31+
32+
## Protocol
33+
34+
1. **Pre-flight — confirm mirrors are healthy.** Run one throwaway build; if
35+
target-platform resolution stalls or errors, **stop** — the window is bad
36+
(this is what voided the 2026-05-09 numbers). Measure only on a clean window.
37+
2. **Warm the caches.** Run 3–5 discard builds first so `~/.m2` + `.cache/tycho`
38+
are populated; cold-cache runs have a different (network-bound) profile.
39+
3. **Sample.** N = 15–20 `workflow_dispatch` runs per candidate, back-to-back in a
40+
≤30-minute window. Run A and B **interleaved within ≤10 minutes** of each other
41+
so they see the same mirror weather and runner-pool load.
42+
4. **Report medians + IQR**, per job *and* total wall-clock. Never a single sample,
43+
never the mean.
44+
5. **Pin `ubuntu-24.04`** (not a matrix) for consistent runner hardware.
45+
6. **Record per-sample metadata:** cache hit/miss, run start time, headSha.
46+
7. **Failure latency:** plant a synthetic PMD/Checkstyle violation, measure how long
47+
until the `lint` check goes red (independent of the build job).
48+
49+
## Why not just trust the old experiment numbers
50+
51+
- Single sample each (no repetition).
52+
- 2026-05-09 mirror-flaky window → resolution time is contaminated and varies per job
53+
even within one dispatch.
54+
- Numbers disagree across rounds (sequential measured 21m one round, 33m another).
55+
- `#1369` reshaped master's CI after the experiments ran, so their baseline is stale.
56+
57+
## What is *not* a timing lever (validated)
58+
59+
- **Local p2 mirror**: with a warm `~/.m2`/`.cache/tycho`, offline resolution is
60+
~equal to online (measured 5s vs 6s) — a mirror's steady-state speedup is ~0.
61+
Its only value is cold-cache + flaky-mirror insurance; deferred per the rare-flake
62+
rule.
63+
- **`-Dtycho.mode=maven` on a gate pass**: marginal on a warm cache (resolution is
64+
already seconds); and it *breaks* PMD type-resolving rules (strips the classpath →
65+
false positives). Not used.

0 commit comments

Comments
 (0)