CAMEL-24290: ci - report tests that only passed after a retry

[ammachado]

Description

Part of CAMEL-24290 (improve CI to support larger workload). This is the visibility step: it makes an existing, currently invisible cost measurable before anything is changed about how tests run.

The problem

Surefire retries failing tests. surefire.rerunFailingTestsCount defaults to 2 in the full profile of parent/pom.xml (activated by !quickly), and both CI systems pass it again explicitly (Jenkinsfile:17, install-mvnd/action.yml:71).

A test that fails and then passes within those attempts is a recovered flake. The build goes green, nothing appears in the console output, and the retry leaves no trace. We are already paying up to 3x the runtime of every flaky test, and the retry is precisely the thing that stops anyone noticing.

parse_errors.sh cannot cover this, for two independent reasons:

1. It parses the .txt reports, which contain no flake data at all (verified: grep -c "Flake" on a surefire 3.5.6 .txt report with a known flake returns 0).
2. It only runs inside the if [[ ${ret} -ne 0 ]] branch, and a recovered flake exits 0.

The change

collect-flakes.py runs on the always-path of incremental-build.sh. It walks the reactor for target/{surefire,failsafe}-reports/TEST-*.xml and reports every <testcase> carrying <flakyFailure>/<flakyError>. Tests with <rerunFailure>/<rerunError> failed every attempt and already fail the build, so they are deliberately excluded.

The walk prunes rather than globbing: after a full build every module's target/ holds thousands of class and generated-source directories, none of which can hold a report, so **/target/... would descend into all of them.

Two outputs:

• A section appended to the PR comment and the job summary, naming module, test, attempt count and first failure message. Nothing is emitted when no test was retried, so clean PRs are unaffected.
• flakes.json, uploaded as flakes-java-<version> on PRs and flakes-main-java-<version> on main. .mvn/develocity.xml publishes build scans only when authenticated, so fork PRs produce no Develocity data. This artifact is the only per-PR record of flakiness we can accumulate.

The section names its JDK

The PR-comment artifact uploads with overwrite: true across the JDK matrix, justified by the existing comment in pr-build-main.yml: the content is identical across entries because the same modules are tested. Flake data is the one part where that does not hold. If JDK 17 flakes and JDK 25 does not, whichever entry finishes last decides what the comment shows.

Rather than restructure the comment/artifact strategy in this PR, the section names the entry it came from (flake-label on the action, --label on the script, also recorded in flakes.json):

:repeat: **1 test passed only after a retry on JDK 25** (1 retried attempt)

Last writer still wins, so the comment shows one JDK's flakes. What the label buys is that a shown section is attributable, and that an empty section can no longer be silently confused between "no flakes" and "the other JDK overwrote it". The per-JDK flakes-* artifacts remain the complete record, and the label in flakes.json lets aggregation distinguish a test that only flakes on one JDK from one that flakes everywhere.

Deliberate non-goals

No time figure is reported. Surefire records no per-attempt timing, and <testcase time> reflects only the final successful attempt. Estimating cost from it would systematically understate timeout-driven flakes, which are the common kind. Counts and test names are honest; a fabricated minutes-lost number would poison the evidence this is meant to gather.

Nothing about test execution changes. No test is skipped, quarantined, or retried differently. This PR only reports.

Notes for reviewers

• The script declares its dependency inline via PEP 723 and is run with uv run (PEP 723 describes inline dependency declaration similar to jbang). uv is installed by the action (astral-sh/setup-uv, pinned by SHA) so the action stays self-contained for downstream repos that reuse it. Both the uv version and the Python version it resolves against are pinned explicitly (version: "0.12.5", python-version: "3.11") — the action's own SHA pin does not fix what uv itself installs at run time, since version defaults to "latest" absent a pyproject.toml/uv.toml pin. The same pins are applied to the Install uv step in pr-ci-scripts-validation.yml, so the tests run against the versions production uses.
• XML is parsed with defusedxml rather than the stdlib, with forbid_dtd=True passed explicitly. defusedxml forbids entity declarations by default but not a bare <!DOCTYPE .. SYSTEM ..>, so the default alone does not enforce "Surefire never emits a DOCTYPE, therefore any report carrying one did not come from the build". testdata/TEST-doctype-no-entities-rejected.xml pins this.
• A pre-parse byte scan for <!DOCTYPE is not sufficient either: it misses a UTF-16 document, where the marker is interleaved with NUL bytes, and the entity then expands normally. testdata/TEST-utf16-doctype-rejected.xml pins that case. (The stdlib-only alternative of installing pyexpat entity handlers is unavailable: ET.XMLParser.parser no longer exists on current CPython.)
• Flaky attempts are read in document order. Collecting per tag (findall("flakyFailure") + findall("flakyError")) returns them grouped by tag, so the "First failure" column named the wrong attempt whenever the two kinds mixed, hiding the timeout that is typically the real cause.
• Failure messages are HTML-escaped. GitHub's renderer treats expected: <true> but was: <false> as raw HTML and strips it, which emptied the column for exactly the assertion messages that produce most flake reports.
• The step cannot fail a job. Parse errors are logged and skipped, and main() always returns 0, because by that point the build verdict is already decided.
• TEST-utf16-doctype-rejected.xml shows as binary in the diff. That is expected for UTF-16 with a BOM.

Verification

• 19 unit tests, run in pr-ci-scripts-validation.yml (path filter extended to .github/actions/incremental-build/**).
• Fixtures derived from real surefire 3.5.6 / JUnit 5.14.4 output rather than hand-written.
• Exercised end to end against a synthetic two-module reactor, and the shell function was executed in both GITHUB_STEP_SUMMARY set and unset states.
• The walk was checked for equivalence with the previous glob across Path, str and relative . roots, at root, one-level and three-level module depths.

Target

• I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

• If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

CAMEL-24290. Note the JIRA issue is currently Unassigned and this PR does not claim it; it implements one bullet from the ticket's list.

Apache Camel coding standards and style

• I checked that each commit in the pull request has a meaningful subject line and body.

• I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.

Not run, and not applicable: this PR touches only .github/** (Python, YAML, shell, Markdown). No Java sources, POMs, or generated files are involved, so no auto-generated content can change. pr-build-main.yml itself carries paths-ignore: .github/** for exactly this reason.

AI-assisted contributions

• If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., Co-authored-by trailers) and the PR description identifies the AI tool used.

Written with Claude Code (Claude Opus 5 and Claude Sonnet 5, across commits). Every AI-assisted commit carries a Co-Authored-By trailer (f03024aa is a reviewer's suggestion applied through the GitHub UI and carries that reviewer's trailer).

---

Claude Code on behalf of @ammachado

[github-actions]

🌟 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.

[apupier]

I think it would be easier to maintain and easier to leverage reported flaky tests by reusing the same tool than for Jenkins. Which is develocity.

[davsclaus]

This is a rules-and-conventions review from OSS Helper — it does not replace CodeRabbit, Sourcery, SonarCloud, or a dedicated static analyzer.

Overall this is a well-scoped, well-tested CI visibility change. To verify rather than just read it, I checked out the branch, ran all 18 unit tests locally (all pass), confirmed the forbid_dtd=True XXE defense actually rejects the UTF-16-hidden-DOCTYPE fixture, confirmed reportRecoveredFlakes genuinely runs on the always-path of incremental-build.sh (after the build's exit code is captured, before the final exit $ret), confirmed the new astral-sh/setup-uv SHA pin matches the v10.0.1 tag, and confirmed both commits carry the Co-Authored-By trailer per project convention. JIRA CAMEL-24290 is Open/Unassigned and the PR's "implements one bullet, doesn't claim the umbrella ticket" framing matches that.

One correctness issue below (inline), plus two non-blocking notes:

Recommend confirming before merge: this is a fork PR, and the only checks that ran are pr-ci-scripts-validation.yml (isolated unit tests) plus the PR-id uploader/dependency-review. pr-build-main.yml — the workflow that actually exercises the new flake-label input, the new install-uv composite-action step, and the new "Upload recovered-flake report" step — has no run at all for this branch yet (checked via gh run list --workflow=pr-build-main.yml), most likely GitHub's approval gate for fork PRs touching .github/workflows/** / .github/actions/**. The unit tests validate collect-flakes.py's logic in isolation but can't prove the composite-action wiring (uv landing on PATH, FLAKE_LABEL threading through, the hashFiles('flakes.json') upload gate) works on a live runner. Worth a maintainer approving/triggering that run before merging.

Minor/optional: in pr-build-main.yml, the new "Upload recovered-flake report" step isn't gated on !matrix.experimental the way the "Save PR number and test comment" step is. Harmless today (the matrix has no experimental: true entry), but worth a thought if an experimental JDK entry is ever added.

Claude Code on behalf of Claus Ibsen (@davsclaus)

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

[ammachado]

I think it would be easier to maintain and easier to leverage reported flaky tests by reusing the same tool than for Jenkins. Which is develocity.

Thanks for the suggestion, @apupier. I checked whether Develocity could stand in for this instead of the custom flake report, and it can't cover the case this feature is actually for.

.mvn/develocity.xml gates publishing on <publishing><onlyIf>authenticated</onlyIf></publishing> - a build scan only reaches develocity.apache.org if the Maven process authenticates. I grepped every workflow and composite action under .github/ for DEVELOCITY, GRADLE_ENTERPRISE, and ACCESS_KEY and found nothing; no GitHub Actions run in this repo ever gets a Develocity key. pr-build-main.yml triggers on plain pull_request (not pull_request_target), and the only secret it uses is secrets.GITHUB_TOKEN. GitHub withholds repo secrets from pull_request runs on fork PRs by design, since fork code effectively controls the run, so switching triggers to expose a Develocity key would just trade this problem for a real exfiltration risk.

So for a fork PR (the majority of external Camel contributions), and the exact audience that can't see Jenkins internals and needs inline visibility most - no Develocity scan ever gets published. There's nothing to leverage there. flakes.json plus the PR comment are the only per-PR record that can exist for that case, which is basically the reasoning already in the original commit message for this change.

Where I think you're right: for merged builds on trusted ASF Jenkins infra, Develocity's cross-build flaky-test view is a better long-term signal than one job's retry report. That's an argument for wiring Develocity on the trusted side too, not a substitute for PR-time visibility on the fork side. I can file a follow-up for that if it's useful.

Claude Code on behalf of Adriano Machado (@ammachado)

This reply was generated by an AI agent and may contain inaccuracies. Please verify before relying on it.

[apupier]

I'm a bit sceptical to the fact that contributors will benefit to the "flaky but still passing" report, especially given that there are a lot of them today. Either they will look and can be confused because it is an existing flaky one, or they won't care as it is passing anyway.
The "flaky but failing" are already looked by almost nobody.

Anyway, if i'm alone to think that that's fine and we can go ahead after other pints clarified/tackled.

Can you try to have these jobs triggered before we merge the PR to ensure that it is not breaking the main branch please?

[ammachado]

Addressing the two points from this review:

CI not triggered: Root cause is pr-build-main.yml's pull_request trigger has paths-ignore: [.github/**, ...] — since this PR only touches files under .github/, it never fires via the normal pull_request event (not the first-time-contributor approval gate the earlier review guessed at). I triggered it manually via the documented workflow_dispatch escape hatch: https://github.com/apache/camel/actions/runs/32856072627

Value of the "flaky but still passing" report: Fair concern, and I don't have a strong counter beyond what's in the original commit message — for a first-time or occasional contributor whose PR fails, then passes on retry, seeing why inline (rather than having to know Jenkins exists, find the right job, and dig through console logs) lowers the bar to understanding "was this me or was this the test." I agree the "flaky but failing" case is the one nobody reads today; I'm not trying to fix that habit with this PR, just make the passing-after-retry case visible where a PR author will actually see it. Open to gating this behind something narrower (e.g. only show it if the flake count is small, or only on first-time-contributor PRs) if that addresses the noise concern — let me know if you'd like that adjustment before merge.

Claude Code on behalf of Adriano Machado (@ammachado)

This reply was generated by an AI agent and may contain inaccuracies. Please verify before relying on it.

[github-actions]

🧪 CI tested the following changed modules:

• docs

---

🔬 Scalpel shadow comparison

⚠️ Scalpel report not found (merge-base may be unreachable in shallow clone)

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

---

⚙️ View full build and test results

[oscerd]

The mechanism is sound and nicely tested — recovered-flake selection (only flakyFailure/flakyError), document-order first-failure, markdown escaping, and DTD rejection all check out, and the step never fails the build. Nice work.

Two things before merge:

1. The PR is currently conflicting with main (mergeStateStatus: DIRTY) — it'll need a rebase/conflict resolution. Likely sites are the recently-changed .github/actions/incremental-build/incremental-build.sh and the PR-build workflows.
2. It bundles an unrelated docs commit (0e60db5, the nav.adoc "Resources & Guides" reorg). Consider splitting that into its own PR/JIRA — the nav change itself looks correct, it's just a separate concern (and uses a chore(docs): subject rather than CAMEL-XXXX:).

---

Reviewed with Claude Code (Opus 4.8) on behalf of @oscerd. This is an AI-generated review and may contain inaccuracies; please verify before applying.

[gnodet-bot]

Re-review after 3 new commits (d0b5664, 60596ee, cbda9e9). All findings from previous reviews are addressed.

Previous findings — status:

• _cell() not applied to module/test/classname: ✅ fixed — all four columns now go through _cell()
• forbid_dtd=True not explicit: ✅ fixed — present in parse_report
• document-order attempt iteration: ✅ fixed — [el for el in testcase if el.tag in FLAKY_TAGS]
• !matrix.experimental guard on artifact upload: ✅ fixed — in pr-build-main.yml
• uv/Python version pinning: ✅ fixed in action.yaml — version: "0.12.5", python-version: "3.11"
• nav.adoc unrelated commit: ✅ removed (no longer in PR files)
• Conflicts with main: ✅ resolved — mergeStateStatus: CLEAN
• pr-build-main.yml live run: ✅ confirmed (CI comment with action run link present)

One inconsistency remains inline.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

test probe

[gnodet-bot]

probe multiline

[gnodet-bot]

Re-review after commit f03024aa ("Update .github/workflows/pr-ci-scripts-validation.yml").

The uv/Python version pinning is now applied to pr-ci-scripts-validation.yml — the intent is correct. However, the commit introduced a broken YAML structure that will prevent the workflow from running at all (see inline). All other previous findings remain addressed.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after commit 6a89cfc ("Update .github/workflows/pr-ci-scripts-validation.yml").

All previous findings are now addressed.

Previous findings — status:

• _cell() not applied to module/test/classname: ✅ fixed — all four columns now go through _cell() (diff lines 329–332 of collect-flakes.py)
• forbid_dtd=True not explicit: ✅ confirmed present
• Broken YAML in pr-ci-scripts-validation.yml (duplicate step + run: on uses: step): ✅ fixed — 6a89cfc splits the combined block into two clean, well-formed steps; pr-ci-scripts-validation.yml CI ran and passed (SUCCESS)
• uv/Python version pinning in validation workflow: ✅ confirmed — both version: "0.12.5" and python-version: "3.11" are present in the Install uv step
• All other previous findings: ✅ remain addressed

Current state:

• mergeStateStatus: CLEAN
• All CI checks passing
• _cell() applied consistently to all four table columns
• DTD rejection tested with three fixtures (entity expansion, bare SYSTEM DOCTYPE, UTF-16-encoded DOCTYPE)
• reportRecoveredFlakes correctly on the always-path of incremental-build.sh (after exit code is captured)
• No unrelated commits in the PR

Code is solid. Ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[apupier]

this is causing build failure

Post job cleanup.
     
    
Run astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
0s
  Post job cleanup.
  UV_CACHE_DIR is already set to /home/runner/work/_temp/setup-uv-cache
  UV_PYTHON_INSTALL_DIR is already set to /home/runner/work/_temp/uv-python-dir
  Error: Cache path /home/runner/work/_temp/setup-uv-cache does not exist on disk. This likely indicates that there are no dependencies to cache. Consider disabling the cache input if it is not needed.

Run astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
0s
  Post job cleanup.
  UV_CACHE_DIR is already set to /home/runner/work/_temp/setup-uv-cache
  UV_PYTHON_INSTALL_DIR is already set to /home/runner/work/_temp/uv-python-dir
  Error: Cache path /home/runner/work/_temp/setup-uv-cache does not exist on disk. This likely indicates that there are no dependencies to cache. Consider disabling the cache input if it is not needed.

https://github.com/apache/camel/actions/runs/36748827776/job/110001865059#step:26:6

[davsclaus]

@apupier thanks for spotting this. uv is only used by the recovered-flake report, which the incremental build does not reach on every path (e.g. a main build that tests no module), so the setup-uv post step found no cache directory and failed the job. Fixed on main in 6d8f350 by setting ignore-nothing-to-cache: true on the setup-uv step in .github/actions/incremental-build/action.yaml.

Claude Code on behalf of davsclaus