From 8bc5949c7f30a64f5af567c6f694b05db2dbbea4 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 10 Jul 2026 11:48:10 +0000 Subject: [PATCH] ci: switch POM dependency detection from grep+Scalpel dual to Scalpel-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the grep-based POM dependency detection and the shadow comparison infrastructure, making Maveniverse Scalpel the sole mechanism for detecting modules affected by POM changes. The grep approach had known limitations that Scalpel resolves: - Managed deps without explicit were missed - Plugin version changes were invisible to child modules - BOM imports were not linked to version properties - Transitive dependency changes were not detected - Non-property version changes in were not caught Scalpel has been running in shadow mode alongside grep since April 2026, validating its detection across hundreds of PRs. The dual approach was designed as a gradual migration path (grep → dual → Scalpel-only), and this completes the final step. This removes ~330 lines of grep detection code and shadow comparison infrastructure that is no longer needed. Co-Authored-By: Claude Opus 4.6 --- .github/CI-ARCHITECTURE.md | 62 +-- .../incremental-build/incremental-build.sh | 402 ++---------------- 2 files changed, 49 insertions(+), 415 deletions(-) diff --git a/.github/CI-ARCHITECTURE.md b/.github/CI-ARCHITECTURE.md index 2e77007d1ad11..fd231a2221ef3 100644 --- a/.github/CI-ARCHITECTURE.md +++ b/.github/CI-ARCHITECTURE.md @@ -90,12 +90,10 @@ PR comment: /component-test kafka http The core test runner. Determines which modules to test using: 1. **File-path analysis**: Maps changed files to Maven modules -2. **POM dependency analysis** (dual detection): - - **Grep-based**: For `parent/pom.xml` changes, detects property changes and finds modules that explicitly reference the affected properties via `${property}` in their `pom.xml` files - - **Scalpel-based**: Uses [Maveniverse Scalpel](https://github.com/maveniverse/scalpel) (Maven extension) for effective POM model comparison — catches managed dependencies, plugin version changes, BOM imports, and transitive dependency impacts that the grep approach misses +2. **POM dependency analysis** via [Maveniverse Scalpel](https://github.com/maveniverse/scalpel): effective POM model comparison that detects changed properties, managed dependencies, plugin version changes, BOM imports, and transitive dependency impacts 3. **Extra modules**: Additional modules passed via `/component-test` -Both detection methods run in parallel. Their results are merged (union), deduplicated, and tested. If Scalpel fails (build error, runtime error), the script falls back to grep-only with no regression. +If Scalpel fails (merge-base unreachable, build error), the PR comment warns about the failure. File-path analysis and `/component-test` modules are always available as fallback. The script also: @@ -123,63 +121,31 @@ Installs system packages required for the build. The CI sets `-Dci.env.name=github.com` via `MVND_OPTS` (in `install-mvnd`). Tests can use `@DisabledIfSystemProperty(named = "ci.env.name")` to skip flaky tests in CI. The test comment warns about these skipped tests. -## POM Dependency Detection: Dual Approach +## POM Dependency Detection via Scalpel -### Grep-based detection (legacy) +[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) is a Maven core extension that compares effective POM models between the base branch and the PR. It detects: -The grep approach searches for `${property-name}` references in module `pom.xml` files. It has known limitations: - -1. **Managed dependencies without explicit ``** — Modules inheriting versions via `` without declaring `${property}` are missed. -2. **Maven plugin version changes** — Plugin version properties consumed in `parent/pom.xml` via `` are invisible to child modules. -3. **BOM imports** — Modules using artifacts from a BOM are not linked to the BOM version property. -4. **Transitive dependency changes** — Only direct property references are detected. -5. **Non-property version changes** — Structural `` edits without property substitution are not caught. - -### Scalpel-based detection (new) - -[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) is a Maven core extension that compares effective POM models between the base branch and the PR. It resolves all 5 grep limitations by: - -- Reading old POM files from the merge-base commit (via JGit) -- Comparing properties, managed dependencies, and managed plugins between old and new POMs -- Resolving the full transitive dependency graph to find all affected modules -- Detecting plugin version changes via `project.getBuildPlugins()` comparison +- **Changed properties** — old/new `` comparison +- **Managed dependencies** — `` changes, including modules inheriting versions without explicit `${property}` +- **Managed plugins** — `` version changes +- **BOM imports** — modules using artifacts from a BOM are linked to the BOM version property +- **Transitive dependency impacts** — resolves the full dependency graph to find all affected modules Scalpel runs in **report mode** (`-Dscalpel.mode=report`), writing a JSON report to `target/scalpel-report.json` without modifying the Maven reactor. The report includes affected modules with reasons (`SOURCE_CHANGE`, `POM_CHANGE`, `TRANSITIVE_DEPENDENCY`, `MANAGED_PLUGIN`). -### Dual-detection strategy - -Both methods run in parallel. Results are merged (union) before testing. This lets us: - -1. **Validate Scalpel** — Compare what each method detects across many PRs -2. **No regression** — If Scalpel fails, grep results are still used -3. **Gradual migration** — Once Scalpel is validated, grep can be removed - 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. -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. - -#### Scalpel features used for shadow comparison +### Scalpel features - **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`. -- **`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. - -#### Shadow comparison - -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. -The shadow comparison section shows: -- How many modules Scalpel would test (direct + downstream) -- How many downstream modules would have tests skipped (generated code, meta-modules) -- Set differences: modules only Scalpel found vs modules only the current approach found -- The full list of modules in each category - -The comparison is apples-to-apples: the current approach's reactor is filtered through the `EXCLUSION_LIST` before comparing, so both sides exclude the same meta/generated modules (catalog, jbang, docs, etc.). - -#### Configuration notes +### Configuration notes 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). -The grep-based script fetches the PR diff via the GitHub REST API (unchanged). Scalpel uses local git history to compare effective POM models and detect source file changes — the CI workflow progressively deepens the shallow clone (`50 → 200 → 1000 → full`) until the merge-base is reachable. Scalpel runs for **all PRs** (not just POM changes) so the shadow comparison covers source-only changes too, building confidence for a future switch to Scalpel-driven builds. Scalpel disables its built-in JGit fetch (`-Dscalpel.fetchBaseBranch=false`) because the CI workflow already pre-fetches the base branch with native git (Scalpel uses JGit 5.x which has limited shallow clone support). +Root `pom.xml` changes are excluded via `-Dscalpel.excludePaths=.github/**` because it contains build-infrastructure config (license plugin, checkstyle, etc.) that does not affect module compilation or test behavior. + +Scalpel uses local git history to compare effective POM models — the CI workflow progressively deepens the shallow clone (`50 → 200 → 1000 → full`) until the merge-base is reachable. Scalpel disables its built-in JGit fetch (`-Dscalpel.fetchBaseBranch=false`) because the CI workflow already pre-fetches the base branch with native git (Scalpel uses JGit 5.x which has limited shallow clone support). ## Manual Integration Test Advisories diff --git a/.github/actions/incremental-build/incremental-build.sh b/.github/actions/incremental-build/incremental-build.sh index c5d0932d79a03..4190e4ef881be 100755 --- a/.github/actions/incremental-build/incremental-build.sh +++ b/.github/actions/incremental-build/incremental-build.sh @@ -19,14 +19,12 @@ # # Determines which modules to test by: # 1. File-path analysis: maps changed files to their Maven modules -# 2. POM dependency analysis (dual detection): -# a. Grep-based: detects property changes in parent/pom.xml and finds -# modules that explicitly reference the affected properties -# b. Scalpel-based: uses Maveniverse Scalpel extension for effective POM -# model comparison — catches managed deps, plugin changes, BOM imports, -# and transitive dependency impacts that grep misses +# 2. POM dependency analysis via Maveniverse Scalpel: uses effective POM +# model comparison to detect changed properties, managed dependencies, +# plugin changes, BOM imports, and transitive dependency impacts +# 3. Extra modules: additional modules passed via /component-test # -# All sets of affected modules are merged and deduplicated before testing. +# Affected modules from all sources are merged and deduplicated before testing. set -euo pipefail @@ -129,116 +127,15 @@ fetchDiff() { echo "$diff_body" } -# ── POM dependency analysis (previously detect-dependencies) ─────────── -# -# Uses the pre-#22022 approach: grep for ${property-name} references in -# module pom.xml files. The Maveniverse Toolbox approach (introduced in -# #22022, reverted in #22279) is intentionally not used here. See -# CI-ARCHITECTURE.md for known limitations of the grep approach. - -# Extract the diff section for a specific pom.xml file from the full diff -extractPomDiff() { - local diff_body="$1" - local pom_path="$2" - - echo "$diff_body" | awk -v target="a/${pom_path}" ' - /^diff --git/ && found { exit } - /^diff --git/ && index($0, target) { found=1 } - found { print } - ' -} - -# Detect which properties changed in a pom.xml diff. -# Returns one property name per line. -# Filters out structural XML elements (groupId, artifactId, version, etc.) -# to only return actual property names (e.g. openai-java-version). -detectChangedProperties() { - local diff_content="$1" - - # Known structural POM elements that are NOT property names - local structural_elements="groupId|artifactId|version|scope|type|classifier|optional|systemPath|exclusions|exclusion|dependency|dependencies|dependencyManagement|parent|modules|module|packaging|name|description|url|relativePath" - - echo "$diff_content" | \ - grep -E '^[+-][[:space:]]*<[^>]+>[^<]*]+>' | \ - grep -vE '^\+\+\+|^---' | \ - sed -E 's/^[+-][[:space:]]*<([^>]+)>.*/\1/' | \ - grep -vE "^(${structural_elements})$" | \ - sort -u || true -} - -# Find modules that reference a property in their pom.xml. -# Searches pom.xml files under catalog/, components/, core/, dsl/ for -# ${property_name} references and extracts the module's artifactId. -# Adds discovered artifactIds to the dep_module_ids variable -# (which must be declared in the caller). -findAffectedModules() { - local property="$1" - - local matches - matches=$(grep -rl "\${${property}}" --include="pom.xml" . 2>/dev/null | \ - grep -v "^\./parent/pom.xml" | \ - grep -v "/target/" || true) - - if [ -z "$matches" ]; then - return - fi - - while read -r pom_file; do - [ -z "$pom_file" ] && continue - - # Only consider catalog, components, core, dsl paths (same as original detect-test.sh) - if [[ "$pom_file" == */catalog/* ]] || \ - [[ "$pom_file" == */components/* ]] || \ - [[ "$pom_file" == */core/* ]] || \ - ([[ "$pom_file" == */dsl/* ]] && [[ "$pom_file" != */dsl/camel-jbang* ]]); then - local mod_artifact - mod_artifact=$(sed -n '//,/<\/parent>/!{ s/.*\([^<]*\)<\/artifactId>.*/\1/p }' "$pom_file" | head -1) - if [ -n "$mod_artifact" ] && ! echo ",$dep_module_ids," | grep -q ",:${mod_artifact},"; then - echo " Property '\${${property}}' referenced by: $mod_artifact" - dep_module_ids="${dep_module_ids:+${dep_module_ids},}:${mod_artifact}" - fi - fi - done <<< "$matches" -} - -# Analyze pom.xml changes to find affected modules via property grep. -# Adds discovered module artifactIds to the dep_module_ids variable -# (which must be declared in the caller). -analyzePomDependencies() { - local diff_body="$1" - local pom_path="$2" # e.g. "parent/pom.xml" or "components/camel-foo/pom.xml" - - local pom_diff - pom_diff=$(extractPomDiff "$diff_body" "$pom_path") - if [ -z "$pom_diff" ]; then - return - fi - - local changed_props - changed_props=$(detectChangedProperties "$pom_diff") - if [ -z "$changed_props" ]; then - return - fi - - echo " Property changes detected in ${pom_path}:" - echo "$changed_props" | while read -r p; do echo " - $p"; done - - while read -r prop; do - [ -z "$prop" ] && continue - findAffectedModules "$prop" - done <<< "$changed_props" -} - -# ── POM dependency analysis via Scalpel (parallel) ───────────────────── +# ── POM dependency analysis via Scalpel ──────────────────────────────── # # Uses Maveniverse Scalpel (Maven extension) for effective POM model # comparison. Detects changed properties, managed dependencies, managed -# plugins, and transitive dependency impacts that the grep approach misses. -# Runs alongside grep — results are merged (union) for testing. +# plugins, and transitive dependency impacts. # See https://github.com/maveniverse/scalpel # Run Scalpel in report mode to detect modules affected by POM changes. -# Sets caller-visible variables: scalpel_module_ids, scalpel_module_paths, +# Sets caller-visible variables: scalpel_module_ids, # scalpel_props, scalpel_managed_deps, scalpel_managed_plugins runScalpelDetection() { echo " Running Scalpel change detection..." @@ -251,15 +148,10 @@ runScalpelDetection() { # - mode=report: write JSON report without trimming the reactor # - fullBuildTriggers="": override .mvn/** default (Scalpel lives in .mvn/extensions.xml) # - fetchBaseBranch=false: base branch is pre-fetched by the CI workflow - # - skipTestsForDownstreamModules: derived from EXCLUSION_LIST — tells Scalpel which - # downstream modules should not run tests in skip-tests mode (for shadow comparison) - # Strip the Maven "!:" prefix from each entry to get bare artifact IDs for Scalpel. - local skip_downstream - skip_downstream=$(echo "$EXCLUSION_LIST" | sed 's/!://g') # Always pass baseBranch explicitly — relying on Scalpel's env.GITHUB_BASE_REF # auto-detection is fragile across Maven wrappers and CI rerun contexts. local base_branch="origin/${GITHUB_BASE_REF:-main}" - local scalpel_args="-Dscalpel.enabled=true -Dscalpel.mode=report -Dscalpel.fullBuildTriggers= -Dscalpel.fetchBaseBranch=false -Dscalpel.baseBranch=${base_branch} -Dscalpel.excludePaths=.github/** -Dscalpel.skipTestsForDownstreamModules=${skip_downstream}" + local scalpel_args="-Dscalpel.enabled=true -Dscalpel.mode=report -Dscalpel.fullBuildTriggers= -Dscalpel.fetchBaseBranch=false -Dscalpel.baseBranch=${base_branch} -Dscalpel.excludePaths=.github/**" # Verify merge base is reachable (pre-fetched by the CI workflow step) if ! git merge-base HEAD "${base_branch}" >/dev/null 2>&1; then @@ -302,29 +194,13 @@ runScalpelDetection() { # Extract affected module artifactIds (colon-prefixed for Maven -pl compatibility) scalpel_module_ids=$(jq -r '.affectedModules[].artifactId' "$report" 2>/dev/null | sort -u | sed 's/^/:/' | tr '\n' ',' | sed 's/,$//' || true) - scalpel_module_paths=$(jq -r '.affectedModules[].path' "$report" 2>/dev/null | sort -u | tr '\n' ',' | sed 's/,$//' || true) scalpel_props=$(jq -r '(.changedProperties // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true) scalpel_managed_deps=$(jq -r '(.changedManagedDependencies // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true) scalpel_managed_plugins=$(jq -r '(.changedManagedPlugins // []) | if length > 0 then join(", ") else "" end' "$report" 2>/dev/null || true) - # Scalpel shadow comparison data: - # - Modules Scalpel skip-tests mode would test (testsSkipped != true) - # - Modules Scalpel would skip (testsSkipped == true, from skipTestsForDownstreamModules) - # - Breakdown by category (DIRECT, DOWNSTREAM) - scalpel_would_test=$(jq -r '[.affectedModules[] | select(.testsSkipped != true)] | map(.artifactId) | sort | join(",")' "$report" 2>/dev/null || true) - scalpel_would_skip=$(jq -r '[.affectedModules[] | select(.testsSkipped == true)] | map(.artifactId) | sort | join(",")' "$report" 2>/dev/null || true) - scalpel_direct_count=$(jq '[.affectedModules[] | select(.category == "DIRECT")] | length' "$report" 2>/dev/null || echo "0") - scalpel_downstream_tested=$(jq '[.affectedModules[] | select(.category == "DOWNSTREAM" and .testsSkipped != true)] | length' "$report" 2>/dev/null || echo "0") - scalpel_downstream_skipped=$(jq '[.affectedModules[] | select(.category == "DOWNSTREAM" and .testsSkipped == true)] | length' "$report" 2>/dev/null || echo "0") - local mod_count mod_count=$(jq '.affectedModules | length' "$report" 2>/dev/null || echo "0") - local test_count=0 - if [ -n "$scalpel_would_test" ]; then - test_count=$(echo "$scalpel_would_test" | tr ',' '\n' | grep -c . || true) - fi - echo " Scalpel detected $mod_count affected modules ($test_count would be tested)" - echo " Direct: $scalpel_direct_count, Downstream tested: $scalpel_downstream_tested, Downstream skipped: $scalpel_downstream_skipped" + echo " Scalpel detected $mod_count affected modules" if [ -n "$scalpel_props" ]; then echo " Changed properties: $scalpel_props" fi @@ -411,160 +287,6 @@ checkManualItTests() { fi } -# ── Scalpel shadow comparison ────────────────────────────────────────── - -# Write Scalpel shadow comparison section to the PR comment. -# Shows what Scalpel would detect vs what the current approach actually tests, -# with a one-line diff summary. Observation only — does not affect test execution. -# Args: $1=comment_file, $2=tested_reactor_ids (newline-separated, already filtered by EXCLUSION_LIST) -writeScalpelComparison() { - local comment_file="$1" - local current_reactor_ids="${2:-}" - - # If Scalpel failed, show why in the PR comment - if [ -n "$scalpel_failure_reason" ]; then - echo "" >> "$comment_file" - echo "---" >> "$comment_file" - echo "" >> "$comment_file" - echo "
:microscope: Scalpel shadow comparison" >> "$comment_file" - echo "" >> "$comment_file" - echo ":warning: $scalpel_failure_reason" >> "$comment_file" - echo "" >> "$comment_file" - echo "> :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)" >> "$comment_file" - echo "" >> "$comment_file" - echo "
" >> "$comment_file" - return - fi - - # Skip if no Scalpel data (Scalpel was not invoked for this PR) - if [ -z "$scalpel_would_test" ] && [ -z "$scalpel_would_skip" ]; then - return - fi - - # Count Scalpel modules - local scalpel_total=0 - local scalpel_test_count=0 - local scalpel_skip_count=0 - if [ -n "$scalpel_module_ids" ]; then - scalpel_total=$(echo "$scalpel_module_ids" | tr ',' '\n' | grep -c . || true) - fi - if [ -n "$scalpel_would_test" ]; then - scalpel_test_count=$(echo "$scalpel_would_test" | tr ',' '\n' | grep -c . || true) - fi - if [ -n "$scalpel_would_skip" ]; then - scalpel_skip_count=$(echo "$scalpel_would_skip" | tr ',' '\n' | grep -c . || true) - fi - - # Compare Scalpel vs current reactor (file-path + grep + -amd expansion) - local current_total=0 - local current_sorted="" - if [ -n "$current_reactor_ids" ]; then - current_sorted=$(echo "$current_reactor_ids" | sort) - current_total=$(echo "$current_sorted" | grep -c . || true) - fi - local scalpel_sorted="" - if [ -n "$scalpel_module_ids" ]; then - scalpel_sorted=$(echo "$scalpel_module_ids" | tr ',' '\n' | sed 's/^://' | sort) - fi - - # Set differences: modules Scalpel found that current missed, and vice versa - local only_in_scalpel="" only_in_current="" - local only_scalpel_count=0 only_current_count=0 - if [ -n "$scalpel_sorted" ] && [ -n "$current_sorted" ]; then - only_in_scalpel=$(comm -23 <(echo "$scalpel_sorted") <(echo "$current_sorted") || true) - only_in_current=$(comm -13 <(echo "$scalpel_sorted") <(echo "$current_sorted") || true) - only_scalpel_count=$(echo "$only_in_scalpel" | grep -c . || true) - only_current_count=$(echo "$only_in_current" | grep -c . || true) - elif [ -n "$scalpel_sorted" ]; then - only_in_scalpel="$scalpel_sorted" - only_scalpel_count=$scalpel_total - elif [ -n "$current_sorted" ]; then - only_in_current="$current_sorted" - only_current_count=$current_total - fi - - # One-line summary: what Scalpel would change - local summary="Scalpel: ${scalpel_test_count} tested, ${scalpel_skip_count} compile-only — current: ${current_total} all tested" - - echo "" >> "$comment_file" - echo "---" >> "$comment_file" - echo "" >> "$comment_file" - echo "
:microscope: Scalpel shadow comparison — ${summary}" >> "$comment_file" - echo "" >> "$comment_file" - - echo "[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **${scalpel_total} affected modules** (current approach: ${current_total})." >> "$comment_file" - echo "" >> "$comment_file" - - # Show modules only Scalpel found (not in current reactor) - if [ "$only_scalpel_count" -gt 0 ]; then - echo "
:warning: Modules only in Scalpel (${only_scalpel_count})" >> "$comment_file" - echo "" >> "$comment_file" - echo "$only_in_scalpel" | while read -r m; do - [ -n "$m" ] && echo "- \`$m\`" >> "$comment_file" - done - echo "" >> "$comment_file" - echo "
" >> "$comment_file" - echo "" >> "$comment_file" - fi - - # Show modules only current approach found (not in Scalpel) - if [ "$only_current_count" -gt 0 ]; then - echo "
Modules only in current approach (${only_current_count})" >> "$comment_file" - echo "" >> "$comment_file" - echo "$only_in_current" | while read -r m; do - [ -n "$m" ] && echo "- \`$m\`" >> "$comment_file" - done - echo "" >> "$comment_file" - echo "
" >> "$comment_file" - echo "" >> "$comment_file" - fi - - # Show Scalpel-detected change details - if [ -n "$scalpel_props" ]; then - echo "Changed properties: ${scalpel_props}" >> "$comment_file" - echo "" >> "$comment_file" - fi - if [ -n "$scalpel_managed_deps" ]; then - echo "Changed managed dependencies: ${scalpel_managed_deps}" >> "$comment_file" - echo "" >> "$comment_file" - fi - if [ -n "$scalpel_managed_plugins" ]; then - echo "Changed managed plugins: ${scalpel_managed_plugins}" >> "$comment_file" - echo "" >> "$comment_file" - fi - - echo "**Skip-tests mode would test ${scalpel_test_count} modules** (${scalpel_direct_count} direct + ${scalpel_downstream_tested} downstream), **skip tests for ${scalpel_skip_count}** (generated code, meta-modules)" >> "$comment_file" - - # Show which modules Scalpel would test - if [ -n "$scalpel_would_test" ]; then - echo "" >> "$comment_file" - echo "
Modules Scalpel would test (${scalpel_test_count})" >> "$comment_file" - echo "" >> "$comment_file" - echo "$scalpel_would_test" | tr ',' '\n' | while read -r m; do - [ -n "$m" ] && echo "- \`$m\`" >> "$comment_file" - done - echo "" >> "$comment_file" - echo "
" >> "$comment_file" - fi - - # Show which modules would have tests skipped - if [ -n "$scalpel_would_skip" ]; then - echo "" >> "$comment_file" - echo "
Modules with tests skipped (${scalpel_skip_count})" >> "$comment_file" - echo "" >> "$comment_file" - echo "$scalpel_would_skip" | tr ',' '\n' | while read -r m; do - [ -n "$m" ] && echo "- \`$m\`" >> "$comment_file" - done - echo "" >> "$comment_file" - echo "
" >> "$comment_file" - fi - - echo "" >> "$comment_file" - echo "> :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)" >> "$comment_file" - echo "" >> "$comment_file" - echo "
" >> "$comment_file" -} - # ── Comment generation ───────────────────────────────────────────────── writeComment() { @@ -574,6 +296,8 @@ writeComment() { local changed_props_summary="$4" local testedDependents="$5" local extra_modules="$6" + local managed_deps_summary="${7:-}" + local managed_plugins_summary="${8:-}" echo "" > "$comment_file" @@ -591,15 +315,23 @@ writeComment() { fi fi - # Section 2: pom dependency-detected modules (grep-based only) + # Section 2: Scalpel-detected POM dependency modules if [ -n "$dep_ids" ]; then echo "" >> "$comment_file" - echo ":white_check_mark: **POM dependency changes: targeted tests included**" >> "$comment_file" + echo ":white_check_mark: **POM dependency changes detected by [Scalpel](https://github.com/maveniverse/scalpel): targeted tests included**" >> "$comment_file" echo "" >> "$comment_file" if [ -n "$changed_props_summary" ]; then echo "Changed properties: ${changed_props_summary}" >> "$comment_file" echo "" >> "$comment_file" fi + if [ -n "$managed_deps_summary" ]; then + echo "Changed managed dependencies: ${managed_deps_summary}" >> "$comment_file" + echo "" >> "$comment_file" + fi + if [ -n "$managed_plugins_summary" ]; then + echo "Changed managed plugins: ${managed_plugins_summary}" >> "$comment_file" + echo "" >> "$comment_file" + fi local dep_count dep_count=$(echo "$dep_ids" | tr ',' '\n' | wc -l | tr -d ' ') echo "
Modules affected by dependency changes (${dep_count})" >> "$comment_file" @@ -611,7 +343,13 @@ writeComment() { echo "
" >> "$comment_file" fi - # Section 3: extra modules (from /component-test) + # Section 3: Scalpel failure warning + if [ -n "${scalpel_failure_reason:-}" ]; then + echo "" >> "$comment_file" + echo ":warning: **Scalpel POM analysis unavailable**: $scalpel_failure_reason" >> "$comment_file" + fi + + # Section 4: extra modules (from /component-test) if [ -n "$extra_modules" ]; then echo "" >> "$comment_file" echo ":heavy_plus_sign: **Additional modules tested** (via \`/component-test\`):" >> "$comment_file" @@ -672,7 +410,6 @@ main() { local pl="" local lastProjectRoot="" local totalAffected=0 - local pom_files="" for project in ${projects}; do if [[ ${project} == */archetype-resources ]]; then @@ -692,81 +429,27 @@ main() { done pl="${pl:1}" # strip leading comma - # Only analyze parent/pom.xml for grep-based dependency detection - # (matches original detect-test.sh behavior) - if echo "$diff_body" | grep -q '^diff --git a/parent/pom.xml'; then - pom_files="parent/pom.xml" - fi - # ── Step 2: POM dependency analysis (dual: grep + Scalpel) ── - # Variables shared with analyzePomDependencies/findAffectedModules + # ── Step 2: POM dependency analysis via Scalpel ── local dep_module_ids="" local all_changed_props="" # Scalpel results (not local — set by runScalpelDetection) scalpel_module_ids="" - scalpel_module_paths="" scalpel_props="" scalpel_managed_deps="" scalpel_managed_plugins="" - # Scalpel shadow comparison data - scalpel_would_test="" - scalpel_would_skip="" - scalpel_direct_count="0" - scalpel_downstream_tested="0" - scalpel_downstream_skipped="0" scalpel_failure_reason="" - # Step 2a: Grep-based detection (existing approach) - if [ -n "$pom_files" ]; then - echo "" - echo "Analyzing parent POM dependency changes (grep)..." - while read -r pom_file; do - [ -z "$pom_file" ] && continue - - # Capture changed props for this pom before calling analyze - local pom_diff - pom_diff=$(extractPomDiff "$diff_body" "$pom_file") - if [ -n "$pom_diff" ]; then - local props - props=$(detectChangedProperties "$pom_diff") - if [ -n "$props" ]; then - all_changed_props="${all_changed_props:+${all_changed_props}, }$(echo "$props" | tr '\n' ',' | sed 's/,$//')" - fi - fi - - analyzePomDependencies "$diff_body" "$pom_file" - done <<< "$pom_files" - fi - - # Step 2b: Scalpel detection (all PRs) - # Scalpel uses effective POM model comparison and source-set analysis — - # catches managed deps, plugin changes, transitive impacts, and - # distinguishes main vs test-only source changes. - # Runs in report (shadow) mode: observes what it would do, does not - # affect actual test execution. Results appear in the PR comment for - # comparison with the grep-based approach. echo "" echo "Running Scalpel change detection..." runScalpelDetection - # Save grep-only results for comment attribution (before merging with Scalpel). - # The comment's top section shows only what the current grep mechanism detected; - # the Scalpel comparison section (after a separator) shows the diff. - local grep_dep_module_ids="$dep_module_ids" - local grep_changed_props="$all_changed_props" - - # Step 2c: Merge grep and Scalpel results (union, deduplicated) — for the build + # Use Scalpel results for dependency-detected modules if [ -n "$scalpel_module_ids" ]; then - dep_module_ids="${dep_module_ids:+${dep_module_ids},}${scalpel_module_ids}" - dep_module_ids=$(echo "$dep_module_ids" | tr ',' '\n' | sort -u | tr '\n' ',' | sed 's/,$//') + dep_module_ids="$scalpel_module_ids" fi if [ -n "$scalpel_props" ]; then - if [ -z "$all_changed_props" ]; then - all_changed_props="$scalpel_props" - else - # Merge and deduplicate property names - all_changed_props=$(printf '%s, %s' "$all_changed_props" "$scalpel_props" | tr ',' '\n' | sed 's/^ *//' | sort -u | tr '\n' ',' | sed 's/,$//' | sed 's/,/, /g') - fi + all_changed_props="$scalpel_props" fi # ── Step 3: Merge and deduplicate ── @@ -808,7 +491,7 @@ main() { if [ -z "$final_pl" ]; then echo "" echo "No modules to test" - writeComment "incremental-test-comment.md" "" "" "" "" "" + writeComment "incremental-test-comment.md" "" "" "" "" "" "" "" exit 0 fi @@ -940,22 +623,7 @@ main() { # ── Step 5: Write comment and summary ── local comment_file="incremental-test-comment.md" - writeComment "$comment_file" "$pl" "$grep_dep_module_ids" "$grep_changed_props" "$testedDependents" "$extraModules" - - # Scalpel shadow comparison (observation only — after separator) - # Filter reactor_ids through EXCLUSION_LIST so the comparison is - # apples-to-apples: both sides exclude the same meta/generated modules. - local tested_reactor_ids="" - if [ -n "$reactor_ids" ]; then - local excl_set - excl_set=$(echo "$EXCLUSION_LIST" | sed 's/!://g' | tr ',' '\n') - tested_reactor_ids=$(echo "$reactor_ids" | while read -r rid; do - if ! echo "$excl_set" | grep -qx "$rid"; then - echo "$rid" - fi - done) - fi - writeScalpelComparison "$comment_file" "$tested_reactor_ids" + writeComment "$comment_file" "$pl" "$dep_module_ids" "$all_changed_props" "$testedDependents" "$extraModules" "$scalpel_managed_deps" "$scalpel_managed_plugins" # Check for tests disabled in CI via @DisabledIfSystemProperty(named = "ci.env.name") local disabled_tests