ci: quarantine flaky tests against a ticket instead of retrying past them

.github/scripts/flake_report.py

@@ -0,0 +1,145 @@
+#!/usr/bin/env python3
+"""Classify a cell's test failures and decide whether they should gate.
+
+Two questions, answered separately:
+
+  Is it flaky?    Failed on one attempt and passed on another. This is what the
+                  retry exists to establish -- it does NOT excuse the failure.
+  Does it gate?   Only the quarantine list answers that. A failure not on the
+                  list turns the job red whether it is flaky or broken, which is
+                  what keeps a flake from being quietly tolerated forever.
+
+So a flaky test still fails CI until somebody quarantines it with a ticket. To
+make that cheap, `report` prints a filled-in quarantine entry to paste.
+"""
+
+import argparse
+import glob
+import json
+import os
+import sys
+import xml.etree.ElementTree as ET
+
+sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
+import quarantine  # noqa: E402
+
+
+def _attempt_number(path):
+    return int(path.rsplit("-", 1)[1])
+
+
+def failed_tests(root_dir):
+    """Map of "class.test" -> first line of the failure message, for JUnit XML
+    anywhere under root_dir."""
+    failures = {}
+    pattern = os.path.join(root_dir, "**", "TEST-*.xml")
+    for path in glob.glob(pattern, recursive=True):
+        try:
+            tree = ET.parse(path)
+        except ET.ParseError:
+            # A JVM that died mid-suite leaves a truncated report. That is not
+            # evidence the tests in it passed, but it is not attributable to a
+            # named test either, so it is left to the exit code to report.
+            continue
+        for case in tree.iter("testcase"):
+            problem = case.find("failure")
+            if problem is None:
+                problem = case.find("error")
+            if problem is None:
+                continue
+            test_id = "{}.{}".format(case.get("classname") or "", case.get("name") or "")
+            message = (problem.get("message") or problem.get("type") or "").strip()
+            failures[test_id] = message.splitlines()[0][:200] if message else "failed"
+    return failures
+
+
+def collect_attempts(evidence_dir):
+    """[(attempt number, failures)] ordered by attempt."""
+    dirs = glob.glob(os.path.join(evidence_dir, "attempt-*"))
+    return [(_attempt_number(d), failed_tests(d)) for d in sorted(dirs, key=_attempt_number)]
+
+
+def cmd_count(args):
+    print(len(failed_tests(args.dir)))
+    return 0
+
+
+def cmd_report(args):
+    attempts = collect_attempts(args.evidence_dir)
+    ran = len(attempts)
+    entries = quarantine.load(args.list)
+
+    results = []
+    for test_id in sorted({t for _, f in attempts for t in f}):
+        failed_in = [n for n, f in attempts if test_id in f]
+        hit = next(
+            (e for e in entries if quarantine.covers(e, test_id) and quarantine.applies_to(e, args.cell)),
+            None,
+        )
+        results.append({
+            "test": test_id,
+            "failed_attempts": failed_in,
+            "message": next(f[test_id] for _, f in attempts if test_id in f),
+            # Passing on any attempt is what makes it flaky, so a test that
+            # failed in fewer attempts than were run has passed at least once.
+            "flaky": len(failed_in) < ran,
+            "quarantined": hit is not None,
+            "ticket": hit.get("ticket") if hit else None,
+        })
+
+    gating = [r for r in results if not r["quarantined"]]
+
+    report = {
+        "cell": args.cell,
+        "attempts": ran,
+        "status": args.final_status,
+        "flaky": [r for r in results if r["flaky"] and not r["quarantined"]],
+        "persistent": [r for r in results if not r["flaky"] and not r["quarantined"]],
+        "quarantined": [r for r in results if r["quarantined"]],
+        "gating_count": len(gating),
+        "failure_count": len(results),
+    }
+
+    os.makedirs(os.path.dirname(args.out) or ".", exist_ok=True)
+    with open(args.out, "w") as handle:
+        json.dump(report, handle, indent=2)
+        handle.write("\n")
+
+    for entry in report["quarantined"]:
+        print("::notice title=Quarantined test failed::{}: {} ({}) — not gating".format(
+            args.cell, entry["test"], entry["ticket"]))
+
+    for entry in report["flaky"]:
+        attempts_desc = ", ".join(str(n) for n in entry["failed_attempts"])
+        print("::error title=Flaky test::{}: {} failed on attempt {} and passed on retry. "
+              "It is not quarantined, so it fails the build. See the PR comment for a "
+              "quarantine entry to paste.".format(args.cell, entry["test"], attempts_desc))
+
+    print("[flake-report] {}: {} flaky, {} persistent, {} quarantined; {} gating".format(
+        args.cell, len(report["flaky"]), len(report["persistent"]),
+        len(report["quarantined"]), report["gating_count"]), file=sys.stderr)
+    return 0
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--list", default=quarantine.DEFAULT_LIST)
+    sub = parser.add_subparsers(dest="command", required=True)
+
+    count = sub.add_parser("count", help="print the number of distinct failed tests")
+    count.add_argument("--dir", required=True)
+    count.set_defaults(func=cmd_count)
+
+    report = sub.add_parser("report", help="classify failures and decide gating")
+    report.add_argument("--cell", required=True)
+    report.add_argument("--evidence-dir", required=True)
+    report.add_argument("--final-status", required=True, choices=["pass", "fail"])
+    report.add_argument("--out", required=True)
+    report.set_defaults(func=cmd_report)
+
+    args = parser.parse_args()
+    return args.func(args)
+
+
+if __name__ == "__main__":
+    sys.exit(main())

.github/scripts/flake_summary.py

@@ -0,0 +1,183 @@
+#!/usr/bin/env python3
+"""Turn the per-cell reports written by flake_report.py into PR-comment markdown.
+
+The matrix runs the same suite across dozens of cells, so the useful unit is the
+test, not the cell: one flaky test shows up as eight red cells, and eight
+unrelated breakages also show up as eight red cells. Grouping by test tells
+those apart.
+
+For anything that looks flaky, this also prints the quarantine entry to paste
+and what to do with it. The judgement -- is this really flaky, is it worth a
+ticket -- stays with a person; the typing does not.
+"""
+
+import argparse
+import datetime
+import glob
+import json
+import os
+import sys
+from collections import OrderedDict
+
+sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
+import quarantine  # noqa: E402
+
+DEFAULT_REVIEW_DAYS = quarantine.DEFAULT_REVIEW_DAYS
+
+
+def load_reports(root_dir):
+    reports = []
+    for path in sorted(glob.glob(os.path.join(root_dir, "**", "*.json"), recursive=True)):
+        try:
+            with open(path) as handle:
+                data = json.load(handle)
+        except (OSError, ValueError):
+            continue
+        if isinstance(data, dict) and "cell" in data:
+            reports.append(data)
+    return reports
+
+
+def group_by_test(reports, key):
+    """OrderedDict of test id -> {cells, message, ticket}."""
+    grouped = OrderedDict()
+    for report in reports:
+        for entry in report.get(key, []):
+            slot = grouped.setdefault(entry["test"], {
+                "cells": [],
+                "message": entry.get("message", ""),
+                "ticket": entry.get("ticket"),
+            })
+            slot["cells"].append(report["cell"])
+    return grouped
+
+
+def short_name(test_id):
+    """com.datadoghq.profiler.FooTest.bar -> FooTest.bar"""
+    parts = test_id.rsplit(".", 2)
+    return ".".join(parts[-2:]) if len(parts) >= 2 else test_id
+
+
+def render_table(grouped, cell_limit=4, row_limit=25, ticket_column=False):
+    header = "| Test | Cells | " + ("Ticket | " if ticket_column else "") + "Message |"
+    rule = "|------|-------|" + ("--------|" if ticket_column else "") + "---------|"
+    lines = [header, rule]
+    for test_id, info in list(grouped.items())[:row_limit]:
+        cells = info["cells"]
+        shown = ", ".join("`{}`".format(c) for c in cells[:cell_limit])
+        if len(cells) > cell_limit:
+            shown += " _+{} more_".format(len(cells) - cell_limit)
+        message = (info["message"] or "").replace("|", "\\|")[:120]
+        ticket = "{} | ".format(info.get("ticket") or "—") if ticket_column else ""
+        lines.append("| `{}` | {} | {}{} |".format(short_name(test_id), shown, ticket, message))
+    if len(grouped) > row_limit:
+        lines.append("")
+        lines.append("_...and {} more. See the job logs._".format(len(grouped) - row_limit))
+    return lines
+
+
+def cells_glob(cells):
+    """A glob covering these cells, when they share an obvious axis.
+
+    Suggesting `*arm64*` for something that only ever failed on arm64 is more
+    useful than listing four cell names, and narrower than quarantining
+    everywhere -- which would hide the same test breaking on x64 tomorrow.
+    """
+    for axis in ("arm64", "aarch64", "musl", "asan", "tsan"):
+        if all(axis in c for c in cells):
+            return ["*{}*".format(axis)]
+    return None
+
+
+def render_proposals(flaky):
+    today = datetime.date.today()
+    review_by = (today + datetime.timedelta(days=DEFAULT_REVIEW_DAYS)).isoformat()
+    out = [
+        "<details>",
+        "<summary><b>Consider quarantining these — click for ready-made entries</b></summary>",
+        "",
+        "A quarantined test still runs and still reports; its failures just stop",
+        "turning CI red. To quarantine one:",
+        "",
+        "1. Open a **PROF** ticket for the test, linking the failing job.",
+        "2. Append the line below to `ddprof-test/quarantine.txt`, replacing",
+        "   `PROF-XXXXX` with the ticket number.",
+        "3. Check the `cells` and `reason` columns — the proposal only knows what",
+        "   failed in this run, and a narrower `cells` glob keeps the same test",
+        "   gating everywhere it has not misbehaved.",
+        "",
+        "CI fails once `review_by` passes, so an entry expires instead of piling up.",
+        "",
+        "```",
+        "# test | ticket | added | review_by | cells | reason",
+    ]
+    for test_id, info in flaky.items():
+        reason = "{} (seen in: {})".format(
+            info["message"] or "intermittent failure",
+            ", ".join(sorted(set(info["cells"]))[:4]),
+        ).replace("|", "/")
+        out.append(quarantine.format_entry(
+            test_id,
+            "PROF-XXXXX",
+            today.isoformat(),
+            review_by,
+            cells_glob(info["cells"]) or [],
+            reason,
+        ))
+    out.append("```")
+    out.append("")
+    out.append("</details>")
+    out.append("")
+    return out
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--dir", required=True, help="directory of downloaded ci-outcome artifacts")
+    args = parser.parse_args()
+
+    reports = load_reports(args.dir)
+    if not reports:
+        return 0
+
+    flaky = group_by_test(reports, "flaky")
+    persistent = group_by_test(reports, "persistent")
+    quarantined = group_by_test(reports, "quarantined")
+
+    out = []
+    if flaky:
+        out.append("### :warning: Flaky tests — failed, then passed on retry")
+        out.append("")
+        out.extend(render_table(flaky))
+        out.append("")
+        out.append(
+            "**These fail the build.** Passing on a second run makes a test flaky, "
+            "not passing. Fix it, or quarantine it against a ticket so the debt is "
+            "tracked rather than forgotten."
+        )
+        out.append("")
+        out.extend(render_proposals(flaky))
+    if persistent:
+        out.append("### :x: Failing tests")
+        out.append("")
+        out.extend(render_table(persistent))
+        out.append("")
+    if quarantined:
+        out.append("### :mute: Quarantined failures — not gating")
+        out.append("")
+        out.extend(render_table(quarantined, ticket_column=True))
+        out.append("")
+
+    retried = [r for r in reports if r.get("attempts", 1) > 1]
+    if retried:
+        out.append("_Retried {} of {} cells._".format(len(retried), len(reports)))
+        out.append("")
+
+    sys.stdout.write("\n".join(out))
+    if out:
+        sys.stdout.write("\n")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())