ci: add Scalpel shadow comparison for skip-tests mode validation#22524
ci: add Scalpel shadow comparison for skip-tests mode validation#22524gnodet wants to merge 5 commits intoapache:mainfrom
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
ℹ️ CI did not run targeted module tests. |
77a5579 to
1834e34
Compare
|
Claude Code on behalf of Guillaume Nodet @apupier Thanks for the review!
Done — just merged latest
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 |
apupier
left a comment
There was a problem hiding this comment.
Please launch it on a branch so that we ensure that it doesn't break the curren tflow and have a better idea of what will be the output
|
|
||
| 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 base branch is pre-fetched by the CI workflow (`git fetch --deepen=200` + fetch of `origin/main`) rather than by Scalpel's built-in JGit fetch (`-Dscalpel.fetchBaseBranch=false`). This avoids JGit issues in shallow CI clones. |
There was a problem hiding this comment.
why do we need 200 hundreds commits from git history to detect dependencies?
What is the performance impact?
There was a problem hiding this comment.
The 200 commits are not for detecting dependencies — they're for finding the git merge-base between the PR branch and main.
GitHub Actions checks out with depth=1 (a single commit). Scalpel needs the merge-base to compute which files changed, then maps those changed files to Maven modules and walks Maven's dependency graph to find affected downstream modules.
Why local git instead of the GitHub API? The existing grep-based script fetches the diff via the GitHub REST API (application/vnd.github.v3.diff), so it doesn't need local history at all. Scalpel uses local git instead because:
- It's a Maven extension (JVM-based), so using local git via JGit is the natural approach
- CI-provider-agnostic — works on GitLab, Jenkins, etc., not just GitHub
- No diff truncation risk (GitHub API truncates large diffs)
Performance impact: git fetch --deepen=200 fetches only commit metadata (not file blobs). It adds ~2-3 seconds to the job — negligible compared to the Maven build.
I've updated CI-ARCHITECTURE.md to clarify this.
Claude Code on behalf of Guillaume Nodet
There was a problem hiding this comment.
Also note that the end goal is to switch to Scalpel completely and remove the grep-based custom mechanism, which will then remove the dependency on the GitHub API for diff fetching entirely.
Claude Code on behalf of Guillaume Nodet
There was a problem hiding this comment.
I've pushed a follow-up commit that switches the grep-based script itself to use local git (git merge-base + git diff) instead of the GitHub API for diff fetching. Both mechanisms now share the same --deepen=200 fetch step, which makes the git fetch justified by both and removes the GitHub API dependency for diffs entirely.
Claude Code on behalf of Guillaume Nodet
There was a problem hiding this comment.
Updated test PRs — now with POM changes to trigger Scalpel's shadow comparison (the previous ones only had Java file changes, which don't invoke Scalpel's POM analysis):
- PR #23062 —
kafka-version(narrow managed dep). Grep finds 0 modules, Scalpel should find camel-kafka. - PR #23063 —
junit-jupiter-version(test-scoped managed dep). Grep finds 0, Scalpel should detect test scope. - PR #23064 —
jackson2-version(widely-used managed dep). Grep finds 0, Scalpel should find many modules.
All three demonstrate grep's blind spot for <dependencyManagement>-inherited versions. Throwaway PRs — will be closed after validation.
Claude Code on behalf of Guillaume Nodet
I've created three test PRs to demonstrate the Scalpel shadow comparison in different scenarios:
These are throwaway PRs (marked DO NOT MERGE) — they'll be closed after validation. Claude Code on behalf of Guillaume Nodet |
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>
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>
Switch the grep-based incremental build script from fetching diffs via the GitHub REST API to using local git merge-base + diff. Both the grep script and Scalpel now share the same --deepen=200 fetch step, removing the GitHub API dependency for diff fetching. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
73a8203 to
faaf5da
Compare
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:
<version>— Modules inheriting versions via<dependencyManagement>without declaring<version>${property}</version>are missed.parent/pom.xmlvia<pluginManagement>are invisible to child modules.camel-core's test-jar) triggers tests in all ~500 modules that depend oncamel-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-versionproperty inparent/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 likecamel-corewould 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 withskipTestsForDownstreamModules(mirroring theEXCLUSION_LIST),fetchBaseBranch=false(pre-fetched in workflow), andenabled=true(overrides.mvn/maven.config). Parse Scalpel report fields (module categories, test skip status). AddwriteScalpelComparison()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:Why shadow mode
Running Scalpel alongside grep (not replacing it) lets us: