diff --git a/.github/workflows/scripts/test-quarantine/README.md b/.github/workflows/scripts/test-quarantine/README.md index f39f7b6d7b56..53a01e34b700 100644 --- a/.github/workflows/scripts/test-quarantine/README.md +++ b/.github/workflows/scripts/test-quarantine/README.md @@ -18,7 +18,7 @@ issue. These correspond to Case A and Case B in 2. A full-history trusted checkout and `collect_case_a_eligibility.py` produce `test-quarantine-case-a-eligibility.json`. Each test receipt records exact - source resolution, method/class/assembly quarantine state, quarantine + source resolution, data-row/method/class/assembly quarantine state, quarantine history category, regression status, raw/excluded/post-cutoff build sets, the conservative freshness cutoff, build-source ancestry against the history cutoff commit, exact evidence identity, and the `origin/main` @@ -41,10 +41,11 @@ issue. These correspond to Case A and Case B in method freshness cutoff and Source B pull-request file checks, which still key off the resolved declaring method and inherited runner files rather than every partial sibling declaration. -3. `collect_requarantine_history.py` enumerates every current method-, type-, - and assembly-level quarantine target from trusted source. It classifies the - exact first-parent history from project-wide commit/parent source snapshots - as `first-quarantine`, `re-quarantined`, or `ambiguous`; method and partial +3. `collect_requarantine_history.py` enumerates every current data-row-, + method-, type-, and assembly-level quarantine target from trusted source. It + classifies the exact first-parent history from project-wide commit/parent + source snapshots as `first-quarantine`, `re-quarantined`, or `ambiguous`; + method and partial type moves between files preserve their logical history, and issue-URL-only replacements are not remove/add transitions. Automated unquarantine requires an exact `first-quarantine` match and fails closed otherwise. @@ -76,11 +77,30 @@ issue. These correspond to Case A and Case B in Agent-provided log excerpts and URLs are for human display only. They are not accepted as validation evidence. -Part 1 still aggregates by normalized test name, not by assembly-qualified -identity. The collector therefore fails closed on ambiguous runner names rather -than using the representative assembly field to choose a project. This does not -redesign aggregation or method-level, file-based quarantine history. Unresolved -historical inheritance is unproven, not evidence that a test was never inherited. +Part 1 aggregates by exact test-case name, preserving theory argument lists, +but not by assembly-qualified identity. The collector correlates those arguments +to an unambiguous `InlineData` row on a `ConditionalTheory`, or an existing +`QuarantinedTestData` row, when possible and +otherwise retains method-level behavior. It fails closed on ambiguous runner or +data-row identities rather than using representative metadata to guess. +Boolean and integer inline constants are normalized to their rendered argument +values while retaining the original source arguments for patch validation. +A `ConditionalTheory` with inline rows that cannot be matched exactly (including +missing argument lists or unsupported constant expressions) is unproven, not a +method-level quarantine candidate. Trailing attribute comments do not hide rows. +Row mapping also fails closed when other method attributes could provide data: +`MemberData`, `ClassData`, and unrecognized attributes can produce the same +rendered arguments as an inline row. Only the recognized theory, inline/data +quarantine, method quarantine, xUnit trait, and known repository non-data condition +attributes (including `MsQuicSupported`, `OSSkipCondition`, and +`FrameworkSkipCondition`) are accepted for automatic row mapping. The known +conditions derive directly from `Attribute`, not `DataAttribute`; implementing +`ITestCondition` alone is not sufficient. Unknown attributes are not assumed to +be non-data metadata. +Matching inline and quarantined rows with identical arguments are ambiguous too. +Renamed row owners receive the same conservative history checks as method targets. +Unresolved historical inheritance is unproven, not evidence that a test was +never inherited. ## Build Insights behavior @@ -113,6 +133,18 @@ into a KBE. ## Safety properties - One exact fully qualified test per new-quarantine issue and PR. +- Row-level quarantine changes preserve the original inline data arguments and + are accepted only when the deterministic receipt resolves that exact row. +- An exact `InlineData` candidate on a `ConditionalTheory` is always row-scoped; + the validator never permits broadening it to a method quarantine. Ordinary + xUnit theories remain method-scoped because they cannot consume quarantine + row metadata. +- The collector recognizes multiline quarantine/data attributes, but automated + row rewrites are deliberately limited to one-line attributes so patch + validation never has to infer unchanged argument lines from diff context. + One-line row replacements may retain trailing line or block comments. The + validator ignores comment brackets when parsing, but requires the exact + comment to remain attached to the same data arguments in the same diff hunk. - The agent cannot author or override new-quarantine eligibility facts. - Every quarantine or unquarantine PR is mechanically bound to deterministic receipts before the privileged PR handler runs. An unquarantine PR may @@ -124,6 +156,18 @@ into a KBE. - At least two distinct post-cutoff failures, exact current quarantine state, regression exclusion, and the new-quarantine category are enforced before KBE rendering. +- Quarantine additions are bound to a deterministic operating-system set. + A subset is emitted only when every retained incident has an unambiguous + platform identity; otherwise the receipt requires all supported platforms. + Source A/B retain per-build `queues` for every distinct result, including + multiple platforms in one build. These come from the Helix job API's `QueueId`, + not the OS-neutral work-item name. Source C records carry the selected job's + `queue`. Job lookups (including failures) are cached across all three sources. + Additional result-detail calls are bounded per source; missing identities, + failed lookups, and budget exhaustion leave explicit unknown entries rather + than borrowing the representative result's OS. Older payloads without queue + metadata also require all supported platforms. + Existing partially scoped targets are not automatically widened or narrowed. - An assembly quarantine removal is treated as a prior unquarantine only if the runner actually inherited or declared the test at that transition. Ambiguous project or historical source association fails closed as unproven. diff --git a/.github/workflows/scripts/test-quarantine/collect_case_a_eligibility.py b/.github/workflows/scripts/test-quarantine/collect_case_a_eligibility.py index 72fdb7a45b68..12afddec37c0 100644 --- a/.github/workflows/scripts/test-quarantine/collect_case_a_eligibility.py +++ b/.github/workflows/scripts/test-quarantine/collect_case_a_eligibility.py @@ -25,16 +25,53 @@ WORK_ITEM_SUFFIX = ".WorkItemExecution" QUARANTINE = "QuarantinedTest" +OPERATING_SYSTEMS = ( + "OperatingSystems.Linux", + "OperatingSystems.MacOSX", + "OperatingSystems.Windows", +) QUARANTINE_ATTRIBUTE_PATTERN = re.compile( r"(?:\[|,)\s*(?:assembly\s*:\s*)?" r"(?:[A-Za-z_][A-Za-z0-9_]*\.)*" - r"QuarantinedTest(?:Attribute)?\s*(?:\(|\])" + r"QuarantinedTest(?:Data)?(?:Attribute)?\s*(?:\(|\])" ) ASSEMBLY_QUARANTINE_PATTERN = re.compile( r"\[\s*assembly\s*:\s*" r"(?:[A-Za-z_][A-Za-z0-9_]*\.)*" r"QuarantinedTest(?:Attribute)?\s*(?:\(|\])" ) +METHOD_QUARANTINE_PATTERN = re.compile( + r"^\s*\[\s*(?:[A-Za-z_][A-Za-z0-9_]*\.)*" + r"QuarantinedTest(?:Attribute)?\s*\(\s*" + r'"(?P[^"]*)"\s*' + r"(?:,\s*(?P[^,\]]+))?\s*\)\s*\]\s*$" +) +DATA_QUARANTINE_PATTERN = re.compile( + r"^\s*\[\s*(?:[A-Za-z_][A-Za-z0-9_]*\.)*" + r"QuarantinedTestData(?:Attribute)?\s*\(\s*" + r'"(?P[^"]*)"\s*,\s*' + r"(?P[^,]+)\s*,\s*" + r"(?P.*)\)\s*\]\s*$" +) +INLINE_DATA_PATTERN = re.compile( + r"^\s*\[\s*(?:[A-Za-z_][A-Za-z0-9_]*\.)*" + r"InlineData(?:Attribute)?\s*\((?P.*)\)\s*\]\s*$" +) +CONDITIONAL_THEORY_PATTERN = re.compile( + r"^\s*\[\s*(?:global::)?(?:[A-Za-z_][A-Za-z0-9_]*\.)*" + r"ConditionalTheory(?:Attribute)?\s*(?:\(.*\))?\s*\]\s*$" +) +TRAIT_PATTERN = re.compile( + r"^\s*\[\s*(?:global::)?(?:Xunit\.)?" + r"Trait(?:Attribute)?\s*\(.*\)\s*\]\s*$" +) +NON_DATA_CONDITION_PATTERN = re.compile( + r"^\s*\[\s*(?:global::)?(?:Microsoft\.AspNetCore\.InternalTesting\.)?" + r"(?:MsQuicSupported|OSSkipCondition|FrameworkSkipCondition|" + r"EnvironmentVariableSkipCondition|MinimumOSVersion|MaximumOSVersion|" + r"DockerOnly|RemoteExecutionSupported|SkipNonHelix|SkipOnHelix|SkipOnCI|SkipOnAlpine)" + r"(?:Attribute)?\s*(?:\(.*\))?\s*\]\s*$" +) QUARANTINE_ISSUE_PATTERN = re.compile( r"https://github\.com/dotnet/aspnetcore/issues/(?P\d+)" ) @@ -58,6 +95,215 @@ def has_quarantine_attribute(text): return QUARANTINE_ATTRIBUTE_PATTERN.search(sanitize_csharp(text)) is not None +def split_arguments(value): + arguments = [] + start = 0 + depth = 0 + quote = None + escaped = False + for index, character in enumerate(value): + if quote is not None: + if escaped: + escaped = False + elif character == "\\": + escaped = True + elif character == quote: + quote = None + continue + if character in ('"', "'"): + quote = character + elif character in "([{": + depth += 1 + elif character in ")]}": + depth -= 1 + elif character == "," and depth == 0: + arguments.append(value[start:index].strip()) + start = index + 1 + arguments.append(value[start:].strip()) + return arguments + + +def normalize_data_value(value): + value = value.strip() + label = re.match(r"^[A-Za-z_][A-Za-z0-9_]*\s*:\s*(.*)$", value) + if label: + value = label.group(1).strip() + while True: + cast = re.match( + r"^\(\s*[A-Za-z_][A-Za-z0-9_.<>\[\]?]*\s*\)\s*(.*)$", + value, + ) + if not cast: + break + value = cast.group(1).strip() + if value.startswith('"') and value.endswith('"'): + return value + if value in ("true", "false"): + return value.title() + integer = re.fullmatch( + r"([+-]?(?:0[xX][0-9a-fA-F_]+|0[bB][01_]+|[0-9][0-9_]*))" + r"(?:[uU][lL]?|[lL][uU]?)?", + re.sub(r"\s+", "", value), + ) + if integer: + digits = integer.group(1).replace("_", "") + unsigned = digits.lstrip("+-").lower() + base = 16 if unsigned.startswith("0x") else 2 if unsigned.startswith("0b") else 10 + return str(int(digits, base)) + if re.fullmatch(r"[A-Za-z_][A-Za-z0-9_.]*", value): + return value.rsplit(".", 1)[-1] + return re.sub(r"\s+", "", value) + + +def data_values(value): + return tuple(normalize_data_value(item) for item in split_arguments(value)) + + +def operating_system_values(value): + if value is None: + return OPERATING_SYSTEMS + values = tuple( + item.strip() + for item in value.split("|") + ) + if ( + len(values) != len(set(values)) + or any(item not in OPERATING_SYSTEMS for item in values) + ): + return None + return tuple( + item for item in OPERATING_SYSTEMS + if item in values + ) + + +def operating_system_from_queue(value): + if not isinstance(value, str): + return None + platform = value.strip().split(".", 1)[0].lower() + if platform in { + "linux", + "ubuntu", + "debian", + "alpine", + "centos", + "rhel", + "azurelinux", + "almalinux", + "fedora", + }: + return "OperatingSystems.Linux" + if platform in {"mac", "macos", "osx"}: + return "OperatingSystems.MacOSX" + if platform in { + "win", + "win7", + "win10", + "win11", + "windows", + }: + return "OperatingSystems.Windows" + return None + + +def square_bracket_delta(value): + depth = 0 + quote = None + escaped = False + for character in value: + if quote is not None: + if escaped: + escaped = False + elif character == "\\": + escaped = True + elif character == quote: + quote = None + continue + if character in ('"', "'"): + quote = character + elif character == "[": + depth += 1 + elif character == "]": + depth -= 1 + return depth + + +def logical_attributes(attributes): + result = [] + start = None + depth = 0 + for index, character in enumerate(sanitize_csharp(attributes)): + if character == "[": + if depth == 0: + start = index + 1 + depth += 1 + elif character == "]" and depth: + depth -= 1 + if depth == 0: + inner = " ".join(attributes[start:index].splitlines()).strip() + result.extend( + f"[{item}]" + for item in split_arguments(inner) + if item + ) + return result + + +def quarantine_attributes(attributes): + result = { + "method": [], + "data": [], + "inline": [], + } + for attribute in logical_attributes(attributes): + method = METHOD_QUARANTINE_PATTERN.fullmatch(attribute) + if method: + result["method"].append({ + "attribute": attribute, + "reason": method.group("reason"), + "operating_systems": method.group("operating_systems"), + "operating_system_values": operating_system_values( + method.group("operating_systems") + ), + }) + continue + data = DATA_QUARANTINE_PATTERN.fullmatch(attribute) + if data: + raw_data = data.group("data").strip() + result["data"].append({ + "attribute": attribute, + "reason": data.group("reason"), + "operating_systems": data.group("operating_systems").strip(), + "operating_system_values": operating_system_values( + data.group("operating_systems") + ), + "data": raw_data, + "values": data_values(raw_data), + }) + continue + inline = INLINE_DATA_PATTERN.fullmatch(attribute) + if inline: + raw_data = inline.group("data").strip() + result["inline"].append({ + "attribute": attribute, + "data": raw_data, + "values": data_values(raw_data), + }) + return result + + +def split_test_name(test_name): + opening = test_name.find("(") + if opening < 0: + return test_name, None + if not test_name.endswith(")"): + return test_name[:opening], None + return test_name[:opening], tuple( + normalize_data_value(item) + for item in split_arguments(test_name[opening + 1:-1]) + ) + + def parse_utc(value): if not value: return None @@ -179,7 +425,7 @@ def attribute_block(lines, declaration_line): bracket_depth = 0 index = declaration_line - 1 while index >= 0: - stripped = lines[index].strip() + stripped = sanitize_csharp(lines[index]).strip() if not stripped or stripped.startswith("//"): if collected: collected.append(lines[index]) @@ -327,15 +573,47 @@ def build_source_index(root): method = method_match.group("method") method_line = clean.count("\n", 0, position) attributes = attribute_block(lines, method_line) + parsed_attributes = quarantine_attributes(attributes) method_index.setdefault(method, []).append({ "path": relative_path, "type": type_name, "method": method, "method_line": method_line + 1, "project_root": project_root, - "method_quarantined": has_quarantine_attribute(attributes), + "method_quarantined": bool(parsed_attributes["method"]), + "method_operating_systems": ( + parsed_attributes["method"][0][ + "operating_system_values" + ] + if len(parsed_attributes["method"]) == 1 + else None + ), "quarantine_attribute": ( - attributes if has_quarantine_attribute(attributes) else "" + "\n".join( + item["attribute"] + for item in parsed_attributes["method"] + ) + ), + "data_quarantines": parsed_attributes["data"], + "inline_data": parsed_attributes["inline"], + "has_row_data": re.search( + r"\b(?:InlineData|QuarantinedTestData)(?:Attribute)?\s*\(", + sanitize_csharp(attributes), + ) is not None, + "conditional_theory": any( + CONDITIONAL_THEORY_PATTERN.fullmatch(attribute) + for attribute in logical_attributes(attributes) + ), + "has_unproven_data_provider": any( + not any(pattern.fullmatch(attribute) for pattern in ( + CONDITIONAL_THEORY_PATTERN, + INLINE_DATA_PATTERN, + DATA_QUARANTINE_PATTERN, + METHOD_QUARANTINE_PATTERN, + TRAIT_PATTERN, + NON_DATA_CONDITION_PATTERN, + )) + for attribute in logical_attributes(attributes) ), "type_quarantined": type_quarantines.get( (project_root, type_name), @@ -437,8 +715,9 @@ def resolve_base_type_name(type_names, current_type, base_name): def resolve_source(root, test_name, source_index=None): - method = test_name.rsplit(".", 1)[-1] - expected_type = normalize_type_name(test_name.rsplit(".", 1)[0]) + source_test_name, test_arguments = split_test_name(test_name) + method = source_test_name.rsplit(".", 1)[-1] + expected_type = normalize_type_name(source_test_name.rsplit(".", 1)[0]) source_index = source_index or build_source_index(root) expected_runner = logical_type(source_index, expected_type) if expected_runner["status"] != "exact": @@ -507,6 +786,40 @@ def resolve_source(root, test_name, source_index=None): result = dict(matches[0]) result["status"] = "exact" + result["test_arguments"] = test_arguments + result["data_quarantine"] = None + result["matching_inline_data"] = None + if ( + result["conditional_theory"] + and result["has_row_data"] + and result["has_unproven_data_provider"] + ): + return {"status": "ambiguous", "matches": matches[:5]} + if test_arguments is not None: + data_matches = [ + item for item in result["data_quarantines"] + if item["values"] == test_arguments + ] + inline_matches = [ + item for item in result["inline_data"] + if result["conditional_theory"] and item["values"] == test_arguments + ] + if len(data_matches) + len(inline_matches) > 1: + return { + "status": "ambiguous", + "matches": matches[:5], + } + if data_matches: + result["data_quarantine"] = data_matches[0] + if inline_matches: + result["matching_inline_data"] = inline_matches[0] + if ( + result["conditional_theory"] + and result["has_row_data"] + and result["data_quarantine"] is None + and result["matching_inline_data"] is None + ): + return {"status": "unmatched-data-row", "matches": matches[:5]} result["declaring_type"] = result["type"] result["type"] = expected_type result["type_quarantined"] = ( @@ -762,6 +1075,8 @@ def historical_project_source_index( methods = {} method_quarantines = set() method_quarantine_issues = {} + data_quarantines = set() + data_quarantine_issues = {} for relative_path in paths: if not relative_path.endswith(".cs"): continue @@ -832,11 +1147,15 @@ def historical_project_source_index( methods[key] = methods.get(key, 0) + 1 declaration_line = clean.count("\n", 0, match.start()) attributes = attribute_block(lines, declaration_line) - if has_quarantine_attribute(attributes): + parsed_attributes = quarantine_attributes(attributes) + if parsed_attributes["method"]: method_quarantines.add(key) issues = { int(match.group("issue")) - for match in QUARANTINE_ISSUE_PATTERN.finditer(attributes) + for item in parsed_attributes["method"] + for match in QUARANTINE_ISSUE_PATTERN.finditer( + item["attribute"] + ) } issue = next(iter(issues)) if len(issues) == 1 else None if ( @@ -846,6 +1165,17 @@ def historical_project_source_index( method_quarantine_issues[key] = None else: method_quarantine_issues[key] = issue + for data_attribute in parsed_attributes["data"]: + data_key = (*key, data_attribute["values"]) + data_quarantines.add(data_key) + issue = quarantine_issue(data_attribute["attribute"]) + if ( + data_key in data_quarantine_issues + and data_quarantine_issues[data_key] != issue + ): + data_quarantine_issues[data_key] = None + else: + data_quarantine_issues[data_key] = issue result = { "status": "exact", @@ -856,6 +1186,8 @@ def historical_project_source_index( "methods": methods, "method_quarantines": method_quarantines, "method_quarantine_issues": method_quarantine_issues, + "data_quarantines": data_quarantines, + "data_quarantine_issues": data_quarantine_issues, } source_cache[cache_key] = result return result @@ -879,7 +1211,8 @@ def historical_test_source( if source_index["status"] != "exact": return {"status": "ambiguous"} - runner_type, method = test_name.rsplit(".", 1) + source_test_name, _ = split_test_name(test_name) + runner_type, method = source_test_name.rsplit(".", 1) current_type = normalize_type_name(runner_type) if current_type not in source_index["types"]: return {"status": "missing"} @@ -1502,6 +1835,7 @@ def method_quarantine_transitions( if history.returncode == 0 else None ), "methods": {}, + "data": {}, } cached_history = history_cache[relative_project_root] target = (type_name, method) @@ -1569,12 +1903,115 @@ def method_quarantine_transitions( return events +def data_quarantine_transitions( + root, + project_root, + type_name, + method, + data_values, + history_ref, + history_cache, + source_cache, + content_cache, +): + root = pathlib.Path(root) + relative_project_root = str( + pathlib.Path(project_root).relative_to(root) + ).replace(os.sep, "/") + if relative_project_root not in history_cache: + history = git_result( + root, + "log", + "--first-parent", + "--format=%H%x09%P%x09%cI", + "-G", + QUARANTINE, + history_ref, + "--", + relative_project_root, + ) + history_cache[relative_project_root] = { + "lines": ( + history.stdout.splitlines() + if history.returncode == 0 else None + ), + "methods": {}, + "data": {}, + } + cached_history = history_cache[relative_project_root] + target = (type_name, method, data_values) + if target in cached_history["data"]: + return cached_history["data"][target] + if cached_history["lines"] is None: + return [{"status": "ambiguous"}] + + events = [] + for line in cached_history["lines"]: + sha, parent_values, timestamp = line.split("\t", 2) + parent = parent_values.split()[0] if parent_values else None + current_index = historical_project_source_index( + root, + relative_project_root, + sha, + source_cache, + content_cache, + quarantine_only=True, + ) + if parent is None: + parent_index = { + "status": "exact", + "data_quarantines": set(), + "data_quarantine_issues": {}, + } + else: + parent_index = historical_project_source_index( + root, + relative_project_root, + parent, + source_cache, + content_cache, + quarantine_only=True, + ) + if ( + current_index["status"] != "exact" + or parent_index["status"] != "exact" + ): + result = [{ + "status": "ambiguous", + "commit": sha, + "utc": timestamp, + }] + cached_history["data"][target] = result + return result + current = target in current_index["data_quarantines"] + previous = target in parent_index["data_quarantines"] + if current == previous: + continue + events.append({ + "status": "added" if current else "removed", + "commit": sha, + "utc": timestamp, + "scope": "data", + "type": type_name, + "method": method, + "data": data_values, + "issue": ( + current_index["data_quarantine_issues"].get(target) + if current + else parent_index["data_quarantine_issues"].get(target) + ), + }) + cached_history["data"][target] = events + return events + + def target_quarantine_transitions( root, project_root, scope, type_name, method, + data_values, history_ref, history_cache, source_cache, @@ -1619,6 +2056,10 @@ def target_state(source, assembly, commit): ("method", item[0], item[1]) for item in source["method_quarantines"] ) + proxies.update( + ("data", item[0], item[1], item[2]) + for item in source["data_quarantines"] + ) return assembly["quarantined"], frozenset(proxies) type_quarantined = type_name in source["type_quarantines"] if scope == "type": @@ -1627,6 +2068,11 @@ def target_state(source, assembly, commit): for item in source["method_quarantines"] if item[0] == type_name } + proxies.update( + ("data", item[0], item[1], item[2]) + for item in source["data_quarantines"] + if item[0] == type_name + ) if proxies or type_quarantined: if assembly["quarantined"]: proxies.add(("assembly",)) @@ -1647,10 +2093,17 @@ def target_state(source, assembly, commit): type_name, method, ) in source["method_quarantines"] + data_quarantined = ( + type_name, + method, + data_values, + ) in source["data_quarantines"] if scope == "data" else False proxies = set() if type_quarantined: proxies.add(("type", type_name)) - if method_quarantined or type_quarantined: + if scope == "data" and method_quarantined: + proxies.add(("method", type_name, method)) + if data_quarantined or method_quarantined or type_quarantined: if assembly["quarantined"]: proxies.add(("assembly",)) elif assembly["quarantined"]: @@ -1665,7 +2118,10 @@ def target_state(source, assembly, commit): return None if (type_name, method) in full_source["methods"]: proxies.add(("assembly",)) - return method_quarantined, frozenset(proxies) + return ( + data_quarantined if scope == "data" else method_quarantined, + frozenset(proxies), + ) events = [] for line in history: @@ -1692,6 +2148,7 @@ def target_state(source, assembly, commit): "methods": {}, "type_quarantines": set(), "method_quarantines": set(), + "data_quarantines": set(), } parent_assembly = { "status": "exact", @@ -1740,7 +2197,7 @@ def target_state(source, assembly, commit): and ( type_name not in parent_source["types"] or ( - scope == "method" + scope in ("method", "data") and (type_name, method) not in parent_source["methods"] ) ) @@ -1766,7 +2223,7 @@ def target_state(source, assembly, commit): full_parent_source["status"] == "exact" and type_name in full_parent_source["types"] and ( - scope != "method" + scope not in ("method", "data") or (type_name, method) in full_parent_source["methods"] ) ) @@ -1909,27 +2366,53 @@ def current_quarantine_targets(source_index): seen = set() for declarations in source_index["methods"].values(): for declaration in declarations: - if not declaration["method_quarantined"]: - continue - key = ( - "method", - declaration["path"], - declaration["type"], - declaration["method"], - ) - if key in seen: - continue - seen.add(key) - targets.append({ - "scope": "method", - "path": declaration["path"], - "project_root": declaration["project_root"], - "type": declaration["type"], - "method": declaration["method"], - "reference": quarantine_reference( - declaration["quarantine_attribute"] - ), - }) + if declaration["method_quarantined"]: + key = ( + "method", + declaration["path"], + declaration["type"], + declaration["method"], + ) + if key not in seen: + seen.add(key) + targets.append({ + "scope": "method", + "path": declaration["path"], + "project_root": declaration["project_root"], + "type": declaration["type"], + "method": declaration["method"], + "reference": quarantine_reference( + declaration["quarantine_attribute"] + ), + "operating_systems": list( + declaration["method_operating_systems"] or () + ), + }) + for data_attribute in declaration["data_quarantines"]: + data_key = ( + "data", + declaration["path"], + declaration["type"], + declaration["method"], + data_attribute["values"], + ) + if data_key in seen: + continue + seen.add(data_key) + targets.append({ + "scope": "data", + "path": declaration["path"], + "project_root": declaration["project_root"], + "type": declaration["type"], + "method": declaration["method"], + "data": data_attribute["data"], + "operating_systems": list( + data_attribute["operating_system_values"] or () + ), + "reference": quarantine_reference( + data_attribute["attribute"] + ), + }) for declarations in source_index["types"].values(): for declaration in declarations: if not declaration["quarantined"]: @@ -1985,74 +2468,145 @@ def collect_requarantine_history( for declarations in source_index["methods"].values(): for declaration in declarations: - if not declaration["method_quarantined"]: - continue - key = ( - "method", - declaration["path"], - declaration["type"], - declaration["method"], - ) - if key in seen: - continue - seen.add(key) - issue = quarantine_issue(declaration["quarantine_attribute"]) - history_complete = project_history_is_complete( - root, - declaration["project_root"], - [declaration["path"]], - history_ref, - project_history_cache, - ) - events = ( - target_quarantine_transitions( - root, - declaration["project_root"], + if declaration["method_quarantined"]: + key = ( "method", + declaration["path"], declaration["type"], declaration["method"], - history_ref, - target_history_cache, - source_cache, - content_cache, - assembly_state_cache, ) - if history_complete else [{"status": "ambiguous"}] - ) - if ( - target_identity_rename_detected( - root, + if key not in seen: + seen.add(key) + issue = quarantine_issue( + declaration["quarantine_attribute"] + ) + history_complete = project_history_is_complete( + root, + declaration["project_root"], + [declaration["path"]], + history_ref, + project_history_cache, + ) + events = ( + target_quarantine_transitions( + root, + declaration["project_root"], + "method", + declaration["type"], + declaration["method"], + None, + history_ref, + target_history_cache, + source_cache, + content_cache, + assembly_state_cache, + ) + if history_complete else [{"status": "ambiguous"}] + ) + if ( + target_identity_rename_detected( + root, + declaration["path"], + declaration["type"], + declaration["method"], + history_ref, + ) + or target_identity_rename_detected( + root, + declaration["path"], + declaration["type"], + None, + history_ref, + ) + or namespace_rename_detected( + root, + declaration["path"], + declaration["type"], + history_ref, + ) + ): + events = [{"status": "ambiguous"}] + targets.append({ + "scope": "method", + "path": declaration["path"], + "type": declaration["type"], + "method": declaration["method"], + "issue": issue, + "status": ( + classify_current_quarantine_history(events) + if issue is not None else "ambiguous" + ), + }) + for data_attribute in declaration["data_quarantines"]: + key = ( + "data", declaration["path"], declaration["type"], declaration["method"], - history_ref, + data_attribute["values"], ) - or target_identity_rename_detected( + if key in seen: + continue + seen.add(key) + issue = quarantine_issue(data_attribute["attribute"]) + history_complete = project_history_is_complete( root, - declaration["path"], - declaration["type"], - None, + declaration["project_root"], + [declaration["path"]], history_ref, + project_history_cache, ) - or namespace_rename_detected( - root, - declaration["path"], - declaration["type"], - history_ref, + events = ( + target_quarantine_transitions( + root, + declaration["project_root"], + "data", + declaration["type"], + declaration["method"], + data_attribute["values"], + history_ref, + target_history_cache, + source_cache, + content_cache, + assembly_state_cache, + ) + if history_complete else [{"status": "ambiguous"}] ) - ): - events = [{"status": "ambiguous"}] - targets.append({ - "scope": "method", - "path": declaration["path"], - "type": declaration["type"], - "method": declaration["method"], - "issue": issue, - "status": ( - classify_current_quarantine_history(events) - if issue is not None else "ambiguous" - ), - }) + if ( + target_identity_rename_detected( + root, + declaration["path"], + declaration["type"], + declaration["method"], + history_ref, + ) + or target_identity_rename_detected( + root, + declaration["path"], + declaration["type"], + None, + history_ref, + ) + or namespace_rename_detected( + root, + declaration["path"], + declaration["type"], + history_ref, + ) + ): + events = [{"status": "ambiguous"}] + targets.append({ + "scope": "data", + "path": declaration["path"], + "type": declaration["type"], + "method": declaration["method"], + "data": data_attribute["data"], + "issue": issue, + "status": ( + classify_current_quarantine_history(events) + if issue is not None else "ambiguous" + ), + }) for declarations in source_index["types"].values(): for declaration in declarations: @@ -2077,6 +2631,7 @@ def collect_requarantine_history( "type", declaration["type"], None, + None, history_ref, target_history_cache, source_cache, @@ -2135,6 +2690,7 @@ def collect_requarantine_history( "assembly", None, None, + None, history_ref, target_history_cache, source_cache, @@ -2173,6 +2729,7 @@ def collect_requarantine_history( item["scope"], item.get("type") or "", item.get("method") or "", + item.get("data") or "", item.get("issue") or 0, ), ), @@ -2437,15 +2994,42 @@ def source_c_failure_records(source_c): if not isinstance(build_id, int): continue for match in SOURCE_C_FAILURE_PATTERN.finditer(item.get("fail_blocks", "")): - test_name = match.group("test").split("(", 1)[0].strip() + test_name = match.group("test").strip() if not test_name or test_name.endswith(WORK_ITEM_SUFFIX): continue record = records.setdefault(test_name, {"builds": []}) if build_id not in record["builds"]: record["builds"].append(build_id) + record.setdefault("queues", {}).setdefault(str(build_id), []).append( + item.get("queue") + ) return records +def quarantine_operating_systems(record_a, record_b, record_c, builds): + by_build = {} + for record in (record_a, record_b, record_c): + if not record: + continue + for build in record.get("builds", []): + queues = record.get("queues", {}).get(str(build)) + operating_systems = { + operating_system_from_queue(queue) for queue in (queues or [None]) + } + by_build.setdefault(build, set()).update(operating_systems) + + resolved = set() + for build in builds: + operating_systems = by_build.get(build) + if operating_systems is None or None in operating_systems: + return list(OPERATING_SYSTEMS) + resolved.update(operating_systems) + return [ + operating_system for operating_system in OPERATING_SYSTEMS + if operating_system in resolved + ] + + def collect( part1, part1_bytes, @@ -2515,6 +3099,7 @@ def collect( "eligible_failure_builds": [], "case_b_eligible": False, "case_b_issue": None, + "quarantine_operating_systems": None, "evidence": None, "reasons": reasons, } @@ -2574,6 +3159,7 @@ def collect( quarantined = ( source["method_quarantined"] + or source["data_quarantine"] is not None or source["type_quarantined"] or source["assembly_quarantined"] ) @@ -2595,6 +3181,25 @@ def collect( historical_content_cache, ) transitions.append(events[0] if events else {"status": "none"}) + data_target = ( + source["data_quarantine"] + or source["matching_inline_data"] + ) + if data_target is not None: + events = data_quarantine_transitions( + root, + source["project_root"], + source["declaring_type"], + source["method"], + data_target["values"], + history_ref, + method_history_cache, + historical_source_cache, + historical_content_cache, + ) + transitions.append( + events[0] if events else {"status": "none"} + ) transitions.append(type_quarantine_transition( root, source["assembly_project_root"], @@ -2759,6 +3364,12 @@ def collect( included.add(build_id) receipt["eligible_failure_builds"] = sorted(included) + receipt["quarantine_operating_systems"] = quarantine_operating_systems( + record_a, + record_b, + record_c, + included, + ) if case_b: receipt["status"] = "ineligible" if included: diff --git a/.github/workflows/scripts/test-quarantine/test_collect_case_a_eligibility.py b/.github/workflows/scripts/test-quarantine/test_collect_case_a_eligibility.py index 025f60590bd2..6368ec5701fd 100644 --- a/.github/workflows/scripts/test-quarantine/test_collect_case_a_eligibility.py +++ b/.github/workflows/scripts/test-quarantine/test_collect_case_a_eligibility.py @@ -1,6 +1,8 @@ #!/usr/bin/env python3 +import ast import datetime +import http.client import importlib.util import io import json @@ -9,6 +11,7 @@ import subprocess import tempfile import textwrap +import urllib.parse from unittest import mock @@ -18,6 +21,10 @@ SPEC.loader.exec_module(MODULE) TEST_NAME = "Microsoft.AspNetCore.Tests.SampleTests.ReturnsExpectedResponse" +THEORY_TEST_NAME = ( + "Microsoft.AspNetCore.Tests.SampleTests." + "ReturnsExpectedResponse(protocol: Http3)" +) TEST_PATH = "src/Sample.Tests/SampleTests.cs" DERIVED_TEST_NAME = ( "Microsoft.AspNetCore.Server.Tests." @@ -67,6 +74,30 @@ def source(quarantine=""): """ +def theory_source(*data_attributes, method_quarantine=""): + attributes = "\n".join( + f" {attribute}" + for attribute in data_attributes + ) + if method_quarantine: + attributes = ( + f" {method_quarantine}\n{attributes}" + if attributes + else f" {method_quarantine}" + ) + return f"""namespace Microsoft.AspNetCore.Tests; + +public class SampleTests +{{ + [ConditionalTheory] +{attributes} + public void ReturnsExpectedResponse(HttpProtocols protocol) + {{ + }} +}} +""" + + def class_quarantined_source(): return """namespace Microsoft.AspNetCore.Tests; @@ -146,7 +177,8 @@ def evidence(regression=False, builds=(101, 102), test_name=TEST_NAME): "evidence_build": builds[-1], "run_id": 2001, "result_id": 3001, - "leg": "Linux_Test", + "leg": "Sample.Tests--net11.0", + "queues": {str(build): ["ubuntu.2404.amd64.open"] for build in builds}, "error": "stable-marker-123", "stack": f"at {type_name.rsplit('.', 1)[-1]}.{method_name}()", "is_consistent_regression": regression, @@ -229,6 +261,672 @@ def assert_already_quarantined(result): assert result["current_quarantine_state"] == "quarantined", result +def test_row_targets_require_conditional_theory(): + for attribute, row_target in [ + ("[Theory]", False), + ("[TheoryAttribute]", False), + ("[ConditionalTheory]", True), + ("[ConditionalTheory(Skip = \"temporary\")]", True), + ("[Microsoft.AspNetCore.InternalTesting.ConditionalTheoryAttribute]", True), + ("[global::Microsoft.AspNetCore.InternalTesting.ConditionalTheoryAttribute()]", True), + ("[ConditionalTheory, Trait(\"Category\", \"Sample\")]", True), + ]: + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text( + theory_source("[InlineData(HttpProtocols.Http3)]").replace( + "[ConditionalTheory]", attribute + ), + encoding="utf-8", + ) + commit(root, "Add theory", "2026-08-01T00:00:00Z") + result = collect_result( + root, evidence(test_name=THEORY_TEST_NAME), + test_name=THEORY_TEST_NAME, + ) + assert result["status"] == "eligible", (attribute, result) + assert bool(result["source_resolution"]["matching_inline_data"]) == ( + row_target + ), (attribute, result) + print(f"PASS row target: {attribute}") + + for attribute in ("Theory", "ConditionalTheory"): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text( + theory_source( + "[InlineData(HttpProtocols.Http3)]", + "[InlineData(HttpProtocols.Http3)]", + ).replace("[ConditionalTheory]", f"[{attribute}]"), + encoding="utf-8", + ) + resolved = MODULE.resolve_source(root, THEORY_TEST_NAME) + assert resolved["status"] == ( + "exact" if attribute == "Theory" else "ambiguous" + ), (attribute, resolved) + + +def part1_functions(): + workflow = (SCRIPT.parents[2] / "test-quarantine.md").read_text(encoding="utf-8") + step = workflow.split(" - name: Aggregate Part 1 failures\n", 1)[1] + script = textwrap.dedent( + step.split(" python3 << 'SCRIPT'\n", 1)[1].split( + "\n SCRIPT", 1 + )[0] + ) + tree = ast.parse(script) + tree.body = [ + node for node in tree.body + if isinstance(node, ast.FunctionDef) + ] + namespace = { + "json": json, "WI_SUFFIX": ".WorkItemExecution", "OCC_CAP": 2, + "OS_DETAIL_BUDGET": 1000, "ERROR_CAP": 1200, "STACK_CAP": 900, + "VSTMR": "https://example.invalid", "scrub_secrets": lambda text: text, + "HELIX": "https://helix.dot.net/api/2019-06-17", + "urllib": urllib, "http": http, "sys": mock.Mock(), + } + exec(compile(tree, "test-quarantine.md", "exec"), namespace) + return namespace + + +def test_workflow_platform_evidence(): + namespace = part1_functions() + linux = "ubuntu.2404.amd64.open" + windows = "windows.amd64.vs2026.open" + scenarios = [ + ("two Linux builds", {101: [linux], 102: [linux]}, ["Linux"]), + ("three Linux builds", {101: [linux], 102: [linux], 103: [linux]}, ["Linux"]), + ("mixed builds", {101: [linux], 102: [windows]}, ["Linux", "Windows"]), + ("mixed same build", {101: [linux, windows], 102: [linux]}, ["Linux", "Windows"]), + ("unknown second queue", {101: [linux, ""], 102: [linux]}, ["Linux", "MacOSX", "Windows"]), + ("detail unavailable", {101: [linux], 102: [None]}, ["Linux", "MacOSX", "Windows"]), + ("missing identity", {101: [linux], 102: ["missing-id"]}, ["Linux", "MacOSX", "Windows"]), + ("missing first identity", {101: ["missing-id"], 102: [linux]}, ["Linux", "MacOSX", "Windows"]), + ("queue unavailable", {101: [linux], 102: ["fetch-error"]}, ["Linux", "MacOSX", "Windows"]), + ("unsupported queue", {101: [linux], 102: ["unknown.amd64.open"]}, ["Linux", "MacOSX", "Windows"]), + ] + for scenario, builds, expected in scenarios: + results = {} + details = {} + for build, legs in builds.items(): + results[build] = [] + for index, leg in enumerate(legs): + result_id = build * 10 + index + results[build].append({ + "automatedTestName": TEST_NAME, + "runId": build, "id": result_id if leg != "missing-id" else None, + }) + details[result_id] = leg + results[build].append(dict(results[build][0])) + + def fetch(url): + if "/jobs/" in url: + result_id = int(url.rsplit("-", 1)[1]) + if details[result_id] == "fetch-error": + raise OSError("fixture queue unavailable") + return {"QueueId": details[result_id]}, {} + result_id = int(url.split("/results/")[1].split("?")[0]) + leg = details[result_id] + if leg is None: + raise OSError("fixture detail unavailable") + return {"comment": json.dumps({ + "HelixJobId": f"job-{result_id}", "HelixWorkItemName": "Sample.Tests--net11.0", + })}, {} + + namespace["failed_results"] = lambda build: iter(results[build]) + namespace["fetch"] = mock.Mock(side_effect=fetch) + aggregated = namespace["enrich"](namespace["aggregate"](list(builds)), {}) + actual = aggregated[TEST_NAME] + assert actual["count"] == len(builds), (scenario, actual) + assert namespace["fetch"].call_count == sum( + leg != "missing-id" for leg in details.values() + ) + sum( + leg not in (None, "missing-id") for leg in details.values() + ), (scenario, namespace["fetch"].call_count) + if scenario == "missing first identity": + assert actual["evidence_build"] == 102, actual + for source in ("source_a", "source_b"): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + commit(root, "Add test", "2026-08-01T00:00:00Z") + data = evidence(builds=tuple(builds)) + data["source_a"] = {} + data[source] = {TEST_NAME: { + **actual, "is_consistent_regression": False, + }} + if source == "source_b": + for metadata in data["builds"].values(): + metadata["pr"] = 42 + result = collect_result(root, data) + assert result["status"] == "eligible", (scenario, source, result) + assert result["quarantine_operating_systems"] == [ + f"OperatingSystems.{name}" for name in expected + ], (scenario, source, result) + print(f"PASS platform evidence: {scenario}") + + namespace["OS_DETAIL_BUDGET"] = 0 + namespace["failed_results"] = lambda build: iter([{ + "automatedTestName": TEST_NAME, "runId": build, "id": build, + }]) + namespace["fetch"] = mock.Mock(side_effect=lambda url: ( + {"QueueId": linux} if "/jobs/" in url else + {"comment": json.dumps({ + "HelixJobId": "job", "HelixWorkItemName": "Sample.Tests--net11.0", + })}, {} + )) + actual = namespace["enrich"](namespace["aggregate"]([101, 102]), {})[TEST_NAME] + assert actual["queues"] == {"101": [linux], "102": [None]}, actual + assert actual["detail_note"] == "platform evidence detail budget exhausted" + assert namespace["fetch"].call_count == 2 + assert MODULE.quarantine_operating_systems(actual, None, None, [101, 102]) == list( + MODULE.OPERATING_SYSTEMS + ) + assert MODULE.quarantine_operating_systems(actual, None, None, [101]) == [ + "OperatingSystems.Linux", + ] + + namespace["failed_results"] = lambda build: iter([{ + "automatedTestName": "Sample.WorkItemExecution", "runId": build, "id": build, + }]) + namespace["fetch"] = mock.Mock(return_value=({ + "comment": json.dumps({"HelixJobId": "job", "HelixWorkItemName": "Sample.Tests--net11.0"}), + }, {})) + work_item = namespace["enrich"](namespace["aggregate"]([101, 102, 103]), {})[ + "Sample.WorkItemExecution" + ] + assert work_item["count"] == 3, work_item + assert len(work_item["probes"]) == 2, work_item + assert namespace["fetch"].call_count == 2 + + +def test_source_c_platform_evidence(): + for second_queue, expected in [ + ("windows.amd64.vs2026.open", ["OperatingSystems.Linux", "OperatingSystems.Windows"]), + ("unknown", list(MODULE.OPERATING_SYSTEMS)), + ]: + record = MODULE.source_c_failure_records([ + {"build": 101, "workitem": "batch_1--net11.0", "queue": queue, + "fail_blocks": f"{TEST_NAME} [FAIL]"} + for queue in ("ubuntu.2404.amd64.open", second_queue) + ])[TEST_NAME] + assert MODULE.quarantine_operating_systems(None, None, record, [101]) == expected + assert MODULE.quarantine_operating_systems( + {"builds": [101], "queues": {"101": [None]}}, + {"builds": [101], "queues": {"101": ["ubuntu.2404.amd64.open"]}}, + None, [101], + ) == list(MODULE.OPERATING_SYSTEMS) + for record in [ + {"builds": [101], "legs": {"101": ["Windows_Test"]}, + "evidence_build": 101, "leg": "Linux_Test"}, + MODULE.source_c_failure_records([{ + "build": 101, "job": "Linux", "workitem": "Windows", + "fail_blocks": f"{TEST_NAME} [FAIL]", + }])[TEST_NAME], + ]: + assert MODULE.quarantine_operating_systems(record, None, None, [101]) == list( + MODULE.OPERATING_SYSTEMS + ), record + + +def test_helix_queue_cache(): + for response, expected, warning in [ + ({"QueueId": "ubuntu.2404.amd64.open"}, "ubuntu.2404.amd64.open", False), + ({"QueueId": "unknown.amd64.open"}, "unknown.amd64.open", False), + ({}, None, True), + ({"QueueId": ""}, None, True), + ({"QueueId": ["ubuntu.2404.amd64.open"]}, None, True), + ([], None, True), + (OSError("fixture queue unavailable"), None, True), + (ValueError("fixture invalid JSON"), None, True), + (http.client.IncompleteRead(b"partial"), None, True), + (http.client.BadStatusLine("fixture invalid status"), None, True), + ]: + namespace = part1_functions() + namespace["fetch"] = mock.Mock( + side_effect=response if isinstance(response, Exception) else None, + return_value=(response, {}), + ) + cache = {} + for _ in range(2): + assert namespace["helix_queue"]("job-id", cache) == expected + namespace["fetch"].assert_called_once_with( + "https://helix.dot.net/api/2019-06-17/jobs/job-id" + ) + assert namespace["sys"].stderr.write.call_count == int(warning) + assert namespace["helix_queue"](None, cache) is None + assert namespace["fetch"].call_count == 1 + + +def test_production_queue_evidence( + queue="ubuntu.2404.amd64.open", + expected="Linux", + sources=("source_a", "source_b", "source_c"), +): + for source in sources: + namespace = part1_functions() + builds = [ + {"id": build, "startTime": f"2026-08-{day}T10:00:00Z", + "sourceVersion": str(build), "definition": {"id": 83}, + "sourceBranch": "refs/pull/42/merge" if source == "source_b" else "refs/heads/main"} + for build, day in ((101, 15), (102, 16)) + ] + workitem = "batch_1--net11.0" + # Shape verified against Helix job 1c70e76a-1985-4b12-943c-1cb84e4c9499. + job_id = "1c70e76a-1985-4b12-943c-1cb84e4c9499" + calls = [] + + def fetch(url): + calls.append(url) + if url == f"{namespace['HELIX']}/jobs/{job_id}": + return {"Name": job_id, "QueueId": queue}, {} + assert "/testresults/runs/" in url, url + result_job = ( + "earlier-job-with-no-fail-blocks" + if source == "source_c" and "/runs/102/" in url + else job_id + ) + return {"comment": json.dumps({ + "HelixJobId": result_job, "HelixWorkItemName": workitem, + })}, {} + + namespace.update({ + "DEFS": [83], + "os": mock.Mock(environ={ + "SOURCE_B_BUILD_IDS": "[101,102]" if source == "source_b" else "", + }), + "datetime": datetime, + "SOURCE_C_DOWNLOAD_BUDGET": 10000, + "SOURCE_C_GLOBAL_CAP": 10000, "WORKITEM_CAP": 10000, + "list_failed_builds": lambda *_args, **_kwargs: ( + [] if source == "source_b" else builds + ), + "builds_by_ids": lambda _ids: builds, + "list_completed_builds": lambda *_args, **_kwargs: [], + "mark_intermittency": lambda agg, *_args: [ + entry.update(is_consistent_regression=False) for entry in agg.values() + ], + "failed_results": lambda build: iter([{ + "automatedTestName": ( + workitem + ".WorkItemExecution" if source == "source_c" else TEST_NAME + ), + "runId": build, "id": build, + }]), + "fetch": fetch, + "helix_console_blocks": lambda job, _wi: ( + [f"{TEST_NAME} [FAIL]"] if job == job_id else [], 100 + ), + "emit": lambda out: json.dumps(out), + }) + data = json.loads(namespace["main"]()) + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + initialize_repository(root) + commit(root, "Add test", "2026-08-01T00:00:00Z") + result = collect_result(root, data) + assert result["eligible_failure_builds"], result + assert result["status"] == ( + "ineligible" if source == "source_c" else "eligible" + ), result + if source == "source_c": + assert result["eligible_failure_builds"] == [101], result + assert result["quarantine_operating_systems"] == [ + f"OperatingSystems.{expected}", + ], (queue, source, result) + assert calls.count(f"{namespace['HELIX']}/jobs/{job_id}") == 1, calls + print(f"PASS production queue: {source}, {queue}") + + +def test_theory_data_quarantine_support(): + assert MODULE.split_test_name(THEORY_TEST_NAME) == ( + TEST_NAME, + ("Http3",), + ) + assert MODULE.data_values( + 'HttpProtocols.Http3, 42, "value,with,commas"' + ) == ( + "Http3", + "42", + '"value,with,commas"', + ) + multiline_attributes = MODULE.quarantine_attributes( + """[QuarantinedTest( + "https://github.com/dotnet/aspnetcore/issues/1", + OperatingSystems.Linux)] +[QuarantinedTestData( + "https://github.com/dotnet/aspnetcore/issues/1", + OperatingSystems.Linux, + HttpProtocols.Http3)] +[InlineData( + HttpProtocols.Http2)]""" + ) + assert len(multiline_attributes["method"]) == 1, multiline_attributes + assert multiline_attributes["data"][0]["values"] == ( + "Http3", + ), multiline_attributes + assert multiline_attributes["inline"][0]["values"] == ( + "Http2", + ), multiline_attributes + combined_attributes = MODULE.quarantine_attributes( + """[Fact, QuarantinedTest( + "https://github.com/dotnet/aspnetcore/issues/1", + OperatingSystems.Linux)] +[ConditionalTheory, QuarantinedTestData( + "https://github.com/dotnet/aspnetcore/issues/1", + OperatingSystems.Linux, + HttpProtocols.Http3)] +[Theory, InlineData(HttpProtocols.Http2)]""" + ) + assert len(combined_attributes["method"]) == 1, combined_attributes + assert combined_attributes["data"][0]["values"] == ( + "Http3", + ), combined_attributes + assert combined_attributes["inline"][0]["values"] == ( + "Http2", + ), combined_attributes + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + project, file_path = initialize_repository(root) + file_path.write_text( + theory_source( + "[InlineData(HttpProtocols.Http2)]", + "[InlineData(HttpProtocols.Http3)]", + ), + encoding="utf-8", + ) + commit(root, "Add theory rows", "2026-08-01T00:00:00Z") + + resolved = MODULE.resolve_source(root, THEORY_TEST_NAME) + assert resolved["status"] == "exact", resolved + assert resolved["matching_inline_data"]["data"] == ( + "HttpProtocols.Http3" + ), resolved + assert resolved["data_quarantine"] is None, resolved + + eligible = collect_result( + root, + evidence(test_name=THEORY_TEST_NAME), + test_name=THEORY_TEST_NAME, + ) + assert eligible["status"] == "eligible", eligible + assert eligible["originating_case"] == "case-a", eligible + assert eligible["source_resolution"]["matching_inline_data"][ + "data" + ] == "HttpProtocols.Http3", eligible + + file_path.write_text( + theory_source( + "[InlineData(HttpProtocols.Http2)]", + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]', + ), + encoding="utf-8", + ) + commit(root, "Quarantine one theory row", "2026-08-02T00:00:00Z") + row_quarantined = collect_result( + root, + evidence(test_name=THEORY_TEST_NAME), + test_name=THEORY_TEST_NAME, + ) + assert_already_quarantined(row_quarantined) + assert row_quarantined["source_resolution"]["data_quarantine"][ + "data" + ] == "HttpProtocols.Http3", row_quarantined + + file_path.write_text( + theory_source( + "[InlineData(HttpProtocols.Http2)]", + "[InlineData(HttpProtocols.Http3)]", + ), + encoding="utf-8", + ) + commit(root, "Unquarantine one theory row", "2026-08-05T00:00:00Z") + removal_commit = run_output(root, "git", "rev-parse", "HEAD") + row_case_b = collect_result( + root, + evidence(test_name=THEORY_TEST_NAME), + test_name=THEORY_TEST_NAME, + ) + assert_case_b(row_case_b, removal_commit) + + file_path.write_text( + theory_source( + "[InlineData(HttpProtocols.Http2)]", + "[InlineData(HttpProtocols.Http3)]", + method_quarantine=( + '[QuarantinedTest(' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + "OperatingSystems.Linux)]" + ), + ), + encoding="utf-8", + ) + commit(root, "Quarantine theory method on Linux", "2026-08-06T00:00:00Z") + method_quarantined = collect_result( + root, + evidence(test_name=THEORY_TEST_NAME), + test_name=THEORY_TEST_NAME, + ) + assert_already_quarantined(method_quarantined) + assert method_quarantined["source_resolution"][ + "method_quarantined" + ], method_quarantined + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text( + theory_source( + "[InlineData(HttpProtocols.Http3)]", + "[InlineData(HttpProtocols.Http3)]", + ), + encoding="utf-8", + ) + commit(root, "Add ambiguous theory rows", "2026-08-01T00:00:00Z") + ambiguous = MODULE.resolve_source(root, THEORY_TEST_NAME) + assert ambiguous["status"] == "ambiguous", ambiguous + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text( + theory_source("[InlineData(HttpProtocols.Http3)]"), + encoding="utf-8", + ) + commit(root, "Add theory row", "2026-08-01T00:00:00Z") + file_path.write_text( + theory_source( + '[QuarantinedTestData(\n' + ' "https://github.com/dotnet/aspnetcore/issues/1",\n' + " OperatingSystems.Linux,\n" + " Microsoft.AspNetCore.Server.Kestrel.Core." + "HttpProtocols.Http3)]" + ), + encoding="utf-8", + ) + commit(root, "Quarantine theory row", "2026-08-02T00:00:00Z") + file_path.write_text( + theory_source("[InlineData(HttpProtocols.Http3)]"), + encoding="utf-8", + ) + commit(root, "Unquarantine theory row", "2026-08-03T00:00:00Z") + file_path.write_text( + theory_source( + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]' + ), + encoding="utf-8", + ) + commit(root, "Re-quarantine theory row", "2026-08-04T00:00:00Z") + history = MODULE.collect_requarantine_history(root, "HEAD") + assert history["targets"] == [{ + "scope": "data", + "path": TEST_PATH, + "type": "Microsoft.AspNetCore.Tests.SampleTests", + "method": "ReturnsExpectedResponse", + "data": "HttpProtocols.Http3", + "issue": 1, + "status": "re-quarantined", + }], history + + +def test_inline_literal_resolution(raw="true", rendered="True"): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text(theory_source(f"[InlineData({raw})]"), encoding="utf-8") + commit(root, "Add inline constant", "2026-08-01T00:00:00Z") + name = TEST_NAME + f"(protocol: {rendered})" + result = collect_result(root, evidence(test_name=name), test_name=name) + assert result["status"] == "eligible", result + row = result["source_resolution"]["matching_inline_data"] + assert row is not None and row["data"] == raw, result + print(f"PASS inline literal: {raw} -> {rendered}") + + +def test_unmatched_inline_row_fails_closed(cases=None): + for raw, name in cases or [ + ("nameof(HttpProtocols.Http3)", TEST_NAME + '(protocol: "Http3")'), + ("1.0f", TEST_NAME + "(protocol: 1)"), + ("HttpProtocols.Http3", TEST_NAME), + ("HttpProtocols.Http3", TEST_NAME + "(protocol: Unknown)"), + ("HttpProtocols.Http3", TEST_NAME + "(protocol: Http3"), + ]: + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text(theory_source(f"[InlineData({raw})]"), encoding="utf-8") + commit(root, "Add unmatched row", "2026-08-01T00:00:00Z") + result = collect_result(root, evidence(test_name=name), test_name=name) + assert result["status"] == "unproven", (raw, name, result) + assert "source-unmatched-data-row" in result["reasons"], result + + +def test_mixed_row_providers(provider='[MemberData(nameof(GetRows))]'): + for row in ( + "[InlineData(HttpProtocols.Http3)]", + '[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]', + ): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text(theory_source(row, provider), encoding="utf-8") + commit(root, "Add mixed row providers", "2026-08-01T00:00:00Z") + result = collect_result( + root, evidence(test_name=THEORY_TEST_NAME), test_name=THEORY_TEST_NAME + ) + assert result["status"] == "unproven", result + assert "source-ambiguous" in result["reasons"], result + print(f"PASS mixed row providers: {row}, {provider}") + + +def test_non_data_condition_rows(condition="[MsQuicSupported]"): + for quarantined in (False, True): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + row = ( + '[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]' + if quarantined else "[InlineData(HttpProtocols.Http3)]" + ) + file_path.write_text(theory_source(condition, row), encoding="utf-8") + commit(root, "Add conditioned row", "2026-08-01T00:00:00Z") + result = collect_result( + root, evidence(test_name=THEORY_TEST_NAME), test_name=THEORY_TEST_NAME + ) + if quarantined: + assert_already_quarantined(result) + else: + assert result["status"] == "eligible", result + assert result["source_resolution"]["matching_inline_data"] is not None, result + file_path.write_text( + theory_source(condition, row, "[MemberData(nameof(GetRows))]"), + encoding="utf-8", + ) + assert MODULE.resolve_source(root, THEORY_TEST_NAME)["status"] == "ambiguous" + print(f"PASS non-data condition: {condition}, quarantined={quarantined}") + + +def test_kestrel_condition_rows(): + kestrel_source = SCRIPT.parents[4] / ( + "src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs" + ) + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text(kestrel_source.read_text(encoding="utf-8"), encoding="utf-8") + for protocol, row_key in (("Http3", "data_quarantine"), ("Http2", "matching_inline_data")): + result = MODULE.resolve_source( + root, + "Interop.FunctionalTests.Http3.Http3RequestTests." + f"POST_ClientCancellationBidirectional_RequestAbortRaised(protocol: {protocol})", + ) + assert result["status"] == "exact", result + assert result[row_key]["data"] == f"HttpProtocols.{protocol}", result + print(f"PASS Kestrel source row: {protocol}") + + +def test_commented_row_attributes(): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + file_path.write_text( + theory_source("[InlineData(HttpProtocols.Http3)] // reason [detail]") + .replace("[ConditionalTheory]", "[ConditionalTheory] // condition"), + encoding="utf-8", + ) + commit(root, "Add commented row", "2026-08-01T00:00:00Z") + result = collect_result( + root, evidence(test_name=THEORY_TEST_NAME), test_name=THEORY_TEST_NAME + ) + assert result["status"] == "eligible", result + assert result["source_resolution"]["matching_inline_data"] is not None, result + quarantine = ( + '[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)] // reason' + ) + file_path.write_text(theory_source(quarantine), encoding="utf-8") + commit(root, "Quarantine commented row", "2026-08-02T00:00:00Z") + result = collect_result( + root, evidence(test_name=THEORY_TEST_NAME), test_name=THEORY_TEST_NAME + ) + assert_already_quarantined(result) + + +def test_renamed_row_history(cases=None): + for old, new in cases or [ + ("ReturnsExpectedResponse", "RenamedResponse"), + ("SampleTests", "RenamedTests"), + ("Microsoft.AspNetCore.Tests", "Microsoft.AspNetCore.Renamed"), + ]: + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, file_path = initialize_repository(root) + inline = theory_source("[InlineData(HttpProtocols.Http3)]") + quarantined = theory_source( + '[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]' + ) + for day, text in enumerate((inline, quarantined, inline), 1): + file_path.write_text(text, encoding="utf-8") + commit(root, "Change row quarantine", f"2026-08-0{day}T00:00:00Z") + file_path.write_text(inline.replace(old, new), encoding="utf-8") + commit(root, "Rename unquarantined row owner", "2026-08-04T00:00:00Z") + file_path.write_text(quarantined.replace(old, new), encoding="utf-8") + commit(root, "Re-quarantine renamed row", "2026-08-05T00:00:00Z") + history = MODULE.collect_requarantine_history(root, "HEAD") + assert len(history["targets"]) == 1, history + assert history["targets"][0]["status"] == "ambiguous", (old, history) + + def test_build_source_ancestry(): with tempfile.TemporaryDirectory() as directory: root = pathlib.Path(directory) @@ -2957,6 +3655,48 @@ def run_output(root, *args): def main(): + test_kestrel_condition_rows() + for condition in ( + "[MsQuicSupported]", + "[Microsoft.AspNetCore.InternalTesting.MsQuicSupportedAttribute()]", + "[global::Microsoft.AspNetCore.InternalTesting.OSSkipCondition(OperatingSystems.Windows)]", + "[FrameworkSkipCondition(RuntimeFrameworks.Mono)]", + ): + test_non_data_condition_rows(condition) + for provider in ( + '[MemberData(nameof(GetRows))]', + '[Xunit.MemberDataAttribute(nameof(GetRows))]', + '[ClassData(typeof(Rows))]', + '[CustomRows]', + '[InlineData(HttpProtocols.Http3)]', + '[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]', + ): + test_mixed_row_providers(provider) + for raw, rendered in [ + ("true", "True"), ("false", "False"), ("-1L", "-1"), + ("1UL", "1"), ("0x10", "16"), ("0b10", "2"), ("1_000L", "1000"), + ]: + test_inline_literal_resolution(raw, rendered) + test_unmatched_inline_row_fails_closed() + test_commented_row_attributes() + test_renamed_row_history() + test_helix_queue_cache() + for queue, expected in [ + ("ubuntu.2404.amd64.open", "Linux"), + ("azurelinux.3.amd64.open", "Linux"), + ("almalinux.10.amd64.open", "Linux"), + ("fedora.44.amd64.open", "Linux"), + ("alpine.323.amd64.open", "Linux"), + ("debian.13.arm64.open", "Linux"), + ("OSX.26.Arm64.Open", "MacOSX"), + ("Windows.Amd64.VS2026.Open", "Windows"), + ]: + test_production_queue_evidence(queue, expected) + test_row_targets_require_conditional_theory() + test_workflow_platform_evidence() + test_source_c_platform_evidence() + test_theory_data_quarantine_support() test_build_source_ancestry() test_history_cutoff_uses_first_parent_order() test_requarantine_history() @@ -2994,10 +3734,16 @@ def main(): assert eligible["status"] == "eligible", eligible assert eligible["originating_case"] == "case-a" assert eligible["eligible_failure_builds"] == [101, 102] + assert eligible["quarantine_operating_systems"] == [ + "OperatingSystems.Linux", + ], eligible one_failure = record(collect(root, evidence(builds=(101,)))) assert one_failure["status"] == "ineligible" assert "fewer-than-two-post-cutoff-failures" in one_failure["reasons"] + assert one_failure["quarantine_operating_systems"] == [ + "OperatingSystems.Linux", + ], one_failure regression = record(collect(root, evidence(regression=True))) assert regression["status"] == "ineligible" diff --git a/.github/workflows/scripts/test-quarantine/test_validate_pull_request_outputs.py b/.github/workflows/scripts/test-quarantine/test_validate_pull_request_outputs.py index 3972080655d4..bfa8b792adc6 100644 --- a/.github/workflows/scripts/test-quarantine/test_validate_pull_request_outputs.py +++ b/.github/workflows/scripts/test-quarantine/test_validate_pull_request_outputs.py @@ -57,6 +57,20 @@ def source(attribute=""): """ +def theory_source(data_attribute="[InlineData(HttpProtocols.Http3)]"): + return f"""namespace Microsoft.AspNetCore.Tests; + +public class SampleTests +{{ + [ConditionalTheory] + {data_attribute} + public void ReturnsExpectedResponse(HttpProtocols protocol) + {{ + }} +}} +""" + + def source_two_quarantined_methods(): return """namespace Microsoft.AspNetCore.Tests; @@ -131,7 +145,7 @@ def receipts(commit, record, history_targets=None): return eligibility, history -def case_a_record(): +def case_a_record(operating_systems=None): return { "status": "eligible", "originating_case": "case-a", @@ -143,10 +157,15 @@ def case_a_record(): }, "current_quarantine_state": "not-quarantined", "latest_quarantine_transition": "none", + "quarantine_operating_systems": operating_systems or [ + "OperatingSystems.Linux", + "OperatingSystems.MacOSX", + "OperatingSystems.Windows", + ], } -def case_b_record(issue=1): +def case_b_record(issue=1, operating_systems=None): return { "status": "ineligible", "originating_case": "case-b", @@ -160,7 +179,25 @@ def case_b_record(issue=1): }, "current_quarantine_state": "not-quarantined", "latest_quarantine_transition": "removed", + "quarantine_operating_systems": operating_systems or [ + "OperatingSystems.Linux", + "OperatingSystems.MacOSX", + "OperatingSystems.Windows", + ], + } + + +def data_record(case, issue=1, operating_systems=None): + record = ( + case_a_record(operating_systems) + if case == "case-a" + else case_b_record(issue, operating_systems) + ) + record["source_resolution"]["matching_inline_data"] = { + "data": "HttpProtocols.Http3", + "values": ["Http3"], } + return record def write_json(path, value): @@ -285,7 +322,161 @@ def assert_rejected(callback, message): raise AssertionError(f"Expected validation failure containing {message!r}") +def test_theory_target_binding(data="HttpProtocols.Http3", displayed="Http3", supported=True, provider=""): + for theory in ("Theory", "ConditionalTheory"): + for case in ("case-a", "case-b"): + for rewrite_row in (True, False): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + initial_source = theory_source().replace( + "[ConditionalTheory]", f"[{theory}]" + ).replace("HttpProtocols.Http3", data) + if provider: + initial_source = initial_source.replace( + f"[InlineData({data})]", f"[InlineData({data})]\n {provider}" + ) + _, commit = initialize_repository(root, initial_source) + record = case_a_record() if case == "case-a" else case_b_record() + record["source_resolution"] = MODULE.ELIGIBILITY.resolve_source( + root, TEST_NAME + f"(protocol: {displayed})" + ) + eligibility, history = receipts(commit, record) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/theory-binding" + issue = "#aw_sample" if case == "case-a" else "1" + arguments = ( + f'"https://github.com/dotnet/aspnetcore/issues/{issue}", ' + 'OperatingSystems.Linux | OperatingSystems.MacOSX | ' + 'OperatingSystems.Windows' + ) + replacement = ( + f"[QuarantinedTestData({arguments}, {data})]" + if rewrite_row + else f"[QuarantinedTest({arguments})]\n" + f" [InlineData({data})]" + ) + create_patch( + root, transport, branch, + initial_source.replace( + f"[InlineData({data})]", replacement + ), + ) + outputs = {"items": [pull_request(branch)]} + if case == "case-a": + outputs["items"].insert(0, case_a_issue()) + + def check(): + return validate( + root, commit, eligibility, history, outputs, transport + ) + + if ( + rewrite_row == (theory == "ConditionalTheory") + and (supported or theory == "Theory") + ): + result = check() + assert result[0]["operation"] == case, result + else: + assert_rejected( + check, + "not bound to an exact eligible test" + if case == "case-a" + else "not bound to one exact eligible test", + ) + print(f"PASS {case}: {theory}, data={data}, row rewrite={rewrite_row}") + + +def test_commented_row_patch(case="case-a", comment=" // reason [detail]", changed_comment=None): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + reference = "#aw_sample" if case == "case-a" else "1" + inline = "[InlineData(HttpProtocols.Http3)]" + quarantine = ( + f'[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/{reference}", ' + "OperatingSystems.Linux, HttpProtocols.Http3)]" + ) + before, after = (quarantine, inline) if case == "unquarantine" else (inline, quarantine) + _, commit = initialize_repository(root, theory_source(before + comment)) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/commented-row" + create_patch( + root, transport, branch, + theory_source(after + (comment if changed_comment is None else changed_comment)), + ) + eligibility, history = receipts( + commit, + data_record("case-b" if case == "case-b" else "case-a", + operating_systems=["OperatingSystems.Linux"]), + [{ + "scope": "data", "path": TEST_PATH, "type": TYPE_NAME, + "method": "ReturnsExpectedResponse", "data": "HttpProtocols.Http3", + "issue": 1, "status": "first-quarantine", + }] if case == "unquarantine" else None, + ) + items = [pull_request(branch)] + if case == "case-a": + items.insert(0, case_a_issue()) + + def check(): + return validate(root, commit, eligibility, history, {"items": items}, transport) + + if changed_comment is None: + result = check() + assert result[0]["operation"] == case, result + else: + assert_rejected(check, "preserve trailing comments") + print(f"PASS commented row: {case}, {comment!r}, changed={changed_comment!r}") + + +def test_row_comment_ownership(): + replacement = ( + '[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, 1)]' + ) + swapped = ( + "@@ -1 +1 @@\n-[InlineData(1)] // first\n" + f"+{replacement} // second\n" + "@@ -10 +10 @@\n-[InlineData(1)] // second\n" + f"+{replacement} // first\n" + ) + assert_rejected( + lambda: MODULE.changed_patch_lines(swapped), "preserve trailing comments" + ) + data = '"https://example.test/path] // text"' + actual = MODULE.changed_patch_lines( + f"@@ -1 +1 @@\n-[InlineData({data})] // reason [\n" + '+[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/1", ' + f"OperatingSystems.Linux, {data})] // reason [\n" + ) + assert actual == [("-", None, "inline", data), ("+", "1", "data", data)], actual + + def main(): + test_theory_target_binding(provider="[MsQuicSupported]") + test_theory_target_binding( + supported=False, provider="[MsQuicSupported]\n [MemberData(nameof(GetRows))]" + ) + for provider in ( + '[MemberData(nameof(GetRows))]', + '[ClassData(typeof(Rows))]', + '[CustomRows]', + ): + test_theory_target_binding(supported=False, provider=provider) + test_row_comment_ownership() + for case in ("case-a", "case-b", "unquarantine"): + for comment in (" // reason", " // reason [detail]", " // unmatched [", + " /* reason [detail] */"): + test_commented_row_patch(case, comment) + for comment, changed in [ + (" // reason", ""), ("", " // new reason"), (" // reason", " // changed reason"), + ]: + test_commented_row_patch(case, comment, changed) + test_theory_target_binding() + test_theory_target_binding("true", "True") + test_theory_target_binding("-1L", "-1") + test_theory_target_binding('nameof(HttpProtocols.Http3)', '"Http3"', supported=False) assert_rejected( lambda: MODULE.changed_patch_lines( "diff --git a/src/A.cs b/src/A.cs\n" @@ -296,6 +487,34 @@ def main(): ), "non-quarantine change", ) + for reference in ("1", "#aw_sample"): + assert_rejected( + lambda reference=reference: MODULE.changed_patch_lines( + "diff --git a/src/A.cs b/src/A.cs\n" + "--- a/src/A.cs\n" + "+++ b/src/A.cs\n" + "@@ -1 +1,4 @@\n" + "-[InlineData(HttpProtocols.Http3)]\n" + "+[QuarantinedTestData(\n" + f'+ "https://github.com/dotnet/aspnetcore/issues/{reference}",\n' + "+ OperatingSystems.Linux,\n" + "+ HttpProtocols.Http3)]\n" + ), + "require one-line attributes", + ) + assert_rejected( + lambda: MODULE.changed_patch_lines( + "diff --git a/src/A.cs b/src/A.cs\n" + "--- a/src/A.cs\n" + "+++ b/src/A.cs\n" + "@@ -1 +1 @@\n" + "-[MemberData(nameof(Data))]\n" + "+[QuarantinedTestData(" + '"https://github.com/dotnet/aspnetcore/issues/1", ' + "OperatingSystems.Linux, 1)]\n" + ), + "non-quarantine change", + ) with tempfile.TemporaryDirectory() as directory: root = pathlib.Path(directory) @@ -341,6 +560,7 @@ def main(): TEST_PATH, TYPE_NAME, "ReturnsExpectedResponse", + None, ) assert_rejected( lambda: validate( @@ -354,6 +574,267 @@ def main(): "cannot run without a validated Case A pull request", ) + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, commit = initialize_repository(root, theory_source()) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/data-case-a" + create_patch( + root, + transport, + branch, + theory_source( + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/#aw_sample", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]' + ), + ) + eligibility, history = receipts( + commit, + data_record( + "case-a", + operating_systems=["OperatingSystems.Linux"], + ), + ) + result = validate( + root, + commit, + eligibility, + history, + { + "items": [ + case_a_issue(), + pull_request(branch), + ], + }, + transport, + ) + assert result[0]["operation"] == "case-a", result + wrong_operating_systems, _ = receipts( + commit, + data_record("case-a"), + ) + assert_rejected( + lambda: validate( + root, + commit, + wrong_operating_systems, + history, + { + "items": [ + case_a_issue(), + pull_request(branch), + ], + }, + transport, + ), + "do not match deterministic failure evidence", + ) + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, commit = initialize_repository(root, theory_source()) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/data-case-b" + create_patch( + root, + transport, + branch, + theory_source( + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Windows | OperatingSystems.Linux, ' + 'HttpProtocols.Http3)]' + ), + ) + eligibility, history = receipts( + commit, + data_record( + "case-b", + operating_systems=[ + "OperatingSystems.Linux", + "OperatingSystems.Windows", + ], + ), + ) + result = validate( + root, + commit, + eligibility, + history, + {"items": [pull_request(branch)]}, + transport, + ) + assert result[0]["operation"] == "case-b", result + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + quarantined_row = ( + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]' + ) + _, commit = initialize_repository( + root, + theory_source(quarantined_row), + ) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/data-unquarantine" + create_patch(root, transport, branch, theory_source()) + history_target = { + "scope": "data", + "path": TEST_PATH, + "type": TYPE_NAME, + "method": "ReturnsExpectedResponse", + "data": "HttpProtocols.Http3", + "issue": 1, + "status": "first-quarantine", + } + eligibility, history = receipts( + commit, + data_record("case-a"), + [history_target], + ) + result = validate( + root, + commit, + eligibility, + history, + {"items": [pull_request(branch)]}, + transport, + ) + assert result[0]["operation"] == "unquarantine", result + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, commit = initialize_repository(root, theory_source()) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/data-mismatch" + create_patch( + root, + transport, + branch, + theory_source( + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/#aw_sample", ' + 'OperatingSystems.Linux, HttpProtocols.Http2)]' + ), + ) + eligibility, history = receipts( + commit, + data_record( + "case-a", + operating_systems=["OperatingSystems.Linux"], + ), + ) + assert_rejected( + lambda: validate( + root, + commit, + eligibility, + history, + { + "items": [ + case_a_issue(), + pull_request(branch), + ], + }, + transport, + ), + "not bound to an exact eligible test", + ) + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + two_rows = theory_source( + "[InlineData(HttpProtocols.Http3)]\n" + " [InlineData(HttpProtocols.Http2)]" + ) + _, commit = initialize_repository(root, two_rows) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/extra-data-row" + create_patch( + root, + transport, + branch, + theory_source( + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/#aw_sample", ' + 'OperatingSystems.Linux, HttpProtocols.Http3)]\n' + ' [QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/#aw_sample", ' + 'OperatingSystems.Linux, HttpProtocols.Http2)]' + ), + ) + eligibility, history = receipts( + commit, + data_record( + "case-a", + operating_systems=["OperatingSystems.Linux"], + ), + ) + assert_rejected( + lambda: validate( + root, + commit, + eligibility, + history, + { + "items": [ + case_a_issue(), + pull_request(branch), + ], + }, + transport, + ), + "must add exactly one target", + ) + + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + _, commit = initialize_repository(root, theory_source()) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/invalid-operating-system" + create_patch( + root, + transport, + branch, + theory_source( + '[QuarantinedTestData(' + '"https://github.com/dotnet/aspnetcore/issues/#aw_sample", ' + 'OperatingSystems.FreeBSD, HttpProtocols.Http3)]' + ), + ) + eligibility, history = receipts( + commit, + data_record( + "case-a", + operating_systems=["OperatingSystems.Linux"], + ), + ) + assert_rejected( + lambda: validate( + root, + commit, + eligibility, + history, + { + "items": [ + case_a_issue(), + pull_request(branch), + ], + }, + transport, + ), + "unique supported OperatingSystems flags", + ) + with tempfile.TemporaryDirectory() as directory: root = pathlib.Path(directory) _, commit = initialize_repository(root) @@ -433,10 +914,16 @@ def main(): branch, source( '[QuarantinedTest(' - '"https://github.com/dotnet/aspnetcore/issues/1")]' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + "OperatingSystems.Linux)]" + ), + ) + eligibility, history = receipts( + commit, + case_b_record( + operating_systems=["OperatingSystems.Linux"], ), ) - eligibility, history = receipts(commit, case_b_record()) result = validate( root, commit, @@ -447,7 +934,13 @@ def main(): ) assert result[0]["operation"] == "case-b", result - wrong_eligibility, _ = receipts(commit, case_b_record(issue=2)) + wrong_eligibility, _ = receipts( + commit, + case_b_record( + issue=2, + operating_systems=["OperatingSystems.Linux"], + ), + ) assert_rejected( lambda: validate( root, @@ -464,7 +957,8 @@ def main(): root = pathlib.Path(directory) quarantined = source( '[QuarantinedTest(' - '"https://github.com/dotnet/aspnetcore/issues/1")]' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + "OperatingSystems.Windows | OperatingSystems.MacOSX)]" ) _, commit = initialize_repository(root, quarantined) transport = root / "transport" @@ -507,6 +1001,40 @@ def main(): "not an exact first quarantine", ) + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + quarantined = source( + '[QuarantinedTest(' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + "OperatingSystems.Linux)]" + ) + _, commit = initialize_repository(root, quarantined) + transport = root / "transport" + transport.mkdir() + branch = "test-quarantine/change-operating-system" + create_patch( + root, + transport, + branch, + source( + '[QuarantinedTest(' + '"https://github.com/dotnet/aspnetcore/issues/1", ' + "OperatingSystems.Windows)]" + ), + ) + eligibility, history = receipts(commit, case_a_record()) + assert_rejected( + lambda: validate( + root, + commit, + eligibility, + history, + {"items": [pull_request(branch)]}, + transport, + ), + "mixes quarantine additions and removals", + ) + with tempfile.TemporaryDirectory() as directory: root = pathlib.Path(directory) _, commit = initialize_repository( diff --git a/.github/workflows/scripts/test-quarantine/validate_pull_request_outputs.py b/.github/workflows/scripts/test-quarantine/validate_pull_request_outputs.py index 28c948d9aba4..5b413616d97d 100644 --- a/.github/workflows/scripts/test-quarantine/validate_pull_request_outputs.py +++ b/.github/workflows/scripts/test-quarantine/validate_pull_request_outputs.py @@ -26,10 +26,22 @@ ATTRIBUTE_LINE = re.compile( r"^\[\s*(?:assembly\s*:\s*)?" r"(?:[A-Za-z_][A-Za-z0-9_]*\.)*" - r"QuarantinedTest(?:Attribute)?\s*\(\s*" + r"(?PQuarantinedTest(?:Data)?)(?:Attribute)?\s*\(\s*" r'"https://github\.com/dotnet/aspnetcore/issues/' - r'(?P\d+|#aw_[A-Za-z0-9_]{3,12})"\s*\)\s*\]$' + r'(?P\d+|#aw_[A-Za-z0-9_]{3,12})"\s*' + r'(?:,\s*(?POperatingSystems\.[A-Za-z]+' + r'(?:\s*\|\s*OperatingSystems\.[A-Za-z]+)*))?' + r'(?:,\s*(?P.*))?\)\s*\]$' ) +INLINE_DATA_LINE = re.compile( + r"^\[\s*(?:[A-Za-z_][A-Za-z0-9_]*\.)*" + r"InlineData(?:Attribute)?\s*\((?P.*)\)\s*\]$" +) +OPERATING_SYSTEMS = { + "OperatingSystems.Linux", + "OperatingSystems.MacOSX", + "OperatingSystems.Windows", +} class ValidationError(ValueError): @@ -110,6 +122,7 @@ def changed_patch_lines(patch): changed = [] attributes = [] in_hunk = False + hunk = 0 for line in patch.splitlines(): if line.startswith("diff --git "): in_hunk = False @@ -118,6 +131,7 @@ def changed_patch_lines(patch): continue if line.startswith("@@"): in_hunk = True + hunk += 1 continue if not in_hunk: continue @@ -126,17 +140,137 @@ def changed_patch_lines(patch): if not line.startswith(("+", "-")): continue content = line[1:].strip() - changed.append((line[0], content)) - for operation, content in changed: + changed.append((line[0], content, hunk)) + logical_changes = [] + current_operation = None + current_lines = [] + bracket_depth = 0 + current_hunk = None + for operation, content, hunk in changed: + syntax = ELIGIBILITY.sanitize_csharp(content) + if current_lines: + if operation != current_operation or hunk != current_hunk: + raise ValidationError( + "Patch splits one attribute across add/remove operations" + ) + current_lines.append(content) + bracket_depth += ELIGIBILITY.square_bracket_delta(syntax) + if bracket_depth == 0: + logical_changes.append(( + current_operation, + " ".join(current_lines), + len(current_lines), + current_hunk, + )) + current_operation = None + current_lines = [] + continue + if content.startswith("["): + bracket_depth = ELIGIBILITY.square_bracket_delta(syntax) + if bracket_depth > 0: + current_operation = operation + current_hunk = hunk + current_lines = [content] + continue + logical_changes.append((operation, content, 1, hunk)) + if current_lines: + raise ValidationError("Patch contains an incomplete attribute") + + row_comments = [] + for operation, content, physical_line_count, hunk in logical_changes: + comment = "" + syntax = ELIGIBILITY.sanitize_csharp(content) + closing = syntax.rfind("]") + 1 + if closing and not syntax[closing:].strip(): + suffix = content[closing:] + if suffix.strip() and re.fullmatch( + r"\s*(?:/\*.*?\*/\s*)*(?://[^\n]*)?", suffix + ): + comment = suffix + content = content[:closing] attribute = ATTRIBUTE_LINE.fullmatch(content) if attribute: - attributes.append((operation, attribute.group("reference"))) + attribute_name = attribute.group("attribute") + operating_systems = attribute.group("operating_systems") + data = attribute.group("data") + if attribute_name == "QuarantinedTestData" and data is None: + raise ValidationError( + "QuarantinedTestData must include one or more data values" + ) + if ( + attribute_name == "QuarantinedTestData" + and operating_systems is None + ): + raise ValidationError( + "QuarantinedTestData must include operating systems" + ) + if attribute_name == "QuarantinedTest" and data is not None: + raise ValidationError( + "QuarantinedTest does not accept test data values" + ) + if ( + attribute_name == "QuarantinedTestData" + and physical_line_count != 1 + ): + raise ValidationError( + "Automated data-row rewrites require one-line attributes" + ) + if comment and attribute_name != "QuarantinedTestData": + raise ValidationError( + "Trailing comments are supported only on data-row replacements" + ) + if operating_systems is not None: + values = [ + value.strip() + for value in operating_systems.split("|") + ] + if ( + len(values) != len(set(values)) + or any(value not in OPERATING_SYSTEMS for value in values) + ): + raise ValidationError( + "Quarantine operating systems must be unique supported " + "OperatingSystems flags" + ) + attributes.append(( + operation, + attribute.group("reference"), + "data" if data is not None else "quarantine", + data, + )) + if data is not None: + row_comments.append((operation, hunk, data, comment)) + continue + inline_data = INLINE_DATA_LINE.fullmatch(content) + if inline_data: + if physical_line_count != 1: + raise ValidationError( + "Automated data-row rewrites require one-line attributes" + ) + attributes.append(( + operation, + None, + "inline", + inline_data.group("data"), + )) + row_comments.append((operation, hunk, inline_data.group("data"), comment)) continue if operation == "+" and content == USING_LINE: continue raise ValidationError( f"Patch contains a non-quarantine change: {operation}{content}" ) + if any(comment for _, _, _, comment in row_comments): + removed_comments = collections.Counter( + (hunk, data, comment) for operation, hunk, data, comment in row_comments + if operation == "-" + ) + added_comments = collections.Counter( + (hunk, data, comment) for operation, hunk, data, comment in row_comments + if operation == "+" + ) + if removed_comments != added_comments: + raise ValidationError("Data-row replacements must preserve trailing comments") return attributes @@ -146,9 +280,29 @@ def target_key(target): target["path"], target.get("type"), target.get("method"), + target.get("data"), ) +def target_operating_systems(target): + operating_systems = target.get("operating_systems") + if not isinstance(operating_systems, list): + return None + return tuple(operating_systems) + + +def validate_operating_systems(target, record, key): + expected = record.get("quarantine_operating_systems") + if ( + not isinstance(expected, list) + or tuple(expected) != target_operating_systems(target) + ): + raise ValidationError( + "Quarantine operating systems do not match deterministic " + f"failure evidence: {key}" + ) + + def target_map(root, source_index): result = {} for target in ELIGIBILITY.current_quarantine_targets(source_index): @@ -191,11 +345,21 @@ def exact_source_target(test_name, record): source = record.get("source_resolution") if not isinstance(source, dict) or source.get("status") != "exact": return None + inline_data = source.get("matching_inline_data") + if isinstance(inline_data, dict): + return ( + "data", + source.get("path"), + source.get("declaring_type") or source.get("type"), + source.get("method"), + inline_data.get("data"), + ) return ( "method", source.get("path"), source.get("declaring_type") or source.get("type"), source.get("method"), + None, ) @@ -237,6 +401,7 @@ def validate_addition(target, eligibility, issue_items): raise ValidationError( f"Case A addition is not bound to an exact eligible test: {key}" ) + validate_operating_systems(target, record, key) return ("case-a", test_name) if not reference.isdigit(): @@ -259,6 +424,7 @@ def validate_addition(target, eligibility, issue_items): raise ValidationError( f"Case B addition is not bound to one exact eligible test: {key}" ) + validate_operating_systems(target, tests[matches[0]], key) return ("case-b", matches[0]) @@ -540,11 +706,21 @@ def validate_outputs( ) removed = { key: target for key, target in before.items() - if key not in after or after[key]["reference"] != target["reference"] + if ( + key not in after + or after[key]["reference"] != target["reference"] + or target_operating_systems(after[key]) + != target_operating_systems(target) + ) } added = { key: target for key, target in after.items() - if key not in before or before[key]["reference"] != target["reference"] + if ( + key not in before + or before[key]["reference"] != target["reference"] + or target_operating_systems(before[key]) + != target_operating_systems(target) + ) } if removed and added: raise ValidationError( @@ -598,8 +774,31 @@ def validate_outputs( eligibility, issue_items, ) - reference = next(iter(added.values()))["reference"] - if attribute_changes != [("+", reference)]: + added_target = next(iter(added.values())) + reference = added_target["reference"] + expected_changes = [ + ( + "+", + reference, + ( + "data" + if added_target["scope"] == "data" + else "quarantine" + ), + added_target.get("data"), + ) + ] + if added_target["scope"] == "data": + expected_changes.insert( + 0, + ( + "-", + None, + "inline", + added_target["data"], + ), + ) + if attribute_changes != expected_changes: raise ValidationError( "Quarantine pull request attribute lines do not " "match its one derived target" @@ -612,10 +811,25 @@ def validate_outputs( "test": test_name, }) else: - expected_attributes = collections.Counter( - ("-", target["reference"]) - for target in removed.values() - ) + expected_attributes = collections.Counter() + for target in removed.values(): + expected_attributes.update([( + "-", + target["reference"], + ( + "data" + if target["scope"] == "data" + else "quarantine" + ), + target.get("data"), + )]) + if target["scope"] == "data": + expected_attributes.update([( + "+", + None, + "inline", + target["data"], + )]) if collections.Counter(attribute_changes) != expected_attributes: raise ValidationError( "Unquarantine pull request attribute lines do not " diff --git a/.github/workflows/test-quarantine.lock.yml b/.github/workflows/test-quarantine.lock.yml index 02e0d86e1de2..3588475e6d66 100644 --- a/.github/workflows/test-quarantine.lock.yml +++ b/.github/workflows/test-quarantine.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"9632397e608a44853d3025d7d9ae42ceb0c743afd08ae3bb1271097143560f64","body_hash":"331d6aaf58d6fa11b9748483a627dfcdfbc0dbbc044bebcee17e0fca721b1bbe","compiler_version":"v0.89.21","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.87"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"ef6ea932bb0871e61cefee5cb60e6cf08dc261f70fbc35f644250f215d5c9554","body_hash":"206f192f7f434b80591083e971321491fcfae9768b6168aae5ad11116877568f","compiler_version":"v0.89.21","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.87"}} # gh-aw-manifest: {"version":1,"secrets":["COPILOT_PAT_0","COPILOT_PAT_1","COPILOT_PAT_2","COPILOT_PAT_3","COPILOT_PAT_4","COPILOT_PAT_5","COPILOT_PAT_6","COPILOT_PAT_7","COPILOT_PAT_8","COPILOT_PAT_9","GH_AW_CI_TRIGGER_TOKEN","GH_AW_DEFAULT_OTLP_ENDPOINT","GH_AW_DEFAULT_OTLP_HEADERS","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN"],"actions":[{"repo":"actions/checkout","sha":"3d3c42e5aac5ba805825da76410c181273ba90b1","version":"v7.0.1"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"github/gh-aw-actions/setup","sha":"924af5fdc64061cfbf66fb584c8b07e2ac230c60","version":"v0.89.21"}],"skills":[".github/skills/create-kbe"],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.28.23","digest":"sha256:2c78aaba1c108e130e2d6d01e4f2cca334ea04c53e6f258913ac34173fe7e3b2","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.28.23@sha256:2c78aaba1c108e130e2d6d01e4f2cca334ea04c53e6f258913ac34173fe7e3b2"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.23","digest":"sha256:c15c3d1208df10c5b588a3657be53742aa982ae0909d1eb1812525268794ca64","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.23@sha256:c15c3d1208df10c5b588a3657be53742aa982ae0909d1eb1812525268794ca64"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.28.23","digest":"sha256:02ffc56dd40158223064ef03a78c2d9717473c93723b4e403d455ea6f0b09ae0","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.28.23@sha256:02ffc56dd40158223064ef03a78c2d9717473c93723b4e403d455ea6f0b09ae0"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.25","digest":"sha256:9be0a86220e807a0ecc89e53d7453468f7a53fbc6b3d1efd2299025ffe01d086","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.25@sha256:9be0a86220e807a0ecc89e53d7453468f7a53fbc6b3d1efd2299025ffe01d086"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:0daa8971fa4732b647150cb6524a6b0804b68d5d24f6f58b5dd1af23bd63fb23","pinned_image":"ghcr.io/github/gh-aw-node@sha256:0daa8971fa4732b647150cb6524a6b0804b68d5d24f6f58b5dd1af23bd63fb23"},{"image":"ghcr.io/github/github-mcp-server:v1.12.2","digest":"sha256:508a0857ec762b1ab1cece29193345b501fab1dd9d1228a7b617062954cecac6","pinned_image":"ghcr.io/github/github-mcp-server:v1.12.2@sha256:508a0857ec762b1ab1cece29193345b501fab1dd9d1228a7b617062954cecac6"}],"mcp_servers":[{"name":"github","tools":["get_commit","get_file_contents","get_latest_release","get_pull_request","get_pull_request_comments","get_pull_request_diff","get_pull_request_files","get_pull_request_review_comments","get_pull_request_reviews","get_pull_request_status","get_release_by_tag","get_tag","issue_read","list_branches","list_commits","list_issue_types","list_issues","list_pull_requests","list_releases","list_starred_repositories","list_tags","pull_request_read","search_code","search_issues","search_pull_requests","search_repositories"]},{"name":"safeoutputs","tools":["add_comment","add_labels","create_pull_request","create_quarantine_issue","missing_data","missing_tool","noop"]}]} # This file was automatically generated by gh-aw (v0.89.21). DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # @@ -126,9 +126,9 @@ on: # seen.add(pr["number"]) # prs.append(pr) # - # # For each PR, get changed files and check for QuarantinedTest additions. - # # Store the added lines containing [QuarantinedTest so the agent can match at - # # method/class/assembly level, not just file level. + # # For each PR, get changed files and check for QuarantinedTest or + # # QuarantinedTestData additions. Store the added lines so the agent can + # # match at data-row/method/class/assembly level, not just file level. # requarantine_data = [] # for pr in prs: # files = get_changed_files(pr["number"]) @@ -184,7 +184,7 @@ on: # # Fetch the numbers of every issue carrying the `re-quarantine` label. A test # # tracked by one of these issues has been deliberately re-quarantined and must # # NEVER be auto-unquarantined. This is a deterministic complement to the - # # re-quarantine-PR diff check: matching a candidate's [QuarantinedTest] issue URL + # # re-quarantine-PR diff check: matching a candidate's quarantine issue URL # # against this set is exact and cannot be missed by fuzzy diff parsing. # python3 << 'SCRIPT' # import json, os, sys, urllib.parse, urllib.request @@ -626,7 +626,7 @@ on: # # optional enrichment (never the per-test counts) and fails loud rather than letting # # GitHub silently truncate into corrupt JSON. Validated ~170KB on 30 days of data. # python3 << 'SCRIPT' - # import json, os, sys, time, datetime, urllib.parse, urllib.request, urllib.error, re + # import json, os, sys, time, datetime, urllib.parse, urllib.request, urllib.error, re, http.client # # ADO = "https://dev.azure.com/dnceng-public/public/_apis" # VSTMR = "https://vstmr.dev.azure.com/dnceng-public/public/_apis" @@ -637,7 +637,8 @@ on: # # ERROR_CAP = 1200 # STACK_CAP = 900 - # OCC_CAP = 2 # occurrences tracked per test (for Source C multi-probe) + # OCC_CAP = 2 # occurrences tracked per work item (for Source C multi-probe) + # OS_DETAIL_BUDGET = 1000 # additional result-detail calls per source for OS evidence # # BLOCK_CAP = 8000 # WORKITEM_CAP = 40000 @@ -786,32 +787,37 @@ on: # # # def norm_name(t): - # # Both automatedTestName and testCaseTitle can carry theory arguments. - # # Quarantine applies to the source method, so normalize every theory row to - # # that method before deduplicating the build-level failure incident. + # # Preserve theory arguments so row-specific failures remain distinct. # name = t.get("automatedTestName") or t.get("testCaseTitle") or "" - # return name.split("(")[0].strip() + # return name.strip() # # # def aggregate(build_ids): # agg = {} # for bid in build_ids: # seen_in_build = set() + # seen_results = set() # for t in failed_results(bid): # name = norm_name(t) - # if not name or name in seen_in_build: + # identity = (name, t.get("runId"), t.get("id")) + # if not name or identity in seen_results: # continue - # # Azure DevOps can publish duplicate rows for one test execution, and - # # theories publish one row per argument set. Neither is independent - # # flakiness evidence: count at most one incident per source method per - # # build, while retaining one representative result for enrichment. - # seen_in_build.add(name) + # if name.endswith(WI_SUFFIX) and name in seen_in_build: + # continue + # seen_results.add(identity) + # # Azure DevOps can publish duplicate rows for one test execution. + # # Count at most one incident per exact test case per build while + # # retaining distinct results so other platform failures are not lost. # e = agg.setdefault(name, {"count": 0, "assembly": t.get("automatedTestStorage", ""), # "builds": [], "occ": []}) + # if name not in seen_in_build: + # seen_in_build.add(name) # e["count"] += 1 # e["builds"].append(bid) - # if t.get("runId") and t.get("id") and len(e["occ"]) < OCC_CAP: - # e["occ"].append({"runId": t["runId"], "resultId": t["id"], "build": bid}) + # if not name.endswith(WI_SUFFIX) or ( + # t.get("runId") and t.get("id") and len(e["occ"]) < OCC_CAP + # ): + # e["occ"].append({"runId": t.get("runId"), "resultId": t.get("id"), "build": bid}) # return agg # # @@ -830,30 +836,63 @@ on: # return data # # - # def enrich(agg): + # def helix_queue(job_id, queue_cache): + # if not isinstance(job_id, str) or not job_id: + # sys.stderr.write("platform evidence: missing Helix job identity\n") + # return None + # if job_id not in queue_cache: + # queue_cache[job_id] = None + # try: + # data, _ = fetch(f"{HELIX}/jobs/{urllib.parse.quote(job_id, safe='')}") + # queue = data.get("QueueId") if isinstance(data, dict) else None + # if not isinstance(queue, str) or not queue.strip(): + # raise ValueError("Helix job response has no QueueId") + # queue_cache[job_id] = queue + # except (OSError, ValueError, http.client.HTTPException) as ex: + # sys.stderr.write(f"platform evidence: Helix queue lookup failed: {type(ex).__name__}\n") + # return queue_cache[job_id] + # + # + # def enrich(agg, queue_cache): # """Attach Helix coords (job+workitem, only when BOTH present) and, for individual - # tests, real error/stack from the representative result detail. For work items, also + # tests, per-build platform evidence and real error/stack from the representative + # result detail. For work items, also # collect candidate (job, workitem, build) probes from every tracked occurrence so # Source C can try more than just the first build.""" + # remaining_details = OS_DETAIL_BUDGET # for name, e in agg.items(): # is_wi = name.endswith(WI_SUFFIX) # probes = [] + # representative_index = next( + # (idx for idx, occ in enumerate(e.get("occ", [])) + # if occ["runId"] and occ["resultId"]), + # None, + # ) # for idx, occ in enumerate(e.get("occ", [])): - # # Individual tests only need the first occurrence (error/stack + coords). - # if not is_wi and idx > 0: - # break + # if not is_wi: + # queues = e.setdefault("queues", {}).setdefault(str(occ["build"]), []) + # queues.append(None) + # if not occ["runId"] or not occ["resultId"]: + # e["detail_note"] = "platform evidence missing result identity" + # continue + # if idx != representative_index: + # if remaining_details == 0: + # e["detail_note"] = "platform evidence detail budget exhausted" + # continue + # remaining_details -= 1 # try: # det = result_detail(occ["runId"], occ["resultId"]) # except Exception as ex: - # if idx == 0: # e["detail_note"] = f"detail fetch failed: {type(ex).__name__}" # continue # job, wi_name = parse_helix(det.get("comment")) - # if idx == 0 and job and wi_name: + # if not is_wi: + # queues[-1] = helix_queue(job, queue_cache) + # if idx == representative_index and job and wi_name: # e["helix"] = {"job": job, "workitem": wi_name} # if is_wi and job and wi_name: # probes.append({"job": job, "workitem": wi_name, "build": occ["build"]}) - # if idx == 0 and not is_wi: + # if idx == representative_index and not is_wi: # e["evidence_build"] = occ["build"] # e["run_id"] = occ["runId"] # e["result_id"] = occ["resultId"] @@ -1021,6 +1060,7 @@ on: # # # def main(): + # queue_cache = {} # # Source A: failed/partial builds on main, both pipelines, last 30 days. # a_builds = [b for d in DEFS for b in list_failed_builds(d, branch="refs/heads/main")] # # Make the representative occurrence deterministic and recent. Any eligible @@ -1032,7 +1072,7 @@ on: # bmeta = {} # for b in a_builds: # bmeta[str(b["id"])] = build_meta(b) - # source_a = enrich(aggregate([b["id"] for b in a_builds])) + # source_a = enrich(aggregate([b["id"] for b in a_builds]), queue_cache) # # Flakiness signal: needs the FULL main timeline (incl. succeeded builds), not just # # the failed/partial builds above, to spot a passing run between two failures. # all_main_builds = [b for d in DEFS for b in list_completed_builds(d, branch="refs/heads/main")] @@ -1057,7 +1097,7 @@ on: # bmeta.get(str(bid), {}).get("startedUtc") or "", # int(bid)), # reverse=True) - # source_b = enrich(aggregate(b_ids)) + # source_b = enrich(aggregate(b_ids), queue_cache) # # # Source C: work items (combined A+B) -> Helix console [FAIL] blocks. Probe each # # tracked occurrence until one yields [FAIL] blocks (the first build is often a @@ -1124,6 +1164,7 @@ on: # continue # total += len(joined) # source_c.append({"workitem": name, "build": chosen["build"], "job": chosen["job"], + # "queue": helix_queue(chosen["job"], queue_cache), # "log_bytes": chosen["log"], "fail_block_count": len(chosen["blocks"]), # "fail_blocks": joined}) # @@ -1320,14 +1361,26 @@ on: # and record.get("case_b_eligible") is True # ) # ) + # eligible_operating_systems = { + # name: receipt["tests"][name].get("quarantine_operating_systems") + # for name in sorted(set(eligible_case_a) | set(eligible_case_b)) + # } # eligible_case_a_json = json.dumps(eligible_case_a, separators=(",", ":")) # eligible_case_b_json = json.dumps(eligible_case_b, separators=(",", ":")) + # eligible_operating_systems_json = json.dumps( + # eligible_operating_systems, + # separators=(",", ":"), + # ) # if len(eligible_case_a_json.encode("utf-8")) > 120000: # raise SystemExit("FATAL: eligible_test_names exceeds the safe job-output limit") # if len(eligible_case_b_json.encode("utf-8")) > 120000: # raise SystemExit( # "FATAL: eligible_case_b_test_names exceeds the safe job-output limit" # ) + # if len(eligible_operating_systems_json.encode("utf-8")) > 120000: + # raise SystemExit( + # "FATAL: eligible_operating_systems exceeds the safe job-output limit" + # ) # output = os.environ.get("GITHUB_OUTPUT") # if not output: # raise SystemExit("FATAL: GITHUB_OUTPUT is not set") @@ -1336,6 +1389,10 @@ on: # stream.write( # f"eligible_case_b_test_names={eligible_case_b_json}\n" # ) + # stream.write( + # "eligible_operating_systems=" + # f"{eligible_operating_systems_json}\n" + # ) # SCRIPT # - name: Upload deterministic evidence for safe output validation # uses: actions/upload-artifact@v7.0.1 @@ -1605,6 +1662,7 @@ jobs: GH_AW_GITHUB_WORKSPACE: ${{ github.workspace }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_CLOSED_QUARANTINE_PRS: ${{ needs.pre_activation.outputs.closed_quarantine_prs }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_CASE_B_TEST_NAMES: ${{ needs.pre_activation.outputs.eligible_case_b_test_names }} + GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_OPERATING_SYSTEMS: ${{ needs.pre_activation.outputs.eligible_operating_systems }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_TEST_NAMES: ${{ needs.pre_activation.outputs.eligible_test_names }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_0: ${{ needs.pre_activation.outputs.part1_data_0 }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_1: ${{ needs.pre_activation.outputs.part1_data_1 }} @@ -1644,6 +1702,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_CLOSED_QUARANTINE_PRS: ${{ needs.pre_activation.outputs.closed_quarantine_prs }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_CASE_B_TEST_NAMES: ${{ needs.pre_activation.outputs.eligible_case_b_test_names }} + GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_OPERATING_SYSTEMS: ${{ needs.pre_activation.outputs.eligible_operating_systems }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_TEST_NAMES: ${{ needs.pre_activation.outputs.eligible_test_names }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_0: ${{ needs.pre_activation.outputs.part1_data_0 }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_1: ${{ needs.pre_activation.outputs.part1_data_1 }} @@ -1689,6 +1748,7 @@ jobs: GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: ${{ needs.pre_activation.outputs.activated }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_CLOSED_QUARANTINE_PRS: ${{ needs.pre_activation.outputs.closed_quarantine_prs }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_CASE_B_TEST_NAMES: ${{ needs.pre_activation.outputs.eligible_case_b_test_names }} + GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_OPERATING_SYSTEMS: ${{ needs.pre_activation.outputs.eligible_operating_systems }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_TEST_NAMES: ${{ needs.pre_activation.outputs.eligible_test_names }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_0: ${{ needs.pre_activation.outputs.part1_data_0 }} GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_1: ${{ needs.pre_activation.outputs.part1_data_1 }} @@ -1735,6 +1795,7 @@ jobs: GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED, GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_CLOSED_QUARANTINE_PRS: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_CLOSED_QUARANTINE_PRS, GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_CASE_B_TEST_NAMES: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_CASE_B_TEST_NAMES, + GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_OPERATING_SYSTEMS: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_OPERATING_SYSTEMS, GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_TEST_NAMES: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ELIGIBLE_TEST_NAMES, GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_0: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_0, GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_1: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_PART1_DATA_1, @@ -3383,6 +3444,7 @@ jobs: closed_quarantine_prs: ${{ steps.closed_quarantine_prs.outputs.closed_quarantine_prs }} closed_quarantine_prs_result: ${{ steps.closed_quarantine_prs.outcome }} eligible_case_b_test_names: ${{ steps.case_a_eligibility.outputs.eligible_case_b_test_names }} + eligible_operating_systems: ${{ steps.case_a_eligibility.outputs.eligible_operating_systems }} eligible_test_names: ${{ steps.case_a_eligibility.outputs.eligible_test_names }} matched_command: '' part1_aggregate_result: ${{ steps.part1_aggregate.outcome }} @@ -3495,9 +3557,9 @@ jobs: seen.add(pr["number"]) prs.append(pr) - # For each PR, get changed files and check for QuarantinedTest additions. - # Store the added lines containing [QuarantinedTest so the agent can match at - # method/class/assembly level, not just file level. + # For each PR, get changed files and check for QuarantinedTest or + # QuarantinedTestData additions. Store the added lines so the agent can + # match at data-row/method/class/assembly level, not just file level. requarantine_data = [] for pr in prs: files = get_changed_files(pr["number"]) @@ -3553,7 +3615,7 @@ jobs: # Fetch the numbers of every issue carrying the `re-quarantine` label. A test # tracked by one of these issues has been deliberately re-quarantined and must # NEVER be auto-unquarantined. This is a deterministic complement to the - # re-quarantine-PR diff check: matching a candidate's [QuarantinedTest] issue URL + # re-quarantine-PR diff check: matching a candidate's quarantine issue URL # against this set is exact and cannot be missed by fuzzy diff parsing. python3 << 'SCRIPT' import json, os, sys, urllib.parse, urllib.request @@ -3995,7 +4057,7 @@ jobs: # optional enrichment (never the per-test counts) and fails loud rather than letting # GitHub silently truncate into corrupt JSON. Validated ~170KB on 30 days of data. python3 << 'SCRIPT' - import json, os, sys, time, datetime, urllib.parse, urllib.request, urllib.error, re + import json, os, sys, time, datetime, urllib.parse, urllib.request, urllib.error, re, http.client ADO = "https://dev.azure.com/dnceng-public/public/_apis" VSTMR = "https://vstmr.dev.azure.com/dnceng-public/public/_apis" @@ -4006,7 +4068,8 @@ jobs: ERROR_CAP = 1200 STACK_CAP = 900 - OCC_CAP = 2 # occurrences tracked per test (for Source C multi-probe) + OCC_CAP = 2 # occurrences tracked per work item (for Source C multi-probe) + OS_DETAIL_BUDGET = 1000 # additional result-detail calls per source for OS evidence BLOCK_CAP = 8000 WORKITEM_CAP = 40000 @@ -4155,32 +4218,37 @@ jobs: def norm_name(t): - # Both automatedTestName and testCaseTitle can carry theory arguments. - # Quarantine applies to the source method, so normalize every theory row to - # that method before deduplicating the build-level failure incident. + # Preserve theory arguments so row-specific failures remain distinct. name = t.get("automatedTestName") or t.get("testCaseTitle") or "" - return name.split("(")[0].strip() + return name.strip() def aggregate(build_ids): agg = {} for bid in build_ids: seen_in_build = set() + seen_results = set() for t in failed_results(bid): name = norm_name(t) - if not name or name in seen_in_build: + identity = (name, t.get("runId"), t.get("id")) + if not name or identity in seen_results: continue - # Azure DevOps can publish duplicate rows for one test execution, and - # theories publish one row per argument set. Neither is independent - # flakiness evidence: count at most one incident per source method per - # build, while retaining one representative result for enrichment. - seen_in_build.add(name) + if name.endswith(WI_SUFFIX) and name in seen_in_build: + continue + seen_results.add(identity) + # Azure DevOps can publish duplicate rows for one test execution. + # Count at most one incident per exact test case per build while + # retaining distinct results so other platform failures are not lost. e = agg.setdefault(name, {"count": 0, "assembly": t.get("automatedTestStorage", ""), "builds": [], "occ": []}) - e["count"] += 1 - e["builds"].append(bid) - if t.get("runId") and t.get("id") and len(e["occ"]) < OCC_CAP: - e["occ"].append({"runId": t["runId"], "resultId": t["id"], "build": bid}) + if name not in seen_in_build: + seen_in_build.add(name) + e["count"] += 1 + e["builds"].append(bid) + if not name.endswith(WI_SUFFIX) or ( + t.get("runId") and t.get("id") and len(e["occ"]) < OCC_CAP + ): + e["occ"].append({"runId": t.get("runId"), "resultId": t.get("id"), "build": bid}) return agg @@ -4199,30 +4267,63 @@ jobs: return data - def enrich(agg): + def helix_queue(job_id, queue_cache): + if not isinstance(job_id, str) or not job_id: + sys.stderr.write("platform evidence: missing Helix job identity\n") + return None + if job_id not in queue_cache: + queue_cache[job_id] = None + try: + data, _ = fetch(f"{HELIX}/jobs/{urllib.parse.quote(job_id, safe='')}") + queue = data.get("QueueId") if isinstance(data, dict) else None + if not isinstance(queue, str) or not queue.strip(): + raise ValueError("Helix job response has no QueueId") + queue_cache[job_id] = queue + except (OSError, ValueError, http.client.HTTPException) as ex: + sys.stderr.write(f"platform evidence: Helix queue lookup failed: {type(ex).__name__}\n") + return queue_cache[job_id] + + + def enrich(agg, queue_cache): """Attach Helix coords (job+workitem, only when BOTH present) and, for individual - tests, real error/stack from the representative result detail. For work items, also + tests, per-build platform evidence and real error/stack from the representative + result detail. For work items, also collect candidate (job, workitem, build) probes from every tracked occurrence so Source C can try more than just the first build.""" + remaining_details = OS_DETAIL_BUDGET for name, e in agg.items(): is_wi = name.endswith(WI_SUFFIX) probes = [] + representative_index = next( + (idx for idx, occ in enumerate(e.get("occ", [])) + if occ["runId"] and occ["resultId"]), + None, + ) for idx, occ in enumerate(e.get("occ", [])): - # Individual tests only need the first occurrence (error/stack + coords). - if not is_wi and idx > 0: - break + if not is_wi: + queues = e.setdefault("queues", {}).setdefault(str(occ["build"]), []) + queues.append(None) + if not occ["runId"] or not occ["resultId"]: + e["detail_note"] = "platform evidence missing result identity" + continue + if idx != representative_index: + if remaining_details == 0: + e["detail_note"] = "platform evidence detail budget exhausted" + continue + remaining_details -= 1 try: det = result_detail(occ["runId"], occ["resultId"]) except Exception as ex: - if idx == 0: - e["detail_note"] = f"detail fetch failed: {type(ex).__name__}" + e["detail_note"] = f"detail fetch failed: {type(ex).__name__}" continue job, wi_name = parse_helix(det.get("comment")) - if idx == 0 and job and wi_name: + if not is_wi: + queues[-1] = helix_queue(job, queue_cache) + if idx == representative_index and job and wi_name: e["helix"] = {"job": job, "workitem": wi_name} if is_wi and job and wi_name: probes.append({"job": job, "workitem": wi_name, "build": occ["build"]}) - if idx == 0 and not is_wi: + if idx == representative_index and not is_wi: e["evidence_build"] = occ["build"] e["run_id"] = occ["runId"] e["result_id"] = occ["resultId"] @@ -4390,6 +4491,7 @@ jobs: def main(): + queue_cache = {} # Source A: failed/partial builds on main, both pipelines, last 30 days. a_builds = [b for d in DEFS for b in list_failed_builds(d, branch="refs/heads/main")] # Make the representative occurrence deterministic and recent. Any eligible @@ -4401,7 +4503,7 @@ jobs: bmeta = {} for b in a_builds: bmeta[str(b["id"])] = build_meta(b) - source_a = enrich(aggregate([b["id"] for b in a_builds])) + source_a = enrich(aggregate([b["id"] for b in a_builds]), queue_cache) # Flakiness signal: needs the FULL main timeline (incl. succeeded builds), not just # the failed/partial builds above, to spot a passing run between two failures. all_main_builds = [b for d in DEFS for b in list_completed_builds(d, branch="refs/heads/main")] @@ -4426,7 +4528,7 @@ jobs: bmeta.get(str(bid), {}).get("startedUtc") or "", int(bid)), reverse=True) - source_b = enrich(aggregate(b_ids)) + source_b = enrich(aggregate(b_ids), queue_cache) # Source C: work items (combined A+B) -> Helix console [FAIL] blocks. Probe each # tracked occurrence until one yields [FAIL] blocks (the first build is often a @@ -4493,6 +4595,7 @@ jobs: continue total += len(joined) source_c.append({"workitem": name, "build": chosen["build"], "job": chosen["job"], + "queue": helix_queue(chosen["job"], queue_cache), "log_bytes": chosen["log"], "fail_block_count": len(chosen["blocks"]), "fail_blocks": joined}) @@ -4688,14 +4791,26 @@ jobs: and record.get("case_b_eligible") is True ) ) + eligible_operating_systems = { + name: receipt["tests"][name].get("quarantine_operating_systems") + for name in sorted(set(eligible_case_a) | set(eligible_case_b)) + } eligible_case_a_json = json.dumps(eligible_case_a, separators=(",", ":")) eligible_case_b_json = json.dumps(eligible_case_b, separators=(",", ":")) + eligible_operating_systems_json = json.dumps( + eligible_operating_systems, + separators=(",", ":"), + ) if len(eligible_case_a_json.encode("utf-8")) > 120000: raise SystemExit("FATAL: eligible_test_names exceeds the safe job-output limit") if len(eligible_case_b_json.encode("utf-8")) > 120000: raise SystemExit( "FATAL: eligible_case_b_test_names exceeds the safe job-output limit" ) + if len(eligible_operating_systems_json.encode("utf-8")) > 120000: + raise SystemExit( + "FATAL: eligible_operating_systems exceeds the safe job-output limit" + ) output = os.environ.get("GITHUB_OUTPUT") if not output: raise SystemExit("FATAL: GITHUB_OUTPUT is not set") @@ -4704,6 +4819,10 @@ jobs: stream.write( f"eligible_case_b_test_names={eligible_case_b_json}\n" ) + stream.write( + "eligible_operating_systems=" + f"{eligible_operating_systems_json}\n" + ) SCRIPT env: CLOSED_QUARANTINE_PRS: ${{ steps.closed_quarantine_prs.outputs.closed_quarantine_prs }} @@ -4848,7 +4967,7 @@ jobs: GH_AW_AGENT_OUTPUT: ${{ steps.setup-agent-output-env.outputs.GH_AW_AGENT_OUTPUT }} - name: Configure Safe Output Scripts run: | - cat > "${RUNNER_TEMP}/gh-aw/actions/safe_output_script_create_quarantine_issue.cjs" << 'GH_AW_SAFE_OUTPUT_SCRIPT_CREATE_QUARANTINE_ISSUE_974179c1865ff752_EOF' + cat > "${RUNNER_TEMP}/gh-aw/actions/safe_output_script_create_quarantine_issue.cjs" << 'GH_AW_SAFE_OUTPUT_SCRIPT_CREATE_QUARANTINE_ISSUE_6856446f5baacc9f_EOF' // @ts-check /// // Auto-generated safe-output script handler: create-quarantine-issue @@ -4995,8 +5114,6 @@ jobs: } const failedName = normalizedLine .slice(0, -"[FAIL]".length) - .trim() - .split("(", 1)[0] .trim(); return failedName === testName; })); @@ -5453,7 +5570,7 @@ jobs: } module.exports = { main }; - GH_AW_SAFE_OUTPUT_SCRIPT_CREATE_QUARANTINE_ISSUE_974179c1865ff752_EOF + GH_AW_SAFE_OUTPUT_SCRIPT_CREATE_QUARANTINE_ISSUE_6856446f5baacc9f_EOF - name: Process Safe Outputs id: process_safe_outputs uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 diff --git a/.github/workflows/test-quarantine.md b/.github/workflows/test-quarantine.md index e405f25c15d8..d191e2a81377 100644 --- a/.github/workflows/test-quarantine.md +++ b/.github/workflows/test-quarantine.md @@ -66,9 +66,9 @@ on: seen.add(pr["number"]) prs.append(pr) - # For each PR, get changed files and check for QuarantinedTest additions. - # Store the added lines containing [QuarantinedTest so the agent can match at - # method/class/assembly level, not just file level. + # For each PR, get changed files and check for QuarantinedTest or + # QuarantinedTestData additions. Store the added lines so the agent can + # match at data-row/method/class/assembly level, not just file level. requarantine_data = [] for pr in prs: files = get_changed_files(pr["number"]) @@ -125,7 +125,7 @@ on: # Fetch the numbers of every issue carrying the `re-quarantine` label. A test # tracked by one of these issues has been deliberately re-quarantined and must # NEVER be auto-unquarantined. This is a deterministic complement to the - # re-quarantine-PR diff check: matching a candidate's [QuarantinedTest] issue URL + # re-quarantine-PR diff check: matching a candidate's quarantine issue URL # against this set is exact and cannot be missed by fuzzy diff parsing. python3 << 'SCRIPT' import json, os, sys, urllib.parse, urllib.request @@ -570,7 +570,7 @@ on: # optional enrichment (never the per-test counts) and fails loud rather than letting # GitHub silently truncate into corrupt JSON. Validated ~170KB on 30 days of data. python3 << 'SCRIPT' - import json, os, sys, time, datetime, urllib.parse, urllib.request, urllib.error, re + import json, os, sys, time, datetime, urllib.parse, urllib.request, urllib.error, re, http.client ADO = "https://dev.azure.com/dnceng-public/public/_apis" VSTMR = "https://vstmr.dev.azure.com/dnceng-public/public/_apis" @@ -581,7 +581,8 @@ on: ERROR_CAP = 1200 STACK_CAP = 900 - OCC_CAP = 2 # occurrences tracked per test (for Source C multi-probe) + OCC_CAP = 2 # occurrences tracked per work item (for Source C multi-probe) + OS_DETAIL_BUDGET = 1000 # additional result-detail calls per source for OS evidence BLOCK_CAP = 8000 WORKITEM_CAP = 40000 @@ -730,32 +731,37 @@ on: def norm_name(t): - # Both automatedTestName and testCaseTitle can carry theory arguments. - # Quarantine applies to the source method, so normalize every theory row to - # that method before deduplicating the build-level failure incident. + # Preserve theory arguments so row-specific failures remain distinct. name = t.get("automatedTestName") or t.get("testCaseTitle") or "" - return name.split("(")[0].strip() + return name.strip() def aggregate(build_ids): agg = {} for bid in build_ids: seen_in_build = set() + seen_results = set() for t in failed_results(bid): name = norm_name(t) - if not name or name in seen_in_build: + identity = (name, t.get("runId"), t.get("id")) + if not name or identity in seen_results: continue - # Azure DevOps can publish duplicate rows for one test execution, and - # theories publish one row per argument set. Neither is independent - # flakiness evidence: count at most one incident per source method per - # build, while retaining one representative result for enrichment. - seen_in_build.add(name) + if name.endswith(WI_SUFFIX) and name in seen_in_build: + continue + seen_results.add(identity) + # Azure DevOps can publish duplicate rows for one test execution. + # Count at most one incident per exact test case per build while + # retaining distinct results so other platform failures are not lost. e = agg.setdefault(name, {"count": 0, "assembly": t.get("automatedTestStorage", ""), "builds": [], "occ": []}) - e["count"] += 1 - e["builds"].append(bid) - if t.get("runId") and t.get("id") and len(e["occ"]) < OCC_CAP: - e["occ"].append({"runId": t["runId"], "resultId": t["id"], "build": bid}) + if name not in seen_in_build: + seen_in_build.add(name) + e["count"] += 1 + e["builds"].append(bid) + if not name.endswith(WI_SUFFIX) or ( + t.get("runId") and t.get("id") and len(e["occ"]) < OCC_CAP + ): + e["occ"].append({"runId": t.get("runId"), "resultId": t.get("id"), "build": bid}) return agg @@ -774,30 +780,63 @@ on: return data - def enrich(agg): + def helix_queue(job_id, queue_cache): + if not isinstance(job_id, str) or not job_id: + sys.stderr.write("platform evidence: missing Helix job identity\n") + return None + if job_id not in queue_cache: + queue_cache[job_id] = None + try: + data, _ = fetch(f"{HELIX}/jobs/{urllib.parse.quote(job_id, safe='')}") + queue = data.get("QueueId") if isinstance(data, dict) else None + if not isinstance(queue, str) or not queue.strip(): + raise ValueError("Helix job response has no QueueId") + queue_cache[job_id] = queue + except (OSError, ValueError, http.client.HTTPException) as ex: + sys.stderr.write(f"platform evidence: Helix queue lookup failed: {type(ex).__name__}\n") + return queue_cache[job_id] + + + def enrich(agg, queue_cache): """Attach Helix coords (job+workitem, only when BOTH present) and, for individual - tests, real error/stack from the representative result detail. For work items, also + tests, per-build platform evidence and real error/stack from the representative + result detail. For work items, also collect candidate (job, workitem, build) probes from every tracked occurrence so Source C can try more than just the first build.""" + remaining_details = OS_DETAIL_BUDGET for name, e in agg.items(): is_wi = name.endswith(WI_SUFFIX) probes = [] + representative_index = next( + (idx for idx, occ in enumerate(e.get("occ", [])) + if occ["runId"] and occ["resultId"]), + None, + ) for idx, occ in enumerate(e.get("occ", [])): - # Individual tests only need the first occurrence (error/stack + coords). - if not is_wi and idx > 0: - break + if not is_wi: + queues = e.setdefault("queues", {}).setdefault(str(occ["build"]), []) + queues.append(None) + if not occ["runId"] or not occ["resultId"]: + e["detail_note"] = "platform evidence missing result identity" + continue + if idx != representative_index: + if remaining_details == 0: + e["detail_note"] = "platform evidence detail budget exhausted" + continue + remaining_details -= 1 try: det = result_detail(occ["runId"], occ["resultId"]) except Exception as ex: - if idx == 0: - e["detail_note"] = f"detail fetch failed: {type(ex).__name__}" + e["detail_note"] = f"detail fetch failed: {type(ex).__name__}" continue job, wi_name = parse_helix(det.get("comment")) - if idx == 0 and job and wi_name: + if not is_wi: + queues[-1] = helix_queue(job, queue_cache) + if idx == representative_index and job and wi_name: e["helix"] = {"job": job, "workitem": wi_name} if is_wi and job and wi_name: probes.append({"job": job, "workitem": wi_name, "build": occ["build"]}) - if idx == 0 and not is_wi: + if idx == representative_index and not is_wi: e["evidence_build"] = occ["build"] e["run_id"] = occ["runId"] e["result_id"] = occ["resultId"] @@ -965,6 +1004,7 @@ on: def main(): + queue_cache = {} # Source A: failed/partial builds on main, both pipelines, last 30 days. a_builds = [b for d in DEFS for b in list_failed_builds(d, branch="refs/heads/main")] # Make the representative occurrence deterministic and recent. Any eligible @@ -976,7 +1016,7 @@ on: bmeta = {} for b in a_builds: bmeta[str(b["id"])] = build_meta(b) - source_a = enrich(aggregate([b["id"] for b in a_builds])) + source_a = enrich(aggregate([b["id"] for b in a_builds]), queue_cache) # Flakiness signal: needs the FULL main timeline (incl. succeeded builds), not just # the failed/partial builds above, to spot a passing run between two failures. all_main_builds = [b for d in DEFS for b in list_completed_builds(d, branch="refs/heads/main")] @@ -1001,7 +1041,7 @@ on: bmeta.get(str(bid), {}).get("startedUtc") or "", int(bid)), reverse=True) - source_b = enrich(aggregate(b_ids)) + source_b = enrich(aggregate(b_ids), queue_cache) # Source C: work items (combined A+B) -> Helix console [FAIL] blocks. Probe each # tracked occurrence until one yields [FAIL] blocks (the first build is often a @@ -1068,6 +1108,7 @@ on: continue total += len(joined) source_c.append({"workitem": name, "build": chosen["build"], "job": chosen["job"], + "queue": helix_queue(chosen["job"], queue_cache), "log_bytes": chosen["log"], "fail_block_count": len(chosen["blocks"]), "fail_blocks": joined}) @@ -1267,14 +1308,26 @@ on: and record.get("case_b_eligible") is True ) ) + eligible_operating_systems = { + name: receipt["tests"][name].get("quarantine_operating_systems") + for name in sorted(set(eligible_case_a) | set(eligible_case_b)) + } eligible_case_a_json = json.dumps(eligible_case_a, separators=(",", ":")) eligible_case_b_json = json.dumps(eligible_case_b, separators=(",", ":")) + eligible_operating_systems_json = json.dumps( + eligible_operating_systems, + separators=(",", ":"), + ) if len(eligible_case_a_json.encode("utf-8")) > 120000: raise SystemExit("FATAL: eligible_test_names exceeds the safe job-output limit") if len(eligible_case_b_json.encode("utf-8")) > 120000: raise SystemExit( "FATAL: eligible_case_b_test_names exceeds the safe job-output limit" ) + if len(eligible_operating_systems_json.encode("utf-8")) > 120000: + raise SystemExit( + "FATAL: eligible_operating_systems exceeds the safe job-output limit" + ) output = os.environ.get("GITHUB_OUTPUT") if not output: raise SystemExit("FATAL: GITHUB_OUTPUT is not set") @@ -1283,6 +1336,10 @@ on: stream.write( f"eligible_case_b_test_names={eligible_case_b_json}\n" ) + stream.write( + "eligible_operating_systems=" + f"{eligible_operating_systems_json}\n" + ) SCRIPT - name: Upload deterministic evidence for safe output validation @@ -1306,6 +1363,7 @@ jobs: source_b_build_ids: ${{ steps.source_b_prs.outputs.source_b_build_ids }} eligible_test_names: ${{ steps.case_a_eligibility.outputs.eligible_test_names }} eligible_case_b_test_names: ${{ steps.case_a_eligibility.outputs.eligible_case_b_test_names }} + eligible_operating_systems: ${{ steps.case_a_eligibility.outputs.eligible_operating_systems }} # part1_data is chunked across fixed outputs to stay under the 131072-byte MAX_ARG_STRLEN # per-env-var limit (see write_part1_chunks above); the prompt concatenates them back. part1_data_0: ${{ steps.part1_aggregate.outputs.part1_data_0 }} @@ -1545,8 +1603,6 @@ safe-outputs: } const failedName = normalizedLine .slice(0, -"[FAIL]".length) - .trim() - .split("(", 1)[0] .trim(); return failedName === testName; })); @@ -2084,7 +2140,7 @@ You are an automated workflow that manages flaky test quarantine in the dotnet/a 1. **Quarantine** tests that are flaky and causing CI failures 2. **Unquarantine** tests that have been reliably passing for 30+ days -Before creating any PRs or issues, check for existing open PRs in dotnet/aspnetcore that already address the same tests. Humans may also open quarantine/unquarantine PRs without the `[test-quarantine]` prefix, so do not rely solely on title matching. For each test you plan to modify, search open PRs for any that touch the same test file by looking at PR changed files. If an open PR already adds or removes a `[QuarantinedTest]` attribute for a test you were about to modify, skip that test. +Before creating any PRs or issues, check for existing open PRs in dotnet/aspnetcore that already address the same tests. Humans may also open quarantine/unquarantine PRs without the `[test-quarantine]` prefix, so do not rely solely on title matching. For each test you plan to modify, search open PRs for any that touch the same test file by looking at PR changed files. If an open PR already adds or removes a `[QuarantinedTest]` or `[QuarantinedTestData]` attribute for a test case you were about to modify, skip that test case. Also check for recently closed (not merged) `[test-quarantine]` PRs from the past 30 days that targeted the same test, and for any `[test-quarantine]` PR (open or closed) carrying the `no-quarantine-for-30-days` or `no-unquarantine-for-30-days` label — skip that test if the signal applicable to what you're about to do (quarantine vs. unquarantine) establishes a cutoff. These two labels are not interchangeable: `no-quarantine-for-30-days` only suppresses quarantine/re-quarantine, and `no-unquarantine-for-30-days` only suppresses unquarantine. See the "Important Rules" section for the exact mapping. @@ -2106,10 +2162,11 @@ The injected object has this shape: - `generated_utc` — when the data was collected. - `builds` — a compact metadata map keyed by build ID (as a string), covering every build referenced below. Each value has `def` (83 or 87), `startedUtc`/`finishedUtc`, `sourceVersion` (the commit the build ran), and `pr` (the PR number for a merged-PR build, or `null` for a `main` build). Use it for the time- and PR-based checks in Step 1.2 (below) so you never need an AzDO call. -- `source_a` — **main branch failures**: an object keyed by test name. Each value has `count` (total failures across defs 83 + 87 on `refs/heads/main` in the last 30 days), `assembly` (e.g. `InMemory.FunctionalTests--net12.0`), `builds` (every Azure DevOps build ID in which this test failed), `helix` (`{job, workitem}` Helix coordinates for the representative failure; present only when both were resolvable), and — for individual test cases — `evidence_build`, `run_id`, `result_id`, `leg`, `error`, and `stack` for the newest exact test result whose details were retrievable (failure text is capped). Individual test cases also carry `is_consistent_regression` — a precomputed boolean that is `true` **only when, on a pipeline (def) where the test failed 2 or more times on `main`, its two most recent failures were in back-to-back runs with no passing run in between**. That is the signature of a real regression (a test that recently started failing *consistently*), so a `true` value means the test must **not** be auto-quarantined under **Case A**. It is computed from the full per-pipeline `main` build timeline: a completed `main` build on that same pipeline that **succeeded or partially succeeded** (so tests actually ran), started strictly between the two failures, and in which the test did not fail, counts as a passing run that *clears* the streak (`failed`/`canceled` builds — e.g. compile/infra breaks that ran no tests — do not count as a pass). The check is conservative: a back-to-back streak on **either** pipeline sets it `true`, so an intermittent pattern on one pipeline can never mask a hard regression on another. A test with fewer than two failures on every single pipeline (e.g. it only flaked in Source B/PR builds) is `false`. +- `source_a` — **main branch failures**: an object keyed by exact test-case name. Parameterized/theory keys retain their argument list, so different rows remain distinct. Each value has `count` (total failures across defs 83 + 87 on `refs/heads/main` in the last 30 days), `assembly` (e.g. `InMemory.FunctionalTests--net12.0`), `builds` (every Azure DevOps build ID in which this exact test case failed), `helix` (`{job, workitem}` Helix coordinates for the representative failure; present only when both were resolvable), and — for individual test cases — `evidence_build`, `run_id`, `result_id`, `leg`, `error`, and `stack` for the newest exact test result whose details were retrievable (failure text is capped). Individual test cases also carry `is_consistent_regression` — a precomputed boolean that is `true` **only when, on a pipeline (def) where the test case failed 2 or more times on `main`, its two most recent failures occurred in back-to-back runs with no passing run in between**. That is the signature of a real regression (a test case that recently started failing *consistently*), so a `true` value means the test case must **not** be auto-quarantined under **Case A**. It is computed from the full per-pipeline `main` build timeline: a completed `main` build on that same pipeline that **succeeded or partially succeeded** (so tests actually ran), started strictly between the two failures, and in which the exact test case did not fail, counts as a passing run that *clears* the streak (`failed`/`canceled` builds — e.g. compile/infra breaks that ran no tests — do not count as a pass). The check is conservative: a back-to-back streak on **either** pipeline sets it `true`, so an intermittent pattern on one pipeline can never mask a hard regression on another. A test case with fewer than two failures on every single pipeline (e.g. it only flaked in Source B/PR builds) is `false`. - `source_b` — **merged-PR failures**: same shape as `source_a`, computed from the already-selected merged-into-`main` PR builds (the `Verify Source B PRs` step did the full B1–B4 selection). It may be empty (`{}`) if no qualifying PR builds failed this run. Source B captures flaky tests that only manifest in PR builds: (1) a PR retried until it passed, and (2) a PR merged on red because the only failures were unrelated flaky tests. - `source_c` — **work-item crash investigation**: a list, one entry per crashed work item (test name ending in `.WorkItemExecution`). Each entry has `workitem`, `build`, `job`, and either `fail_block_count` + `fail_blocks` (the extracted `[FAIL]` blocks from the Helix console log, capped per block and overall) or a `note` explaining why no blocks were extracted. **A work item with `fail_block_count` of 0 is almost always macOS-hang / "test host process crashed" infrastructure flakiness with no clean test-level failure — it is NOT a quarantine signal on its own; do not invent a culprit test from it.** - `source_c_truncated` — `true` if the global Source C size cap was hit and some work items were omitted; call this out in your analysis if it affects a decision. +- Source A/B individual test records also contain `queues`, mapping each build ID to the Helix job's `QueueId` for its distinct failed results. Source C records with failure blocks carry the selected job's `queue`. Job lookups are cached across all three sources. A null or unrecognized queue means that platform could not be proven (including missing identity, lookup failure, or exhaustion of the additional result-detail budget). The eligibility collector uses every retained incident's queue, never work-item names or the representative `leg`, to determine OS scope. - `trim` — present only if the whole payload approached the 1MB injection limit and optional enrichment had to be shed (e.g. `stack_dropped`, `error_dropped`). The per-test failure counts are never dropped; if you see this, error/stack for some tests may be missing and you can fetch them for a final candidate via its `helix` coordinates (Part 3). The deterministic collector also resolved current source/history and applied @@ -2145,13 +2202,27 @@ not sufficient: the build's source snapshot must contain the unquarantine commit. Do not re-quarantine a test absent from this list, even if the raw Part 1 data contains later-starting failures. +The deterministic operating-system scope for every eligible Case A or Case B +test is: + +```json +${{ needs.pre_activation.outputs.eligible_operating_systems }} +``` + +Use exactly the listed flags when adding a quarantine attribute. The collector +lists a strict subset only when every retained failure incident has one +unambiguous platform identity; otherwise it lists all three supported flags. +The safe-output validator rejects a different scope. Existing partially scoped +quarantines are treated as already quarantined and are never automatically +widened or narrowed. + **Names ending in `.WorkItemExecution` are work-item (whole-assembly) crashes, not individual tests.** Use `source_c` `fail_blocks` to find the specific `[FAIL]` test inside a crashed work item; an individual test only becomes a quarantine candidate under the rules in Step 1.2. If you later need the per-test `.log` file for a **final** candidate when writing its issue (Part 3), the `helix` `{job, workitem}` coordinates in its `source_a` / `source_b` entry let you fetch it directly (see the API Reference) — but do this only for the handful of confirmed candidates, never as part of Part 1 gathering. ### Step 1.2 — Combine and identify quarantine candidates -**IMPORTANT: Aggregate all failure data before identifying candidates, using distinct builds as the unit of evidence.** For each normalized source test method, union the Azure DevOps build IDs from Source A, Source B, and any matching Source C `[FAIL]` blocks. Count each build ID at most once for that method, even if Azure DevOps published duplicate result rows, multiple theory/parameter rows failed, or the same incident appears in more than one source. The injected Source A/B `count` fields already follow this rule, but when combining sources you **must recompute the unified count from the union of build IDs rather than adding the `count` fields**. A test with failures in two distinct builds qualifies for the 2-failure threshold; two or more rows from one build count as one incident. Do not evaluate sources separately. Only after combining all sources into a single per-test set of failure builds should you apply the cutoffs and thresholds below. +**IMPORTANT: Aggregate all failure data before identifying candidates, using distinct builds as the unit of evidence.** For each exact test case, including the argument list for a parameterized/theory row, union the Azure DevOps build IDs from Source A, Source B, and any matching Source C `[FAIL]` blocks. Count each build ID at most once for that exact case, even if Azure DevOps published duplicate result rows or the same incident appears in more than one source. Do not combine different theory rows. The injected Source A/B `count` fields already follow this rule, but when combining sources you **must recompute the unified count from the union of build IDs rather than adding the `count` fields**. A test case with failures in two distinct builds qualifies for the 2-failure threshold; duplicate rows from one build count as one incident. Do not evaluate sources separately. Only after combining all sources into a single per-test-case set of failure builds should you apply the cutoffs and thresholds below. **Mandatory evidence cutoff — perform this before Case A or Case B classification.** Perform this for every non-quarantined individual test with **at least one** raw build incident. Case B needs only one fresh incident, so do not prefilter this step using Case A's 2-build threshold. @@ -2160,7 +2231,7 @@ If you later need the per-test `.log` file for a **final** candidate when writin git log refs/remotes/origin/main --first-parent --follow -p --format="commit %H %ci %s" -- ``` Find the latest `main` commit that either: - - removed this exact test's `[QuarantinedTest]` attribute, or + - removed this exact test target's `[QuarantinedTest]` or `[QuarantinedTestData]` attribute, or - changed this exact test method/class in a way that plausibly fixes the observed failure, including a commit whose subject or patch explicitly identifies the test or failure being fixed. Do not treat an unrelated edit elsewhere in the same file as a fix. If no such commit exists, there is no history-derived cutoff. 2. Use that commit's committer timestamp as the `main` landing time. Because this is a first-parent walk of `refs/remotes/origin/main`, do not use an earlier topic-branch author/committer timestamp. @@ -2179,24 +2250,24 @@ All of the following are true: - It is an **individual test case** (not a `.WorkItemExecution`) - It has failed **2 or more times** total across all sources - It is **flaky, not a consistent regression**. Its `source_a` entry must **not** have `is_consistent_regression == true`. A `true` value means that, on a pipeline where the test failed 2+ times on `main`, its two most recent `main` failures occurred in **back-to-back runs with no passing run in between** — it is failing *consistently*, the signature of a real regression, so it must **not** be quarantined (auto-quarantining it would hide the regression). Wait until evidence of intermittency accumulates (a later passing run lands between failures) before quarantining. When `is_consistent_regression` is `false` (or absent) this gate does not block the candidate — including a test that only flaked in Source B/PR builds, which is not a `main` regression — so judge it on the other criteria. This gate applies to **Case A only**; it never applies to Case B and relaxes no other Case A requirement. -- It is **not already quarantined** (check the source code for existing `[QuarantinedTest]` attributes) +- It is **not already quarantined** at matching data-row, method, class, or assembly scope (check the source code for existing `[QuarantinedTest]` and `[QuarantinedTestData]` attributes) - It does **not** match Case B (it is not a re-quarantine of a previously unquarantined test — see Case B below; evaluate Case B first) - The failures are **not** from a PR that modified the test itself. For a Source B failure, map each of its `builds` IDs to `builds[].pr` / `builds[].sourceVersion` and, using the checked-out repo, check whether that change touched the test's file; exclude the failure if so. **Case B – Re-quarantine of a previously unquarantined test** -**Classify Case B before Case A.** A test whose latest per-test `[QuarantinedTest]` history change removed the attribute is permanently classified as **previously unquarantined / Case B** until it is quarantined again — *regardless of how long ago the unquarantine happened and regardless of whether any post-unquarantine failures remain after the evidence cutoff*. There is **no time limit**. Such a test must never fall back to Case A and receive a new issue merely because its stale pre-unquarantine failures were discarded. If it lacks the required fresh post-cutoff evidence, it is not a candidate this run. +**Classify Case B before Case A.** A test case whose latest exact-target `[QuarantinedTest]` or `[QuarantinedTestData]` history change removed the attribute is permanently classified as **previously unquarantined / Case B** until it is quarantined again — *regardless of how long ago the unquarantine happened and regardless of whether any post-unquarantine failures remain after the evidence cutoff*. There is **no time limit**. Such a test case must never fall back to Case A and receive a new issue merely because its stale pre-unquarantine failures were discarded. If it lacks the required fresh post-cutoff evidence, it is not a candidate this run. All of the following are true: - The exact fully qualified test appears in the deterministic Case B eligibility list above. This is a hard gate; manual reasoning may exclude a listed candidate because of a later relevant fix or stronger contrary evidence, but it may never add a candidate that the collector omitted. -- The test was **previously unquarantined**: it currently has **no** `[QuarantinedTest]` attribute, and the most recent commit that changed *this test's* `[QuarantinedTest]` attribute **removed** it (an unquarantine). Detect this **per-test**, not per-file — a single source file usually contains **many** tests, each with an independent quarantine history, so you **must not** key off "the file's newest `[QuarantinedTest]` commit" (that commit may belong to a different test). Inspect the test's own source file across its **full history** — do **not** add a `--since` cutoff (the old 14-day window wrongly excluded tests unquarantined more than two weeks ago). Pass `--follow` so the walk traverses renames/moves of the file (a test whose file was renamed would otherwise lose its earlier quarantine/unquarantine commits): +- The test case was **previously unquarantined**: its exact data-row/method/class/assembly target is not currently quarantined, and the most recent commit that changed that target's `[QuarantinedTest]` or `[QuarantinedTestData]` attribute **removed** it (an unquarantine). Detect this **per-target**, not per-file — a single source file usually contains **many** independently quarantined methods and data rows, so you **must not** key off the file's newest quarantine commit. Inspect the test's own source file across its **full history** — do **not** add a `--since` cutoff. Pass `--follow` so the walk traverses renames/moves of the file: ``` git log refs/remotes/origin/main --first-parent --follow -p -G 'QuarantinedTest' --format="commit %H %ci %s" -- ``` - The `refs/remotes/origin/main --first-parent` walk makes this the actual main-branch landing commit and timestamp, not an earlier topic-branch commit. The fully qualified ref avoids ambiguity with a local branch named `origin/main`. The `-p` flag prints each commit's patch inline using the file's **historical** path at that commit, so it works correctly across renames — do **not** issue a separate `git show -- `, which would return an empty diff for any commit from before a rename. Walk the matching commits from **newest to oldest**, inspecting the inline patch of each, and stop at the most recent commit whose patch **adds or removes the `[QuarantinedTest]` attribute on _this specific_ test method/class** (ignore commits that only touch *other* tests in the same file). If that commit **removed** this test's attribute, Case B applies and that commit is the **unquarantine commit**; if it **added** the attribute, the test is currently/most-recently quarantined and Case B does **not** apply. + The `refs/remotes/origin/main --first-parent` walk makes this the actual main-branch landing commit and timestamp, not an earlier topic-branch commit. The fully qualified ref avoids ambiguity with a local branch named `origin/main`. The `-p` flag prints each commit's patch inline using the file's **historical** path at that commit, so it works correctly across renames — do **not** issue a separate `git show -- `, which would return an empty diff for any commit from before a rename. Walk the matching commits from **newest to oldest**, inspecting the inline patch of each, and stop at the most recent commit whose patch **adds or removes the exact `[QuarantinedTest]` method/class/assembly target or `[QuarantinedTestData]` row target** (ignore commits that only touch other targets in the same file). If that commit removed the exact target, Case B applies; if it added the target, the test case is currently/most-recently quarantined and Case B does not apply. - It has **at least one distinct-build failure incident remaining after the mandatory evidence cutoff above**. The history-derived cutoff includes the unquarantine landing time and any later relevant fix, and the final cutoff also includes any later trusted prior-attempt decision. Failures from before that cutoff do not count — they are stale evidence from before the test was repaired/unquarantined or before a maintainer rejected an earlier attempt. If no incidents remain, do not re-quarantine and do not evaluate the test under Case A. - **Respect any prior-attempt cutoff (see the "Check for recently closed (not merged) PRs" rule).** If a trusted contributor already closed a recent re-quarantine attempt for this same test, only failures whose build `startedUtc` is strictly after that PR's `closed_at` count toward re-quarantining. If no failure post-dates that cutoff, do **not** re-quarantine this run — defer until there is fresh flakiness after the maintainer's decision. -- **Reuse the original tracking issue — do not create a new one.** Determine the issue **directly from the unquarantine commit's patch** (the inline `-p` patch already obtained above for that commit — do not run a separate `git show -- `, which fails for pre-rename commits), which shows the exact attribute that was removed. The removed line `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/N")]` names issue **N** — that is the original tracking issue to reuse in Step 3.1. Do **not** rely only on searching for a `"Quarantine {test}"`-titled issue: the original tracking issue may be a `[Known Build Error]` / `Known Build Error`-labeled issue (or otherwise not match that title), so a title search would miss it. Confirm issue **N** exists (it may be **open or closed**) before reusing it. If the removed attribute has **no valid numeric issue URL** (e.g. an empty or non-`issues/N` argument), fail closed and skip automated re-quarantine for this test. It remains Case B and must not fall back to Case A or receive a duplicate issue. +- **Reuse the original tracking issue — do not create a new one.** Determine the issue **directly from the exact `[QuarantinedTest]` or `[QuarantinedTestData]` line removed by the unquarantine commit's patch**. Its numeric issue URL names the original tracking issue to reuse in Step 3.1. Do **not** rely only on issue-title searches. Confirm the issue exists (it may be open or closed) before reusing it. If the removed attribute has no valid numeric issue URL, fail closed and skip automated re-quarantine for this test case. **Case A identity boundary** @@ -2236,37 +2307,37 @@ For each pipeline, query only builds on the **main branch**: 3. Aggregate per test name **per pipeline**: total pass count, total fail count, total "other" count, and number of builds the test appeared in. Group strictly by the **exact, full `automatedTestName`** — use the exact AzDO string as-is (for parameterized/theory tests it also includes the argument list, e.g. `...MyTests.Foo(variant: X)`; keep that intact at this stage and do not strip it). Never group by the leaf method name or a truncated display name, since different classes may declare methods that share a leaf name. The `{DeclaringClass}.{Method}` normalization used for source-identity matching happens later (Step 2.3), not here. Track these counts separately for each pipeline (84 and 87) — do not combine them. A quarantined test will only run in one of the two pipelines, so combining counts would dilute the appearance rate and cause valid candidates to be incorrectly excluded. -**Note:** Since pipeline 87 runs non-quarantined tests too, those will appear in the data but will be filtered out in Step 2.3 when we verify each candidate has a `[QuarantinedTest]` attribute in source. +**Note:** Since pipeline 87 runs non-quarantined tests too, those will appear in the data but will be filtered out in Step 2.3 when we verify each candidate has a matching `[QuarantinedTest]` or `[QuarantinedTestData]` attribute in source. ### Step 2.2 — Identify unquarantine candidates A test is a candidate for unquarantining if ALL of the following are true: -- It has a **100% pass rate** (zero failures) across the past 30 days, computed over the test's **fully-qualified name** (namespace + declaring class + method). When the same leaf method name appears under more than one fully-qualified name in the AzDO data — for example a base class and a derived subclass that overrides it (such as `Microsoft.AspNetCore.Components.E2ETest.Tests.RoutingTest.Foo` vs `Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests.ServerRoutingTest.Foo`), or two unrelated classes that each define a method named `Foo` (such as `...StartupTests.HelloWorld` vs `...HelloWorldTests.HelloWorld`) — each fully-qualified name is a **distinct test**. Never merge their pass/fail counts, and never substitute one test's results for another's (the **only** exception is the IIS multi-assembly case described below, where the *same* `{DeclaringClass}.{Method}` legitimately appears under several assembly/namespace prefixes). The pass rate for a candidate must be computed **only** from AzDO rows whose fully-qualified name resolves to that test's identity in source — i.e. the declaring class + method that carry the `[QuarantinedTest]` attribute, **or**, when the attribute is inherited from a base method, the concrete subclass rows that exercise it (see Step 2.3 for the exact subclass/base-method resolution). -- It has **real run evidence**: at least one **passing** result among the rows that **resolve to this test's source identity** — the same row set used for the pass-rate check above (the matching `{DeclaringClass}.{Method}` rows, including the allowed IIS multi-assembly prefix variants and, for an inherited base method, the concrete subclass rows). A candidate whose entire resolved row set has **zero** pass *and* zero fail rows (it never actually ran, or produced only "other"/skipped outcomes — e.g. every IIS variant was `[ConditionalFact]`-skipped) has no evidence of reliability — **fail closed and do NOT unquarantine it.** Individual variant rows that are 0 pass / 0 fail do not by themselves disqualify the candidate, as long as **at least one** row in the resolved set passed and **none** failed. Do not borrow an unrelated test's pass count to satisfy this requirement. +- It has a **100% pass rate** (zero failures) across the past 30 days for the exact source target. Method/class/assembly targets use all AzDO rows resolving to the same concrete declaring class + method; a data-row target uses only the exact matching argument list. Never merge unrelated classes or argument rows. The only prefix exception is the IIS multi-assembly case described below. +- It has **real run evidence**: at least one passing result in that same exact row set. A target whose resolved row set has zero pass and zero fail results has no evidence of reliability; fail closed. For a method/class/assembly target, skipped parameter rows are allowed if at least one row passed and none failed. For a data-row target, that exact row itself must have passed. - It does **not** have a suspiciously low total count — it appeared in at least 66% of the builds **for the pipeline that actually runs it**. Since a quarantined test only runs in one of the two pipelines (84 or 87), compare its build count against the total builds for that specific pipeline, not the combined total across both pipelines. - It is **not** `AlwaysTestTests.SuccessfulTests.GuaranteedQuarantinedTest` (this test must always stay quarantined) - It is an **individual test case**, not a work item (exclude names ending in `.WorkItemExecution`) -- The `[QuarantinedTest]` attribute has been present for **at least 60 days**. To check this, use `git log -G` with a regex matching the issue URL from the attribute to find the commit that introduced it (pass `--follow` so renames of the file are traversed): +- The exact `[QuarantinedTest]` or `[QuarantinedTestData]` target has been present for **at least 60 days**. To check this, use `git log -G` with a regex matching the issue URL from the attribute to find the commit that introduced it (pass `--follow` so renames of the file are traversed): ``` git log --follow --format="%H %ai" -1 -G 'QuarantinedTest.*{ISSUE_NUMBER}' -- {FILE_PATH} ``` If the commit date is less than 60 days ago, skip this test — it was recently quarantined and needs more time to establish reliability. -- The test has **never been re-quarantined**. A test is considered re-quarantined if its exact per-test history shows that the current `[QuarantinedTest]` attribute was added after an earlier removal. PR titles and labels are supplementary signals only; a re-quarantine can be bundled into a differently titled PR. Any confirmed re-quarantine permanently blocks automated unquarantining. To check this: +- The test has **never been re-quarantined**. A target is considered re-quarantined if its exact per-target history shows that the current `[QuarantinedTest]` or `[QuarantinedTestData]` attribute was added after an earlier removal. PR titles and labels are supplementary signals only; a re-quarantine can be bundled into a differently titled PR. Any confirmed re-quarantine permanently blocks automated unquarantining. To check this: **Check (a) first — deterministic exact-target history is authoritative.** - The pre-activation step injects one entry for every current method-, type-, - or assembly-level quarantine target: + The pre-activation step injects one entry for every current data-row, method, + type, or assembly quarantine target: ```json ${{ needs.pre_activation.outputs.requarantine_history }} ``` Match the candidate's current attribute to exactly one entry by `scope`, - `path`, `type`, `method`, and `issue`. Its `status` must be exactly + `path`, `type`, `method`, optional `data`, and `issue`. Its `status` must be exactly `first-quarantine`. If the status is `re-quarantined` or `ambiguous`, or the target is missing, duplicated, or mismatched, fail closed and do not unquarantine it. Agent reasoning may disqualify a candidate but may not override any status other than `first-quarantine`. - **Check (b) — labeled issue numbers as defense in depth.** The pre-activation step injects the numbers of every issue carrying the `re-quarantine` label as a JSON array. Read the candidate's current `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/")]` attribute in the source file, extract ``, and if `` appears in this array, the test is permanently re-quarantined — **skip it immediately, do not evaluate pass rates, and do not open an unquarantine PR.** If the list is missing or unparseable, fail closed and do not unquarantine any test. + **Check (b) — labeled issue numbers as defense in depth.** Read the numeric issue from the candidate's current `[QuarantinedTest]` or `[QuarantinedTestData]` attribute. If it appears in the injected re-quarantine issue array, skip the target immediately. **Re-quarantine issue numbers (from pre-activation step):** ```json @@ -2293,13 +2364,13 @@ A test is a candidate for unquarantining if ALL of the following are true: **Check (d) — merged re-quarantine PR diffs as defense in depth.** The re-quarantine data is injected below from the pre-activation step. Parse the JSON — it contains an array of objects, each with: - `number`: PR number - `title`: PR title - - `quarantine_entries`: array of `{filename, added_lines, patch_truncated}` — each entry represents a file where `[QuarantinedTest` was added + - `quarantine_entries`: array of `{filename, added_lines, patch_truncated}` — each entry represents a file where `[QuarantinedTest` or `[QuarantinedTestData` was added **If the data is missing (empty string or unset) or cannot be parsed as valid JSON, do NOT unquarantine any tests — fail closed and report the error.** An empty array (`[]`) is valid and means no re-quarantine PRs were found — unquarantining may proceed. For each entry's `quarantine_entries`, determine whether the re-quarantine applies to the candidate test: - If `patch_truncated` is `true`, the patch was too large for the API to return. **Fail closed**: treat this as matching any test in that file. - - Otherwise, examine `added_lines` (the actual source lines that were added). Since `[QuarantinedTest]` is an attribute placed above a method or class declaration, the added line alone won't name the target. To identify which method/class it applies to, find the matching `[QuarantinedTest` line in the current source file (by `filename`) and look at the next non-attribute, non-blank line — that will be the method or class declaration (e.g., `public async Task FooTest()` or `public class FooTests`). If the candidate test matches that declaration, it's a match. As an additional signal, if an added `[QuarantinedTest` line references an issue URL whose number is in the re-quarantine issue-number list above, and the candidate's tracking issue is that same number, treat it as a match. + - Otherwise, examine `added_lines` (the actual source lines that were added). For `[QuarantinedTest]`, resolve the following method/class declaration. For `[QuarantinedTestData]`, match both the containing method and the data arguments. If the candidate's target and issue match, it is a re-quarantine. If any check matches the candidate, this test must be permanently excluded from automated unquarantining. Only a human may unquarantine such a test. @@ -2314,7 +2385,7 @@ For IIS tests compiled into multiple assemblies (Common.LongTests, Common.Functi ### Step 2.3 — Match candidates to source code -Search the repository for `[QuarantinedTest(` attributes. The `[QuarantinedTest]` attribute can be applied at three levels: +Search the repository for `[QuarantinedTest(` and `[QuarantinedTestData(` attributes. Quarantine can be applied at four levels: 1. **Method level** — on an individual test method (most common). Example: ```csharp @@ -2331,19 +2402,22 @@ Search the repository for `[QuarantinedTest(` attributes. The `[QuarantinedTest] 3. **Assembly level** — applied via `[assembly: QuarantinedTest(...)]`, which quarantines all tests in the assembly. -For each unquarantine candidate from Step 2.2, find the corresponding `[QuarantinedTest]` attribute in source: +4. **Theory data-row level** — `[QuarantinedTestData(reason, operatingSystems, ...data)]` replaces one `[InlineData(...)]` row on a `ConditionalTheory` and quarantines only that row on the listed operating systems. + +For each unquarantine candidate from Step 2.2, find the corresponding exact quarantine target in source: -**Establish the fully-qualified identity first.** The `[QuarantinedTest]` attribute's location in source defines the test's identity: the namespace and declaring class it sits in (this is the *concrete* class — for a subclass override such as `ServerRoutingTest : RoutingTest`, the identity is the subclass `ServerRoutingTest`, **not** the base `RoutingTest`) plus the method name. The pass-rate and run-evidence checks from Step 2.2 must be evaluated against **only** the AzDO rows whose fully-qualified name resolves to that same declaring class + method (allowing for the IIS multi-assembly prefix variants noted above, which share an identical `{DeclaringClass}.{Method}` suffix and differ only by assembly/namespace prefix). If the AzDO data contains rows for a base class (or any other class) that share the leaf method name but not the declaring class, ignore them — they belong to a different test. For **parameterized/theory** tests, the AzDO `automatedTestName` carries the argument list (e.g. `...MyTests.Foo(variant: X)`); treat every parameterized row with the same `{DeclaringClass}.{Method}` as belonging to this one source method, and require **all** of them to have **zero failures** (a row that is 0 pass / 0 fail — e.g. a skipped variant — is allowed, as long as at least one row in the set passed). If, after this filtering, the test has no passing rows (or any failing row), it is **not** an unquarantine candidate; do not remove the attribute. If the `[QuarantinedTest]` attribute is on a base method that multiple concrete subclasses inherit, each concrete subclass produces its own fully-qualified rows — **every** such concrete row must have **zero failures** (again, a 0 pass / 0 fail subclass row is allowed provided at least one concrete row passed), and if the mapping from attribute to concrete rows is uncertain, fail closed and do not unquarantine. +**Establish the fully-qualified identity first.** The quarantine attribute's location in source defines the test identity: namespace, concrete declaring class, method, and — for `[QuarantinedTestData]` — the exact data arguments. The pass-rate and run-evidence checks must use only AzDO rows resolving to that identity. For a method-level `[QuarantinedTest]` on a parameterized theory, every row for that method must have zero failures and at least one row must pass. For a `[QuarantinedTestData]` target, evaluate only the exact matching argument row and require it to have at least one pass and zero failures. If row-to-source matching is ambiguous, fail closed. Base-method and IIS multi-assembly handling otherwise remain as described above. - If the attribute is on an **individual method**, unquarantine that method by removing the attribute. +- If the attribute is a **data row**, require 100% passing evidence for that exact parameterized row, then replace `[QuarantinedTestData(reason, operatingSystems, ...data)]` with `[InlineData(...data)]`. Do not use other rows from the same method as evidence. - If the attribute is on a **class**, only remove it if **every test method in that class** appears in the quarantine pipeline data with a 100% pass rate over the past 30 days. Verify by counting the distinct test methods for that class in the AzDO data and confirming all have zero failures. - If the attribute is at the **assembly level**, only remove it if every test in that assembly has 100% pass rate. This is rare and should be handled conservatively. -Extract the **issue URL** from the `QuarantinedTest` attribute argument (e.g., `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/12345")]`). +Extract the **issue URL** from the exact `QuarantinedTest` or `QuarantinedTestData` attribute. ### Step 2.4 — Group candidates by issue -Group the unquarantine candidates by their associated GitHub issue number. Extract the **issue URL** from each `QuarantinedTest` attribute argument (e.g., `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/12345")]`). +Group the unquarantine candidates by their associated GitHub issue number. Extract the issue URL from each exact quarantine attribute. **Do not create any PRs or issues yet.** Record the grouped candidates for later — they will be actioned in Part 3 after budget planning. @@ -2355,15 +2429,15 @@ Group the unquarantine candidates by their associated GitHub issue number. Extra - **Always exclude** `AlwaysTestTests.SuccessfulTests.GuaranteedQuarantinedTest` from all analysis. This test must never be unquarantined. - **Never unquarantine a test that has ever been re-quarantined.** If the deterministic exact-target first-parent history shows quarantine → unquarantine → quarantine, it is permanently excluded from automated unquarantining. PR titles and the `re-quarantine` label are defense in depth only and are not required for this classification. Only a human may unquarantine such a test. This rule applies regardless of how long the test has been passing or how many times it has been re-quarantined. - **Always exclude** tests under `Microsoft.AspNetCore.SignalR.Specification.Tests` from all analysis. These are abstract base classes inherited by other test projects — there is no good way to quarantine them, so they must be ignored entirely. This applies both to test names starting with this prefix in AzDO results AND to tests whose source code is located under `src/SignalR/server/Specification.Tests/`. A test may appear in AzDO under a different namespace (e.g., `StackExchangeRedis.Tests`) but still be defined in `Specification.Tests` — check the actual source file before quarantining. -- **`[QuarantinedTest]` attributes must reference a GitHub issue URL that *ultimately resolves* to a numeric issue number** (e.g., `https://github.com/dotnet/aspnetcore/issues/12345`). For a newly created issue (Case A) you write the `#{temporary_id}` token while editing (see below); the framework resolves it to the numeric URL before the PR is opened, so the final committed code is numeric. Never write placeholder strings, descriptive text, or any other non-numeric identifier — the only permitted non-numeric value is the required `#{temporary_id}` token. +- **`[QuarantinedTest]` and `[QuarantinedTestData]` attributes must reference a GitHub issue URL that *ultimately resolves* to a numeric issue number** (e.g., `https://github.com/dotnet/aspnetcore/issues/12345`). For a newly created issue (Case A) you write the `#{temporary_id}` token while editing; the framework resolves it before the PR is opened. Never write placeholder strings, descriptive text, or any other non-numeric identifier — the only permitted non-numeric value is the required `#{temporary_id}` token. - **For a newly created quarantine issue (Case A), you MUST write the `#{temporary_id}` reference — never a literal numeric issue number.** Here `#{temporary_id}` means a literal `#` immediately followed by the **exact** `temporary_id` string you passed to the corresponding `create_quarantine_issue` call (do **not** add any extra `aw_` prefix — the `temporary_id` already includes it). The issue's real number is assigned by the framework *after* the agent finishes, so it is impossible for you to know it while editing code. For example, if you called `create_quarantine_issue(temporary_id: "aw_http2ign", ...)`, write `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/#aw_http2ign")]`. The framework resolves `#aw_http2ign` to the real numeric URL before opening the PR, so the final committed code will contain the numeric URL. - **The `temporary_id` must be exactly the literal prefix `aw_` followed by a *slug* of 3 to 12 characters from `[A-Za-z0-9_]`** — i.e. it must match `^aw_[A-Za-z0-9_]{3,12}$` (the framework's exact rule is "'aw_' followed by 3 to 12 alphanumeric or underscore characters"). The **3–12 count applies only to the slug portion after `aw_`**, so the full token is 6–15 characters long (e.g. `aw_http2ign` = slug `http2ign`, 8 chars). This limit is enforced by the framework. If you pass a `temporary_id` whose slug is too long or otherwise malformed, the framework **silently discards it, auto-generates a *different* id, and registers your new issue under that auto-generated id** — so the `#{temporary_id}` token you wrote into the code no longer matches any registered id, is treated as a malformed reference, and is **committed verbatim as a broken placeholder**. Keep the slug short and abbreviate. The identical `temporary_id` must be used for the `create_quarantine_issue` call, the `add_comment` `item_number`, the `#{temporary_id}` in the attribute, and the `Associated issue: #{temporary_id}` in the PR body. - **A literal numeric issue URL is allowed ONLY when reusing an already-existing tracking issue (Case B re-quarantine), and only after you have confirmed in this run that the issue exists and is the original tracking issue for this test.** The authoritative way to identify it is the issue number in the `[QuarantinedTest("…/issues/N")]` line removed by the unquarantine commit (see Case B in Step 1.2); that issue is correct to reuse whether it is labeled `test-failure`, `Known Build Error`, or otherwise. The reused issue may be **closed** — a prior unquarantine PR (Step 3.2) can auto-close the tracking issue on merge, and re-quarantine still reuses that original issue. Never write a literal number for an issue you created (or will create) in this run. - **Never** use placeholder text like `TODO`, `TBD`, or descriptive strings. - **Never guess, predict, probe for, or reverse-engineer a GitHub issue number.** Do not try to discover "what number my new issue will get" by listing issues, incrementing the latest issue/PR number, or probing candidate issue numbers via the issue/PR APIs to find an "unused" one. New-issue numbers are assigned asynchronously by the framework and are unknowable while you are editing code — the only correct way to reference a newly created issue is the `#{temporary_id}` token (see above). (Looking up a **known, specific** issue number to confirm the original tracking issue for Case B reuse — identified from the unquarantine commit's removed `[QuarantinedTest]` attribute, whether labeled `test-failure` or `Known Build Error` — is fine — what is forbidden is probing for, or guessing, the number of an issue you are creating in this run.) - **Treat any "not found", "filtered", "lower integrity", "not accessible", "integrity policy", or permission-denied response from an issue or PR lookup as access-denied — NOT as evidence that an issue number is free, unused, or available.** Such responses tell you nothing about whether a number is allocated. Never conclude that a probed number is "available", and never write a probed or inferred number into code. -- **When checking the 60-day quarantine age**, verify that the `[QuarantinedTest]` attribute in the repository contains a valid numeric issue URL. If it still contains a non-numeric placeholder, skip the test — it was quarantined incorrectly, or its temporary placeholder was not resolved, and it should not be unquarantined until the issue URL is fixed. -- **Check for existing open PRs** before creating new ones. Search all open PRs for any that modify the same test file. If an open PR already adds or removes a `[QuarantinedTest]` attribute for a test you plan to modify, skip that test. +- **When checking the 60-day quarantine age**, verify that the current quarantine attribute contains a valid numeric issue URL. If it still contains a non-numeric placeholder, skip the target until the issue URL is fixed. +- **Check for existing open PRs** before creating new ones. Search all open PRs for any that modify the same test file. If an open PR already adds or removes a `[QuarantinedTest]` or `[QuarantinedTestData]` attribute for a target you plan to modify, skip that target. - **Check for recently closed (not merged) PRs and labeled PRs — apply a prior-attempt cutoff.** The pre-activation step injects, as JSON, (a) every closed-but-unmerged PR from the past 30 days whose title contains `test-quarantine` (the workflow's own previously-rejected attempts), and (b) any `test-quarantine` PR, open or closed, carrying the `no-quarantine-for-30-days` or `no-unquarantine-for-30-days` label. Each object has `number`, `title`, `body` (the PR description, capped), `closed_at` (ISO-8601 close time, or null if still open), `closed_by` (the login that closed it), `trusted_closed` (a precomputed boolean — `true` only when the login that closed the PR is a non-author, non-bot human; both source queries are restricted to PRs authored by this workflow's own bot, so closing one requires triage/write access), `quarantine_label_added_at` (ISO-8601 timestamp of the most recent time `no-quarantine-for-30-days` was added, or null), and `unquarantine_label_added_at` (same, for `no-unquarantine-for-30-days`). **This data does not rely on comment text at all** — comments are free text anyone can post regardless of permission, so they are never read or trusted for this check. `trusted_closed`, `quarantine_label_added_at`, and `unquarantine_label_added_at` are all derived only from signals that require GitHub triage/write access to produce (closing someone else's PR, or adding a label). **Use this injected data — do NOT use `search_pull_requests` (MCP: github) for this check.** The MCP tool applies a DIFC integrity filter that silently drops PRs authored by this workflow's own bot (`app/github-actions`), which is exactly how prior maintainer "do not (un)quarantine" signals get missed and a rejected PR gets re-created. For each candidate, find injected PRs that targeted the same test (match on the test method/class name in the PR `title` or `body`). A matching PR establishes: @@ -2383,7 +2457,7 @@ Group the unquarantine candidates by their associated GitHub issue number. Extra - **One PR per issue** for unquarantining. Group tests by their quarantine issue. - **One issue + one PR per exact test for Case A.** Never group new quarantines or apply a class-level Case A quarantine. Case B may reuse its one original issue only as described below. - **Never combine unrelated quarantine/unquarantine actions into a single PR.** Each quarantine action and each unquarantine action must be a separate PR. Do not bundle multiple independent test changes into one PR, even if it seems more efficient — separate PRs are easier to review, revert, and track. -- **Re-quarantine (Case B) actions must ALWAYS get their own dedicated PR.** Never combine a re-quarantine (Case B) with a new quarantine (Case A), with an unquarantine, or with a re-quarantine for a *different* issue, in the same PR. The reason is critical: the `re-quarantine` label is applied to the entire PR, and the unquarantine-exclusion check treats **every** test whose `[QuarantinedTest]` attribute is added in a PR carrying that label (or with "Re-quarantine" in the title) as permanently barred from automated unquarantining. Bundling a brand-new Case A quarantine into a re-quarantine PR would therefore silently and permanently prevent that new test from ever being auto-unquarantined. One PR may carry the `re-quarantine` label **only if every `[QuarantinedTest]` attribute it adds is a Case B re-quarantine reusing the same single issue**. +- **Re-quarantine (Case B) actions must ALWAYS get their own dedicated PR.** Never combine a re-quarantine (Case B) with a new quarantine (Case A), with an unquarantine, or with a re-quarantine for a *different* issue, in the same PR. The reason is critical: the `re-quarantine` label is applied to the entire PR, and the unquarantine-exclusion check treats **every** target whose quarantine attribute is added in a PR carrying that label (or with "Re-quarantine" in the title) as permanently barred from automated unquarantining. Bundling a brand-new Case A quarantine into a re-quarantine PR would therefore silently and permanently prevent that target from ever being auto-unquarantined. One PR may carry the `re-quarantine` label **only if every quarantine attribute it adds is a Case B re-quarantine reusing the same single issue**. - When modifying IIS tests in `Common.LongTests` or `Common.FunctionalTests`, be aware these are compiled into multiple test assemblies (IIS.FunctionalTests, IISExpress.FunctionalTests, IIS.NewHandler.FunctionalTests, IIS.NewShim.FunctionalTests). A single source change affects all variants. ## Security: Untrusted Input Handling @@ -2469,12 +2543,12 @@ Follow these rules mechanically for each PR: git clean -fd git checkout -f main ``` - `git checkout -- .` and `git reset --hard HEAD` clean the *current* branch's tracked working tree and index; `git clean -fd` removes any **untracked** files/directories a prior candidate may have created (a `reset`/`checkout` leaves those in place, so a stray new file could otherwise be `git add`-ed into the next PR — note `git clean -fd` deliberately omits `-x`, so gitignored build artifacts are preserved); the `git checkout -f main` then switches to `main` (the `-f` is a safety net that discards any residual tracked changes). Do **not** run `git checkout main` before the cleanup, and do not rely on `git checkout -- .`/`git reset --hard HEAD` alone — they do **not** switch branches, so without the `git checkout -f main` you would stay on the previous candidate's branch and its commits would leak into the next PR. Then make only this candidate's edits. Never begin a new PR's edits while a prior candidate's `[QuarantinedTest]` add/removal is still present in the working tree, the index, or the branch you are about to submit. + `git checkout -- .` and `git reset --hard HEAD` clean the *current* branch's tracked working tree and index; `git clean -fd` removes any **untracked** files/directories a prior candidate may have created (a `reset`/`checkout` leaves those in place, so a stray new file could otherwise be `git add`-ed into the next PR — note `git clean -fd` deliberately omits `-x`, so gitignored build artifacts are preserved); the `git checkout -f main` then switches to `main` (the `-f` is a safety net that discards any residual tracked changes). Do **not** run `git checkout main` before the cleanup, and do not rely on `git checkout -- .`/`git reset --hard HEAD` alone — they do **not** switch branches, so without the `git checkout -f main` you would stay on the previous candidate's branch and its commits would leak into the next PR. Then make only this candidate's edits. Never begin a new PR's edits while a prior candidate's quarantine add/removal is still present in the working tree, the index, or the branch you are about to submit. 2. **One candidate per branch, one logical change per branch history.** A Case A branch must contain changes for **only** its one exact test. If you notice a commit for a *different* test on the branch, do **not** "fix" it by adding a revert commit — that leaves both the stray commit and the revert in history. Instead, return to clean `main` (rule 1) and rebuild the branch from scratch with only this candidate's change. 3. **Pre-submit diff verification (do this before EVERY `create_pull_request`).** Run `git status`, `git diff` (or `git diff --cached`), **and `git log main..HEAD` / `git diff main...HEAD`** and confirm that both the working tree **and the branch's commit history relative to `main`**: - touch **only** the source file(s) for this one candidate, and - - contain **only** the intended `[QuarantinedTest]` addition or removal for this candidate (plus, for a quarantine, the `using Microsoft.AspNetCore.InternalTesting;` line if needed). - If the diff or `main..HEAD` history shows any unrelated file, any unrelated `[QuarantinedTest]` add/remove, or a revert of an unrelated change, **stop**: return to clean `main` (rule 1) and rebuild this PR's change from scratch. Do not submit a PR whose diff or branch history contains anything beyond this candidate's change. + - contain **only** the intended quarantine attribute addition/removal or `InlineData`/`QuarantinedTestData` replacement for this candidate (plus, for a quarantine, the `using Microsoft.AspNetCore.InternalTesting;` line if needed). + If the diff or `main..HEAD` history shows any unrelated file, any unrelated quarantine add/remove, or a revert of an unrelated change, **stop**: return to clean `main` (rule 1) and rebuild this PR's change from scratch. Do not submit a PR whose diff or branch history contains anything beyond this candidate's change. The safe-output job independently repeats this as an executable gate. It applies each authoritative patch to the deterministic `main` snapshot, derives @@ -2489,7 +2563,7 @@ For each quarantine/re-quarantine candidate, in priority order (Case B re-quaran #### Pre-PR self-check (perform before every quarantine/re-quarantine PR) -Before you call `create_pull_request` for any quarantine or re-quarantine, re-read the exact diff you are about to submit and verify **every** added `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/")]` line: +Before you call `create_pull_request` for any quarantine or re-quarantine, re-read the exact diff you are about to submit and verify **every** added `[QuarantinedTest(...)]` and `[QuarantinedTestData(...)]` line: 1. If `` is for an issue you created in this run, it **must** be `#{temporary_id}` — a literal `#` followed by the *exact* `temporary_id` you passed to a `create_quarantine_issue` call in this same run (e.g., `#aw_http2ign`; do not add an extra `aw_` prefix). A bare number here is a bug — fix it before submitting. 2. If `` is a literal number, it **must** be the original tracking issue for this test, confirmed in this run (Case B reuse only; identified from the issue URL removed by the unquarantine commit, and it may be labeled `test-failure` or `Known Build Error`; the issue may be closed). If you cannot confirm that, do not submit the PR. @@ -2504,6 +2578,15 @@ Before writing the issue body (Case A) or investigation comment (Case B), comput 1. **Failure frequency.** The size of the test's unified, post-cutoff distinct-build set from Step 1.2. Phrase it as: `Failed {N} times over the past 30 days.` 2. **Most recent failing build.** Use that same unified, post-cutoff distinct-build set, including a Source C build when it supplied the matching individual `[FAIL]` evidence. Find the build ID whose `builds[].startedUtc` (from the injected metadata map) is latest. Do not re-derive this from Source A/B counts or include a build discarded by the cutoff. +#### Select the narrowest supported quarantine target + +- For any `ConditionalTheory` row that maps unambiguously to one `[InlineData(...)]`, replace that line with `[QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/", , ...)]`. Preserve the original data arguments exactly. Never broaden an exact inline-row candidate to the whole method, even when sibling rows also qualify; each exact row remains its own candidate and PR. +- Automatic row rewrites are limited to attributes written on one physical source line. If the matching `InlineData` or `QuarantinedTestData` spans multiple lines, skip the candidate rather than reformatting it. +- Add `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/", )]` only when the candidate has no exact `ConditionalTheory`/`InlineData` mapping, such as an ordinary xUnit `Theory`, a non-parameterized test, or unsupported external/member data. Do not convert `Theory` to `ConditionalTheory`. Use exactly the flags in `eligible_operating_systems`. +- When all three flags are listed for a method target, the one-argument `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/")]` form is also allowed. +- `QuarantinedTestData` requires an operating-system value. Use exactly the listed flags; all three preserve all-platform quarantine while keeping healthy rows enabled. +- Never infer or alter platform scope yourself. The deterministic mapping is the safe-output contract. + #### Case B — Re-quarantine of a previously unquarantined test For re-quarantines, **reuse the original quarantine issue** instead of creating a new one. You identified this issue in Step 1.2. @@ -2513,7 +2596,7 @@ For re-quarantines, **reuse the original quarantine issue** instead of creating 1. **Post an investigation comment** on the **existing** issue using `add_comment` with `item_number` set to the existing numeric issue number (e.g., `item_number: 66035`). Explain that the test was unquarantined but is failing again, include the recent failure details, and note which unquarantine PR removed the attribute. Also include the **failure frequency** sentence and a link to the **most recent failing build** (`https://dev.azure.com/dnceng-public/public/_build/results?buildId={BUILD_ID}`), both computed above — do not omit these even though this is a re-quarantine of an existing issue. 2. **Create a PR** that: - - Adds `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/{ISSUE_NUMBER}")]` to the exact individual test method, using the **existing issue's numeric URL** directly (e.g., `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/66035")]`) — not a temporary ID. Do not broaden an individual Case B failure to a class- or assembly-level quarantine. + - Adds the narrowest supported quarantine target described above, using the **existing issue's numeric URL** directly — not a temporary ID. Do not broaden an individual Case B failure to a class- or assembly-level quarantine. - Adds `using Microsoft.AspNetCore.InternalTesting;` if not already present in the file - References the existing issue in the PR body with a literal issue reference (e.g., `Associated issue: #66035`). - Adds the `re-quarantine` label to the PR. **Only ever apply this label to a PR whose every change is a Case B re-quarantine for this single issue.** @@ -2550,7 +2633,7 @@ For re-quarantines, **reuse the original quarantine issue** instead of creating - Do not include potentially sensitive information such as access tokens. 3. **Create a PR** that: - - Adds `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/#{TEMPORARY_ID}")]` to the individual test method, where `{TEMPORARY_ID}` is the `temporary_id` you used when calling `create_quarantine_issue` in step 1 (e.g., `aw_http2ign`). The framework will resolve `#{TEMPORARY_ID}` to the actual numeric issue number before creating the PR. For example, if you called `create_quarantine_issue(temporary_id: "aw_http2ign", ...)`, use `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/#aw_http2ign")]`. **Never write a literal numeric issue number here** — the issue you just created does not have a number yet, and guessing or probing for one is forbidden. **Never** use placeholder text like `TODO`, `TBD`, or descriptive strings. **Before finishing, verify the token you wrote is `#` + the *exact* `temporary_id` from step 1 and that the id matches `^aw_[A-Za-z0-9_]{3,12}$`** — if it does not match this pattern the reference will not resolve and a broken placeholder will be committed. + - Adds the narrowest supported quarantine target described above, using `https://github.com/dotnet/aspnetcore/issues/#{TEMPORARY_ID}` as its reason. The framework resolves the temporary ID before creating the PR. **Never write a literal numeric issue number here**. **Before finishing, verify the token is `#` + the exact `temporary_id` and that the ID matches `^aw_[A-Za-z0-9_]{3,12}$`**. - Adds `using Microsoft.AspNetCore.InternalTesting;` if not already present in the file - References the issue in the PR body with `Associated issue: #{TEMPORARY_ID}` (using the same `temporary_id` from `create_quarantine_issue`, e.g., `Associated issue: #aw_http2ign`). Do **not** use the word `Fixes` or `Closes` — quarantine PRs open tracking issues, they do not fix them, and GitHub would auto-close the issue when the PR merges. - When referencing build IDs in the PR body, always use full clickable URLs: `https://dev.azure.com/dnceng-public/public/_build/results?buildId={BUILD_ID}&view=results`. Never reference build IDs as plain numbers. @@ -2560,21 +2643,21 @@ For re-quarantines, **reuse the original quarantine issue** instead of creating For each unquarantine candidate group (from Step 2.4), using remaining budget: -**Mandatory pre-PR re-quarantine self-check (perform before EVERY unquarantine PR — this is a hard stop).** Immediately before you call `create_pull_request` for an unquarantine, inspect the exact diff you are about to submit. For **every** removed line containing a `QuarantinedTest` attribute that references an issue URL — in **any** form, including method-level `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/N")]`, class-level, and assembly-level `[assembly: QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/N")]`: +**Mandatory pre-PR re-quarantine self-check (perform before EVERY unquarantine PR — this is a hard stop).** Immediately before you call `create_pull_request` for an unquarantine, inspect the exact diff you are about to submit. For **every** removed `[QuarantinedTest]` or `[QuarantinedTestData]` line that references an issue URL: -1. Match the exact `scope`, `path`, `type`, `method`, and numeric issue `N` against the injected `requarantine_history`. The single matching entry must have status `first-quarantine`; otherwise drop the removal. +1. Match the exact `scope`, `path`, `type`, `method`, optional `data`, and numeric issue `N` against the injected `requarantine_history`. The single matching entry must have status `first-quarantine`; otherwise drop the removal. 2. Check `N` against the injected `requarantine_issue_numbers` array. If it appears, drop the removal. -3. Repeat the exact first-parent history check from Step 2.2 for the method/class/assembly whose attribute is being removed. If the current attribute was added after an earlier removal, drop the removal even when the containing merged PR had no re-quarantine title or label. +3. Repeat the exact first-parent history check from Step 2.2 for the data row, method, class, or assembly whose attribute is being removed. If the current attribute was added after an earlier removal, drop the removal even when the containing merged PR had no re-quarantine title or label. 4. Check the injected merged-PR re-quarantine data as defense in depth. If any check is missing, unparseable, or ambiguous, **fail closed** and drop the removal. If no removals remain, abandon the PR entirely. This diff-level gate stands independently of candidate selection and must catch any re-quarantined test that slipped through earlier analysis. -1. **Create a PR** that removes the `[QuarantinedTest(...)]` attribute(s) from the test method(s) or class. Do NOT remove the `using Microsoft.AspNetCore.InternalTesting;` statement — it may be used by other attributes. +1. **Create a PR** that removes method/class/assembly `[QuarantinedTest(...)]` attributes, or replaces each data-row `[QuarantinedTestData(reason, operatingSystems, ...data)]` with `[InlineData(...data)]`. Do NOT remove the `using Microsoft.AspNetCore.InternalTesting;` statement — it may be used by other attributes. 2. In the PR body, explain that the test(s) have been passing 100% for 30+ days in the quarantined pipeline and are being unquarantined. Include a note that a maintainer/contributor can add the `no-unquarantine-for-30-days` label to this PR to tell the workflow not to touch this test again for 30 days. 3. For each issue referenced: - - Search the entire repository for any **remaining** `[QuarantinedTest]` attributes that reference that issue URL. + - Search the entire repository for any **remaining** `[QuarantinedTest]` or `[QuarantinedTestData]` attributes that reference that issue URL. - If **no other** quarantined tests reference that issue, include `Closes https://github.com/dotnet/aspnetcore/issues/{ISSUE_NUMBER}` in the PR body so the issue is automatically closed when the PR merges. Do **not** close the issue manually — let GitHub close it via the PR merge. - If other tests still reference the issue, do **not** include a `Closes` reference for it. diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs index fc14acc98ce1..de4fedb50a6f 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs @@ -1123,9 +1123,8 @@ await ServerRetryHelper.BindPortsWithRetry(async port => // Verify HTTP/2 and HTTP/3 match behavior [ConditionalTheory] [MsQuicSupported] - [InlineData(HttpProtocols.Http3)] + [QuarantinedTestData("https://github.com/dotnet/aspnetcore/issues/38008", OperatingSystems.Windows | OperatingSystems.Linux | OperatingSystems.MacOSX, HttpProtocols.Http3)] [InlineData(HttpProtocols.Http2)] - [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/38008")] public async Task POST_ClientCancellationBidirectional_RequestAbortRaised(HttpProtocols protocol) { // Arrange