diff --git a/.github/scripts/flake_report.py b/.github/scripts/flake_report.py new file mode 100755 index 0000000000..ece0e44388 --- /dev/null +++ b/.github/scripts/flake_report.py @@ -0,0 +1,390 @@ +#!/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 re +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 _numbered_attempt_dirs(evidence_dir): + """attempt- directories, numeric suffixes only. + + A stray `attempt-tmp` left by a tool must not raise out of sorted() and + take the whole classification -- and with it the job -- down with it. + """ + found = [] + for path in glob.glob(os.path.join(evidence_dir, "attempt-*")): + suffix = path.rsplit("-", 1)[1] + if suffix.isdigit(): + found.append(path) + return sorted(found, key=_attempt_number) + + +def attempt_results(root_dir): + """(observed test ids, {failed test id: message}) from JUnit XML under root_dir. + + `observed` is every testcase the attempt recorded a result for, pass or + fail. Knowing a test ran and passed is what distinguishes a flake from a + test that simply never got reached on the retry. + """ + observed = set() + 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"): + name = case.get("name") + classname = case.get("classname") + if not name or not classname: + # A testcase element missing either half of its id cannot be + # attributed to any real test; "Class." and ".method" are + # equally unusable as a test id worth counting, tabling, or + # proposing for quarantine. + continue + test_id = "{}.{}".format(classname, name) + if case.find("skipped") is None: + observed.add(test_id) + problem = case.find("failure") + if problem is None: + problem = case.find("error") + if problem is None: + continue + message = (problem.get("message") or problem.get("type") or "").strip() + failures[test_id] = message.splitlines()[0][:200] if message else "failed" + return observed, failures + + +def failed_tests(root_dir): + """Just the failures, for callers that do not care what else ran.""" + return attempt_results(root_dir)[1] + + +def collect_attempts(evidence_dir): + """[(attempt number, observed, failures)] ordered by attempt. + + An attempt that recorded no testcase at all is dropped: it tells us nothing + about any individual test, and counting it would make every failure from + the other attempts look as though it had passed somewhere. + """ + attempts = [] + for path in _numbered_attempt_dirs(evidence_dir): + observed, failures = attempt_results(path) + if observed: + attempts.append((_attempt_number(path), observed, failures)) + return attempts + + +def cmd_count(args): + print(len(failed_tests(args.dir))) + return 0 + + +_NON_TEST_TASK_FAILURE_RE = re.compile(r"Execution failed for task '([^']+)'") + +# Gradle reporting that a forked test JVM died, and the JVM's own crash banner. +# An ordinary test failure makes Gradle exit non-zero too, so the exit code by +# itself cannot separate "the quarantined test failed" from "the JVM died with +# tests still unrun" -- but Gradle names the latter explicitly. +_CUT_SHORT_RE = re.compile( + r"finished with non-zero exit value" + r"|A fatal error has been detected by the Java Runtime Environment" + r"|hs_err_pid" +) + + +def non_test_task_failures(log_path, test_task_pattern): + """Gradle task names blamed for a failure, other than the test task itself. + + Quarantine excuses the tests it names; it does not excuse the build. If + this invocation's log also blames a compile, a native gtest, or a + verification task, that failure has nothing to do with the list. + """ + if not log_path or not os.path.isfile(log_path): + return [] + found = set() + with open(log_path, errors="replace") as handle: + for line in handle: + m = _NON_TEST_TASK_FAILURE_RE.search(line) + if m and test_task_pattern not in m.group(1): + found.add(m.group(1)) + return sorted(found) + + +def cut_short_marker(log_path): + """The log line fragment showing this invocation was cut short, or None. + + A missing or unreadable log yields None, matching non_test_task_failures() + above: with no log there is no evidence either way, and the alternative + would be to gate every quarantined failure on the strength of a file the + caller happened not to pass. + """ + if not log_path or not os.path.isfile(log_path): + return None + with open(log_path, errors="replace") as handle: + for line in handle: + m = _CUT_SHORT_RE.search(line) + if m: + return m.group(0) + return None + + +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] + # Flaky means seen both ways: failed here, ran and passed there. A test + # that is merely missing from the retry never re-ran -- an attempt that + # aborted early, a filtered suite -- and claiming that as a pass would + # hand out quarantine proposals for tests nobody has cleared. + passed_in = [n for n, seen, f in attempts if test_id in seen and test_id not in f] + hit = quarantine.find_entry(entries, test_id, args.cell) + results.append({ + "test": test_id, + "failed_attempts": failed_in, + "message": next(f[test_id] for _, _, f in attempts if test_id in f), + "flaky": bool(passed_in), + "quarantined": hit is not None, + "ticket": hit.get("ticket") if hit else None, + }) + + gating = [r for r in results if not r["quarantined"]] + + # The caller's exit-code decision must never be made from failures + # aggregated across every attempt: those can all be quarantined while the + # final attempt itself failed for a reason that named no test at all (a + # docker or Gradle configuration failure, an OOM-killed daemon, an ASan + # init abort). Report the final attempt's own standing separately so the + # caller can require it to have actually produced test results, with every + # one of its own named failures quarantined, before trusting the list. + final = attempts[-1] if attempts else None + final_attempt_ran = final is not None and final[0] == args.final_attempt + final_attempt_gating_count = None + final_attempt_failure_count = None + if final_attempt_ran: + _, _, final_failures = final + final_attempt_failure_count = len(final_failures) + final_attempt_gating_count = sum( + 1 for test_id in final_failures + if quarantine.find_entry(entries, test_id, args.cell) is None + ) + + other_task_failures = non_test_task_failures(args.attempt_log, args.test_task_pattern) + + # Two independent signs that the final attempt stopped early rather than + # running to completion and failing tests: Gradle saying so in the log, and + # the attempt having reached fewer tests than another attempt managed. The + # latter needs more than one attempt to compare against, which slow suites + # (MAX_ATTEMPTS=1) do not have, so the log is the primary signal. + final_attempt_cut_short = cut_short_marker(args.attempt_log) + observed_shortfall = None + if final_attempt_ran and ran > 1: + best_observed = max(len(seen) for _, seen, _ in attempts) + final_observed = len(final[1]) + if final_observed < best_observed: + observed_shortfall = (final_observed, best_observed) + + # The gating verdict, owned here rather than re-derived by the caller from + # raw counts: three independent readers of this file re-deciding the same + # thing is how a schema change turns into shotgun surgery. + # + # any un-quarantined failure -> gate, even if a retry passed. A flake + # nobody has quarantined is still a + # failure. + # every failure quarantined -> excuse, but only when the *final* + # attempt itself ran and recorded results + # (final_attempt_ran), and every failure + # it did name is quarantined. A final + # attempt that crashed before recording a + # single testcase drops out of `attempts` + # entirely, so final_attempt_ran is False + # and it is never waved through just + # because an *earlier* attempt's failures + # all happen to be quarantined. A final + # attempt that ran and simply passed + # outright (zero failures of its own) is + # the ordinary flaky-then-fixed case and + # must not gate. + # final attempt exited -> gate only with evidence that it was cut + # non-zero and was cut short short: Gradle reporting a dead test JVM, + # or the attempt having reached fewer + # tests than another attempt managed. A + # quarantined test that fails makes Gradle + # exit non-zero all by itself, so treating + # every non-zero exit as a crash would + # gate the ordinary case the list exists + # to excuse. Gradle blames a crash on the + # test task itself, so the non-test task + # check above cannot see it. + # no failure named -> no opinion; the caller keeps its own + # exit code (a compile error or a dead + # runner is nothing to do with + # quarantine). + if gating: + gates = True + gate_reason = "{} un-quarantined failure(s)".format(len(gating)) + elif results: + # Suspect evidence belongs here, inside the branch that has something + # to excuse: it is a reason to distrust the quarantine excuse, not a + # test failure of its own. A run where everything passed but the + # results tree could not be cleared has nothing to excuse, so its own + # exit code stands. + if args.evidence_suspect: + gates = True + gate_reason = ( + "flake evidence for this cell is suspect (the results directory " + "could not be reliably cleared between attempts), so a prior " + "attempt's results may be mistaken for the final attempt's own" + ) + elif not final_attempt_ran or final_attempt_gating_count != 0: + gates = True + gate_reason = ( + "the final attempt did not itself pass with only quarantined " + "failures (ran={}, its own named failures={}, its own " + "unquarantined count={}); failures from an earlier attempt " + "cannot be trusted instead" + ).format(final_attempt_ran, final_attempt_failure_count, final_attempt_gating_count) + elif other_task_failures: + gates = True + gate_reason = "all failing tests are quarantined, but the build also failed in {}".format( + ", ".join(other_task_failures)) + elif args.final_attempt_exit_code not in (None, 0) and final_attempt_cut_short: + gates = True + gate_reason = ( + "the final attempt exited {} and its log shows the run was cut " + "short ({!r}), so the tests missing from its results cannot be " + "read as quarantined" + ).format(args.final_attempt_exit_code, final_attempt_cut_short) + elif args.final_attempt_exit_code not in (None, 0) and observed_shortfall: + gates = True + gate_reason = ( + "the final attempt exited {} having reached only {} of the {} " + "tests another attempt ran, so it stopped early and the tests " + "missing from its results cannot be read as quarantined" + ).format(args.final_attempt_exit_code, *observed_shortfall) + else: + gates = False + gate_reason = "all {} failing test(s) are quarantined".format(len(results)) + else: + gates = None + gate_reason = None + + report = { + "cell": args.cell, + "attempts": ran, + "attempts_run": args.final_attempt, + # A single attempt cannot distinguish flaky from broken: passed_in is + # empty for every failure because nothing re-ran. Calling those + # "persistent" would assert something the run never measured, so they + # get their own bucket and their own proposals. + "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"] and ran > 1 + ], + "unclassified": [ + r for r in results + if not r["flaky"] and not r["quarantined"] and ran <= 1 + ], + "quarantined": [r for r in results if r["quarantined"]], + "gating_count": len(gating), + "failure_count": len(results), + "final_attempt_ran": final_attempt_ran, + "final_attempt_gating_count": final_attempt_gating_count, + "final_attempt_failure_count": final_attempt_failure_count, + "other_task_failures": other_task_failures, + "final_attempt_cut_short": final_attempt_cut_short, + "final_attempt_observed_shortfall": observed_shortfall, + "final_attempt_exit_code": args.final_attempt_exit_code, + "gates": gates, + "gate_reason": gate_reason, + } + + 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-attempt", required=True, type=int, + help="the attempt number the caller actually ran last") + report.add_argument("--out", required=True) + report.add_argument("--final-attempt-exit-code", default=None, type=int, + help="the exit status of the final attempt's command; a non-zero " + "status with no named failure of its own means the run was " + "cut short and must not be excused by the list") + report.add_argument("--attempt-log", default=None, + help="the final attempt's raw log, to check for non-test task failures") + report.add_argument("--test-task-pattern", default=":ddprof-test:test", + help="Gradle task name the quarantine list is entitled to excuse") + report.add_argument("--evidence-suspect", action="store_true", + help="the caller could not reliably isolate this attempt's own " + "results (e.g. a stale results directory it could not clear); " + "never let the quarantine list excuse the exit code") + report.set_defaults(func=cmd_report) + + args = parser.parse_args() + return args.func(args) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/scripts/flake_summary.py b/.github/scripts/flake_summary.py new file mode 100755 index 0000000000..f7429c15cf --- /dev/null +++ b/.github/scripts/flake_summary.py @@ -0,0 +1,354 @@ +#!/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 re +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 + +# Test ids and failure messages come from the PR's own test code, not from +# anything CI controls, and this comment is rendered as markdown and offered +# up as a ready-to-paste quarantine entry. Neither may carry markdown, HTML, or +# a fence-breaking ``` sequence into that render. +_SAFE_TEST_ID_RE = re.compile(r"[^A-Za-z0-9_.$-]") + +# The display width for a failure message in this module's tables. +# flake_report.py stores messages at a wider cap (200 chars) for anyone reading +# the raw JSON. +MESSAGE_DISPLAY_WIDTH = 120 + + +def sanitize_test_id(test_id): + return _SAFE_TEST_ID_RE.sub("_", test_id) + + +def sanitize_quarantine_test_pattern(test_id): + """Sanitize a test id for the *paste-ready quarantine entry*, not display. + + sanitize_test_id() is safe for markdown but rewrites JUnit's parameterized- + and dynamic-test punctuation (brackets, parens, commas) to '_', producing a + pattern quarantine.covers() (exact string equality, or a trailing '.*') + can never match against the real id. When the method name would need that + rewriting to render safely, fall back to the class-wide '.*' pattern + instead, which is still an exact, matchable pattern and rendering-safe as + is (it contains no character _SAFE_TEST_ID_RE would touch). + """ + # A plain JUnit 5 method arrives as `Class.method()`. Those parentheses are + # the only unsafe characters in it, and quarantine.covers() normalises them + # away, so proposing the documented `Class.method` keeps the entry precise + # rather than muting the whole class. + if test_id.endswith("()") and not _SAFE_TEST_ID_RE.search(test_id[:-2]): + return test_id[:-2] + if not _SAFE_TEST_ID_RE.search(test_id): + return test_id + classname = test_id.rsplit(".", 1)[0] + if classname and not _SAFE_TEST_ID_RE.search(classname): + return classname + ".*" + return sanitize_test_id(test_id) + + +def sanitize_inline(text): + """Strip newlines and backticks so text can't break a table row, a code + span, or the ``` fence around the quarantine proposals.""" + return text.replace("`", "'").replace("\n", " ").replace("\r", " ") + + +def load_reports(root_dir): + """(reports, files found, files skipped). + + Skipped covers anything that looked like a report but wasn't usable: JSON + that failed to parse, or parsed into something that isn't a report at all. + Distinguishing "found nothing" from "found reports, all clean" from "found + reports, some unreadable" is the point -- a total artifact-download failure + must not render the same as a spotless run. + """ + reports = [] + skipped = 0 + paths = sorted(glob.glob(os.path.join(root_dir, "**", "*.json"), recursive=True)) + for path in paths: + try: + with open(path) as handle: + data = json.load(handle) + except (OSError, ValueError): + skipped += 1 + continue + if isinstance(data, dict) and "cell" in data: + reports.append(data) + else: + skipped += 1 + return reports, len(paths), skipped + + +def group_by_test(reports, key): + """OrderedDict of test id -> {cells, message, ticket, tickets}. + + quarantine.py deliberately allows the same test to carry different + tickets on disjoint cell globs, so this keeps every ticket seen (not just + the first report's) and every message, rather than collapsing them to + whichever report happened to load first. + """ + 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"), + "tickets": [], + "messages": [], + }) + slot["cells"].append(report["cell"]) + ticket = entry.get("ticket") + if ticket and ticket not in slot["tickets"]: + slot["tickets"].append(ticket) + message = entry.get("message", "") + if message and message not in slot["messages"]: + slot["messages"].append(message) + 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"] + # cell names come from this PR's own workflow file and must go + # through the same sanitizer as everything else rendered here. + shown = ", ".join("`{}`".format(sanitize_test_id(c)) for c in cells[:cell_limit]) + if len(cells) > cell_limit: + shown += " _+{} more_".format(len(cells) - cell_limit) + message = sanitize_inline((info["message"] or "").replace("|", "\\|"))[:MESSAGE_DISPLAY_WIDTH] + message_cell = "`{}`".format(message) if message else "" + if ticket_column: + # ticket comes from this PR's own quarantine.txt line and is + # rendered into the same PR comment -- it must not be trusted + # unescaped any more than the test id or the message are. + tickets = info.get("tickets") or ([info["ticket"]] if info.get("ticket") else []) + ticket_text = ", ".join(sanitize_test_id(t) for t in tickets) or "—" + ticket = "{} | ".format(ticket_text) + else: + ticket = "" + lines.append("| `{}` | {} | {}{} |".format( + sanitize_test_id(short_name(test_id)), shown, ticket, message_cell)) + 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 one or more obvious axes. + + Suggesting `*aarch64*` for something that only ever failed on aarch64 is more + useful than listing four cell names, and narrower than quarantining + everywhere -- which would hide the same test breaking on x64 tomorrow. + Every shared axis narrows the glob further: a test failing only on + musl+aarch64 gets `*musl*aarch64*` rather than the wider `*aarch64*` + (which would also cover glibc aarch64). + + Cell names are `---[-slow]`, so the axes are read + positionally out of that grammar rather than matched against a hardcoded + token list. A list would silently omit whichever axis nobody thought of -- + the JDK (which the matrix varies along most) and the regular/slow suffix + were both missing, so a flake seen only on `glibc-8-j9-debug-amd64` + proposed `*glibc*debug*amd64*` and quarantined it on all 13 JDKs and on the + slow suite too. + + A field the cells disagree on becomes `*`; one they share is kept + literally. The suffix is an axis in its own right: a proposal derived from + regular cells ends in the arch so it cannot also match that cell's `-slow` + twin. + """ + fields = [c.split("-") for c in cells] + # A cell that does not parse as ---[-slow] (a jdk + # like "8-j9" makes that five or six fields) is not something to guess at. + widths = {len(f) for f in fields} + if len(widths) != 1: + return None + width = widths.pop() + if width < 4: + return None + shared = [ + fields[0][i] if all(f[i] == fields[0][i] for f in fields) else "*" + for i in range(width) + ] + if all(part == "*" for part in shared): + return None + glob = "-".join(shared) + # Anchored at the end so a regular-suite proposal cannot match `-slow`; + # leading `*` only if the first field itself is unconstrained. + return [glob if shared[0] != "*" else "*" + glob.lstrip("*")] + + +def widened_note(test_id, pattern): + """A warning line when the proposed pattern covers more than was observed. + + covers() understands an exact id or a class-wide `.*` and nothing else, so + an id it cannot express exactly (a parameterized invocation index) can only + be proposed class-wide. That mutes every test in the class, which is a + different decision from the one the evidence supports -- so it is stated + rather than left for the reviewer to notice. + """ + if pattern.endswith(".*") and not test_id.endswith(".*"): + return ("# WIDENED: observed `{}`, which covers() cannot match exactly; " + "this entry mutes the whole class".format(test_id)) + return None + + +def render_proposals(flaky, proposal_limit=25): + today = datetime.date.today() + review_by = (today + datetime.timedelta(days=DEFAULT_REVIEW_DAYS)).isoformat() + out = [ + "
", + "Consider quarantining these — click for ready-made entries", + "", + "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", + ] + items = list(flaky.items()) + for test_id, info in items[:proposal_limit]: + reason = sanitize_inline("{} (seen in: {})".format( + info["message"] or "intermittent failure", + ", ".join(sorted(set(info["cells"]))[:4]), + )).replace("|", "/") + pattern = sanitize_quarantine_test_pattern(test_id) + note = widened_note(test_id, pattern) + if note: + out.append(note) + out.append(quarantine.format_entry( + pattern, + "PROF-XXXXX", + today.isoformat(), + review_by, + cells_glob(info["cells"]) or [], + reason, + )) + if len(items) > proposal_limit: + out.append("# ...and {} more. See the job logs.".format(len(items) - proposal_limit)) + out.append("```") + out.append("") + out.append("
") + 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, files_found, files_skipped = load_reports(args.dir) + if not files_found: + # Distinct from "reports loaded, all clean": this is what a total + # ci-outcome artifact-download failure looks like, and it must not + # render as a silent, spotless run. + sys.stdout.write("_No CI outcome reports were found for this run._\n") + return 0 + if not reports: + if files_skipped: + sys.stdout.write( + "_{} CI outcome report(s) were found but could not be parsed._\n" + .format(files_skipped)) + return 0 + + flaky = group_by_test(reports, "flaky") + persistent = group_by_test(reports, "persistent") + unclassified = group_by_test(reports, "unclassified") + quarantined = group_by_test(reports, "quarantined") + + out = [] + if files_skipped: + out.append("_{} of {} CI outcome report(s) could not be parsed and were skipped._".format( + files_skipped, files_found)) + out.append("") + 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 unclassified: + out.append("### :grey_question: Failing tests — flakiness not measured") + out.append("") + out.extend(render_table(unclassified)) + out.append("") + out.append( + "**These fail the build.** The suite was not retried (slow suites run " + "once), so whether they are flaky or broken was never measured — the " + "entry below is offered on the same terms as a flake's, and the " + "judgement is still yours." + ) + out.append("") + out.extend(render_proposals(unclassified)) + if quarantined: + out.append("### :mute: Quarantined failures — not gating") + out.append("") + out.extend(render_table(quarantined, ticket_column=True)) + out.append("") + + # attempts_run counts every attempt the runner actually executed; + # `attempts` counts only attempts that produced JUnit results, which + # undercounts a cell whose first attempt aborted before writing any XML + # (e.g. an ASan init abort) and only produced results on the retry. + retried = [r for r in reports if r.get("attempts_run", 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()) diff --git a/.github/scripts/generate-test-summary.sh b/.github/scripts/generate-test-summary.sh index a6cbcfcc58..19fc6b710d 100755 --- a/.github/scripts/generate-test-summary.sh +++ b/.github/scripts/generate-test-summary.sh @@ -89,18 +89,26 @@ while IFS= read -r job; do started_at=$(echo "$job" | jq -r '.started_at') completed_at=$(echo "$job" | jq -r '.completed_at') - # Only process test jobs (match pattern: test-linux-{libc}-{arch} ({java}, {config})) + # Only process test jobs (match pattern: + # test-linux-{libc}-{arch} ({java}, {config}, {slow|regular})) # Note: regex stored in variable to avoid bash parsing issues with ) character # Note: No ^ anchor because reusable workflow jobs are prefixed with caller job name - # e.g., "test-matrix / test-linux-glibc-amd64 (8, debug)" - test_job_pattern='test-linux-([a-z]+)-([a-z0-9]+) \(([^,]+), ([^)]+)\)$' + # e.g., "test-matrix / test-linux-glibc-amd64 (8, debug, regular)" + # The trailing slow/regular comes from a workflow input, not a matrix axis, + # so it has to be captured here too -- test_workflow.yml is called twice + # with overlapping configs in the same run (nightly, release-validated), + # and without it two different jobs collapse onto the same cell. + test_job_pattern='test-linux-([a-z]+)-([a-z0-9]+) \(([^,]+), ([^,]+), (slow|regular)\)$' if [[ "$name" =~ $test_job_pattern ]]; then libc="${BASH_REMATCH[1]}" arch="${BASH_REMATCH[2]}" java_version="${BASH_REMATCH[3]}" config="${BASH_REMATCH[4]}" + suite="${BASH_REMATCH[5]}" + suite_suffix="" + [[ "$suite" == "slow" ]] && suite_suffix="-slow" - platform="${libc}-${arch}/${config}" + platform="${libc}-${arch}/${config}${suite_suffix}" # Calculate duration if [[ -n "$started_at" && "$started_at" != "null" && -n "$completed_at" && "$completed_at" != "null" ]]; then @@ -172,55 +180,15 @@ for key in "${!job_status[@]}"; do fi done -# --- Download failure artifacts (if any failures) --- -declare -A failure_details=() -failure_details["__init__"]=1; unset 'failure_details[__init__]' - -if ((failed_count > 0)); then - log "Downloading failure artifacts..." - mkdir -p ./failure-artifacts - - # Try to download test reports - gh run download "$RUN_ID" --pattern '(test-reports)*' --dir ./failure-artifacts 2>/dev/null || true - - # Parse JUnit XML for failure details - for key in "${failed_jobs[@]}"; do - IFS='|' read -r platform java_version <<< "$key" - - # Find matching test report directory - # Pattern: (test-reports) test-linux-{libc}-{arch} ({java}, {config}) - IFS='/' read -r libc_arch config <<< "$platform" - report_pattern="./failure-artifacts/*${libc_arch}*${java_version}*${config}*" - - failures="" - for report_dir in $report_pattern; do - if [[ -d "$report_dir" ]]; then - # Parse JUnit XML files - for xml_file in "$report_dir"/**/TEST-*.xml; do - if [[ -f "$xml_file" ]]; then - # Extract failed test cases - while IFS= read -r testcase; do - classname=$(echo "$testcase" | grep -oP 'classname="\K[^"]+' || echo "") - testname=$(echo "$testcase" | grep -oP 'name="\K[^"]+' || echo "") - # Get failure message (first line only, truncated) - failure_msg=$(echo "$testcase" | grep -oP ']*message="\K[^"]*' | head -c 100 || echo "") - - if [[ -n "$classname" && -n "$testname" ]]; then - short_class="${classname##*.}" - failures+="| \`${short_class}.${testname}\` | ${failure_msg:-Test failed} |"$'\n' - fi - done < <(grep -Pzo '(?s)]*>.*?' "$xml_file" 2>/dev/null | tr '\0' '\n' | grep -E '<(failure|error)' || true) - fi - done - fi - done - - failure_details["$key"]="$failures" - done - - # Cleanup - rm -rf ./failure-artifacts -fi +# --- Download outcome artifacts (for flake_summary.py below) --- +# Per-cell outcome reports, written by run_tests_with_retry.sh and uploaded +# whether the cell passed or failed. A cell that only went green on a retry +# produces no failure artifact at all, so this is the one place its flaky test +# is recorded. +OUTCOME_DIR="./ci-outcome-artifacts" +log "Downloading CI outcome reports..." +mkdir -p "$OUTCOME_DIR" +gh run download "$RUN_ID" --pattern '(ci-outcome)*' --dir "$OUTCOME_DIR" 2>/dev/null || true # --- Generate markdown --- log "Generating markdown summary..." @@ -284,35 +252,29 @@ log "Generating markdown summary..." echo "" fi - # Failed tests details - if ((failed_count > 0)); then - echo "### Failed Tests" + # Flaky and failing tests, grouped by test rather than by cell. One flaky + # test reddens a dozen cells and so does a dozen unrelated breakages; only + # grouping by test tells those apart. + python3 "$(dirname "$0")/flake_summary.py" --dir "$OUTCOME_DIR" \ + || echo "_Could not render the flaky-test summary; see the job log._" + + # Failed jobs, linked to their logs. Which tests failed and why is the + # flaky/persistent/quarantined tables above -- rendering it a second time + # here, grouped by job instead of by test, only gave the same failure two + # different messages if the two renderers' sanitizing ever drifted. + if ((${#failed_jobs[@]} > 0)); then + echo "### Failed Jobs" echo "" - for key in "${failed_jobs[@]}"; do IFS='|' read -r platform java_version <<< "$key" url="${job_url[$key]:-}" - details="${failure_details[$key]:-}" - - echo "
" - echo "${platform} / ${java_version}" - echo "" if [[ -n "$url" ]]; then - echo "**Job:** [View logs]($url)" - echo "" - fi - - if [[ -n "$details" ]]; then - echo "| Test | Error |" - echo "|------|-------|" - echo -n "$details" + echo "- **${platform} / ${java_version}** — [view logs]($url)" else - echo "_No detailed failure information available. Check the job logs._" + echo "- **${platform} / ${java_version}**" fi - echo "" - echo "
" - echo "" done + echo "" fi # Summary statistics (single line) @@ -331,5 +293,7 @@ log "Generating markdown summary..." } > "$OUTPUT_FILE" +rm -rf "$OUTCOME_DIR" + log "Summary written to $OUTPUT_FILE" log "Total jobs: $total_jobs, Passed: $passed_jobs, Failed: $failed_count" diff --git a/.github/scripts/prepare_reports.sh b/.github/scripts/prepare_reports.sh index 4ff852450e..b64dd69c2a 100755 --- a/.github/scripts/prepare_reports.sh +++ b/.github/scripts/prepare_reports.sh @@ -12,6 +12,22 @@ cp ddprof-test/javacore*.txt test-reports/ || true cp ddprof-test/build/hs_err* test-reports/ || true cp -r ddprof-lib/build/tmp test-reports/native_build || true cp -r ddprof-test/build/reports/tests test-reports/tests || true +# The JUnit XML of every attempt, for reading by hand, normally comes from +# flake-evidence/ alone (copied below): run_tests_with_retry.sh snapshots +# each attempt's build/test-results there, and flake-evidence/attempt- +# holds exactly what build/test-results itself holds once the run is over. +# The one case that snapshots nothing at all is a suite that passed outright +# on its first attempt (skipped as a needless copy with no other attempt to +# compare against) -- copy build/test-results directly only then, so a green +# run still ships its JUnit XML. +# Keyed on real evidence rather than on the attempt-* directory: snapshot() +# creates that with mkdir -p *before* the copy that may fail, so an empty or +# partial snapshot would otherwise skip this fallback and ship an artifact with +# no JUnit XML in it at all. +if [ -z "$(find flake-evidence -name 'TEST-*.xml' 2>/dev/null | head -1)" ]; then + cp -r ddprof-test/build/test-results test-reports/test-results || true +fi +cp -r flake-evidence test-reports/flake-evidence || true cp build/logs/gdb-watchdog.log test-reports/ || true cp -r /tmp/recordings test-reports/recordings || true find ddprof-lib/build -name 'libjavaProfiler.*' -exec cp {} test-reports/ \; || true diff --git a/.github/scripts/quarantine.py b/.github/scripts/quarantine.py new file mode 100755 index 0000000000..fb048073f1 --- /dev/null +++ b/.github/scripts/quarantine.py @@ -0,0 +1,383 @@ +#!/usr/bin/env python3 +"""The quarantine list: which failing tests do not turn CI red. + +Its `validate` subcommand enforces the format, the ticket, and the review_by +date. The gating decision itself (find_entry(), covers(), applies_to()) is a +library used in-process by flake_report.py -- there is no CLI for it, so the +rule CI actually runs cannot drift from a separate CLI wrapper. + +The paste-ready entry a PR comment proposes for a flaky test is rendered by +flake_summary.py's own call to format_entry() below, not by this module's CLI. + +The list is a plain text table (see ddprof-test/quarantine.txt) rather than +JSON or YAML: it is edited by hand far more often than by machine, so real +comments, one-line diffs and clean `git blame` matter more than a schema. It +also has to parse inside the Alpine test containers, where PyYAML cannot be +assumed -- this needs nothing but str.split. +""" + +import argparse +import datetime +import fnmatch +import os +import re +import sys + +DEFAULT_LIST = os.path.join("ddprof-test", "quarantine.txt") +TICKET_RE = re.compile(r"^PROF-\d+$") +DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") +FIELDS = ("test", "ticket", "added", "review_by", "cells", "reason") +# Long enough not to be busywork, short enough that a quarantine outlives +# neither the release it was added in nor the memory of why. +DEFAULT_REVIEW_DAYS = 90 +# A review_by further out than this is not a review date, it is a way to write +# "never" without saying so. Padded above DEFAULT_REVIEW_DAYS since a proposal +# is dated `added` at the moment it is written, and review_by is measured from +# whenever the entry is actually appended -- which is not the same day. +MAX_REVIEW_DAYS = DEFAULT_REVIEW_DAYS + 30 +# The `test` field is an exact test id, optionally ending in a class-wide +# ".*" -- that is all covers() understands. Anything else (a bare "*", a "?", +# or a wildcard anywhere but as the final two characters) passes validate() +# today and then silently quarantines nothing at runtime. +BAD_TEST_WILDCARD_RE = re.compile(r"[*?]") + +# Cell names are ---. Only libc and arch are a closed +# set -- jdk and config come from the workflow inputs and grow without warning +# -- so those two are the only axes worth checking a glob against. +KNOWN_ARCHES = ("amd64", "aarch64") +KNOWN_LIBCS = ("glibc", "musl") +# Anything that reads like an architecture. A glob naming one that CI never +# builds silently quarantines nothing, which is how "*arm64*" shipped in this +# file's own example: the arch is spelled aarch64. +ARCH_LIKE_RE = re.compile(r"(?:x86|x64|amd|arm|aarch|i386|ppc|s390)[\w_]*") + +# A synthetic universe of cell names, used only to ask whether two entries' +# cell globs could both match the same real cell. Wide enough to catch a glob +# written against any axis (jdk, config, the libc/arch pair, or the slow/regular +# suite suffix) without having to enumerate the workflow's actual, ever-growing +# matrix. Over-inclusive on purpose: a synthetic cell that never occurs for +# real only makes overlap detection more conservative, never less. Under- +# inclusive is the dangerous direction -- cells_overlap() fails closed for a +# glob matching nothing synthetic, so a JDK variant missing from here makes two +# genuinely disjoint entries look like duplicates and fail validation. +_SYNTHETIC_JDK_BASES = ("8", "11", "17", "21", "25") +_SYNTHETIC_JDK_SUFFIXES = ("", "-orcl", "-j9", "-ibm", "-graal") +_SYNTHETIC_JDKS = tuple( + base + suffix + for base in _SYNTHETIC_JDK_BASES + for suffix in _SYNTHETIC_JDK_SUFFIXES +) +_SYNTHETIC_CONFIGS = ("debug", "release", "asan", "tsan") +_SYNTHETIC_SUITE_SUFFIXES = ("", "-slow") +SYNTHETIC_CELLS = tuple( + "{}-{}-{}-{}{}".format(libc, jdk, config, arch, suffix) + for libc in KNOWN_LIBCS + for jdk in _SYNTHETIC_JDKS + for config in _SYNTHETIC_CONFIGS + for arch in KNOWN_ARCHES + for suffix in _SYNTHETIC_SUITE_SUFFIXES +) + + +def _matches_any_synthetic_cell(globs): + return any(any(fnmatch.fnmatch(cell, g) for g in globs) for cell in SYNTHETIC_CELLS) + + +def cells_overlap(globs_a, globs_b): + """Could some real cell match both sets of globs? No globs means every cell. + + Equal glob lists always overlap without needing the synthetic universe, + which matters when a glob names an axis (like a jdk or config) that + SYNTHETIC_CELLS does not model. And when a glob's axis is genuinely + unmodelled -- it matches nothing in the synthetic universe at all -- this + fails closed (treats it as overlapping) rather than open: a duplicate that + cells_overlap cannot evaluate is exactly the case validate() must not wave + through, since find_entry() would still only honour the first entry. + """ + if not globs_a or not globs_b: + return True + if sorted(globs_a) == sorted(globs_b): + return True + if not _matches_any_synthetic_cell(globs_a) or not _matches_any_synthetic_cell(globs_b): + return True + return any( + any(fnmatch.fnmatch(cell, g) for g in globs_a) + and any(fnmatch.fnmatch(cell, g) for g in globs_b) + for cell in SYNTHETIC_CELLS + ) + + +def parse(path): + """([entry], [(line number, message)]) — entries and malformed lines. + + Each entry carries `_line` so validate() can point at the offender. + """ + entries, errors = [], [] + if not os.path.exists(path): + return entries, errors + + with open(path) as handle: + for number, raw in enumerate(handle, start=1): + line = raw.strip() + if not line or line.startswith("#"): + continue + + parts = [p.strip() for p in line.split("|")] + if len(parts) != len(FIELDS): + errors.append((number, "expected {} fields separated by '|', found {}".format( + len(FIELDS), len(parts)))) + continue + + entry = dict(zip(FIELDS, parts)) + entry["cells"] = [c.strip() for c in entry["cells"].split(",") + if c.strip() and c.strip() != "-"] + entry["_line"] = number + entries.append(entry) + + return entries, errors + + +def load(path): + """Entries only, for callers that just need to match against the list.""" + return parse(path)[0] + + +def applies_to(entry, cell): + """Does this entry cover the given cell? No globs means everywhere.""" + globs = entry.get("cells") + if not globs: + return True + return any(fnmatch.fnmatch(cell, g) for g in globs) + + +_INVOCATION_INDEX_RE = re.compile(r"^\[\d+\]$") + + +def normalise_test_id(test_id): + """A JUnit XML test id reduced to the shape entries are written in. + + Gradle writes a JUnit 5 @Test method as `name="method()"`, so the id built + from the report is `Class.method()`, while quarantine.txt documents -- and + a human writes -- `Class.method`. Stripping the parentheses here, in the + one function every caller's match goes through, keeps the documented shape + matching the id JUnit actually produces while reports and annotations go + on showing the real name. + """ + return test_id[:-2] if test_id.endswith("()") else test_id + + +def covers(entry, test_id): + pattern = entry["test"] + test_id = normalise_test_id(test_id) + if pattern.endswith(".*"): + # "covers every method in the class", per quarantine.txt -- so the + # remainder after the class name must be a single method segment. An + # unbounded prefix match would make "com.datadoghq.profiler.*" suspend + # the merge gate for the entire repository from one validate-clean + # line. + prefix = pattern[:-1] + if not test_id.startswith(prefix): + return False + return "." not in test_id[len(prefix):] + return test_id == pattern + + +def is_expired(entry, today=None): + """True once this entry's review_by date has passed. + + validate-quarantine (quarantine.py's own CLI) only runs from ci.yml, but + find_entry() is called from every workflow that reuses run_tests_with_retry.sh + (nightly.yml, release-validated.yml included). Enforcing expiry here, at + match time, means a stale mute cannot keep quarantining a failure just + because the workflow that hit it never runs the separate validator. + """ + review_by = entry.get("review_by", "") + if not DATE_RE.match(review_by): + # Blank or not a date at all. parse() accepts both, and validate() + # only runs in PR CI, so treating an unreadable expiry as "never + # expires" would let the one entry nobody can review outlive every + # entry that can be. An expiry that cannot be read has passed. + return True + try: + due = datetime.date.fromisoformat(review_by) + except ValueError: + return True + return due < (today or datetime.date.today()) + + +def find_entry(entries, test_id, cell): + """The first entry quarantining this test on this cell, or None. + + Every caller that decides whether a failure gates goes through here, so the + matching rule cannot drift between the subcommand and flake_report.py. An + expired entry is treated as absent rather than as a hit, so it can never + excuse a failure outside the PR CI that happens to run validate-quarantine. + """ + return next( + (e for e in entries + if covers(e, test_id) and applies_to(e, cell) and not is_expired(e)), + None, + ) + + +def format_entry(test, ticket, added, review_by, cells, reason): + return " | ".join([test, ticket, added, review_by, ",".join(cells) or "-", reason]) + + +def cmd_validate(args): + # parse() tolerates a missing file so that matching still works before the + # first entry lands. Validation must not: "0 quarantined test(s), all + # valid" for a list that has been renamed or deleted would report success + # at the exact moment gating silently stopped applying everywhere. + if not os.path.exists(args.list): + print("::error::quarantine list '{}' does not exist".format(args.list)) + return 1 + + entries, problems = parse(args.list) + today = datetime.date.today() + seen_by_name = [] + + def complain(line, message): + problems.append((line, message)) + + for entry in entries: + line = entry["_line"] + name = entry["test"] + + for field in FIELDS: + if field == "cells": + continue # optional, normalised to [] above + if not entry[field]: + complain(line, "field '{}' is empty".format(field)) + + # Two entries shadow each other on cells where they overlap when either + # pattern covers() the other -- not just when the `test` strings are + # identical. A trailing ".*" entry covers individual methods too, and + # find_entry() only ever returns the first match, so the second + # entry's ticket and review_by silently never take effect on the + # cells the two share. + for prior in seen_by_name: + if not (covers(prior, entry["test"]) or covers(entry, prior["test"])): + continue + if cells_overlap(prior["cells"], entry["cells"]): + where = ", ".join(entry["cells"]) or "every cell" + complain(line, "'{}' is already quarantined (as '{}') for {} on line {}".format( + name, prior["test"], where, prior["_line"])) + break + seen_by_name.append(entry) + + if entry["ticket"] and not TICKET_RE.match(entry["ticket"]): + complain(line, "ticket '{}' is not a PROF-".format(entry["ticket"])) + + for field in ("added", "review_by"): + if entry[field] and not DATE_RE.match(entry[field]): + complain(line, "{} '{}' is not YYYY-MM-DD".format(field, entry[field])) + + stem = entry["test"][:-2] if entry["test"].endswith(".*") else entry["test"] + if entry["test"] and (not stem or BAD_TEST_WILDCARD_RE.search(stem)): + complain(line, ( + "test pattern '{}' is not an exact id or a class-wide '.*'; " + "covers() understands nothing else, so this would silently " + "quarantine nothing" + ).format(entry["test"])) + + # ".*" is not a class. covers() scopes a trailing '.*' to one + # class's methods, so a package-level pattern quarantines nothing -- + # while looking like it quarantines a great deal. + if entry["test"].endswith(".*") and stem: + last = stem.rsplit(".", 1)[-1] + if last and not last[:1].isupper(): + complain(line, ( + "test pattern '{}' reads as a package, not a class: a " + "trailing '.*' covers the methods of one class, so this " + "matches nothing. Name the class, or list its tests" + ).format(entry["test"])) + + if entry["test"].endswith("()"): + complain(line, ( + "test '{}' carries the parentheses JUnit puts in its XML; " + "entries are written as ., and covers() " + "normalises the report's id to that shape -- drop the '()'" + ).format(entry["test"])) + + if _INVOCATION_INDEX_RE.match(entry["test"].rsplit(".", 1)[-1]): + complain(line, ( + "test '{}' names an invocation index. @ParameterizedTest and " + "@RetryingTest invocations appear in the XML as '[1]', '[2]' " + "with no method name at all, so the index identifies neither " + "the method nor a stable case -- quarantine the class with " + "'.*' instead" + ).format(entry["test"])) + + if entry["added"] and DATE_RE.match(entry["added"]): + try: + datetime.date.fromisoformat(entry["added"]) + except ValueError: + complain(line, "added '{}' is not a real calendar date".format(entry["added"])) + + for pattern in entry["cells"]: + unknown_arch_tokens = [ + t for t in ARCH_LIKE_RE.findall(pattern) if t not in KNOWN_ARCHES + ] + # A token like "amd" or "aarch" (from "*amd*"/"*aarch*") is a + # legitimate abbreviation of a real arch and matches real cells; + # only complain when the glob, as actually evaluated by fnmatch, + # matches nothing in the synthetic universe -- that is what + # distinguishes a working glob from one like "*arm64*" that + # genuinely names an architecture CI never builds. + if unknown_arch_tokens and not _matches_any_synthetic_cell([pattern]): + complain(line, ( + "cell glob '{}' names architecture '{}', which CI never " + "builds (cells end in {}); it would quarantine nothing" + ).format(pattern, unknown_arch_tokens[0], " or ".join(KNOWN_ARCHES))) + head = pattern.split("-", 1)[0] + if head and "*" not in head and "?" not in head and head not in KNOWN_LIBCS: + complain(line, ( + "cell glob '{}' starts with '{}'; cell names start with {}" + ).format(pattern, head, " or ".join(KNOWN_LIBCS))) + + if DATE_RE.match(entry["review_by"]): + try: + due = datetime.date.fromisoformat(entry["review_by"]) + except ValueError: + complain(line, "review_by '{}' is not a real calendar date".format( + entry["review_by"])) + else: + if due < today: + complain(line, ( + "'{}' has been quarantined since {} and its review was due {} " + "({} days ago). Fix the test and delete this line, or renew " + "review_by with a note on {}." + ).format(name, entry["added"], entry["review_by"], + (today - due).days, entry["ticket"] or "the ticket")) + elif due > today + datetime.timedelta(days=MAX_REVIEW_DAYS): + complain(line, ( + "review_by '{}' is more than {} days out; that is not a " + "review date, it defeats the point of an expiring quarantine" + ).format(entry["review_by"], MAX_REVIEW_DAYS)) + + for line, message in sorted(problems): + print("::error file={},line={}::{}".format(args.list, line, message)) + + if problems: + print("\n{} problem(s) in {}".format(len(problems), args.list), file=sys.stderr) + return 1 + + print("{}: {} quarantined test(s), all valid".format(args.list, len(entries))) + return 0 + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--list", default=DEFAULT_LIST) + sub = parser.add_subparsers(dest="command", required=True) + + validate = sub.add_parser("validate", help="check the list's format and review dates") + validate.set_defaults(func=cmd_validate) + + args = parser.parse_args() + return args.func(args) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/scripts/run_tests_with_retry.sh b/.github/scripts/run_tests_with_retry.sh new file mode 100755 index 0000000000..a788ab0536 --- /dev/null +++ b/.github/scripts/run_tests_with_retry.sh @@ -0,0 +1,285 @@ +#!/usr/bin/env bash +# Run a test suite, retry once to find out whether a failure reproduces, and +# let the quarantine list -- not the retry -- decide whether the job goes red. +# +# Usage: run_tests_with_retry.sh [--list ] -- +# +# The command is passed through verbatim, so a caller can hand over a plain +# ./gradlew invocation, one wrapped in setarch, or the docker run that drives +# the Alpine aarch64 suite. +# +# Environment: +# MAX_ATTEMPTS attempts to allow (default 2; 1 disables retry) +# MAX_FAILURES_TO_RETRY don't retry past this many failed tests (default 3) +# RETRY_ON_NO_TEST_FAILURES retry a failure that named no test (default 0) +# +# The retry buys a label, not a pass. A test that fails then passes is flaky; a +# test that fails twice is broken. Both still fail the build unless quarantined +# -- the difference decides what the PR comment advises, not whether CI is green. +# +# A retry is spent only when the shape of the failure suggests it might not +# reproduce: a handful of failed tests. A suite where fifty tests went red, or +# where none did (a compile error, an OOM-killed runner, a JVM that never +# started), is not flakiness and a second run only doubles the wait. +# +# The retry is a full re-run rather than a `--tests` filter over the failures. +# Re-running a test alone would clear any failure that only happens in company +# -- an ordering or shared-state bug -- and a test mislabelled "flaky" invites a +# quarantine entry that buries a real defect. + +set -uo pipefail + +QUARANTINE_LIST="ddprof-test/quarantine.txt" +if [ "${1:-}" = "--list" ]; then + QUARANTINE_LIST="$2" + shift 2 +fi + +CELL="${1:?usage: run_tests_with_retry.sh [--list ] -- }" +shift +[ "${1:-}" = "--" ] && shift +# Without a command the pipeline below degrades to a bare redirection, exits 0, +# records no attempt, and the cell goes green having run no test at all. Every +# other unexamined-pass path in this script fails loudly; so does this one. +if [ "$#" -eq 0 ]; then + echo "::error::no command given: run_tests_with_retry.sh [--list ] -- " + exit 1 +fi + +MAX_ATTEMPTS="${MAX_ATTEMPTS:-2}" +MAX_FAILURES_TO_RETRY="${MAX_FAILURES_TO_RETRY:-3}" +RETRY_ON_NO_TEST_FAILURES="${RETRY_ON_NO_TEST_FAILURES:-0}" + +case "$MAX_ATTEMPTS" in + ''|*[!0-9]*|0) + echo "::error::MAX_ATTEMPTS must be a positive integer, got '${MAX_ATTEMPTS}'" + exit 1 + ;; +esac + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# Overridable so a caller outside :ddprof-test's own Gradle layout can point +# this at its own results directory instead of silently classifying an empty, +# never-populated evidence set as "no observed tests". +RESULTS_DIR="${RESULTS_DIR:-ddprof-test/build/test-results}" +EVIDENCE_DIR="flake-evidence" +OUTCOME_FILE="ci-outcome/${CELL}.json" +# Set when an attempt's evidence cannot be trusted as belonging to that +# attempt alone (e.g. a stale RESULTS_DIR that could not be cleared) -- the +# quarantine excuse must never fire on suspect evidence, no matter what the +# counts say. +EVIDENCE_SUSPECT=0 + +# Snapshot this attempt's JUnit XML before the next one overwrites it -- the +# whole point is to compare attempts, and Gradle reuses the same directory. +# The Alpine aarch64 suite runs as root inside Docker while this script runs as +# the host user, so the XML it writes is root-owned. Without this the snapshot +# and the next attempt's cleanup both fail, and the cell loses flake +# classification entirely -- silently, since both used to discard their errors. +make_results_readable() { + [ -d "$RESULTS_DIR" ] || return 0 + # The permission problem this exists to fix is per-file (Docker writes the + # XML as root while the directory it lands in stays host-owned), so a + # directory-level writability check would miss it. Probe by ownership rather + # than with find's -writable, which busybox does not implement -- there the + # test would fail open into a silent no-op, which is the failure this whole + # function exists to stop. A probe that cannot decide takes ownership anyway. + if foreign=$(find "$RESULTS_DIR" ! -user "$(id -u)" -print 2>/dev/null | head -n 1); then + [ -n "$foreign" ] || { [ -w "$RESULTS_DIR" ] && [ -w "$(dirname "$RESULTS_DIR")" ] && return 0; } + fi + # Returning non-zero here is the whole contract: the tree holds files this + # user cannot read, so any snapshot taken from it is partial, and a partial + # snapshot is exactly what lets a final attempt's missing failures read back + # as a quarantined pass. The caller turns that into EVIDENCE_SUSPECT. + if ! command -v sudo >/dev/null 2>&1; then + echo "::warning::${RESULTS_DIR} has files not owned by $(id -un) and sudo is unavailable to fix that; flake evidence may be incomplete" + return 1 + fi + # Non-recursive on the parent: it only needs its own write bit so the + # pre-attempt `rm -rf "$RESULTS_DIR"` below can unlink the directory itself. + # Recursing over the whole module build tree (classes, jars, native libs, + # kept JFRs) would be orders of magnitude more inodes than needed and makes + # unrelated build output world-writable. + local ok=0 + sudo chmod a+rwX "$(dirname "$RESULTS_DIR")" 2>/dev/null \ + || { ok=1; echo "::warning::Could not make $(dirname "$RESULTS_DIR") writable; flake evidence may be incomplete"; } + sudo chmod -R a+rwX "$RESULTS_DIR" 2>/dev/null \ + || { ok=1; echo "::warning::Could not take ownership of ${RESULTS_DIR}; flake evidence may be incomplete"; } + return "$ok" +} + +snapshot() { + local attempt="$1" + local dest="${EVIDENCE_DIR}/attempt-${attempt}" + # The XML being snapshotted was written by the command that just ran, after + # the loop-top make_results_readable() -- under Docker it lands root-owned, + # so read access has to be taken again here or the cp below fails and the + # cell silently loses its flake evidence. + make_results_readable || EVIDENCE_SUSPECT=1 + if ! rm -rf "$dest"; then + # cp -r below merges into whatever survives, so this attempt's evidence + # becomes a union with an earlier one's. attempt_results() then reports + # tests as observed-and-passed that this attempt never ran, which is how a + # persistent failure acquires a "flaky" label and a paste-ready quarantine + # entry that buries a real defect. + echo "::warning::Could not clear ${dest}; attempt ${attempt} evidence may be merged with an earlier attempt's" + EVIDENCE_SUSPECT=1 + fi + mkdir -p "$dest" + if [ -d "$RESULTS_DIR" ]; then + cp -r "$RESULTS_DIR"/. "$dest"/ \ + || { EVIDENCE_SUSPECT=1; echo "::warning::Could not snapshot ${RESULTS_DIR} for attempt ${attempt}; flake classification for this cell will be incomplete"; } + fi +} + +# Which Gradle tasks are the tests. A failure in anything else is not something +# the quarantine list has any business excusing. +TEST_TASK_PATTERN="${TEST_TASK_PATTERN:-:ddprof-test:test}" + +# Self-contained state: a leftover attempt-2 from an earlier run on a reused +# workspace would be read back as this run's evidence, inflating the attempt +# count and importing failures that never happened here. Root-owned leftovers +# from a Docker-run cell are exactly how that happens, so a failure to clear +# is recorded rather than discarded. +if ! rm -rf "$EVIDENCE_DIR" "$(dirname "$OUTCOME_FILE")"; then + echo "::warning::Could not clear ${EVIDENCE_DIR}; this run may inherit an earlier run's attempts as its own evidence" + EVIDENCE_SUSPECT=1 +fi + +EXIT_CODE=1 +# A single, per-attempt-truncated log: only the final attempt's is ever read +# (by flake_report.py's non-test-task-failure check below), and keeping one +# copy per attempt on disk earned nothing but wasted space. +ATTEMPT_LOG="build/logs/attempt.log" +for attempt in $(seq 1 "$MAX_ATTEMPTS"); do + mkdir -p build/logs + make_results_readable || EVIDENCE_SUSPECT=1 + if ! rm -rf "$RESULTS_DIR"; then + echo "::warning::Could not clear ${RESULTS_DIR} before attempt ${attempt}; it may inherit the previous attempt's results and a flake will look persistent" + # A stale RESULTS_DIR here means this attempt's snapshot can end up being + # the *previous* attempt's JUnit XML, which would let a final attempt that + # actually crashed without running a single test be read back as having + # "passed with only quarantined failures". Never let the quarantine excuse + # fire on evidence that might not be this attempt's own. + EVIDENCE_SUSPECT=1 + fi + : > "$ATTEMPT_LOG" + + "$@" 2>&1 \ + | tee -a build/test-raw.log \ + | tee "$ATTEMPT_LOG" \ + | python3 -u "${HERE}/filter_gradle_log.py" + EXIT_CODE=${PIPESTATUS[0]} + + # A first-attempt pass has no prior attempt to compare against, so its + # snapshot could only ever yield an empty flake report; skip the find + # traversal, possible sudo chmod, and recursive copy that nobody will read. + # A later-attempt pass still needs its snapshot -- that is the evidence that + # proves the earlier failure was a flake. + if [ "$EXIT_CODE" -ne 0 ] || [ "$attempt" -gt 1 ]; then + snapshot "$attempt" + fi + + if [ "$EXIT_CODE" -eq 0 ]; then + break + fi + + if [ "$attempt" -ge "$MAX_ATTEMPTS" ]; then + break + fi + + failed=$(python3 "${HERE}/flake_report.py" count --dir "${EVIDENCE_DIR}/attempt-${attempt}") || failed="" + case "$failed" in + ''|*[!0-9]*) + # Every guard below is a numeric comparison; on a non-number they would + # all quietly evaluate false and retry the very failures meant to be + # taken at face value. + echo "::warning::Could not count test failures for attempt ${attempt}; not retrying" + break + ;; + esac + if [ "$failed" -eq 0 ] && [ "$RETRY_ON_NO_TEST_FAILURES" != "1" ]; then + # No test was named, so the suite did not get far enough to have one fail: + # a compile error, a missing toolchain, a runner that ran out of disk. None + # of those get better on a second run. + echo "::notice::Attempt ${attempt} failed with no named test failures (build or infrastructure); not retrying" + break + fi + if [ "$failed" -gt "$MAX_FAILURES_TO_RETRY" ]; then + echo "::notice::Attempt ${attempt} failed ${failed} tests (> ${MAX_FAILURES_TO_RETRY}); a break, not a flake — not retrying" + break + fi + + if [ "$failed" -eq 0 ]; then + echo "::warning::Attempt ${attempt} failed before any test ran, retrying once" + else + echo "::warning::Attempt ${attempt} failed ${failed} test(s), retrying once to tell a flake from a break" + fi + ./gradlew --stop 2>/dev/null || true +done + +REPORT_ARGS=(--list "$QUARANTINE_LIST" report + --cell "$CELL" + --evidence-dir "$EVIDENCE_DIR" + --final-attempt "$attempt" + --out "$OUTCOME_FILE" + --attempt-log "$ATTEMPT_LOG" + --final-attempt-exit-code "$EXIT_CODE" + --test-task-pattern "$TEST_TASK_PATTERN") +[ "$EVIDENCE_SUSPECT" = "1" ] && REPORT_ARGS+=(--evidence-suspect) + +python3 "${HERE}/flake_report.py" "${REPORT_ARGS[@]}" +REPORT_STATUS=$? + +# A classifier that did not run cannot vouch for a green suite: it is the only +# thing that would have noticed a test failing on the first attempt and passing +# on the second. Fail loudly rather than inherit a pass we cannot justify. +if [ "$REPORT_STATUS" -ne 0 ]; then + echo "::error::Could not classify results for ${CELL} (flake_report.py exited ${REPORT_STATUS}); failing the job rather than trusting an unexamined pass" + exit 1 +fi + +# flake_report.py owns the gating verdict -- it has every count and the +# quarantine list in hand, so re-deriving the decision here (as this script +# used to, with an inline python, two case sanity checks, and a bash +# if/elif chain) is one more independent reader of the outcome-JSON schema +# for no benefit. "gates" is true/false to override EXIT_CODE, or the string +# "none" when there is no failure to have an opinion about, in which case the +# command's own exit code stands (a compile error or a dead runner is nothing +# to do with quarantine). +if [ -f "$OUTCOME_FILE" ]; then + decision=$(python3 -c " +import json, sys +try: + d = json.load(open(sys.argv[1])) + if 'gates' not in d: + sys.exit('missing key: gates') +except Exception as e: + sys.exit(str(e)) +gates = d['gates'] +print('none' if gates is None else ('true' if gates else 'false')) +print(d.get('gate_reason') or '') +" "$OUTCOME_FILE") || { + echo "::error::Could not read ${OUTCOME_FILE} (${decision:-no output}); failing the job rather than guessing whether its failures gate" + exit 1 + } + gates=$(echo "$decision" | sed -n '1p') + reason=$(echo "$decision" | sed -n '2p') + case "$gates" in + true) + echo "::error::${CELL} fails: ${reason}" + EXIT_CODE=1 + ;; + false) + echo "::warning::${CELL} is not failing the job: ${reason}" + EXIT_CODE=0 + ;; + none) ;; + *) + echo "::error::${OUTCOME_FILE} did not yield a usable gating decision (got '${gates}'); failing the job" + exit 1 + ;; + esac +fi + +exit "$EXIT_CODE" diff --git a/.github/scripts/tests/test_generate_test_summary.sh b/.github/scripts/tests/test_generate_test_summary.sh new file mode 100755 index 0000000000..e424dbf100 --- /dev/null +++ b/.github/scripts/tests/test_generate_test_summary.sh @@ -0,0 +1,171 @@ +#!/usr/bin/env bash + +set -euo pipefail + +# Copyright 2026, Datadog, Inc + +# Hermetic tests for generate-test-summary.sh's handling of downloaded +# ci-outcome reports. +# Run with: .github/scripts/tests/test_generate_test_summary.sh +# +# `gh` is the only external dependency this script has that can't run inside a +# sandbox, so it is the only thing stubbed out below; everything else (jq, +# the report-parsing logic) runs for real against fixture data. + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd) +SCRIPT="$ROOT/.github/scripts/generate-test-summary.sh" +TEMP_DIR=$(mktemp -d) +TESTS=0 + +cleanup() { + rm -rf "$TEMP_DIR" +} +trap cleanup EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +pass() { + TESTS=$((TESTS + 1)) + echo " ok: $*" +} + +# A stub `gh` good enough for this script's two call sites: the jobs listing +# and the ci-outcome artifact download. Real GitHub is never reached. +STUB_BIN="$TEMP_DIR/stub-bin" +mkdir -p "$STUB_BIN" +cat > "$STUB_BIN/gh" <<'EOS' +#!/usr/bin/env bash +if [ "$1" = "api" ]; then + cat "$GH_JOBS_FIXTURE" + exit 0 +fi +if [ "$1" = "run" ] && [ "$2" = "download" ]; then + dir="" + prev="" + for arg in "$@"; do + if [ "$prev" = "--dir" ]; then dir="$arg"; fi + prev="$arg" + done + mkdir -p "$dir" + cp "$GH_OUTCOME_FIXTURE_DIR"/*.json "$dir/" 2>/dev/null || true + exit 0 +fi +echo "stub gh: unexpected invocation: $*" >&2 +exit 1 +EOS +chmod +x "$STUB_BIN/gh" +PATH="$STUB_BIN:$PATH" +export PATH + +write_jobs_fixture() { + # write_jobs_fixture + cat > "$1" < "$CASE/outcomes/glibc-17-debug-amd64.json" <<'EOJ' +{"cell": "glibc-17-debug-amd64", "attempts": 1, "persistent": + [{"test": "com.dd.FooTest.bar", "message": "assertion failed: boom"}], + "flaky": [], "quarantined": [], "gating_count": 1, "failure_count": 1, + "final_attempt_ran": true, "final_attempt_gating_count": 1} +EOJ +( + cd "$CASE/work" + export GH_JOBS_FIXTURE="$CASE/jobs/jobs.json" GH_OUTCOME_FIXTURE_DIR="$CASE/outcomes" + export GITHUB_REPOSITORY="DataDog/java-profiler" GITHUB_SHA="deadbeefcafef00dfeedfacebeefcafebeefcafe" + "$SCRIPT" 12345 "$CASE/work/summary.md" +) || fail "generate-test-summary.sh exited non-zero on a valid outcome report" +summary=$(cat "$CASE/work/summary.md") +echo "$summary" | grep -q "FooTest.bar" \ + || fail "expected the real failing test in the summary, got: $summary" +echo "$summary" | grep -q "assertion failed: boom" \ + || fail "expected the real failure message in the summary, got: $summary" +echo "$summary" | grep -q "Failed Jobs" \ + || fail "expected the failed job to be listed and linked, got: $summary" +echo "$summary" | grep -q "https://example.invalid/job/1" \ + || fail "expected the failed job's log link, got: $summary" +pass "a valid outcome report renders its real failure via flake_summary.py, and the job is linked" + +# A malformed outcome report (invalid JSON) must be flagged rather than +# silently dropped -- this is now flake_summary.py's own "could not be +# parsed" fallback, the one reader of ci-outcome JSON left in this pipeline. +CASE="$TEMP_DIR/case-malformed-report" +mkdir -p "$CASE/jobs" "$CASE/outcomes" "$CASE/work" +write_jobs_fixture "$CASE/jobs/jobs.json" failure +printf 'this is not json\n' > "$CASE/outcomes/glibc-17-debug-amd64.json" +( + cd "$CASE/work" + export GH_JOBS_FIXTURE="$CASE/jobs/jobs.json" GH_OUTCOME_FIXTURE_DIR="$CASE/outcomes" + export GITHUB_REPOSITORY="DataDog/java-profiler" GITHUB_SHA="deadbeefcafef00dfeedfacebeefcafebeefcafe" + "$SCRIPT" 12345 "$CASE/work/summary.md" +) || fail "generate-test-summary.sh exited non-zero on a malformed outcome report" +summary=$(cat "$CASE/work/summary.md") +echo "$summary" | grep -q "could not be parsed" \ + || fail "expected a malformed outcome report to be flagged unreadable, got: $summary" +pass "a malformed outcome report is flagged unreadable rather than silently ignored" + +# A run where every test job passed must not print an empty "Failed Jobs" +# section -- that guard is what stands between a green run and a stray, +# empty heading (or, on a bash whose indexed-array expansion under `set -u` +# minds an empty array, a hard failure). +CASE="$TEMP_DIR/case-all-green" +mkdir -p "$CASE/jobs" "$CASE/outcomes" "$CASE/work" +write_jobs_fixture "$CASE/jobs/jobs.json" success +( + cd "$CASE/work" + export GH_JOBS_FIXTURE="$CASE/jobs/jobs.json" GH_OUTCOME_FIXTURE_DIR="$CASE/outcomes" + export GITHUB_REPOSITORY="DataDog/java-profiler" GITHUB_SHA="deadbeefcafef00dfeedfacebeefcafebeefcafe" + "$SCRIPT" 12345 "$CASE/work/summary.md" +) || fail "generate-test-summary.sh exited non-zero on an all-passing run" +summary=$(cat "$CASE/work/summary.md") +if echo "$summary" | grep -q "Failed Jobs"; then + fail "an all-passing run must not print a Failed Jobs section, got: $summary" +fi +echo "$summary" | grep -q "All 1 test jobs passed" \ + || fail "expected the all-passed banner, got: $summary" +pass "an all-passing run prints no Failed Jobs section" + +# The job-name regex and test_workflow.yml's `name:` expressions are a contract +# with no enforcement: a non-matching job is skipped silently, so a rename +# renders a summary reporting zero test jobs while the run stays green. Read +# the real workflow rather than a fabricated name, so drift fails here. +pattern=$(sed -n "s/.*test_job_pattern='\(.*\)'.*/\1/p" "$SCRIPT") +[ -n "$pattern" ] || fail "could not read test_job_pattern out of generate-test-summary.sh" +names=$(grep -hE "^ *name: test-linux" "$ROOT/.github/workflows/test_workflow.yml") +[ -n "$names" ] || fail "no test-linux job names found in test_workflow.yml" +count=0 +while IFS= read -r line; do + rendered=${line#*name: } + rendered=${rendered//'${{ matrix.java_version }}'/17} + rendered=${rendered//'${{ matrix.config }}'/debug} + rendered=${rendered//"\${{ inputs.slow_tests && 'slow' || 'regular' }}"/regular} + [[ "$rendered" =~ $pattern ]] \ + || fail "job name '$rendered' does not match generate-test-summary.sh's test_job_pattern" + count=$((count + 1)) +done <<< "$names" +[ "$count" -eq 4 ] || fail "expected 4 test-linux jobs in test_workflow.yml, matched $count" +pass "every test-linux job name in test_workflow.yml matches the summary's regex" + +echo +echo "All $TESTS generate-test-summary tests passed." diff --git a/.github/scripts/tests/test_prepare_reports.sh b/.github/scripts/tests/test_prepare_reports.sh new file mode 100755 index 0000000000..fe3d5c8ba4 --- /dev/null +++ b/.github/scripts/tests/test_prepare_reports.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# Hermetic checks for prepare_reports.sh's artifact staging. +# +# The one thing that must never happen quietly: shipping a test-reports +# artifact with no JUnit XML in it, which is what made every failed job render +# "No detailed failure information available". +set -uo pipefail + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd) +SCRIPT="$ROOT/.github/scripts/prepare_reports.sh" +TESTS=0 +TEMP_DIR=$(mktemp -d) +trap 'rm -rf "$TEMP_DIR"' EXIT + +pass() { TESTS=$((TESTS + 1)); echo " ok: $1"; } +fail() { echo "FAIL: $1" >&2; exit 1; } + +write_xml() { + mkdir -p "$1" + cat > "$1/TEST-com.dd.Some.xml" <<'EOS' + + + + +EOS +} + +echo "== prepare_reports.sh stages JUnit XML ==" + +# snapshot() creates attempt- with mkdir -p *before* the copy that may fail, +# so an empty or partial snapshot must still fall back to build/test-results. +CASE="$TEMP_DIR/case-empty-snapshot" +mkdir -p "$CASE/flake-evidence/attempt-1" "$CASE/build/logs" +write_xml "$CASE/ddprof-test/build/test-results/testDebug" +( cd "$CASE" && "$SCRIPT" >/dev/null 2>&1 ) +[ -n "$(find "$CASE/test-reports" -name 'TEST-*.xml' 2>/dev/null | head -1)" ] \ + || fail "an empty attempt-* snapshot must still ship build/test-results, found none in test-reports" +pass "an empty snapshot falls back to build/test-results" + +# With real evidence in flake-evidence there is nothing to fall back to: the +# snapshot already holds what build/test-results holds. +CASE="$TEMP_DIR/case-real-snapshot" +mkdir -p "$CASE/build/logs" +write_xml "$CASE/flake-evidence/attempt-1" +write_xml "$CASE/ddprof-test/build/test-results/testDebug" +( cd "$CASE" && "$SCRIPT" >/dev/null 2>&1 ) +[ -n "$(find "$CASE/test-reports" -name 'TEST-*.xml' 2>/dev/null | head -1)" ] \ + || fail "a populated snapshot must still reach the artifact" +pass "a populated snapshot is shipped as-is" + +echo +echo "All $TESTS prepare-reports tests passed." diff --git a/.github/scripts/tests/test_quarantine.sh b/.github/scripts/tests/test_quarantine.sh new file mode 100755 index 0000000000..b3adaf2e02 --- /dev/null +++ b/.github/scripts/tests/test_quarantine.sh @@ -0,0 +1,1045 @@ +#!/usr/bin/env bash + +set -euo pipefail + +# Copyright 2026, Datadog, Inc + +# Hermetic tests for the flaky-test quarantine machinery. +# Run with: .github/scripts/tests/test_quarantine.sh +# +# The gating decision here is the one that can let a real defect through, and +# the retry path only executes when something has already failed -- which is to +# say, never on a green CI run. So it is exercised against fixtures instead. + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd) +SCRIPTS="$ROOT/.github/scripts" +TEMP_DIR=$(mktemp -d) +TESTS=0 + +cleanup() { + rm -rf "$TEMP_DIR" +} +trap cleanup EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +pass() { + TESTS=$((TESTS + 1)) + echo " ok: $*" +} + +today() { python3 -c 'import datetime; print(datetime.date.today())'; } +day_offset() { python3 -c "import datetime,sys; print(datetime.date.today()+datetime.timedelta(days=int(sys.argv[1])))" "$1"; } + +write_list() { + # write_list [entry line...] + local path="$1"; shift + printf '# test | ticket | added | review_by | cells | reason\n' > "$path" + local line + for line in "$@"; do + printf '%s\n' "$line" >> "$path" + done +} + +entry() { + # entry [cells] + printf '%s | %s | %s | %s | %s | flaky under test\n' \ + "$1" "$2" "$(today)" "$3" "${4:--}" +} + +# Writes a JUnit XML report naming one failed test. +write_failure_xml() { + # write_failure_xml + mkdir -p "$1" + cat > "$1/TEST-$2.xml" < + + + + + +EOF +} + +write_pass_xml() { + mkdir -p "$1" + cat > "$1/TEST-$2.xml" < + + + +EOF +} + +echo "== quarantine.py validate ==" + +LIST="$TEMP_DIR/list.txt" + +write_list "$LIST" +python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null \ + || fail "empty list should be valid" +pass "an empty list is valid" + +write_list "$LIST" "$(entry a.B.c PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null \ + || fail "a complete, unexpired entry should be valid" +pass "a complete entry is valid" + +write_list "$LIST" "a.B.c | | $(today) | $(day_offset 30) | - | no ticket" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "an entry with no ticket should be rejected" +fi +pass "an entry with no ticket is rejected" + +write_list "$LIST" "$(entry a.B.c JIRA-1 "$(day_offset 30)")" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "a ticket outside the PROF project should be rejected" +fi +pass "a non-PROF ticket is rejected" + +write_list "$LIST" "$(entry a.B.c PROF-1 "$(day_offset -1)")" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "an entry past review_by should be rejected" +fi +pass "an expired entry is rejected" + +write_list "$LIST" "$(entry a.B.c PROF-1 "$(day_offset 30)")" "$(entry a.B.c PROF-2 "$(day_offset 30)")" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "the same test listed twice should be rejected" +fi +pass "a duplicate entry is rejected" + +# The list that ships in the repo must itself be valid, or CI is lying. +python3 "$SCRIPTS/quarantine.py" --list "$ROOT/ddprof-test/quarantine.txt" validate >/dev/null \ + || fail "the committed quarantine list is invalid" +pass "the committed quarantine list is valid" + +echo "== quarantine.find_entry (the rule the gating decision actually uses) ==" + +write_list "$LIST" "$(entry a.B.c PROF-1 "$(day_offset 30)" '*aarch64*')" + +result=$(python3 -c " +import sys; sys.path.insert(0, '$SCRIPTS') +import quarantine +entries = quarantine.load('$LIST') +print('hit' if quarantine.find_entry(entries, 'a.B.c', 'glibc-17-debug-aarch64') else 'miss') +") +[ "$result" = "hit" ] || fail "expected a.B.c quarantined on an aarch64 cell, got: $result" +pass "a cell glob matches the cells it names" + +result=$(python3 -c " +import sys; sys.path.insert(0, '$SCRIPTS') +import quarantine +entries = quarantine.load('$LIST') +print('hit' if quarantine.find_entry(entries, 'a.B.c', 'glibc-17-debug-amd64') else 'miss') +") +[ "$result" = "miss" ] || fail "expected a.B.c gating (not quarantined) on an amd64 cell, got: $result" +pass "a cell glob does not match other cells" + +write_list "$LIST" "$(entry 'a.B.*' PROF-1 "$(day_offset 30)")" +result=$(python3 -c " +import sys; sys.path.insert(0, '$SCRIPTS') +import quarantine +entries = quarantine.load('$LIST') +tests = ['a.B.c', 'a.B.d', 'a.C.e'] +gating = [t for t in tests if quarantine.find_entry(entries, t, 'any') is None] +print(','.join(gating)) +") +[ "$result" = "a.C.e" ] || fail "expected only a.C.e to gate under a class wildcard, got: $result" +pass "a class wildcard covers that class only" + +# validate-quarantine only runs from ci.yml, but find_entry() is the function +# every workflow's flake_report.py invocation actually calls -- nightly.yml +# and release-validated.yml reuse the retry/quarantine machinery without ever +# running validate-quarantine, so an expired entry must stop matching here +# too, not just be caught by the separate CLI check above. +write_list "$LIST" "$(entry a.B.c PROF-1 "$(day_offset -1)")" +result=$(python3 -c " +import sys; sys.path.insert(0, '$SCRIPTS') +import quarantine +entries = quarantine.load('$LIST') +print('hit' if quarantine.find_entry(entries, 'a.B.c', 'any') else 'miss') +") +[ "$result" = "miss" ] || fail "expected an expired entry to gate rather than match, got: $result" +pass "find_entry() treats an expired entry as absent, not as a hit" + +echo "== gating: run_tests_with_retry.sh ==" + +# A suite that fails one test on the first attempt and passes on the second. +make_flaky_suite() { + local dir="$1" + mkdir -p "$dir" + cat > "$dir/suite.sh" </dev/null || echo 0) + 1 )); echo \$n > .n +OUT=ddprof-test/build/test-results/testDebug +mkdir -p "\$OUT" +if [ "\$n" -eq 1 ]; then +$(declare -f write_failure_xml) + write_failure_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" "got 2 samples, wanted 50" + exit 1 +fi +$(declare -f write_pass_xml) +write_pass_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" +exit 0 +EOS + chmod +x "$dir/suite.sh" +} + +# A clean run on the first attempt must exit 0 and report nothing gating. +# Every other case in this section starts from a failure; without this one, a +# regression that made a clean run report a gating failure would leave every +# other assertion here passing. +CASE="$TEMP_DIR/case-green" +mkdir -p "$CASE" +cat > "$CASE/suite.sh" </dev/null 2>&1) +rc=$? +set -e +[ "$rc" -eq 0 ] || fail "a suite that passes on the first attempt must exit 0 (got exit $rc)" +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gating_count'] == 0 and d['failure_count'] == 0, d +assert not d['flaky'] and not d['persistent'] and not d['unclassified'], d +" "$CASE/ci-outcome/glibc-17-debug-amd64.json" || fail "a clean run was not reported as clean" +pass "a suite that passes on the first attempt is green and reports no failures" + +# Not quarantined: passing on the retry must not rescue the job. +CASE="$TEMP_DIR/case-gating" +make_flaky_suite "$CASE" +write_list "$CASE/list.txt" +set +e +(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh >/dev/null 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "an un-quarantined flaky test must fail the job (got exit $rc)" +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gating_count'] == 1, d +assert d['flaky'][0]['test'] == 'com.dd.WobblyTest.sometimesFails', d +" "$CASE/ci-outcome/glibc-17-debug-amd64.json" || fail "flaky test not classified as gating" +pass "an un-quarantined flake fails the job and is recorded as flaky" + +# Same suite, now quarantined: the job goes green and the failure is recorded. +CASE="$TEMP_DIR/case-quarantined" +make_flaky_suite "$CASE" +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +set +e +(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh >/dev/null 2>&1) +rc=$? +set -e +[ "$rc" -eq 0 ] || fail "a quarantined test must not fail the job (got exit $rc)" +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gating_count'] == 0, d +assert d['quarantined'][0]['ticket'] == 'PROF-1', d +" "$CASE/ci-outcome/glibc-17-debug-amd64.json" || fail "quarantined failure not recorded" +pass "a quarantined failure keeps the job green and is still recorded" + +# A build error names no test, so quarantine has nothing to say about it. +CASE="$TEMP_DIR/case-build-error" +mkdir -p "$CASE" +printf '#!/usr/bin/env bash\necho "error: cannot find symbol"\nexit 1\n' > "$CASE/suite.sh" +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry 'com.dd.WobblyTest.*' PROF-1 "$(day_offset 30)")" +set +e +(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh >/dev/null 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a build error must fail the job regardless of the quarantine list" +pass "a failure naming no test is never excused by quarantine" + +# Regression: an unreadable list once made flake_report.py exit non-zero, and a +# `|| true` turned that into a silent green on a suite whose first attempt had +# failed. A classifier that did not run must never be mistaken for a clean run. +CASE="$TEMP_DIR/case-broken-list" +make_flaky_suite "$CASE" +printf 'this line has too few fields\n' > "$CASE/list.txt" +set +e +output=$(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a malformed list must not yield a green job (got exit $rc)" +pass "a list with a malformed line still fails the job" +# A malformed line is skipped rather than fatal, so here the flake is what +# gates. The classifier-failure path is a separate case below. +echo "$output" | grep -q "Flaky test" \ + || fail "expected the flake to be reported, got: $output" +pass "the reason for the red is reported" + +# The guard above only bites when flake_report.py itself exits non-zero, which +# a merely malformed line does not do. Point --list at a directory so the +# classifier genuinely fails: the suite passes on its retry, so without the +# REPORT_STATUS guard this job would be green. +CASE="$TEMP_DIR/case-unreadable-list" +make_flaky_suite "$CASE" +mkdir -p "$CASE/list.txt" +set +e +output=$(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a classifier that could not run must not yield a green job (got exit $rc)" +echo "$output" | grep -q "Could not classify results for" \ + || fail "expected the classifier failure to be named, got: $output" +pass "a classifier that cannot run fails the job rather than passing unexamined" + +# Quarantine excuses the tests it names, never the build around them. A suite +# whose only named failure is quarantined but which also failed a non-test +# Gradle task must stay red. +CASE="$TEMP_DIR/case-quarantined-plus-build-failure" +mkdir -p "$CASE" +cat > "$CASE/suite.sh" < Task :ddprof-lib:verifyNative FAILED" +echo "Execution failed for task ':ddprof-lib:verifyNative'." +exit 1 +EOS +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry 'com.dd.WobblyTest.*' PROF-1 "$(day_offset 30)")" +set +e +output=$(cd "$CASE" && MAX_ATTEMPTS=1 "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a non-test task failure must not be excused by quarantine (got exit $rc)" +echo "$output" | grep -q "verifyNative" \ + || fail "expected the offending task to be named, got: $output" +pass "quarantine excuses the tests it names, not a build failure alongside them" + +# Regression: a failure aggregated from an EARLIER attempt must never rescue a +# FINAL attempt that failed for a reason naming no test at all (here: nothing +# that prints "Execution failed for task", so the non_test_task_failures grep +# alone would miss it). Attempt 1 fails a named, quarantined test; attempt 2 +# aborts before writing any JUnit XML, the way a docker or JVM-init failure +# would. The job must stay red even though every named failure is quarantined. +CASE="$TEMP_DIR/case-final-attempt-no-tests" +mkdir -p "$CASE" +cat > "$CASE/suite.sh" </dev/null || echo 0) + 1 )); echo \$n > .n +OUT=ddprof-test/build/test-results/testDebug +mkdir -p "\$OUT" +if [ "\$n" -eq 1 ]; then +$(declare -f write_failure_xml) + write_failure_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" "got 2 samples, wanted 50" + exit 1 +fi +rm -rf "\$OUT" +echo "docker: Error response from daemon: OCI runtime create failed" +exit 1 +EOS +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +set +e +output=$(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a final attempt that named no test must not be excused by an earlier attempt's quarantined failure (got exit $rc)" +pass "a final attempt naming no test is never excused by an earlier attempt's quarantine hit" + +# Regression: a final attempt that crashes part-way through still writes JUnit +# XML for the tests it got to, and those all passed -- so it names no failure +# of its own and every aggregated failure is quarantined. Gradle blames the +# abort on the test task itself, so the non-test-task check cannot see it +# either. Only the attempt's own non-zero exit distinguishes this from the +# ordinary flaky-then-passed case, and it must stay red. +CASE="$TEMP_DIR/case-final-attempt-crashed-mid-run" +mkdir -p "$CASE" +cat > "$CASE/suite.sh" </dev/null || echo 0) + 1 )); echo \$n > .n +OUT=ddprof-test/build/test-results/testDebug +mkdir -p "\$OUT" +$(declare -f write_failure_xml) +$(declare -f write_pass_xml) +if [ "\$n" -eq 1 ]; then + write_failure_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" "got 2 samples, wanted 50" + exit 1 +fi +write_pass_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" +echo "# A fatal error has been detected by the Java Runtime Environment: SIGSEGV" +echo "Execution failed for task ':ddprof-test:test'." +echo "> Process 'Gradle Test Executor 3' finished with non-zero exit value 134" +exit 134 +EOS +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +set +e +output=$(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a final attempt that crashed after recording only passes must not be excused by quarantine (got exit $rc)" +pass "a final attempt that crashed mid-run is not read as a quarantined pass" + +# The counterpart: the same shape without the crash is the ordinary +# flaky-then-passed case a quarantine entry exists to excuse, and must be +# green. Without this the guard above could be satisfied by gating everything. +CASE="$TEMP_DIR/case-quarantined-flake-recovers" +mkdir -p "$CASE" +cat > "$CASE/suite.sh" </dev/null || echo 0) + 1 )); echo \$n > .n +OUT=ddprof-test/build/test-results/testDebug +mkdir -p "\$OUT" +$(declare -f write_failure_xml) +$(declare -f write_pass_xml) +if [ "\$n" -eq 1 ]; then + write_failure_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" "got 2 samples, wanted 50" + exit 1 +fi +write_pass_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" +exit 0 +EOS +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +set +e +output=$(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -eq 0 ] || fail "a quarantined flake that passed on retry must stay green (got exit $rc): $output" +pass "a quarantined flake that recovers on a clean retry is still excused" + +# A final attempt can name only quarantined failures and still exit non-zero +# -- a JVM abort partway through, after writing XML for the one test it +# reached. The tests it never got to are missing from the XML, not passing; +# a quarantined name or two must not paper over that crash. +CASE="$TEMP_DIR/case-final-attempt-quarantined-failure-plus-crash" +mkdir -p "$CASE" +cat > "$CASE/suite.sh" </dev/null || echo 0) + 1 )); echo \$n > .n +OUT=ddprof-test/build/test-results/testDebug +mkdir -p "\$OUT" +$(declare -f write_failure_xml) +write_failure_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" "got 2 samples, wanted 50" +if [ "\$n" -gt 1 ]; then + echo "# A fatal error has been detected by the Java Runtime Environment: SIGSEGV" + echo "Execution failed for task ':ddprof-test:test'." + echo "> Process 'Gradle Test Executor 3' finished with non-zero exit value 134" + exit 134 +fi +exit 1 +EOS +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +set +e +output=$(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a final attempt that crashed after naming only a quarantined failure must not be excused (got exit $rc)" +pass "a final attempt naming a quarantined failure is still gated if it also crashed" + +# ...but only when the evidence it rests on is complete. Docker writes the +# JUnit XML as root; if this user cannot take ownership of it, the snapshot is +# partial, and a partial snapshot is indistinguishable from an attempt whose +# missing tests all passed. make_results_readable() must report that rather +# than fail open, and the run must go red. +CASE="$TEMP_DIR/case-unreadable-results" +mkdir -p "$CASE/stub-bin" +cat > "$CASE/stub-bin/find" <<'EOS' +#!/usr/bin/env bash +for a in "$@"; do + if [ "$a" = "-user" ]; then echo "/root-owned/TEST-Foo.xml"; exit 0; fi +done +exec /usr/bin/find "$@" +EOS +cat > "$CASE/stub-bin/sudo" <<'EOS' +#!/usr/bin/env bash +exit 1 +EOS +chmod +x "$CASE/stub-bin/find" "$CASE/stub-bin/sudo" +cat > "$CASE/suite.sh" </dev/null || echo 0) + 1 )); echo \$n > .n +OUT=ddprof-test/build/test-results/testDebug +mkdir -p "\$OUT" +$(declare -f write_failure_xml) +$(declare -f write_pass_xml) +if [ "\$n" -eq 1 ]; then + write_failure_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" "got 2 samples, wanted 50" + exit 1 +fi +write_pass_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" +exit 0 +EOS +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +set +e +output=$(cd "$CASE" && PATH="$CASE/stub-bin:$PATH" "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "results that could not be made readable must not be excused by quarantine (got exit $rc)" +echo "$output" | grep -q "suspect" \ + || fail "expected the suspect evidence to be named as the reason, got: $output" +pass "evidence that could not be made readable is never excused by the quarantine list" + +# A `cp -r` that fails partway through RESULTS_DIR must poison the evidence +# the same way an unreadable results directory does above -- even when the +# one failure it *did* manage to copy is quarantined, the files it could not +# copy are missing from this attempt's snapshot, not passing. Stubbed like +# the find/sudo cases above rather than chmod'd, since a root-run CI job +# would not actually be denied read access by chmod. +CASE="$TEMP_DIR/case-partial-snapshot-copy" +mkdir -p "$CASE/stub-bin" +cat > "$CASE/stub-bin/cp" <<'EOS' +#!/usr/bin/env bash +exit 1 +EOS +chmod +x "$CASE/stub-bin/cp" +cat > "$CASE/suite.sh" </dev/null || echo 0) + 1 )); echo \$n > .n +OUT=ddprof-test/build/test-results/testDebug +mkdir -p "\$OUT" +$(declare -f write_failure_xml) +write_failure_xml "\$OUT" "com.dd.WobblyTest" "sometimesFails" "got 2 samples, wanted 50" +exit 1 +EOS +chmod +x "$CASE/suite.sh" +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +set +e +output=$(cd "$CASE" && PATH="$CASE/stub-bin:$PATH" "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- ./suite.sh 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "a failed snapshot copy must not be excused even though the named failure is quarantined (got exit $rc)" +echo "$output" | grep -q "Could not snapshot" \ + || fail "expected the lost evidence to be reported, got: $output" +pass "a failed snapshot copy leaves nothing to excuse, so the command's own failure stands" + +# Suspect evidence is a reason to distrust a quarantine excuse, not a failure +# of its own. A suite that passed has nothing to excuse, so an unreadable +# results tree must not turn its exit 0 into a red job. +CASE="$TEMP_DIR/case-suspect-evidence-all-passed" +mkdir -p "$CASE/stub-bin" +cat > "$CASE/stub-bin/find" <<'EOS' +#!/usr/bin/env bash +for a in "$@"; do + if [ "$a" = "-user" ]; then echo "/root-owned/TEST-Foo.xml"; exit 0; fi +done +exec /usr/bin/find "$@" +EOS +cat > "$CASE/stub-bin/sudo" <<'EOS' +#!/usr/bin/env bash +exit 1 +EOS +chmod +x "$CASE/stub-bin/find" "$CASE/stub-bin/sudo" +cat > "$CASE/suite.sh" <&1) +rc=$? +set -e +[ "$rc" -eq 0 ] || fail "suspect evidence must not fail a run in which every test passed (got exit $rc): $output" +pass "suspect evidence does not redden a run with nothing to excuse" + +# A test missing from the retry never re-ran, so it is not evidence of a flake. +CASE="$TEMP_DIR/case-absent-is-not-passed" +mkdir -p "$CASE/flake-evidence/attempt-1" "$CASE/flake-evidence/attempt-2" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.GoneTest" "vanishes" "boom" +write_pass_xml "$CASE/flake-evidence/attempt-2" "com.dd.OtherTest" "unrelated" +write_list "$CASE/list.txt" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 2 --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert not d['flaky'], 'a test absent from the retry must not be called flaky: %r' % d['flaky'] +assert len(d['persistent']) == 1, d +" "$CASE/out.json" || fail "absence from a later attempt was treated as a pass" +pass "a test missing from the retry is not mistaken for a flake" + +# A missing name or classname cannot be attributed to any real +# test; it must be skipped rather than counted as a failure or crashing the +# classifier, while a properly-identified failure alongside it still counts. +CASE="$TEMP_DIR/case-unnamed-testcase" +mkdir -p "$CASE/flake-evidence/attempt-1" +cat > "$CASE/flake-evidence/attempt-1/TEST-com.dd.Weird.xml" <<'EOS' + + + + + + + + + +EOS +write_list "$CASE/list.txt" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --out "$CASE/out.json" >/dev/null 2>&1 \ + || fail "a testcase with no name attribute must not crash the classifier" +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['failure_count'] == 1, d +assert d['unclassified'] and d['unclassified'][0]['test'] == 'com.dd.Weird.realFailure', d +" "$CASE/out.json" || fail "the unnamed testcase was not ignored, or the real failure was missed" +pass "a testcase with no name attribute is ignored, not mistaken for a failure" + +# A stray attempt-* directory must not abort classification, and attempt-1 +# must still be read as the (only, and so final) real attempt. +CASE="$TEMP_DIR/case-stray-attempt" +mkdir -p "$CASE/flake-evidence/attempt-tmp" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.WobblyTest" "sometimesFails" "boom" +write_list "$CASE/list.txt" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --out "$CASE/out.json" >/dev/null 2>&1 \ + || fail "a non-numeric attempt directory must be ignored, not fatal" +pass "a stray attempt directory is ignored" +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['attempts'] == 1, d +assert d['unclassified'] and d['unclassified'][0]['test'] == 'com.dd.WobblyTest.sometimesFails', d +" "$CASE/out.json" || fail "attempt-1 was not read back despite the stray attempt-tmp" +pass "attempt-1 is still read as evidence while the stray directory is skipped" + +# The ordinary quarantine case, and the one the exit-code guard must not eat: +# a quarantined test fails on the final attempt, so Gradle exits non-zero +# *because of that very test*. No test passed --final-attempt-exit-code before, +# so it defaulted to None and this shape went untested while the guard gated +# every one of them. +CASE="$TEMP_DIR/case-quarantined-failure-exit-nonzero" +mkdir -p "$CASE/flake-evidence/attempt-1" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.WobblyTest" "sometimesFails" "boom" +cat > "$CASE/attempt.log" <<'EOS' +> Task :ddprof-test:testDebug FAILED +Execution failed for task ':ddprof-test:testDebug'. +> There were failing tests. +EOS +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --attempt-log "$CASE/attempt.log" \ + --final-attempt-exit-code 1 --test-task-pattern test \ + --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gates'] is False, 'a quarantined test failing on the final attempt must not gate: %r' % d['gate_reason'] +assert len(d['quarantined']) == 1, d +" "$CASE/out.json" || fail "the non-zero exit caused by the quarantined test's own failure was read as a crash" +pass "a quarantined failure still excuses the non-zero exit it caused" + +# The shape the guard exists for: the same all-quarantined failure list, but +# Gradle reports the test JVM died, so tests it never reached are absent from +# the XML rather than passing. +CASE="$TEMP_DIR/case-quarantined-but-cut-short" +mkdir -p "$CASE/flake-evidence/attempt-1" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.WobblyTest" "sometimesFails" "boom" +cat > "$CASE/attempt.log" <<'EOS' +> Task :ddprof-test:testDebug FAILED +Process 'Gradle Test Executor 1' finished with non-zero exit value 134 +EOS +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --attempt-log "$CASE/attempt.log" \ + --final-attempt-exit-code 1 --test-task-pattern test \ + --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gates'] is True, 'a cut-short final attempt must gate even with every named failure quarantined: %r' % d['gate_reason'] +assert d['final_attempt_cut_short'], d +" "$CASE/out.json" || fail "a dead test JVM was excused by the quarantine list" +pass "a cut-short final attempt gates despite its failures being quarantined" + +# Same intent, without the log saying so: the final attempt reached fewer tests +# than an earlier one managed, so it stopped early. +CASE="$TEMP_DIR/case-quarantined-but-short-run" +mkdir -p "$CASE/flake-evidence/attempt-1" "$CASE/flake-evidence/attempt-2" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.WobblyTest" "sometimesFails" "boom" +write_pass_xml "$CASE/flake-evidence/attempt-1" "com.dd.OtherTest" "stable" +write_failure_xml "$CASE/flake-evidence/attempt-2" "com.dd.WobblyTest" "sometimesFails" "boom" +cat > "$CASE/attempt.log" <<'EOS' +> Task :ddprof-test:testDebug FAILED +Execution failed for task ':ddprof-test:testDebug'. +EOS +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 2 --attempt-log "$CASE/attempt.log" \ + --final-attempt-exit-code 1 --test-task-pattern test \ + --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gates'] is True, 'a final attempt that reached fewer tests must gate: %r' % d['gate_reason'] +assert d['final_attempt_observed_shortfall'], d +" "$CASE/out.json" || fail "a final attempt that stopped early was excused by the quarantine list" +pass "a final attempt reaching fewer tests than an earlier one gates" + +# Gradle writes a JUnit 5 @Test method as name="method()", so the id built from +# the report is Class.method() while an entry is written Class.method. Without +# normalisation covers() (exact equality) matches nothing and the quarantine +# silently excuses nothing, with validate still reporting the list as valid. +CASE="$TEMP_DIR/case-method-parens" +mkdir -p "$CASE/flake-evidence/attempt-1" +cat > "$CASE/flake-evidence/attempt-1/TEST-com.dd.WobblyTest.xml" <<'EOS' + + + + + + +EOS +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert len(d['quarantined']) == 1, 'an entry written as documented must match the id JUnit produces: %r' % d +assert d['quarantined'][0]['test'] == 'com.dd.WobblyTest.sometimesFails()', d +assert d['gates'] is False, d['gate_reason'] +" "$CASE/out.json" || fail "the documented . form did not match Gradle's method() id" +pass "an entry written without parentheses matches JUnit's method() id" + +# The paste-ready proposal must be precise for such a method, not widened to +# the whole class: the parens are the only unsafe characters and covers() +# normalises them away. +CASE="$TEMP_DIR/case-proposal-parens" +python3 -c " +import sys +sys.path.insert(0, '$SCRIPTS') +import flake_summary +got = flake_summary.sanitize_quarantine_test_pattern('com.dd.WobblyTest.sometimesFails()') +assert got == 'com.dd.WobblyTest.sometimesFails', got +idx = flake_summary.sanitize_quarantine_test_pattern('com.dd.WobblyTest.[1]') +assert idx == 'com.dd.WobblyTest.*', idx +" || fail "the proposal for a method() id was not the documented form, or an indexed invocation was not widened class-wide" +pass "a method() id proposes the documented form; an indexed invocation proposes the class" + +# The other-task guard, asserted on the verdict rather than on stdout: the +# previous check grepped for a task name the fixture echoes itself, which +# filter_gradle_log.py passes through verbatim, so deleting the guard left the +# test green. Exit 0 from the fixture isolates this from the exit-code branch. +CASE="$TEMP_DIR/case-other-task-guard" +mkdir -p "$CASE/flake-evidence/attempt-1" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.WobblyTest" "sometimesFails" "boom" +cat > "$CASE/attempt.log" <<'EOS' +> Task :ddprof-lib:verifyNative FAILED +Execution failed for task ':ddprof-lib:verifyNative'. +EOS +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --attempt-log "$CASE/attempt.log" \ + --final-attempt-exit-code 0 --test-task-pattern ":ddprof-test:test" \ + --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gates'] is True, 'a non-test task failure must gate even with every test quarantined: %r' % d['gate_reason'] +assert 'the build also failed in' in (d['gate_reason'] or ''), d['gate_reason'] +assert d['other_task_failures'] == [':ddprof-lib:verifyNative'], d['other_task_failures'] +" "$CASE/out.json" || fail "the non-test-task guard did not gate, or did not name the task" +pass "a build failure outside the test task is never excused by the list" + +# b-12, hermetically: suspect evidence with no failure recorded has nothing to +# excuse, so the classifier must hold no opinion and let the command's own exit +# code stand. +CASE="$TEMP_DIR/case-suspect-no-failures" +mkdir -p "$CASE/flake-evidence/attempt-1" +write_pass_xml "$CASE/flake-evidence/attempt-1" "com.dd.SteadyTest" "alwaysPasses" +write_list "$CASE/list.txt" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --evidence-suspect --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gates'] is None, 'suspect evidence with no failure must not gate: %r' % d['gate_reason'] +" "$CASE/out.json" || fail "suspect evidence gated a run in which nothing failed" +pass "suspect evidence holds no opinion when there is no failure to excuse" + +# s-22: a single attempt cannot tell flaky from broken, so the failure must not +# also appear as persistent -- that label asserts a measurement never made. +CASE="$TEMP_DIR/case-single-attempt-not-persistent" +mkdir -p "$CASE/flake-evidence/attempt-1" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.SlowTest" "onlyRunOnce" "boom" +write_list "$CASE/list.txt" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "glibc-17-debug-amd64-slow" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert not d['persistent'], 'a single-attempt failure must not be labelled persistent: %r' % d['persistent'] +assert len(d['unclassified']) == 1, d +" "$CASE/out.json" || fail "a single-attempt failure was labelled broken rather than unclassified" +pass "a single-attempt failure is unclassified, not persistent" + +# s-19, through the renderer: the WIDENED marker has to reach the rendered +# proposal, not merely exist as a helper. +python3 -c " +import sys +sys.path.insert(0, '$SCRIPTS') +from flake_summary import render_proposals +out = '\n'.join(render_proposals({'com.dd.WobblyTest.[1]': {'message': 'boom', 'cells': ['glibc-17-debug-amd64']}})) +assert 'WIDENED' in out, 'the rendered proposal does not mark the widening:\n' + out +assert 'com.dd.WobblyTest.*' in out, out +" || fail "a class-wide proposal was rendered without its WIDENED marker" +pass "the rendered proposal carries the WIDENED marker" + +# g-14: no command at all must fail loudly rather than record a green cell that +# ran nothing. +CASE="$TEMP_DIR/case-no-command" +mkdir -p "$CASE" +write_list "$CASE/list.txt" +set +e +output=$(cd "$CASE" && "$SCRIPTS/run_tests_with_retry.sh" --list list.txt "glibc-17-debug-amd64" -- 2>&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "an empty command list must not exit 0 (got $rc): $output" +echo "$output" | grep -q "no command given" \ + || fail "expected an explicit error about the missing command, got: $output" +pass "an empty command list fails loudly instead of passing without running a test" + +# s-9: a destination that cannot be cleared makes cp merge stale evidence into +# this attempt's, which is how a persistent failure acquires a flaky label. +CASE="$TEMP_DIR/case-dest-clear-fails" +mkdir -p "$CASE/stub-bin" +cat > "$CASE/stub-bin/rm" <<'EOS' +#!/usr/bin/env bash +for a in "$@"; do + case "$a" in *flake-evidence/attempt-*) exit 1 ;; esac +done +exec /bin/rm "$@" +EOS +chmod +x "$CASE/stub-bin/rm" +cat > "$CASE/suite.sh" <&1) +rc=$? +set -e +[ "$rc" -ne 0 ] || fail "evidence that may be merged with an earlier attempt's must not be excused (got exit $rc): $output" +echo "$output" | grep -q "merged with an earlier attempt" \ + || fail "expected the merged-evidence warning, got: $output" +pass "a destination that cannot be cleared poisons the evidence rather than being excused" + +echo "== quarantine.py fails closed ==" + +# An unreadable review_by must stop excusing rather than excuse forever: +# parse() accepts a blank or non-date value, and validate() only runs in PR CI. +for bad in "" "not-a-date" "2026-13-45"; do + python3 -c " +import sys +sys.path.insert(0, '$SCRIPTS') +import quarantine +entry = {'test': 'a.B.c', 'ticket': 'PROF-1', 'added': '2026-01-01', + 'review_by': '$bad', 'cells': [], '_line': 1} +assert quarantine.is_expired(entry), 'review_by %r must count as expired' % '$bad' +assert quarantine.find_entry([entry], 'a.B.c', 'glibc-17-debug-amd64') is None, \ + 'an entry with an unreadable review_by must not excuse anything' +" || fail "an unreadable review_by (${bad:-}) was treated as never expiring" +done +pass "an unreadable review_by counts as expired, not as eternal" + +# A trailing '.*' covers one class's methods, as documented -- not a package. +python3 -c " +import sys +sys.path.insert(0, '$SCRIPTS') +import quarantine +pkg = {'test': 'com.datadoghq.profiler.*'} +assert not quarantine.covers(pkg, 'com.datadoghq.profiler.cpu.CpuTest.sampling'), \ + 'a package-level pattern must not quarantine a whole subtree' +cls = {'test': 'com.datadoghq.profiler.cpu.CpuTest.*'} +assert quarantine.covers(cls, 'com.datadoghq.profiler.cpu.CpuTest.sampling') +assert quarantine.covers(cls, 'com.datadoghq.profiler.cpu.CpuTest.sampling()') +assert not quarantine.covers(cls, 'com.datadoghq.profiler.cpu.CpuTest.Inner.sampling') +" || fail "a trailing '.*' is not scoped to one class's methods" +pass "a trailing '.*' covers one class's methods, not a package" + +write_list "$LIST" "$(entry 'com.datadoghq.profiler.*' PROF-1 "$(day_offset 30)")" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "a package-level '.*' pattern should be rejected" +fi +pass "a package-level '.*' pattern is rejected" + +write_list "$LIST" "$(entry '.*' PROF-1 "$(day_offset 30)")" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "a bare '.*' pattern should be rejected" +fi +pass "a bare '.*' pattern is rejected" + +# cells_overlap() fails closed for a glob matching nothing synthetic, so a JDK +# variant missing from the synthetic universe makes disjoint entries look like +# duplicates. +write_list "$LIST" \ + "$(entry a.B.c PROF-1 "$(day_offset 30)" '*17-j9*')" \ + "$(entry a.B.c PROF-2 "$(day_offset 30)" '*21-graal*')" +python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1 \ + || fail "two entries on genuinely disjoint JDK variants must validate" +pass "disjoint JDK-variant cells are not mistaken for duplicates" + +echo "== cells_glob narrows on every axis ==" + +python3 -c " +import sys +sys.path.insert(0, '$SCRIPTS') +import fnmatch +from flake_summary import cells_glob +# One cell: every axis is shared, so nothing else may match. +g = cells_glob(['glibc-8-j9-debug-amd64'])[0] +assert fnmatch.fnmatch('glibc-8-j9-debug-amd64', g), g +assert not fnmatch.fnmatch('glibc-17-debug-amd64', g), 'JDK axis not narrowed: %s' % g +# Differing only in JDK: that field opens up, the rest stays put. +g = cells_glob(['glibc-8-debug-amd64', 'glibc-17-debug-amd64'])[0] +assert fnmatch.fnmatch('glibc-8-debug-amd64', g) and fnmatch.fnmatch('glibc-17-debug-amd64', g), g +assert not fnmatch.fnmatch('musl-8-debug-amd64', g), 'libc axis not narrowed: %s' % g +assert not fnmatch.fnmatch('glibc-8-release-amd64', g), 'config axis not narrowed: %s' % g +assert not fnmatch.fnmatch('glibc-8-debug-aarch64', g), 'arch axis not narrowed: %s' % g +assert not fnmatch.fnmatch('glibc-8-debug-amd64-slow', g), 'suite suffix not narrowed: %s' % g +" || fail "cells_glob does not narrow on the JDK or the slow/regular axis" +pass "a proposal excludes cells differing only in JDK or in the slow suffix" + +echo "== a widened proposal says so ==" + +python3 -c " +import sys +sys.path.insert(0, '$SCRIPTS') +from flake_summary import widened_note, sanitize_quarantine_test_pattern +idx = 'com.dd.WobblyTest.[1]' +pat = sanitize_quarantine_test_pattern(idx) +assert pat == 'com.dd.WobblyTest.*', pat +note = widened_note(idx, pat) +assert note and 'WIDENED' in note and idx in note, note +exact = 'com.dd.WobblyTest.sometimesFails()' +assert widened_note(exact, sanitize_quarantine_test_pattern(exact)) is None +" || fail "a class-wide proposal for an inexpressible id carries no warning" +pass "a proposal widened to the class is marked as widened" + +echo "== validate rejects unmatchable test patterns ==" + +write_list "$LIST" "$(entry 'com.dd.WobblyTest.sometimesFails()' PROF-1 "$(day_offset 30)")" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "a test field carrying JUnit's parentheses should be rejected" +fi +pass "a test pattern written with parentheses is rejected" + +write_list "$LIST" "$(entry 'com.dd.WobblyTest.[1]' PROF-1 "$(day_offset 30)")" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "a test field naming an invocation index should be rejected" +fi +pass "a test pattern naming an invocation index is rejected" + +echo "== validate rejects unmatchable cell globs ==" + +write_list "$LIST" "$(entry a.B.c PROF-1 "$(day_offset 30)" '*arm64*')" +if python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null 2>&1; then + fail "a cell glob naming an architecture CI never builds should be rejected" +fi +pass "an unmatchable cell glob is rejected" + +write_list "$LIST" "$(entry a.B.c PROF-1 "$(day_offset 30)" '*aarch64*')" +python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null \ + || fail "a real cell glob should be accepted" +pass "a real cell glob is accepted" + +if python3 "$SCRIPTS/quarantine.py" --list "$TEMP_DIR/does-not-exist.txt" validate >/dev/null 2>&1; then + fail "validating a missing list should fail rather than report success" +fi +pass "a missing list fails validation instead of reporting zero problems" + +# Two entries for one test are legitimate when they cover different cells. +write_list "$LIST" \ + "$(entry a.B.c PROF-1 "$(day_offset 30)" '*aarch64*')" \ + "$(entry a.B.c PROF-2 "$(day_offset 30)" '*amd64*')" +python3 "$SCRIPTS/quarantine.py" --list "$LIST" validate >/dev/null \ + || fail "the same test on disjoint cells should be allowed" +pass "one test may have separate entries for separate cells" + +echo "== flake_summary.py renders ==" + +CASE="$TEMP_DIR/case-summary" +mkdir -p "$CASE/outcomes" +cat > "$CASE/outcomes/glibc-17-debug-aarch64.json" <<'EOS' +{"cell": "glibc-17-debug-aarch64", "attempts": 2, "attempts_run": 2, + "flaky": [{"test": "com.dd.WobblyTest.sometimesFails", "failed_attempts": [1], + "message": "got 2 | wanted 50", + "flaky": true, "quarantined": false, "ticket": null}], + "persistent": [], "quarantined": [], "gating_count": 1, "failure_count": 1, + "final_attempt_ran": true, "final_attempt_gating_count": 0, + "final_attempt_failure_count": 0, "other_task_failures": [], + "gates": true, "gate_reason": "1 un-quarantined failure(s)"} +EOS +summary=$(python3 "$SCRIPTS/flake_summary.py" --dir "$CASE/outcomes") \ + || fail "flake_summary.py must render without error" +echo "$summary" | grep -q "sometimesFails" \ + || fail "expected the flaky test in the summary, got: $summary" +echo "$summary" | grep -q "PROF-XXXXX" \ + || fail "expected a paste-ready quarantine proposal, got: $summary" +echo "$summary" | grep -q 'got 2 \\| wanted 50' \ + || fail "expected the pipe in the message to be escaped, got: $summary" +pass "the PR summary renders the flaky table and a proposal" + +echo "== flake_summary.cells_glob ==" + +# Every glob cells_glob() proposes must actually fnmatch the cells it was +# derived from -- a glob built with axes in the wrong order (or missing an +# axis real cells vary on, like config) looks plausible but can never match, +# silently proposing a quarantine entry that quarantines nothing. +python3 -c " +import fnmatch, sys +sys.path.insert(0, '$SCRIPTS') +import flake_summary as fs + +cases = [ + ['glibc-17-debug-amd64', 'glibc-21-debug-amd64'], + ['musl-17-release-aarch64', 'musl-11-release-aarch64'], + ['glibc-17-debug-amd64-slow'], + ['glibc-17-asan-amd64', 'musl-17-asan-amd64'], +] +for cells in cases: + globs = fs.cells_glob(cells) or [] + for cell in cells: + assert any(fnmatch.fnmatch(cell, g) for g in globs), ( + 'cells_glob(%r) = %r does not match %r' % (cells, globs, cell)) +print('ok') +" | grep -q '^ok$' || fail "a cells_glob() proposal did not fnmatch the cells it was derived from" +pass "every cells_glob() proposal fnmatches the cells it was derived from" + +echo +echo "All $TESTS quarantine tests passed." diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 603579f378..39d79e4f36 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,6 +45,30 @@ jobs: .github/scripts/tests/test_release_automation.sh .github/scripts/tests/test_release_automation.sh + # Fails when a quarantine entry is malformed, ticketless, or past its + # review_by date. Without this the list only ever grows, and a quarantine + # becomes a permanent mute rather than tracked debt. + validate-quarantine: + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Validate the test quarantine list + run: | + bash -n .github/scripts/run_tests_with_retry.sh + bash -n .github/scripts/generate-test-summary.sh + python3 -m py_compile .github/scripts/quarantine.py \ + .github/scripts/flake_report.py \ + .github/scripts/flake_summary.py + python3 .github/scripts/quarantine.py validate + .github/scripts/tests/test_quarantine.sh + .github/scripts/tests/test_generate_test_summary.sh + .github/scripts/tests/test_prepare_reports.sh + check-for-pr: runs-on: ubuntu-latest outputs: diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index d611384666..d9a13216dd 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -43,6 +43,11 @@ jobs: echo "configs=$configs" >> $GITHUB_OUTPUT test-linux-glibc-amd64: needs: cache-jdks + # The default job name has no room for slow_tests -- it is a workflow + # input, not a matrix axis -- so two calls to this workflow in the same + # run (one regular, one slow) would otherwise show identical job names + # and be indistinguishable to generate-test-summary.sh. + name: test-linux-glibc-amd64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) strategy: fail-fast: false matrix: @@ -155,26 +160,38 @@ jobs: exit 0 fi - MAX_ATTEMPTS=1 + # Default: a failure naming no test is a build error, not worth a + # retry. + export RETRY_ON_NO_TEST_FAILURES=0 if [[ "${{ matrix.config }}" == "asan" ]]; then - # ASan init can nondeterministically collide with the JVM's ASLR-influenced - # mmap layout (google/sanitizers#856); retry once before failing the job. - MAX_ATTEMPTS=2 + # ASan init can nondeterministically collide with the JVM's + # ASLR-influenced mmap layout (google/sanitizers#856). The JVM + # aborts before any test runs, so that failure names no test and + # would otherwise be classed as a build error and left unretried. + export RETRY_ON_NO_TEST_FAILURES=1 fi - for attempt in $(seq 1 $MAX_ATTEMPTS); do - mkdir -p build/logs - ${GRADLEW_PREFIX} ./gradlew -PCI -PkeepJFRs ${{ inputs.skip_gtest == true && '-Pskip-gtest' || '' }} :ddprof-test:test${{ inputs.slow_tests && 'Slow' || '' }}${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs 2>&1 \ - | tee -a build/test-raw.log \ - | python3 -u .github/scripts/filter_gradle_log.py - EXIT_CODE=${PIPESTATUS[0]} - - if [ $EXIT_CODE -eq 0 ]; then break; fi - if [ $attempt -lt $MAX_ATTEMPTS ]; then - echo "::warning::Attempt $attempt failed (exit $EXIT_CODE), retrying..." - ./gradlew --stop 2>/dev/null || true + # The slow/e2e suite already runs the best part of an hour, so a + # retry would risk the 180-minute job timeout, and it records + # failures without re-running them. ASan keeps its second attempt + # even when slow, but only for the init abort above (0 named + # failures): MAX_FAILURES_TO_RETRY=0 stops a slow ASan run that + # actually failed named tests from being retried too, which would + # re-run the full slow suite a second time and risk that same + # timeout. + export MAX_ATTEMPTS=2 + if [[ "${{ inputs.slow_tests }}" == "true" ]]; then + if [[ "${{ matrix.config }}" == "asan" ]]; then + export MAX_FAILURES_TO_RETRY=0 + else + export MAX_ATTEMPTS=1 fi - done + fi + + .github/scripts/run_tests_with_retry.sh \ + "glibc-${{ matrix.java_version }}-${{ matrix.config }}-amd64${{ inputs.slow_tests && '-slow' || '' }}" -- \ + ${GRADLEW_PREFIX} ./gradlew -PCI -PkeepJFRs ${{ inputs.skip_gtest == true && '-Pskip-gtest' || '' }} :ddprof-test:test${{ inputs.slow_tests && 'Slow' || '' }}${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs + EXIT_CODE=$? # Kill the watchdog if tests finished before it fired if [[ -n "${GDB_WATCHDOG_PID:-}" ]]; then @@ -222,6 +239,16 @@ jobs: with: name: (test-reports) test-linux-glibc-amd64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) path: test-reports + # Always uploaded, unlike the reports above: a cell that went green only + # because of a retry produces no failure artifact, and that is exactly the + # run whose evidence the summary needs. The file is a few hundred bytes. + - name: Upload CI outcome + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: (ci-outcome) test-linux-glibc-amd64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) + path: ci-outcome + if-no-files-found: ignore - name: Upload signal-safety violation log uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: failure() @@ -240,6 +267,8 @@ jobs: test-linux-musl-amd64: needs: [cache-jdks, filter-musl-configs] if: needs.filter-musl-configs.outputs.has_configs == 'true' + # See the comment on test-linux-glibc-amd64's name above. + name: test-linux-musl-amd64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) strategy: fail-fast: false matrix: @@ -310,11 +339,12 @@ jobs: export JAVA_VERSION echo "JAVA_VERSION=${JAVA_VERSION}" - mkdir -p build/logs - ./gradlew -PCI -PkeepJFRs :ddprof-test:test${{ inputs.slow_tests && 'Slow' || '' }}${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs 2>&1 \ - | tee -a build/test-raw.log \ - | python3 -u .github/scripts/filter_gradle_log.py - EXIT_CODE=${PIPESTATUS[0]} + export MAX_ATTEMPTS=${{ inputs.slow_tests && 1 || 2 }} + + .github/scripts/run_tests_with_retry.sh \ + "musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64${{ inputs.slow_tests && '-slow' || '' }}" -- \ + ./gradlew -PCI -PkeepJFRs :ddprof-test:test${{ inputs.slow_tests && 'Slow' || '' }}${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs + EXIT_CODE=$? if [ $EXIT_CODE -ne 0 ]; then echo "musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt @@ -357,6 +387,16 @@ jobs: with: name: (test-reports) test-linux-musl-amd64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) path: test-reports + # Always uploaded, unlike the reports above: a cell that went green only + # because of a retry produces no failure artifact, and that is exactly the + # run whose evidence the summary needs. The file is a few hundred bytes. + - name: Upload CI outcome + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: (ci-outcome) test-linux-musl-amd64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) + path: ci-outcome + if-no-files-found: ignore - name: Upload signal-safety violation log uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: failure() @@ -374,6 +414,8 @@ jobs: test-linux-glibc-aarch64: needs: cache-jdks + # See the comment on test-linux-glibc-amd64's name above. + name: test-linux-glibc-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) strategy: fail-fast: false matrix: @@ -483,26 +525,38 @@ jobs: exit 0 fi - MAX_ATTEMPTS=1 + # Default: a failure naming no test is a build error, not worth a + # retry. + export RETRY_ON_NO_TEST_FAILURES=0 if [[ "${{ matrix.config }}" == "asan" ]]; then - # ASan init can nondeterministically collide with the JVM's ASLR-influenced - # mmap layout (google/sanitizers#856); retry once before failing the job. - MAX_ATTEMPTS=2 + # ASan init can nondeterministically collide with the JVM's + # ASLR-influenced mmap layout (google/sanitizers#856). The JVM + # aborts before any test runs, so that failure names no test and + # would otherwise be classed as a build error and left unretried. + export RETRY_ON_NO_TEST_FAILURES=1 fi - for attempt in $(seq 1 $MAX_ATTEMPTS); do - mkdir -p build/logs - ${GRADLEW_PREFIX} ./gradlew -PCI -PkeepJFRs ${{ inputs.skip_gtest == true && '-Pskip-gtest' || '' }} :ddprof-test:test${{ inputs.slow_tests && 'Slow' || '' }}${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs 2>&1 \ - | tee -a build/test-raw.log \ - | python3 -u .github/scripts/filter_gradle_log.py - EXIT_CODE=${PIPESTATUS[0]} - - if [ $EXIT_CODE -eq 0 ]; then break; fi - if [ $attempt -lt $MAX_ATTEMPTS ]; then - echo "::warning::Attempt $attempt failed (exit $EXIT_CODE), retrying..." - ./gradlew --stop 2>/dev/null || true + # The slow/e2e suite already runs the best part of an hour, so a + # retry would risk the 180-minute job timeout, and it records + # failures without re-running them. ASan keeps its second attempt + # even when slow, but only for the init abort above (0 named + # failures): MAX_FAILURES_TO_RETRY=0 stops a slow ASan run that + # actually failed named tests from being retried too, which would + # re-run the full slow suite a second time and risk that same + # timeout. + export MAX_ATTEMPTS=2 + if [[ "${{ inputs.slow_tests }}" == "true" ]]; then + if [[ "${{ matrix.config }}" == "asan" ]]; then + export MAX_FAILURES_TO_RETRY=0 + else + export MAX_ATTEMPTS=1 fi - done + fi + + .github/scripts/run_tests_with_retry.sh \ + "glibc-${{ matrix.java_version }}-${{ matrix.config }}-aarch64${{ inputs.slow_tests && '-slow' || '' }}" -- \ + ${GRADLEW_PREFIX} ./gradlew -PCI -PkeepJFRs ${{ inputs.skip_gtest == true && '-Pskip-gtest' || '' }} :ddprof-test:test${{ inputs.slow_tests && 'Slow' || '' }}${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs + EXIT_CODE=$? # Kill the watchdog if tests finished before it fired if [[ -n "${GDB_WATCHDOG_PID:-}" ]]; then @@ -550,6 +604,16 @@ jobs: with: name: (test-reports) test-linux-glibc-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) path: test-reports + # Always uploaded, unlike the reports above: a cell that went green only + # because of a retry produces no failure artifact, and that is exactly the + # run whose evidence the summary needs. The file is a few hundred bytes. + - name: Upload CI outcome + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: (ci-outcome) test-linux-glibc-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) + path: ci-outcome + if-no-files-found: ignore - name: Upload signal-safety violation log uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: failure() @@ -568,6 +632,8 @@ jobs: test-linux-musl-aarch64: needs: [cache-jdks, filter-musl-configs] if: needs.filter-musl-configs.outputs.has_configs == 'true' + # See the comment on test-linux-glibc-amd64's name above. + name: test-linux-musl-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) strategy: fail-fast: false matrix: @@ -608,16 +674,18 @@ jobs: set +e # the effective JAVA_VERSION is computed in the test_alpine_aarch64.sh script mkdir -p build/logs - docker run --cpus 4 --rm -v /tmp:/tmp -v "${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}" -w "${GITHUB_WORKSPACE}" alpine:3.21 /bin/sh -c " - \"$GITHUB_WORKSPACE/.github/scripts/test_alpine_aarch64.sh\" \ - \"${{ github.sha }}\" \"musl/${{ matrix.java_version }}-${{ matrix.config }}-aarch64\" \ - \"${{ matrix.config }}\" \"${{ env.JAVA_HOME }}\" \"${{ env.JAVA_TEST_HOME }}\" \ - \"${{ inputs.slow_tests }}\" - " 2>&1 \ - | tee -a build/test-raw.log \ - | python3 -u .github/scripts/filter_gradle_log.py + export MAX_ATTEMPTS=${{ inputs.slow_tests && 1 || 2 }} - EXIT_CODE=${PIPESTATUS[0]} + .github/scripts/run_tests_with_retry.sh \ + "musl-${{ matrix.java_version }}-${{ matrix.config }}-aarch64${{ inputs.slow_tests && '-slow' || '' }}" -- \ + docker run --cpus 4 --rm -v /tmp:/tmp -v "${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}" -w "${GITHUB_WORKSPACE}" alpine:3.21 /bin/sh -c " + \"$GITHUB_WORKSPACE/.github/scripts/test_alpine_aarch64.sh\" \ + \"${{ github.sha }}\" \"musl/${{ matrix.java_version }}-${{ matrix.config }}-aarch64\" \ + \"${{ matrix.config }}\" \"${{ env.JAVA_HOME }}\" \"${{ env.JAVA_TEST_HOME }}\" \ + \"${{ inputs.slow_tests }}\" + " + + EXIT_CODE=$? if [ $EXIT_CODE -ne 0 ]; then echo "musl-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_musl-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt @@ -674,6 +742,16 @@ jobs: with: name: (test-reports) test-linux-musl-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) path: test-reports + # Always uploaded, unlike the reports above: a cell that went green only + # because of a retry produces no failure artifact, and that is exactly the + # run whose evidence the summary needs. The file is a few hundred bytes. + - name: Upload CI outcome + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: (ci-outcome) test-linux-musl-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }}, ${{ inputs.slow_tests && 'slow' || 'regular' }}) + path: ci-outcome + if-no-files-found: ignore - name: Upload signal-safety violation log uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: failure() diff --git a/.gitignore b/.gitignore index 1c9dd43f2d..38ff268849 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,13 @@ datadog/maven/resources # Temporary documentation and work state doc/temp/ +# Working state left by run_tests_with_retry.sh +/flake-evidence/ +/ci-outcome/ + +# Bytecode from the CI scripts, left by running them or their tests locally +__pycache__/ + # CLAUDE.md is auto-generated from AGENTS.md bootstrap instructions CLAUDE.md diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt index e0b9691433..b7fbae4632 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt @@ -368,6 +368,12 @@ class ProfilerTestPlugin : Plugin { allArgs.add("-D$key=$value") } + // Same build/test-results/ layout Gradle's own Test task uses, + // so run_tests_with_retry.sh's TEST-*.xml scan finds musl's results the + // same way it finds every other cell's. + val reportsDir = project.layout.buildDirectory.dir("test-results/$taskName").get().asFile + allArgs.add("-Dtest.reportsDir=${reportsDir.absolutePath}") + // UNIFIED INTERFACE: Test filter from -Ptests property val testsFilter = project.findProperty("tests") as String? if (testsFilter != null) { diff --git a/ddprof-test/quarantine.txt b/ddprof-test/quarantine.txt new file mode 100644 index 0000000000..7fbd2121da --- /dev/null +++ b/ddprof-test/quarantine.txt @@ -0,0 +1,46 @@ +# Tests whose failures do not turn CI red. +# +# FORMAT — one entry per line, six fields separated by "|", whitespace around +# each field ignored. Blank lines and lines starting with "#" are ignored. +# +# test | ticket | added | review_by | cells | reason +# +# test Fully qualified ., written without the +# parentheses JUnit puts in its own XML report -- CI normalises +# the report's id to this shape. A trailing ".*" covers every +# method in the class. +# +# @ParameterizedTest and @RetryingTest invocations cannot be +# named individually: the XML records them as "[1]", "[2]" with +# no method name at all, so the index identifies neither the +# method nor a stable case. Quarantine those class-wide with +# ".*"; CI's suggested line already does. +# ticket PROF-. Required — a quarantine without a ticket is just +# a test nobody runs. +# added YYYY-MM-DD, the day it went in. +# review_by YYYY-MM-DD. CI FAILS once this date passes, so staying +# quarantined is a decision somebody renews rather than the +# default. 90 days is the usual span. +# cells Comma-separated globs against the cell name +# (---[-slow]) -- the slow/e2e suite gets +# its own cells, suffixed "-slow", distinct from the regular +# suite's -- e.g. "*aarch64*", "musl-*,*-asan-*", or "*-slow" to +# target only the slow suite. Leave as "-" to quarantine +# everywhere; prefer narrowing it, so the same test breaking +# elsewhere still gates. +# reason Free text — what is unreliable and how often. Last field, so it +# may contain anything but "|". +# +# A quarantined test STILL RUNS and still reports; only the gating is +# suspended. That keeps the pass rate visible, which is how you find out the +# test got fixed, or that a "flake" has quietly become permanently broken. +# +# Quarantine is for a test that fails intermittently and that nobody has time +# to fix right now. It is not for a test that is simply wrong — fix or delete +# that one. The intended exit from this file is a fix and a deleted line. +# +# When CI sees a flake it prints a ready-made line in the PR comment. The +# ticket and the judgement are still yours. +# +# Example (delete when the first real entry lands): +# com.datadoghq.profiler.cpu.CpuSamplingTest.testSampling | PROF-12345 | 2026-09-02 | 2026-12-02 | *aarch64* | Under-samples on emulated aarch64; 2 of 40 runs diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/test/ProfilerTestRunner.java b/ddprof-test/src/test/java/com/datadoghq/profiler/test/ProfilerTestRunner.java index ff7c0a2e68..d03991d52a 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/test/ProfilerTestRunner.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/test/ProfilerTestRunner.java @@ -18,8 +18,13 @@ import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; import org.junit.platform.launcher.core.LauncherFactory; import org.junit.platform.launcher.listeners.SummaryGeneratingListener; +import org.junit.platform.reporting.legacy.xml.LegacyXmlReportGeneratingListener; +import java.io.IOException; import java.io.PrintWriter; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -41,6 +46,10 @@ * - -Dtest.filter=*.Pattern* - Pattern matching on class names * - -Dtest.tags.include=tag1,tag2 - Only run tests tagged with one of these tags * - -Dtest.tags.exclude=tag1,tag2 - Skip tests tagged with any of these tags + * - -Dtest.reportsDir=path - Write JUnit XML reports here (same TEST-*.xml format + * Gradle's Test task produces), so flake_report.py can + * classify musl failures the same way it classifies every + * other cell */ public class ProfilerTestRunner { public static void main(String[] args) { @@ -132,6 +141,31 @@ private static void runTests() { SummaryGeneratingListener listener = new SummaryGeneratingListener(); launcher.registerTestExecutionListeners(new GradleStyleTestListener(), listener); + // Without this, musl runs produce no TEST-*.xml at all: flake_report.py has + // nothing to read, so every failure on musl is invisible to flake + // classification and quarantine, and a passing musl attempt looks + // indistinguishable from one that never ran. + String reportsDir = System.getProperty("test.reportsDir"); + if (reportsDir != null && !reportsDir.isEmpty()) { + Path reportsPath = Paths.get(reportsDir); + try { + Files.createDirectories(reportsPath); + } catch (IOException e) { + // Silently producing no XML is the one outcome that must not + // happen: flake_report.py would read zero observed tests for + // this cell, which is indistinguishable from a cell that was + // never retried, so a failure here would disable flake + // classification and quarantine for musl without a trace. + System.err.println("::error::cannot write JUnit reports to " + reportsPath + ": " + e); + System.exit(1); + } + if (!Files.isDirectory(reportsPath)) { + System.err.println("::error::JUnit report directory is not a directory: " + reportsPath); + System.exit(1); + } + launcher.registerTestExecutionListeners(new LegacyXmlReportGeneratingListener(reportsPath, new PrintWriter(System.err))); + } + // Execute tests launcher.execute(request); diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 1f8c21b5dd..b3efc0d0f1 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -39,6 +39,7 @@ junit-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "jun junit-engine = { module = "org.junit.jupiter:junit-jupiter-engine", version.ref = "junit" } junit-params = { module = "org.junit.jupiter:junit-jupiter-params", version.ref = "junit" } junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher", version.ref = "junit-platform" } +junit-platform-reporting = { module = "org.junit.platform:junit-platform-reporting", version.ref = "junit-platform" } junit-pioneer = { module = "org.junit-pioneer:junit-pioneer", version.ref = "junit-pioneer" } # Logging @@ -66,7 +67,7 @@ jmh-annprocess = { module = "org.openjdk.jmh:jmh-generator-annprocess", version. [bundles] # Core testing framework (JUnit + logging + launcher API for custom test runner) -testing = ["junit-api", "junit-engine", "junit-params", "junit-platform-launcher", "junit-pioneer", "slf4j-simple"] +testing = ["junit-api", "junit-engine", "junit-params", "junit-platform-launcher", "junit-platform-reporting", "junit-pioneer", "slf4j-simple"] # Profiler runtime dependencies (JFR analysis + compression) profiler-runtime = ["jmc-flightrecorder", "jol-core", "lz4", "snappy", "zstd"]