chore(ci): rationalize CI into single workflow with unified comment#22247
Draft
chore(ci): rationalize CI into single workflow with unified comment#22247
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
changed the title
apupier
reviewed
Mar 25, 2026
apupier
reviewed
Mar 25, 2026
apupier
reviewed
Mar 25, 2026
apupier
reviewed
Mar 25, 2026
Contributor
apupier
left a comment
apupier
left a comment
There was a problem hiding this comment.
I think we will need to be able to test it in some ways before merging as it is quite a complex change
oscerd
approved these changes
Mar 25, 2026
Contributor
oscerd
left a comment
oscerd
left a comment
There was a problem hiding this comment.
This looks a really good improvement for maintainability. I think it would be important to have a document under .github folder where we show the architecture of our GH actions ecosystem. That way it could be used for knowledge in the future.
Contributor
Author
|
Claude Code on behalf of Guillaume Nodet Regarding testing: since Regarding the architecture doc: added All review comments have been addressed:
|
4 tasks
Contributor
|
What is the output of the way indicated to test it? |
Contributor
Author
|
Claude Code on behalf of Guillaume Nodet The Pushed a dummy |
Contributor
Author
|
🧪 CI tested the following changed modules:
✅ POM dependency changes: targeted tests included Changed properties: arangodb-java-version Modules affected by dependency changes (2)
All tested modules (936 modules)
|
apupier
reviewed
Mar 26, 2026
Contributor
apupier
left a comment
apupier
left a comment
There was a problem hiding this comment.
by looking to the genrated output, some feedback:
- inside the build log, there are some tests played:
[INFO] -------------------------------------------------------
[INFO] T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.camel.catalog.CamelCatalogCacheTest
[INFO] Tests run: 98, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.953 s -- in org.apache.camel.catalog.CamelCatalogCacheTest
[INFO] Running org.apache.camel.catalog.CamelCatalogJsonSchemaTest
[INFO] Tests run: 755, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.966 s -- in org.apache.camel.catalog.CamelCatalogJsonSchemaTest
[INFO] Running org.apache.camel.catalog.CamelCatalogTest
[INFO] Tests run: 98, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.057 s -- in org.apache.camel.catalog.CamelCatalogTest
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 951, Failures: 0, Errors: 0, Skipped: 0
- given that the changes wasn't impacting a component dependency, there are no tests played in the incremental-test, which I think is the main objective of reworking on this github actions. So we still do not know if it is working now.
- unless I missed some parts, we do not know if the /component-test feature is still working
so unless a human takes the responsibility to monitor the result after the merge and be ready to revert in case of problem, I think this is still not ready to be merged
gnodet
added a commit
that referenced
this pull request
Mar 26, 2026
This reverts the detect-dependencies CI action back to its original implementation. The reworked version is causing issues in CI. A proper fix will be done in #22247. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 task
gnodet
force-pushed
the
ci-merge-incremental-build-detect-dependencies
branch
from
070c8c3 to
989fe6f
Compare
gnodet
added a commit
that referenced
this pull request
Mar 26, 2026
Reverts the detect-dependencies CI action back to its original implementation. The reworked version was causing issues in CI. A proper fix will be done in #22247.
gnodet
force-pushed
the
ci-merge-incremental-build-detect-dependencies
branch
from
989fe6f to
265f055
Compare
gnodet
changed the title
gnodet
force-pushed
the
ci-merge-incremental-build-detect-dependencies
branch
from
265f055 to
9e69933
Compare
gnodet
force-pushed
the
ci-merge-incremental-build-detect-dependencies
branch
2 times, most recently
from
49ebd68 to
8cc70dd
Compare
Merge three separate CI actions (incremental-build, detect-dependencies, component-test) into one unified incremental-build script and simplify the workflow architecture. Deleted: .github/actions/component-test/ .github/actions/detect-dependencies/ Added: .github/workflows/pr-test-commenter.yml .github/CI-ARCHITECTURE.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
force-pushed
the
ci-merge-incremental-build-detect-dependencies
branch
from
8cc70dd to
463ac2d
Compare
- Restore maxNumberOfTestableProjects to 50 (was accidentally changed to 1000) - Restore root project change handling: exit early with informative comment - Restrict POM dependency analysis to parent/pom.xml only (matches original detect-test.sh behavior; detection improvements deferred to follow-up PR) - Add explicit comment about intentional Toolbox removal (pre-#22022 approach) - Add skip_full_build input for /component-test dispatches to avoid full regen.sh build (uses quick targeted build with -Dquickly instead) - Document multi-JDK artifact upload behavior (intentional overwrite for resilience when one JDK fails) - Update CI-ARCHITECTURE.md to reflect all changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…epo default
- Fix bash arithmetic error when ./mvnw pipeline fails with pipefail:
wc -l output was concatenated with the || echo "0" fallback, producing
multi-line values like "24\n0" that broke the [[ -gt ]] comparison.
- Change github-repo action input default from hardcoded 'apache/camel'
to ${{ github.repository }} for fork compatibility.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
Fork Testing ReportClaude Code on behalf of Guillaume Nodet Ran manual tests on Test 1: Component file change detection ✅Run: 23613433728 — All 3 JDKs passed Changed
Test 2: Parent POM dependency detection ✅ (script worked, tests failed)Run: 23613434374 — JDK 21 failed (test failures) Changed
Test 3: skip_full_build + extra_modules (
|
The EXCLUSION_LIST uses Maven's !:module exclusion syntax, which only works when the excluded modules are in the reactor (i.e., with -amd). When testing POM-dependency-detected modules without -amd, appending the exclusion list causes "Could not find the selected project in the reactor: :camel-allcomponents" errors. Move the exclusion list append into the -amd branch only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Merge three separate CI actions (
incremental-build,detect-dependencies,component-test) into one unifiedincremental-build.shand simplify the workflow architecture. The change detection mechanism itself is unchanged — this is purely structural.What changed
detect-dependenciesintoincremental-build.sh— property change detection now runs inline, feeding its results into the same Maven invocation instead of a separate one/component-testinto the main "Build and test" workflow viaworkflow_dispatchwithextra_modulesparameterworkflow_runcommenter (pr-test-commenter.yml)build-all,build-dependents) from the commenter welcome messageCI-ARCHITECTURE.mddocumenting the workflow structureBenefits
Single Maven invocation — previously,
incremental-buildanddetect-dependenciesran as separate steps with independent Maven calls. Now both file-path-detected and POM-dependency-detected modules are combined into onemvn install, avoiding redundant reactor resolution and compilation.One unified PR comment — instead of up to 3 separate bot comments (incremental-build, detect-dependencies, component-test), there is now a single comment updated in place. This reduces noise and makes it easier to see what CI actually tested.
Fork PR comments work — the inline
actions/github-scriptcomment step ran in the PR's context, which lackspull-requests: writepermission for fork PRs. The newworkflow_runcommenter (pr-test-commenter.yml) runs in the base repo context with proper permissions./component-testreuses the full pipeline — instead of a separate action that only ran tests (skipping the build, regen, and uncommitted-changes checks), component-test now dispatches the same "Build and test" workflow with extra modules appended. This ensures component tests go through the same validation as PR builds.Concurrency group separation —
/component-testdispatches get a distinct concurrency group suffix (-component-test), so they don't cancel in-flight PR builds and vice versa.Less code to maintain — 3 actions → 1 action, 3 shell scripts → 1 script. The deleted
detect-test.sh(96 lines) andcomponent-test.sh(71 lines) are folded intoincremental-build.sh.Workflow overview
Before → After
Files
.github/actions/component-test/,.github/actions/detect-dependencies/.github/workflows/pr-test-commenter.yml,.github/CI-ARCHITECTURE.mdincremental-build/,pr-build-main.yml,pr-manual-component-test.yml,main-build.yml,pr-commenter.ymlTest plan
pull_requesteventsworkflow_dispatchworks for CI-only file changes (.github/**is inpaths-ignore)/component-test kafkadispatches the main workflow with correctextra_modulesskip-testsandtest-dependentslabels still work