Skip to content

ci: add Scalpel shadow comparison for skip-tests mode validation#22524

Open
gnodet wants to merge 3 commits intoapache:mainfrom
gnodet:worktree-scalpel-skip-tests
Open

ci: add Scalpel shadow comparison for skip-tests mode validation#22524
gnodet wants to merge 3 commits intoapache:mainfrom
gnodet:worktree-scalpel-skip-tests

Conversation

@gnodet
Copy link
Copy Markdown
Contributor

@gnodet gnodet commented Apr 9, 2026

Summary

Add a shadow comparison section to CI PR comments that shows what Scalpel's skip-tests mode would have tested, without affecting actual test execution. This lets us validate Scalpel's module detection across many PRs before switching to Scalpel-driven test execution.

Motivation

The current grep-based POM dependency detection has known limitations:

  1. Managed dependencies without explicit <version> — Modules inheriting versions via <dependencyManagement> without declaring <version>${property}</version> are missed.
  2. Maven plugin version changes — Plugin version properties consumed in parent/pom.xml via <pluginManagement> 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; transitive impacts are missed.
  5. Test-jar vs regular dependencies — Grep treats all dependencies equally. A change to a test utility class (e.g., camel-core's test-jar) triggers tests in all ~500 modules that depend on camel-core, even though only modules using the test-jar are actually affected.

Scalpel resolves all of these by comparing effective POM models between the base branch and the PR using Maven's own dependency resolution. It also distinguishes test-jar dependencies from regular dependencies via source-set-aware propagation, avoiding unnecessary test runs.

The goal is to eventually replace the fragile grep checks with Scalpel's more robust approach. This PR adds the shadow comparison as a validation step toward that migration.

Example: BOM version bumps and transitive dependencies

A concrete example of a gap the current approach misses: PR #22947 bumps the jackson3-version property in parent/pom.xml. Grep correctly finds the 4 direct modules (camel-jackson3, camel-jackson3-avro, camel-jackson3-protobuf, camel-jackson3xml) because they explicitly reference ${jackson3-version} in their BOM import. However, grep does not trace Maven's dependency graph, so it stops there.

The CI script intentionally does not use -amd (also-make-dependents) for POM-only changes — if it did, a property change affecting a widely-used module like camel-core would pull in ~500 modules. But this means downstream modules that transitively depend on the changed modules are not tested at all.

Scalpel fills this gap by walking the actual dependency graph. For PR #22947, it identifies 36 additional downstream modules (camel-kafka, camel-rest, camel-jbang-core, etc.) that transitively depend on the Jackson 3 modules and could be affected by the version change — without the all-or-nothing explosion of -amd.

Changes

  • incremental-build.sh: Configure Scalpel with skipTestsForDownstreamModules (mirroring the EXCLUSION_LIST), fetchBaseBranch=false (pre-fetched in workflow), and enabled=true (overrides .mvn/maven.config). Parse Scalpel report fields (module categories, test skip status). Add writeScalpelComparison() to generate a collapsible PR comment section.
  • pr-build-main.yml / sonar-build.yml: Add base branch fetch step for Scalpel's merge-base detection in shallow CI clones.
  • CI-ARCHITECTURE.md: Document the shadow comparison approach, Scalpel features used, and configuration notes.

How it works

Scalpel runs in report mode during mvn validate — it compares effective POM models between the base branch and the PR, then writes a JSON report. The script reads this report and adds a collapsible section to the PR comment showing:

  • How many modules Scalpel would test (direct + downstream)
  • How many downstream modules would have tests skipped (generated code, meta-modules)
  • The full list of modules in each category

Why shadow mode

Running Scalpel alongside grep (not replacing it) lets us:

  1. Validate — Compare what each method detects across many PRs
  2. No regression — If Scalpel fails, grep results are still used
  3. Gradual migration — Once validated, grep can be removed

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

ℹ️ CI did not run targeted module tests.


⚙️ View full build and test results

@gnodet gnodet marked this pull request as draft April 9, 2026 23:37
Upgrade Scalpel from extension3:0.1.0 to extension:0.3.0 and run it in
shadow mode — observing what skip-tests mode would do without affecting
actual test execution. This allows validating Scalpel's decisions across
many PRs before switching to Scalpel-driven test execution.

Changes:
- Upgrade Scalpel to 0.3.0 with source-set-aware propagation and
  skipTestsForDownstreamModules support
- Pre-fetch base branch in CI workflow (avoids JGit issues in shallow clones)
- Remove persist-credentials: false (needed for git fetch operations)
- Add shadow comparison section to PR comments showing what Scalpel
  skip-tests mode would have tested vs the current approach
- Update CI-ARCHITECTURE.md with shadow comparison documentation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet gnodet force-pushed the worktree-scalpel-skip-tests branch from 77a5579 to 1834e34 Compare April 10, 2026 10:16
@gnodet gnodet changed the title ci: use Scalpel skip-tests mode for single-invocation CI builds ci: add Scalpel 0.3.0 shadow comparison alongside grep-based detection Apr 10, 2026
Copy link
Copy Markdown
Contributor

@apupier apupier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requires to resolve conflict.

Does this PR mean that we should not update the maven extension without other changes? #22572

gnodet and others added 2 commits May 5, 2026 11:32
…-tests

# Conflicts:
#	.github/actions/incremental-build/incremental-build.sh
#	.mvn/extensions.xml
Scalpel 0.3.0 upgrade is already on main. This PR focuses on the shadow
comparison feature, not the version bump.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gnodet gnodet changed the title ci: add Scalpel 0.3.0 shadow comparison alongside grep-based detection ci: add Scalpel shadow comparison for skip-tests mode validation May 5, 2026
@gnodet gnodet requested review from apupier and davsclaus May 5, 2026 09:57
@gnodet gnodet marked this pull request as ready for review May 5, 2026 09:58
@gnodet
Copy link
Copy Markdown
Contributor Author

gnodet commented May 5, 2026

Claude Code on behalf of Guillaume Nodet

@apupier Thanks for the review!

requires to resolve conflict.

Done — just merged latest main and resolved the conflicts.

Does this PR mean that we should not update the maven extension without other changes? #22572

No, the Scalpel extension can be updated independently — that PR (#22572) was already merged and is included in the merge we just did. The CI script is version-agnostic: it uses mode=report and reads the JSON output with jq fallbacks, so new Scalpel versions adding fields won't break anything. This PR just adds a shadow comparison section to the PR comment showing what Scalpel's skip-tests mode would have tested — purely observational, no change to which tests actually run.

@davsclaus davsclaus requested review from Croway and orpiske May 5, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants