From 3c4d65592761ca8ad84592a413456a0a8b5288c1 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 13 Jul 2026 18:54:34 +0200 Subject: [PATCH 1/3] =?UTF-8?q?feat(ci):=E2=80=AFReport=20failed=20flaky?= =?UTF-8?q?=20tests=20as=20skip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitlab/collect-result/CollectResults.java | 7 ++++-- .gitlab/collect-result/JUnitReport.java | 26 ++++++++++++++++++++- .gitlab/collect-result/ResultCollector.java | 14 ++++++++++- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/.gitlab/collect-result/CollectResults.java b/.gitlab/collect-result/CollectResults.java index ad7c6429cda..ac8d1cea04a 100644 --- a/.gitlab/collect-result/CollectResults.java +++ b/.gitlab/collect-result/CollectResults.java @@ -3,12 +3,15 @@ class CollectResults { public static void main(String[] args) throws Exception { + // Detect flaky jobs + var continueOnFailure = Boolean.parseBoolean(System.getenv("CONTINUE_ON_FAILURE")); + // Run collector var collector = new ResultCollector( Path.of("results"), Path.of("workspace"), - List.of(Path.of("workspace"), Path.of("buildSrc"))); - + List.of(Path.of("workspace"), Path.of("buildSrc")), + continueOnFailure); collector.collect(); } } diff --git a/.gitlab/collect-result/JUnitReport.java b/.gitlab/collect-result/JUnitReport.java index b79b8a551ad..efc6800f0f2 100644 --- a/.gitlab/collect-result/JUnitReport.java +++ b/.gitlab/collect-result/JUnitReport.java @@ -153,6 +153,26 @@ void tagFinalStatuses() { } } + /// Downgrades failing testcases to `test.final_status=skip` for jobs that tolerate failures. + /// + /// The flaky test jobs (`test_flaky`, `test_flaky_inst`) run with `CONTINUE_ON_FAILURE=true`, so + /// Gradle test failures are swallowed and a failing flaky test does not fail the job. Reporting + /// those failures to Test Optimization as `fail` produces false-positive failure notifications + /// and skews SLIs, so we record them as `skip` instead. Passing and skipped tests keep their + /// natural status, so the tests stay visible in Test Optimization. + /// + /// **Must run before {@link #tagFinalStatuses()}** so the natural `fail` status is never assigned + /// to these testcases. Testcases already tagged by {@link #tagRetriedAttempts()} or + /// {@link #tagSyntheticFailures()} are left untouched. + void tagFailuresAsSkipped() { + for (var testcase : testcases()) { + if (hasFinalStatusProperty(testcase) || !isFailed(testcase)) { + continue; + } + addFinalStatusProperty(testcase, "skip", MissingPropertiesPlacement.FIRST_CHILD); + } + } + void write(Path xmlFile) throws Exception { Files.createDirectories(xmlFile.getParent()); var tmpFile = Files.createTempFile(xmlFile.getParent(), "collect-results-", ".xml"); @@ -239,7 +259,7 @@ private static boolean propertiesHasFinalStatusProperty(Element properties) { } private static String finalStatus(Element testcase) { - if (hasChildElement(testcase, "failure") || hasChildElement(testcase, "error")) { + if (isFailed(testcase)) { return "fail"; } if (hasChildElement(testcase, "skipped")) { @@ -248,6 +268,10 @@ private static String finalStatus(Element testcase) { return "pass"; } + private static boolean isFailed(Element testcase) { + return hasChildElement(testcase, "failure") || hasChildElement(testcase, "error"); + } + private static Element firstChildElement(Element parent, String tagName) { var children = parent.getChildNodes(); for (var i = 0; i < children.getLength(); i++) { diff --git a/.gitlab/collect-result/ResultCollector.java b/.gitlab/collect-result/ResultCollector.java index 423b44343d8..2e82dc67fb5 100644 --- a/.gitlab/collect-result/ResultCollector.java +++ b/.gitlab/collect-result/ResultCollector.java @@ -13,12 +13,15 @@ final class ResultCollector { private final Path resultsDir; private final Path workspaceDir; private final List searchDirs; + private final boolean continueOnFailure; private final SourceFileResolver sourceFileResolver; - ResultCollector(Path resultsDir, Path workspaceDir, List searchDirs) { + ResultCollector( + Path resultsDir, Path workspaceDir, List searchDirs, boolean continueOnFailure) { this.resultsDir = resultsDir; this.workspaceDir = workspaceDir; this.searchDirs = searchDirs; + this.continueOnFailure = continueOnFailure; this.sourceFileResolver = new SourceFileResolver(workspaceDir); } @@ -32,6 +35,11 @@ void collect() throws Exception { return; } + if (continueOnFailure) { + System.out.println( + "CONTINUE_ON_FAILURE=true: reporting failed tests as skip"); + } + System.out.println("Saving test results:"); for (var sourceXml : findXmlFiles(testResultDirs)) { collect(sourceXml); @@ -51,6 +59,10 @@ private void collect(Path sourceXml) throws Exception { report.tagRetriedAttempts(); reportChangedBeforeFinalStatus |= report.normalizeStableTestNames(); report.tagSyntheticFailures(); + // On flaky jobs, downgrade failures to skip before assigning natural statuses (APMLP-1267). + if (continueOnFailure) { + report.tagFailuresAsSkipped(); + } report.tagFinalStatuses(); report.write(targetXml); From 7d30705e8f8094175cc25bb527cd61fbdadcabf5 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 13 Jul 2026 18:59:23 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(ci):=E2=80=AFFix=20label=20documentatio?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 69e6e48aac3..4eee2471c3d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,7 +68,7 @@ docs/ Developer documentation (see below) - Title: imperative verb sentence describing user-visible change (e.g. "Fix span sampling rule parsing") - Labels: always add `tag: ai generated` and at least one `comp:` or `inst:` label + one `type:` label -- Use `tag: no release note` for internal/refactoring changes +- Use `tag: no release notes` for internal/refactoring changes - Open as draft first, convert to ready when reviewable ## Bootstrap constraints (critical) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f5f9bdd15ab..4dde1824dd7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,7 +118,7 @@ Both pull requests and issues should be labelled with at least a component or an Labels are used to not only categorize but also alter the continuous integration behavior: -* `tag: no release note` to exclude a pull request from the next release changelog. Use it when changes are not relevant to the users like: +* `tag: no release notes` to exclude a pull request from the next release changelog. Use it when changes are not relevant to the users like: * Internal features changes * Refactoring pull requests * CI and build tools improvements From 94dbdf3c49667d1f30956f4b3a85686a81e49e37 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 13 Jul 2026 19:46:24 +0200 Subject: [PATCH 3/3] =?UTF-8?q?feat(ci):=E2=80=AFReport=20all=20tests=20as?= =?UTF-8?q?=20skip=20instead?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitlab/collect-result/JUnitReport.java | 29 ++++++++++----------- .gitlab/collect-result/ResultCollector.java | 8 +++--- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/.gitlab/collect-result/JUnitReport.java b/.gitlab/collect-result/JUnitReport.java index efc6800f0f2..a77a54af4b3 100644 --- a/.gitlab/collect-result/JUnitReport.java +++ b/.gitlab/collect-result/JUnitReport.java @@ -153,20 +153,23 @@ void tagFinalStatuses() { } } - /// Downgrades failing testcases to `test.final_status=skip` for jobs that tolerate failures. + /// Marks every testcase as `test.final_status=skip` for jobs that tolerate failures. /// /// The flaky test jobs (`test_flaky`, `test_flaky_inst`) run with `CONTINUE_ON_FAILURE=true`, so - /// Gradle test failures are swallowed and a failing flaky test does not fail the job. Reporting - /// those failures to Test Optimization as `fail` produces false-positive failure notifications - /// and skews SLIs, so we record them as `skip` instead. Passing and skipped tests keep their - /// natural status, so the tests stay visible in Test Optimization. + /// their result never gates the pipeline: it runs to completion regardless of pass or fail. + /// `test.final_status` records that CI impact rather than the raw outcome, so every test in these + /// jobs is `skip` — a failure is a non-blocking failure and a pass is a non-blocking pass. This + /// keeps flaky failures from creating false-positive notifications and skewing SLIs, while the + /// real per-test outcome stays available in `test.status` (derived from the ``, + /// ``, and `` children, which are left in place). Always-green tests that could + /// leave the flaky pipeline are then found with `@test.status:pass @test.final_status:skip`. /// - /// **Must run before {@link #tagFinalStatuses()}** so the natural `fail` status is never assigned - /// to these testcases. Testcases already tagged by {@link #tagRetriedAttempts()} or - /// {@link #tagSyntheticFailures()} are left untouched. - void tagFailuresAsSkipped() { + /// **Must run before {@link #tagFinalStatuses()}** so the natural pass/fail status is never + /// assigned. Testcases already tagged by {@link #tagRetriedAttempts()} or + /// {@link #tagSyntheticFailures()} are left untouched (already `skip`). + void tagAllAsSkipped() { for (var testcase : testcases()) { - if (hasFinalStatusProperty(testcase) || !isFailed(testcase)) { + if (hasFinalStatusProperty(testcase)) { continue; } addFinalStatusProperty(testcase, "skip", MissingPropertiesPlacement.FIRST_CHILD); @@ -259,7 +262,7 @@ private static boolean propertiesHasFinalStatusProperty(Element properties) { } private static String finalStatus(Element testcase) { - if (isFailed(testcase)) { + if (hasChildElement(testcase, "failure") || hasChildElement(testcase, "error")) { return "fail"; } if (hasChildElement(testcase, "skipped")) { @@ -268,10 +271,6 @@ private static String finalStatus(Element testcase) { return "pass"; } - private static boolean isFailed(Element testcase) { - return hasChildElement(testcase, "failure") || hasChildElement(testcase, "error"); - } - private static Element firstChildElement(Element parent, String tagName) { var children = parent.getChildNodes(); for (var i = 0; i < children.getLength(); i++) { diff --git a/.gitlab/collect-result/ResultCollector.java b/.gitlab/collect-result/ResultCollector.java index 2e82dc67fb5..c851e95cdc2 100644 --- a/.gitlab/collect-result/ResultCollector.java +++ b/.gitlab/collect-result/ResultCollector.java @@ -36,8 +36,7 @@ void collect() throws Exception { } if (continueOnFailure) { - System.out.println( - "CONTINUE_ON_FAILURE=true: reporting failed tests as skip"); + System.out.println("CONTINUE_ON_FAILURE=true: reporting all tests as skip"); } System.out.println("Saving test results:"); @@ -59,9 +58,10 @@ private void collect(Path sourceXml) throws Exception { report.tagRetriedAttempts(); reportChangedBeforeFinalStatus |= report.normalizeStableTestNames(); report.tagSyntheticFailures(); - // On flaky jobs, downgrade failures to skip before assigning natural statuses (APMLP-1267). + // Flaky jobs (CONTINUE_ON_FAILURE=true) never gate CI, so record every test as skip before + // assigning natural statuses (APMLP-1267). if (continueOnFailure) { - report.tagFailuresAsSkipped(); + report.tagAllAsSkipped(); } report.tagFinalStatuses(); report.write(targetXml);