Skip to content

Commit 4585eeb

Browse files
committed
ci: add Scalpel shadow comparison for skip-tests mode validation
Add a shadow comparison section to CI PR comments showing what Maveniverse Scalpel's skip-tests mode would have tested — without affecting actual test execution. Changes: - incremental-build.sh: configure Scalpel with skipTestsForDownstreamModules and fetchBaseBranch=false, add writeScalpelComparison() for collapsible PR comment section with failure reporting - pr-build-main.yml / sonar-build.yml: add base branch fetch step for Scalpel's merge-base detection in shallow CI clones, restore checkout v7 - CI-ARCHITECTURE.md: document shadow comparison approach and configuration - Scalpel upgraded to 0.3.7: fixes inflated affectedModules count for parent POM property changes (scalpel#39) and skipTestsForDownstreamModules
1 parent 0a38c5e commit 4585eeb

5 files changed

Lines changed: 147 additions & 15 deletions

File tree

.github/CI-ARCHITECTURE.md

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,30 @@ Both methods run in parallel. Results are merged (union) before testing. This le
154154
2. **No regression** — If Scalpel fails, grep results are still used
155155
3. **Gradual migration** — Once Scalpel is validated, grep can be removed
156156

157-
Scalpel is configured permanently in `.mvn/extensions.xml` (version `0.1.0`). On developer machines it is a no-op — without CI environment variables (`GITHUB_BASE_REF`), no base branch is detected and Scalpel returns immediately. The `mvn validate` with report mode adds ~60-90 seconds in CI.
158-
159-
Note: the script overrides `fullBuildTriggers` to empty (`-Dscalpel.fullBuildTriggers=`) because Scalpel's default (`.mvn/**`) would trigger a full build whenever `.mvn/extensions.xml` itself changes (e.g., Dependabot bumping Scalpel).
157+
Scalpel is configured permanently in `.mvn/extensions.xml`. On developer machines it is a no-op (disabled via `-Dscalpel.enabled=false` in `.mvn/maven.config`). The CI script overrides this with `-Dscalpel.enabled=true`. The `mvn validate` with report mode adds ~60-90 seconds in CI.
160158

161159
Scalpel is only invoked when a **subdirectory** `pom.xml` is changed (e.g. `parent/pom.xml`, `components/camel-kafka/pom.xml`). Changes to the **root** `pom.xml` are excluded because it contains build-infrastructure config (license plugin, checkstyle, etc.) that does not affect module compilation or test behavior. Without this filter, Scalpel would report every module as affected since they all inherit from the root POM.
162160

161+
#### Scalpel features used for shadow comparison
162+
163+
- **Source-set-aware propagation**: Distinguishes test-jar dependencies from regular dependencies. A module that depends only on another module's test-jar (e.g., `camel-core`'s test-jar with test utilities) is propagated through the `TEST` source set, not the `MAIN` source set. This prevents a change to test utilities from triggering tests in all ~500 modules that depend on `camel-core`.
164+
- **`skipTestsForDownstreamModules`**: Allows specifying modules whose tests should be skipped when they appear as downstream dependents (mirrors the `EXCLUSION_LIST` in `incremental-build.sh`). This gives Scalpel an accurate picture of what skip-tests mode would actually test.
165+
166+
#### Shadow comparison
167+
168+
Scalpel runs in **shadow mode**: it observes what skip-tests mode *would* have done and reports it in a collapsible section of the PR comment, without affecting actual test execution. This allows the team to validate Scalpel's decisions across many PRs before switching to Scalpel-driven test execution.
169+
170+
The shadow comparison section shows:
171+
- How many modules Scalpel would test (direct + downstream)
172+
- How many downstream modules would have tests skipped (generated code, meta-modules)
173+
- The full list of modules in each category
174+
175+
#### Configuration notes
176+
177+
The script overrides `fullBuildTriggers` to empty (`-Dscalpel.fullBuildTriggers=`) because Scalpel's default (`.mvn/**`) would trigger a full build whenever `.mvn/extensions.xml` itself changes (e.g., Dependabot bumping Scalpel).
178+
179+
The grep-based script fetches the PR diff via the GitHub REST API (unchanged). Scalpel uses local git history to compare effective POM models — the CI workflow pre-fetches the base branch (`git fetch --deepen=200` + fetch of `origin/main`) so Scalpel's JGit can find the merge-base. Scalpel disables its built-in JGit fetch (`-Dscalpel.fetchBaseBranch=false`) to avoid JGit issues in shallow CI clones. The `--deepen=200` fetches only commit metadata (not file blobs), adding ~2-3 seconds to the job.
180+
163181
## Manual Integration Test Advisories
164182

165183
Some modules are excluded from CI's `-amd` expansion (the `EXCLUSION_LIST`) because they are generated code, meta-modules, or expensive integration test suites. When a contributor changes one of these modules, CI cannot automatically test all downstream effects.

.github/actions/incremental-build/incremental-build.sh

Lines changed: 114 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -243,19 +243,20 @@ analyzePomDependencies() {
243243
runScalpelDetection() {
244244
echo " Running Scalpel change detection..."
245245

246-
# Ensure sufficient git history for JGit merge-base detection
247-
# (CI uses shallow clones; Scalpel needs to find the merge base)
248-
git fetch origin main:refs/remotes/origin/main --depth=200 2>/dev/null || true
249-
git fetch --deepen=200 2>/dev/null || true
250-
251246
# Scalpel is permanently configured in .mvn/extensions.xml.
252-
# On developer machines it's a no-op (no GITHUB_BASE_REF → no base branch detected).
247+
# On developer machines it's a no-op (disabled via -Dscalpel.enabled=false in .mvn/maven.config).
248+
# The CI script overrides this with -Dscalpel.enabled=true.
249+
# Base branch is pre-fetched by the CI workflow (fetchBaseBranch=false).
253250
# Run Maven validate with Scalpel in report mode:
254251
# - mode=report: write JSON report without trimming the reactor
255252
# - fullBuildTriggers="": override .mvn/** default (Scalpel lives in .mvn/extensions.xml)
256-
# - alsoMake/alsoMakeDependents=false: we only want directly affected modules
257-
# (our script handles -amd expansion separately)
258-
local scalpel_args="-Dscalpel.enabled=true -Dscalpel.mode=report -Dscalpel.fullBuildTriggers= -Dscalpel.alsoMake=false -Dscalpel.alsoMakeDependents=false"
253+
# - fetchBaseBranch=false: base branch is pre-fetched by the CI workflow
254+
# - skipTestsForDownstreamModules: derived from EXCLUSION_LIST — tells Scalpel which
255+
# downstream modules should not run tests in skip-tests mode (for shadow comparison)
256+
# Strip the Maven "!:" prefix from each entry to get bare artifact IDs for Scalpel.
257+
local skip_downstream
258+
skip_downstream=$(echo "$EXCLUSION_LIST" | sed 's/!://g')
259+
local scalpel_args="-Dscalpel.enabled=true -Dscalpel.mode=report -Dscalpel.fullBuildTriggers= -Dscalpel.fetchBaseBranch=false -Dscalpel.excludePaths=.github/** -Dscalpel.skipTestsForDownstreamModules=${skip_downstream}"
259260
# For workflow_dispatch, GITHUB_BASE_REF may not be set
260261
if [ -z "${GITHUB_BASE_REF:-}" ]; then
261262
scalpel_args="$scalpel_args -Dscalpel.baseBranch=origin/main"
@@ -265,6 +266,7 @@ runScalpelDetection() {
265266
./mvnw -B -q validate $scalpel_args ${MAVEN_EXTRA_ARGS:-} -l /tmp/scalpel-validate.log 2>/dev/null || {
266267
echo " WARNING: Scalpel detection failed (exit $?), skipping"
267268
grep -i "scalpel" /tmp/scalpel-validate.log 2>/dev/null | head -5 || true
269+
scalpel_failure_reason="Scalpel detection failed (mvn validate exited with error)"
268270
return
269271
}
270272

@@ -273,6 +275,7 @@ runScalpelDetection() {
273275
if [ ! -f "$report" ]; then
274276
echo " WARNING: Scalpel report not found at $report"
275277
grep -i "scalpel" /tmp/scalpel-validate.log 2>/dev/null | head -5 || true
278+
scalpel_failure_reason="Scalpel report not found (merge-base may be unreachable in shallow clone)"
276279
return
277280
fi
278281

@@ -283,6 +286,7 @@ runScalpelDetection() {
283286
local trigger_file
284287
trigger_file=$(jq -r '.triggerFile // "unknown"' "$report")
285288
echo " Scalpel: Full build triggered by change to $trigger_file"
289+
scalpel_failure_reason="Scalpel triggered a full build (changed file: $trigger_file)"
286290
return
287291
fi
288292

@@ -293,9 +297,24 @@ runScalpelDetection() {
293297
scalpel_managed_deps=$(jq -r '(.changedManagedDependencies // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true)
294298
scalpel_managed_plugins=$(jq -r '(.changedManagedPlugins // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true)
295299

300+
# Scalpel shadow comparison data:
301+
# - Modules Scalpel skip-tests mode would test (testsSkipped != true)
302+
# - Modules Scalpel would skip (testsSkipped == true, from skipTestsForDownstreamModules)
303+
# - Breakdown by category (DIRECT, DOWNSTREAM)
304+
scalpel_would_test=$(jq -r '[.affectedModules[] | select(.testsSkipped != true)] | map(.artifactId) | sort | join(",")' "$report" 2>/dev/null || true)
305+
scalpel_would_skip=$(jq -r '[.affectedModules[] | select(.testsSkipped == true)] | map(.artifactId) | sort | join(",")' "$report" 2>/dev/null || true)
306+
scalpel_direct_count=$(jq '[.affectedModules[] | select(.category == "DIRECT")] | length' "$report" 2>/dev/null || echo "0")
307+
scalpel_downstream_tested=$(jq '[.affectedModules[] | select(.category == "DOWNSTREAM" and .testsSkipped != true)] | length' "$report" 2>/dev/null || echo "0")
308+
scalpel_downstream_skipped=$(jq '[.affectedModules[] | select(.category == "DOWNSTREAM" and .testsSkipped == true)] | length' "$report" 2>/dev/null || echo "0")
309+
296310
local mod_count
297311
mod_count=$(jq '.affectedModules | length' "$report" 2>/dev/null || echo "0")
298-
echo " Scalpel detected $mod_count affected modules"
312+
local test_count=0
313+
if [ -n "$scalpel_would_test" ]; then
314+
test_count=$(echo "$scalpel_would_test" | tr ',' '\n' | grep -c . || true)
315+
fi
316+
echo " Scalpel detected $mod_count affected modules ($test_count would be tested)"
317+
echo " Direct: $scalpel_direct_count, Downstream tested: $scalpel_downstream_tested, Downstream skipped: $scalpel_downstream_skipped"
299318
if [ -n "$scalpel_props" ]; then
300319
echo " Changed properties: $scalpel_props"
301320
fi
@@ -382,6 +401,81 @@ checkManualItTests() {
382401
fi
383402
}
384403

404+
# ── Scalpel shadow comparison ──────────────────────────────────────────
405+
406+
# Write Scalpel shadow comparison section to the PR comment.
407+
# Shows what Scalpel skip-tests mode would have tested vs what the current
408+
# approach actually tested — observation only, does not affect test execution.
409+
writeScalpelComparison() {
410+
local comment_file="$1"
411+
412+
# If Scalpel failed, show why in the PR comment
413+
if [ -n "$scalpel_failure_reason" ]; then
414+
echo "" >> "$comment_file"
415+
echo "<details><summary>:microscope: Scalpel shadow comparison (skip-tests mode)</summary>" >> "$comment_file"
416+
echo "" >> "$comment_file"
417+
echo ":warning: $scalpel_failure_reason" >> "$comment_file"
418+
echo "" >> "$comment_file"
419+
echo "> :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)" >> "$comment_file"
420+
echo "" >> "$comment_file"
421+
echo "</details>" >> "$comment_file"
422+
return
423+
fi
424+
425+
# Skip if no Scalpel data (Scalpel was not invoked for this PR)
426+
if [ -z "$scalpel_would_test" ] && [ -z "$scalpel_would_skip" ]; then
427+
return
428+
fi
429+
430+
local scalpel_test_count=0
431+
local scalpel_skip_count=0
432+
if [ -n "$scalpel_would_test" ]; then
433+
scalpel_test_count=$(echo "$scalpel_would_test" | tr ',' '\n' | grep -c . || true)
434+
fi
435+
if [ -n "$scalpel_would_skip" ]; then
436+
scalpel_skip_count=$(echo "$scalpel_would_skip" | tr ',' '\n' | grep -c . || true)
437+
fi
438+
439+
echo "" >> "$comment_file"
440+
echo "<details><summary>:microscope: Scalpel shadow comparison (skip-tests mode)</summary>" >> "$comment_file"
441+
echo "" >> "$comment_file"
442+
echo "**Scalpel skip-tests mode would test ${scalpel_test_count} modules** (${scalpel_direct_count} direct + ${scalpel_downstream_tested} downstream)" >> "$comment_file"
443+
444+
if [ "$scalpel_downstream_skipped" -gt 0 ]; then
445+
echo "" >> "$comment_file"
446+
echo "${scalpel_downstream_skipped} downstream module(s) would have tests skipped (generated code, meta-modules)" >> "$comment_file"
447+
fi
448+
449+
# Show which modules Scalpel would test
450+
if [ -n "$scalpel_would_test" ]; then
451+
echo "" >> "$comment_file"
452+
echo "<details><summary>Modules Scalpel would test (${scalpel_test_count})</summary>" >> "$comment_file"
453+
echo "" >> "$comment_file"
454+
echo "$scalpel_would_test" | tr ',' '\n' | while read -r m; do
455+
[ -n "$m" ] && echo "- \`$m\`" >> "$comment_file"
456+
done
457+
echo "" >> "$comment_file"
458+
echo "</details>" >> "$comment_file"
459+
fi
460+
461+
# Show which modules would have tests skipped
462+
if [ -n "$scalpel_would_skip" ]; then
463+
echo "" >> "$comment_file"
464+
echo "<details><summary>Modules with tests skipped (${scalpel_skip_count})</summary>" >> "$comment_file"
465+
echo "" >> "$comment_file"
466+
echo "$scalpel_would_skip" | tr ',' '\n' | while read -r m; do
467+
[ -n "$m" ] && echo "- \`$m\`" >> "$comment_file"
468+
done
469+
echo "" >> "$comment_file"
470+
echo "</details>" >> "$comment_file"
471+
fi
472+
473+
echo "" >> "$comment_file"
474+
echo "> :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)" >> "$comment_file"
475+
echo "" >> "$comment_file"
476+
echo "</details>" >> "$comment_file"
477+
}
478+
385479
# ── Comment generation ─────────────────────────────────────────────────
386480

387481
writeComment() {
@@ -539,6 +633,13 @@ main() {
539633
scalpel_props=""
540634
scalpel_managed_deps=""
541635
scalpel_managed_plugins=""
636+
# Scalpel shadow comparison data
637+
scalpel_would_test=""
638+
scalpel_would_skip=""
639+
scalpel_direct_count="0"
640+
scalpel_downstream_tested="0"
641+
scalpel_downstream_skipped="0"
642+
scalpel_failure_reason=""
542643

543644
# Step 2a: Grep-based detection (existing approach)
544645
if [ -n "$pom_files" ]; then
@@ -762,6 +863,9 @@ main() {
762863
local comment_file="incremental-test-comment.md"
763864
writeComment "$comment_file" "$pl" "$dep_module_ids" "$all_changed_props" "$testedDependents" "$extraModules" "$scalpel_managed_deps" "$scalpel_managed_plugins"
764865

866+
# Scalpel shadow comparison (observation only)
867+
writeScalpelComparison "$comment_file"
868+
765869
# Check for tests disabled in CI via @DisabledIfSystemProperty(named = "ci.env.name")
766870
local disabled_tests
767871
disabled_tests=$(detectDisabledTests "$final_pl")

.github/workflows/pr-build-main.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ jobs:
8080
with:
8181
persist-credentials: false
8282
ref: ${{ inputs.pr_ref || '' }}
83+
- name: Fetch base branch for Scalpel change detection
84+
if: ${{ !inputs.skip_full_build }}
85+
run: |
86+
# Scalpel needs the merge base between HEAD and the base branch.
87+
# The checkout is depth=1, so deepen both sides for merge-base reachability.
88+
git fetch --deepen=200 2>/dev/null || true
89+
git fetch --no-tags --depth=200 origin "${GITHUB_BASE_REF:-main}:refs/remotes/origin/${GITHUB_BASE_REF:-main}"
8390
- id: install-packages
8491
uses: ./.github/actions/install-packages
8592
- id: install-mvnd

.github/workflows/sonar-build.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ jobs:
4545
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
4646
with:
4747
persist-credentials: false
48-
48+
- name: Fetch base branch for Scalpel change detection
49+
run: |
50+
git fetch --deepen=200 2>/dev/null || true
51+
git fetch --no-tags --depth=200 origin "${GITHUB_BASE_REF:-main}:refs/remotes/origin/${GITHUB_BASE_REF:-main}"
4952
- id: install-packages
5053
uses: ./.github/actions/install-packages
5154

.mvn/extensions.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@
2323
<extension>
2424
<groupId>eu.maveniverse.maven.scalpel</groupId>
2525
<artifactId>extension3</artifactId>
26-
<version>0.3.5</version>
26+
<version>0.3.7</version>
2727
</extension>
2828
</extensions>

0 commit comments

Comments
 (0)