diff --git a/.github/scripts/flake_report.py b/.github/scripts/flake_report.py new file mode 100755 index 0000000000..efaccbc7af --- /dev/null +++ b/.github/scripts/flake_report.py @@ -0,0 +1,321 @@ +#!/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 '([^']+)'") + + +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 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) + + # 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. It ran, recorded results, and + # non-zero having named named no failure of its own, yet the + # no failure of its own command still failed: the JVM aborted + # part-way through, so the tests it never + # reached are absent from the XML rather + # than passing. Gradle blames the 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 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 gating: + gates = True + gate_reason = "{} un-quarantined failure(s)".format(len(gating)) + elif results: + if 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 not final_attempt_failure_count: + gates = True + gate_reason = ( + "the final attempt named no failure of its own yet exited {}; " + "the run was cut short rather than passing, so the tests missing " + "from its results cannot be read as quarantined" + ).format(args.final_attempt_exit_code) + 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, + "flaky": [r for r in results if r["flaky"] and not r["quarantined"]], + "persistent": [r for r in results if not r["flaky"] and not r["quarantined"]], + "quarantined": [r for r in results if r["quarantined"]], + "gating_count": len(gating), + "failure_count": len(results), + "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_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..52c5a86e29 --- /dev/null +++ b/.github/scripts/flake_summary.py @@ -0,0 +1,290 @@ +#!/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 one place a failure message is truncated for display. flake_report.py +# stores messages at a wider cap (200 chars) for anyone reading the raw JSON; +# every renderer of this data (this module's tables, generate-test-summary.sh's +# per-job table) uses this same, narrower display width so the same failure +# does not render at two different lengths in one PR comment. +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). + """ + 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). + """ + axes = ["aarch64", "amd64", "musl", "glibc", "asan", "tsan", "slow"] + shared = [axis for axis in axes if all(axis in c for c in cells)] + if not shared: + return None + return ["*" + "*".join(shared) + "*"] + + +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("|", "/") + out.append(quarantine.format_entry( + sanitize_quarantine_test_pattern(test_id), + "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") + 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 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..e016a46da6 100755 --- a/.github/scripts/prepare_reports.sh +++ b/.github/scripts/prepare_reports.sh @@ -12,6 +12,18 @@ 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. +if [ -z "$(find flake-evidence -mindepth 1 -maxdepth 1 -name 'attempt-*' 2>/dev/null)" ]; 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..f5a7890b2a --- /dev/null +++ b/.github/scripts/quarantine.py @@ -0,0 +1,297 @@ +#!/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. Deliberately over-inclusive (e.g. jdk variants like "8-j9" beyond the +# base list below): a synthetic cell that never occurs for real only makes +# overlap detection more conservative, never less. +_SYNTHETIC_JDKS = ("8", "8-graal", "11", "17", "17-graal", "21", "25") +_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) + + +def covers(entry, test_id): + pattern = entry["test"] + if pattern.endswith(".*"): + return test_id.startswith(pattern[:-1]) + return test_id == pattern + + +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. + """ + return next( + (e for e in entries if covers(e, test_id) and applies_to(e, cell)), + 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])) + + if entry["test"] and BAD_TEST_WILDCARD_RE.search( + entry["test"][:-2] if entry["test"].endswith(".*") else entry["test"] + ): + complain(line, ( + "test pattern '{}' has a wildcard outside a single trailing " + "'.*'; covers() only understands an exact id or a class-wide " + "'.*', so this would silently quarantine nothing" + ).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..0fea0325ba --- /dev/null +++ b/.github/scripts/run_tests_with_retry.sh @@ -0,0 +1,265 @@ +#!/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 + +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 + rm -rf "$dest" || echo "::warning::Could not clear ${dest}; attempt ${attempt} evidence may be stale" + mkdir -p "$dest" + if [ -d "$RESULTS_DIR" ]; then + cp -r "$RESULTS_DIR"/. "$dest"/ \ + || 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. +rm -rf "$EVIDENCE_DIR" "$(dirname "$OUTCOME_FILE")" + +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..594d960ae3 --- /dev/null +++ b/.github/scripts/tests/test_generate_test_summary.sh @@ -0,0 +1,150 @@ +#!/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" + +echo +echo "All $TESTS generate-test-summary tests passed." diff --git a/.github/scripts/tests/test_quarantine.sh b/.github/scripts/tests/test_quarantine.sh new file mode 100755 index 0000000000..6b102cbbf1 --- /dev/null +++ b/.github/scripts/tests/test_quarantine.sh @@ -0,0 +1,566 @@ +#!/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" + +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'], 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" + +# ...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 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['persistent'] and d['persistent'][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['persistent'] and d['persistent'][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" + +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 +echo "All $TESTS quarantine tests passed." diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 603579f378..69990a911e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,6 +45,29 @@ 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 + 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..ee257fca7c 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,10 @@ datadog/maven/resources # Temporary documentation and work state doc/temp/ +# Working state left by run_tests_with_retry.sh +/flake-evidence/ +/ci-outcome/ + # CLAUDE.md is auto-generated from AGENTS.md bootstrap instructions CLAUDE.md diff --git a/ddprof-test/quarantine.txt b/ddprof-test/quarantine.txt new file mode 100644 index 0000000000..74504f057c --- /dev/null +++ b/ddprof-test/quarantine.txt @@ -0,0 +1,38 @@ +# 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 .. A trailing ".*" covers every +# method in the class. +# 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