From c8540223b40b2b4ac2a1b7e4911793c3d41198f8 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 29 Oct 2026 17:20:08 +0000 Subject: [PATCH] ci: check potential conflicts on a schedule instead of per PR event Each pull_request_target or pull_request_review run of predict-conflicts rewrote the advisory comments on other PRs, so the job sat in one global concurrency group. GitHub keeps one pending run per group and cancels the older pending run, and each cancelled run stays as a cancelled check on its PR's head commit, so many green PRs showed a cancelled predict_conflicts. Run on a schedule instead. Each run checks every same-base pair of open, ready PRs that touch the same files, once per pair since a merge conflict is symmetric, and rewrites each PR's comment from the results. A pair the run cannot check, for example because one PR already needs a rebase, keeps what the comments already say, and comments on PRs that are no longer open and ready are deleted. Scheduled runs are not attached to any PR, so the job no longer appears in PR checks at all. Co-Authored-By: Claude Opus 5.5 --- .../workflows/handle_potential_conflicts.py | 510 ++++++----------- .github/workflows/predict-conflicts.yml | 24 +- .../test_handle_potential_conflicts.py | 518 ++++++------------ 3 files changed, 349 insertions(+), 703 deletions(-) diff --git a/.github/workflows/handle_potential_conflicts.py b/.github/workflows/handle_potential_conflicts.py index 2acec3e56727..5d93ed59d02f 100755 --- a/.github/workflows/handle_potential_conflicts.py +++ b/.github/workflows/handle_potential_conflicts.py @@ -4,14 +4,14 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """ -Validate candidate PR conflicts and maintain advisory PR comments. +Find open PRs that conflict with each other and maintain advisory PR comments. Usage: - $ ./handle_potential_conflicts.py --pr-number + $ ./handle_potential_conflicts.py [--dry-run] -The overlap pass only finds same-base PRs that touch the same files. This -script checks whether each candidate PR can still merge after the current PR is -pre-merged, then updates one managed comment per affected PR. +Each run checks every pair of open, ready-for-review PRs that share a base +branch and touch the same files, then rewrites one managed comment per PR from +the results. A pair the run cannot check keeps what the comments already say. """ from __future__ import annotations @@ -26,6 +26,7 @@ import sys import urllib.request from dataclasses import dataclass +from datetime import datetime, timedelta, timezone from typing import Any from urllib.parse import quote @@ -35,15 +36,17 @@ COMMENT_END = "-->" TRUSTED_COMMENT_AUTHORS = {"github-actions[bot]"} MAX_FILE_DETAILS = 12 +# How long a PR that was closed or made a draft keeps being checked for a comment to delete. +CLEANUP_WINDOW = timedelta(days=1) # Error statuses, network errors and timeouts are OSError; a malformed body is ValueError. REQUEST_ERRORS = (OSError, http.client.HTTPException, ValueError) @dataclass class ManagedComment: - comment_id: int | None + comment_id: int body: str - state: dict[str, Any] + listed_prs: set[int] def github_headers() -> dict[str, str]: @@ -119,85 +122,53 @@ def github_delete(path: str) -> None: http_request("DELETE", api_url(path), github_headers()) -def normalize_state_item(item: Any) -> dict[str, Any] | None: - if not isinstance(item, dict): - return None - - try: - number = int(item["number"]) - except (KeyError, TypeError, ValueError): - return None - - files = item.get("files", []) - if not isinstance(files, list): - files = [] - - return { - "number": number, - "title": " ".join(str(item.get("title", "Unknown title")).split()), - "url": str(item.get("url") or f"https://github.com/{repo_name()}/pull/{number}"), - "files": [str(filename) for filename in files], - } - - -def normalize_state(state: Any) -> dict[str, Any]: +def listed_prs_from_state(state: Any) -> set[int]: if not isinstance(state, dict): - state = {} - - outbound = state.get("outbound", []) - inbound = state.get("inbound", {}) - - if not isinstance(outbound, list): - outbound = [] - if not isinstance(inbound, dict): - inbound = {} - - normalized_outbound = [] - for item in outbound: - normalized_item = normalize_state_item(item) - if normalized_item is not None: - normalized_outbound.append(normalized_item) - - normalized_inbound = {} - for key, value in inbound.items(): - normalized_item = normalize_state_item(value) - if normalized_item is not None: - normalized_inbound[str(key)] = normalized_item + return set() - return { - "outbound": normalized_outbound, - "inbound": normalized_inbound, - } + if "conflicts" in state: + entries = state["conflicts"] + else: + # Comments written when each PR event ran its own check stored both merge orders. + outbound = state.get("outbound") + inbound = state.get("inbound") + entries = [ + *(outbound if isinstance(outbound, list) else []), + *(inbound.values() if isinstance(inbound, dict) else []), + ] + + numbers = set() + for entry in entries if isinstance(entries, list) else []: + try: + numbers.add(int(entry["number"] if isinstance(entry, dict) else entry)) + except (KeyError, TypeError, ValueError): + continue + return numbers -def extract_state(body: str) -> dict[str, Any]: +def extract_listed_prs(body: str) -> set[int]: # The state block is rendered last, but comments written before that change # start with it. Anchor on the last marker unless the body opens with one, # so marker-like text in advisory content cannot shadow the real block. marker_index = 0 if body.startswith(COMMENT_START) else body.rfind(COMMENT_START) if marker_index == -1: - return normalize_state({}) + return set() json_start = body.find("\n", marker_index) marker_end = body.find(COMMENT_END, marker_index) if json_start == -1 or marker_end == -1 or json_start > marker_end: - return normalize_state({}) + return set() encoded_state = body[json_start:marker_end].strip() if not encoded_state: - return normalize_state({}) + return set() try: raw_state = base64.b64decode(encoded_state).decode("utf8") - return normalize_state(json.loads(raw_state)) - except (ValueError, json.JSONDecodeError): + return listed_prs_from_state(json.loads(raw_state)) + except ValueError: print("Warning: managed conflict comment has invalid state; replacing it", file=sys.stderr) - return normalize_state({}) - - -def state_is_empty(state: dict[str, Any]) -> bool: - normalized = normalize_state(state) - return not normalized["outbound"] and not normalized["inbound"] + return set() def compact_title(title: str) -> str: @@ -233,36 +204,18 @@ def format_pr_line(item: dict[str, Any]) -> str: return f"- [#{number}: {title}]({url}){format_files(files)}" -def render_comment_body(state: dict[str, Any]) -> str: - state = normalize_state(state) - state_json = json.dumps(state, sort_keys=True, separators=(",", ":")) +def render_comment_body(conflicts: list[dict[str, Any]]) -> str: + conflicts = sorted(conflicts, key=lambda item: int(item["number"])) + state_json = json.dumps({"conflicts": [int(item["number"]) for item in conflicts]}, separators=(",", ":")) state_encoded = base64.b64encode(state_json.encode("utf8")).decode("ascii") lines = [ "## Potential PR merge conflicts", "", - "This is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order.", + "This is advisory only. It does not block CI. This PR and each open PR below change the same files and " + "cannot be merged together cleanly, so whichever of the two merges second will likely need a rebase:", + "", ] - - if state["outbound"]: - lines.extend([ - "", - "### If this PR merges first", - "", - "These open PRs will likely need a rebase:", - "", - ]) - lines.extend(format_pr_line(item) for item in sorted(state["outbound"], key=lambda item: int(item["number"]))) - - if state["inbound"]: - lines.extend([ - "", - "### If these PRs merge first", - "", - "This PR will likely need a rebase:", - "", - ]) - inbound_items = sorted(state["inbound"].values(), key=lambda item: int(item["number"])) - lines.extend(format_pr_line(item) for item in inbound_items) + lines.extend(format_pr_line(item) for item in conflicts) # The state block goes last so notification previews start with the advisory text. lines.extend([ @@ -272,15 +225,7 @@ def render_comment_body(state: dict[str, Any]) -> str: COMMENT_END, ]) - return "\n".join(lines).rstrip() + "\n" - - -def get_pr_json(pr_num: int) -> dict[str, Any] | None: - try: - return github_get(f"pulls/{pr_num}") - except REQUEST_ERRORS as e: - print(f"Warning: Error fetching PR {pr_num}: {e}", file=sys.stderr) - return None + return "\n".join(lines) + "\n" def list_issue_comments(pr_num: int) -> list[dict[str, Any]]: @@ -304,51 +249,30 @@ def find_managed_comments(pr_num: int) -> list[ManagedComment]: body = comment.get("body") or "" if COMMENT_START in body: managed_comments.append(ManagedComment( - comment_id=comment.get("id"), + comment_id=comment["id"], body=body, - state=extract_state(body), + listed_prs=extract_listed_prs(body), )) return managed_comments -def select_managed_comment(managed_comments: list[ManagedComment]) -> ManagedComment: - if managed_comments: - return managed_comments[-1] - return ManagedComment(comment_id=None, body="", state=normalize_state({})) - - -def get_managed_comment(pr_num: int) -> ManagedComment: - return select_managed_comment(find_managed_comments(pr_num)) - - -def save_managed_comment(pr_num: int, state: dict[str, Any]) -> None: - managed_comments = find_managed_comments(pr_num) - managed_comment = select_managed_comment(managed_comments) - state = normalize_state(state) - - if state_is_empty(state): - for comment in managed_comments: - if comment.comment_id is not None: - github_delete(f"issues/comments/{comment.comment_id}") - if managed_comments: - print(f"Deleted stale conflict comments on PR #{pr_num}", file=sys.stderr) - return - - body = render_comment_body(state) - if managed_comment.comment_id is None: - github_post(f"issues/{pr_num}/comments", {"body": body}) - print(f"Created conflict comment on PR #{pr_num}", file=sys.stderr) - elif managed_comment.body != body: - github_patch(f"issues/comments/{managed_comment.comment_id}", {"body": body}) - print(f"Updated conflict comment on PR #{pr_num}", file=sys.stderr) - else: - print(f"Conflict comment on PR #{pr_num} is already up to date", file=sys.stderr) +def save_managed_comment(pr_num: int, managed_comments: list[ManagedComment], conflicts: list[dict[str, Any]]) -> None: + """Make the PR's managed comment list `conflicts`, or delete it if there are none.""" + kept = managed_comments[-1] if conflicts and managed_comments else None + if conflicts: + body = render_comment_body(conflicts) + if kept is None: + github_post(f"issues/{pr_num}/comments", {"body": body}) + print(f"Created conflict comment on PR #{pr_num}", file=sys.stderr) + elif kept.body != body: + github_patch(f"issues/comments/{kept.comment_id}", {"body": body}) + print(f"Updated conflict comment on PR #{pr_num}", file=sys.stderr) for comment in managed_comments: - if comment.comment_id is not None and comment.comment_id != managed_comment.comment_id: + if comment is not kept: github_delete(f"issues/comments/{comment.comment_id}") - print(f"Deleted duplicate conflict comment on PR #{pr_num}", file=sys.stderr) + print(f"Deleted {'duplicate' if conflicts else 'stale'} conflict comment on PR #{pr_num}", file=sys.stderr) def build_pr_item(pr_json: dict[str, Any], files: list[str]) -> dict[str, Any]: @@ -360,13 +284,6 @@ def build_pr_item(pr_json: dict[str, Any], files: list[str]) -> dict[str, Any]: } -def item_number(item: dict[str, Any]) -> int | None: - try: - return int(item["number"]) - except (KeyError, TypeError, ValueError): - return None - - def file_names_from_pr_file(file_info: dict[str, Any]) -> set[str]: names = set() @@ -381,45 +298,25 @@ def file_names_from_pr_file(file_info: dict[str, Any]) -> set[str]: return names -def get_pr_files(pr_num: int) -> set[str]: +def get_pr_files(pr_num: int) -> set[str] | None: files = set() - for file_info in github_get_paginated(f"pulls/{pr_num}/files?per_page=100"): - files.update(file_names_from_pr_file(file_info)) + try: + for file_info in github_get_paginated(f"pulls/{pr_num}/files?per_page=100"): + files.update(file_names_from_pr_file(file_info)) + except REQUEST_ERRORS as e: + print(f"Warning: Error fetching the files of PR {pr_num}: {e}", file=sys.stderr) + return None return files -def discover_conflict_candidates(our_pr_json: dict[str, Any]) -> list[dict[str, Any]]: - """Find same-base open PRs that touch at least one of the same files.""" - our_pr_num = int(our_pr_json["number"]) - our_base = our_pr_json["base"]["ref"] - our_files = get_pr_files(our_pr_num) - - if not our_files: - print(f"PR #{our_pr_num} has no changed files", file=sys.stderr) - return [] - - candidates = [] - open_prs = github_get_paginated(f"pulls?state=open&base={quote(our_base, safe='')}&per_page=100") - for pr_json in open_prs: - conflict_pr_num = int(pr_json["number"]) - if conflict_pr_num == our_pr_num: - continue - if pr_json.get("draft", False): - print(f"PR #{conflict_pr_num} is a draft. Skipping conflict check", file=sys.stderr) - continue - - conflict_files = get_pr_files(conflict_pr_num) - overlap = sorted(our_files & conflict_files) - if not overlap: - continue - - candidates.append({ - "number": conflict_pr_num, - "files": sorted(conflict_files), - "conflicts": overlap, - }) - - return candidates +def pr_needs_rebase(pr_num: int) -> bool | None: + """Return None if the PR could not be fetched or GitHub has not yet checked it against its base.""" + try: + mergeable_state = github_get(f"pulls/{pr_num}").get("mergeable_state") + except REQUEST_ERRORS as e: + print(f"Warning: Error fetching PR {pr_num}: {e}", file=sys.stderr) + return None + return None if mergeable_state == "unknown" else mergeable_state == "dirty" def branches_can_merge(our_pr_label: str, conflict_pr_label: str) -> bool | None: @@ -444,193 +341,106 @@ def branches_can_merge(our_pr_label: str, conflict_pr_label: str) -> bool | None return None -def validate_conflicts(candidates: list[dict[str, Any]], our_pr_json: dict[str, Any]) -> list[dict[str, Any]]: - our_pr_label = our_pr_json["head"]["label"] - validated_conflicts = [] - - for conflict in candidates: - if "number" not in conflict: - print("Warning: Skipping conflict entry without 'number' field", file=sys.stderr) - continue - - conflict_pr_num = int(conflict["number"]) - conflict_pr_json = get_pr_json(conflict_pr_num) - if conflict_pr_json is None: - print(f"Warning: Failed to fetch PR {conflict_pr_num}, skipping", file=sys.stderr) - continue - - if "head" not in conflict_pr_json or "label" not in conflict_pr_json["head"]: - print(f"Warning: Invalid PR data structure for PR {conflict_pr_num}, skipping", file=sys.stderr) - continue - - if conflict_pr_json.get("mergeable_state") == "dirty": - print(f"PR #{conflict_pr_num} already needs rebase. Skipping conflict check", file=sys.stderr) - continue - - if conflict_pr_json.get("draft", False): - print(f"PR #{conflict_pr_num} is a draft. Skipping conflict check", file=sys.stderr) - continue - - can_merge = branches_can_merge(our_pr_label, conflict_pr_json["head"]["label"]) - if can_merge is True: - print(f"PR #{conflict_pr_num} still merges after PR #{our_pr_json['number']}", file=sys.stderr) - continue - if can_merge is None: - continue - - files = conflict.get("conflicts", []) - item = build_pr_item(conflict_pr_json, files if isinstance(files, list) else []) - validated_conflicts.append(item) - - return validated_conflicts - - -def update_comments(our_pr_json: dict[str, Any], validated_conflicts: list[dict[str, Any]]) -> None: - our_pr_num = int(our_pr_json["number"]) - our_managed_comment = get_managed_comment(our_pr_num) - previous_outbound_numbers = { - number - for item in our_managed_comment.state["outbound"] - if isinstance(item, dict) - for number in [item_number(item)] - if number is not None - } - current_outbound_numbers = {int(item["number"]) for item in validated_conflicts} - - our_state = normalize_state(our_managed_comment.state) - our_state["outbound"] = validated_conflicts - save_managed_comment(our_pr_num, our_state) - - targets_to_update = previous_outbound_numbers | current_outbound_numbers - our_item_by_target = { - int(item["number"]): build_pr_item(our_pr_json, [str(filename) for filename in item.get("files", [])]) - for item in validated_conflicts - } - - for target_pr_num in sorted(targets_to_update): - target_comment = get_managed_comment(target_pr_num) - target_state = normalize_state(target_comment.state) - inbound = target_state["inbound"] - - if target_pr_num in our_item_by_target: - inbound[str(our_pr_num)] = our_item_by_target[target_pr_num] - else: - inbound.pop(str(our_pr_num), None) - - target_state["inbound"] = inbound - save_managed_comment(target_pr_num, target_state) - - -def remove_outbound_reference(state: dict[str, Any], pr_num: int) -> dict[str, Any]: - state = normalize_state(state) - outbound = [] - - for item in state["outbound"]: - if not isinstance(item, dict): - continue - if item_number(item) == pr_num: - continue - outbound.append(item) - - state["outbound"] = outbound - return state - - -def cleanup_closed_pr_comments(pr_num: int) -> None: - managed_comment = get_managed_comment(pr_num) - state = normalize_state(managed_comment.state) - outbound_numbers = { - number - for item in state["outbound"] - if isinstance(item, dict) - for number in [item_number(item)] - if number is not None - } - inbound_numbers = { - int(number) - for number in state["inbound"] - if str(number).isdigit() - } - - for target_pr_num in sorted(outbound_numbers): - target_comment = get_managed_comment(target_pr_num) - target_state = normalize_state(target_comment.state) - target_state["inbound"].pop(str(pr_num), None) - save_managed_comment(target_pr_num, target_state) - - for source_pr_num in sorted(inbound_numbers): - source_comment = get_managed_comment(source_pr_num) - source_state = remove_outbound_reference(source_comment.state, pr_num) - save_managed_comment(source_pr_num, source_state) - - deleted_comment = False - for comment in find_managed_comments(pr_num): - if comment.comment_id is not None: - github_delete(f"issues/comments/{comment.comment_id}") - deleted_comment = True - if deleted_comment: - print(f"Deleted conflict comment on closed PR #{pr_num}", file=sys.stderr) +def find_conflicts(prs: list[dict[str, Any]]) -> tuple[dict[int, list[dict[str, Any]]], dict[int, dict[int, dict[str, Any]]]]: + """Return the PRs each PR conflicts with, and the PRs each one overlaps but could not be checked against.""" + files = {int(pr["number"]): get_pr_files(int(pr["number"])) for pr in prs} + conflicts: dict[int, list[dict[str, Any]]] = {number: [] for number in files} + unchecked: dict[int, dict[int, dict[str, Any]]] = {number: {} for number in files} + needs_rebase: dict[int, bool | None] = {} + + for index, pr in enumerate(prs): + for other in prs[index + 1:]: + if pr["base"]["ref"] != other["base"]["ref"]: + continue + a, b = int(pr["number"]), int(other["number"]) + files_a, files_b = files[a], files[b] + if files_a is None or files_b is None: + # Without both file lists the overlap is unknown, so the pair stays unchecked. + overlap = [] + else: + overlap = sorted(files_a & files_b) + if not overlap: + continue + + can_merge = None + if overlap: + for number in (a, b): + if number not in needs_rebase: + needs_rebase[number] = pr_needs_rebase(number) + # A PR that already needs a rebase is checked again once it is rebased. + if needs_rebase[a] is False and needs_rebase[b] is False: + # A merge conflict is symmetric, so one check covers both merge orders. + can_merge = branches_can_merge(pr["head"]["label"], other["head"]["label"]) + + if can_merge is None: + unchecked[a][b] = build_pr_item(other, overlap) + unchecked[b][a] = build_pr_item(pr, overlap) + elif not can_merge: + conflicts[a].append(build_pr_item(other, overlap)) + conflicts[b].append(build_pr_item(pr, overlap)) + + return conflicts, unchecked + + +def recently_updated(pr: dict[str, Any], since: datetime) -> bool: + return datetime.fromisoformat(pr["updated_at"].replace("Z", "+00:00")) >= since def parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser( - description="Validate potential PR conflicts and maintain advisory comments.", - ) - parser.add_argument( - "--pr-number", - type=int, - required=True, - help="Pull request number to inspect.", + description="Find conflicting open PRs and maintain advisory comments.", ) parser.add_argument( "--dry-run", action="store_true", - help="Print the current PR comment body instead of creating, updating, or deleting comments.", - ) - parser.add_argument( - "--cleanup-closed", - action="store_true", - help="Remove advisory state for a PR that has been closed.", + help="Print the comment bodies instead of creating, updating, or deleting comments.", ) return parser.parse_args() def main() -> None: args = parse_args() - our_pr_num = args.pr_number - - if args.cleanup_closed: + open_prs = github_get_paginated("pulls?state=open&per_page=100") + ready_prs = [pr for pr in open_prs if not pr.get("draft", False)] + # Closing a PR or making it a draft updates it, so this finds every PR that left the + # checked set lately, even if no comment lists it or an earlier run failed to clean it. + since = datetime.now(timezone.utc) - CLEANUP_WINDOW + closed_prs = github_get("pulls?state=closed&sort=updated&direction=desc&per_page=100") + left_prs = [pr for pr in open_prs if pr.get("draft", False)] + closed_prs + recently_left = {int(pr["number"]) for pr in left_prs if recently_updated(pr, since)} + conflicts, unchecked = find_conflicts(ready_prs) + + failed = [] + previously_listed = set() + for number, items in sorted(conflicts.items()): + try: + managed_comments = find_managed_comments(number) + listed = set().union(*(comment.listed_prs for comment in managed_comments)) + previously_listed |= listed + # Keep what the comment already says about PRs this run could not check. + items = items + [item for other, item in unchecked[number].items() if other in listed] + if args.dry_run: + print(f"PR #{number}:\n{render_comment_body(items) if items else 'no comment'}\n") + else: + save_managed_comment(number, managed_comments, items) + except REQUEST_ERRORS as e: + print(f"Warning: could not update the conflict comment on PR #{number}: {e}", file=sys.stderr) + failed.append(number) + + # Drafts and closed PRs are not checked; clear any comment they still have + # from when they were open and ready for review. + for number in sorted((previously_listed | recently_left) - conflicts.keys()): if args.dry_run: - print(f"Would clean advisory conflict comments for closed PR #{our_pr_num}") - else: - cleanup_closed_pr_comments(our_pr_num) - return - - our_pr_json = get_pr_json(our_pr_num) - - if our_pr_json is None: - print(f"Error: Failed to fetch PR {our_pr_num}", file=sys.stderr) - sys.exit(1) - - if "head" not in our_pr_json or "label" not in our_pr_json["head"]: - print(f"Error: Invalid PR data structure for PR {our_pr_num}", file=sys.stderr) - sys.exit(1) - - if our_pr_json.get("draft", False): - if args.dry_run: - print(f"Would clean advisory conflict comments for draft PR #{our_pr_num}") - else: - cleanup_closed_pr_comments(our_pr_num) - return - - candidates = discover_conflict_candidates(our_pr_json) - validated_conflicts = validate_conflicts(candidates, our_pr_json) - print(f"Conflicting PRs after validation: {[item['number'] for item in validated_conflicts]}", file=sys.stderr) - - if args.dry_run: - print(render_comment_body({"outbound": validated_conflicts, "inbound": {}})) - else: - update_comments(our_pr_json, validated_conflicts) + print(f"PR #{number}: delete any conflict comment\n") + continue + try: + save_managed_comment(number, find_managed_comments(number), []) + except REQUEST_ERRORS as e: + print(f"Warning: could not delete the conflict comment on PR #{number}: {e}", file=sys.stderr) + failed.append(number) + + if failed: + sys.exit(f"Error: could not update conflict comments on PRs {failed}") if __name__ == "__main__": diff --git a/.github/workflows/predict-conflicts.yml b/.github/workflows/predict-conflicts.yml index d2191ed245e4..f9bf9dde931c 100644 --- a/.github/workflows/predict-conflicts.yml +++ b/.github/workflows/predict-conflicts.yml @@ -1,10 +1,11 @@ name: "Check Potential Conflicts" +# A scheduled run is not attached to any PR, so a slow or cancelled run never +# shows up in a PR's checks. Each run rechecks every open PR. on: - pull_request_target: # zizmor: ignore[dangerous-triggers] needs a write token to comment on fork PRs; runs base-branch code only - types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed] - pull_request_review: - types: [submitted] + schedule: + - cron: '7,37 * * * *' + workflow_dispatch: permissions: contents: read @@ -21,7 +22,10 @@ permissions: jobs: predict_conflicts: + # Forks also run scheduled workflows. + if: github.repository == 'dashpay/dash' runs-on: ${{ vars.DASH_LIGHT_RUNNER || 'ubuntu-latest' }} + timeout-minutes: 20 concurrency: group: predict-conflicts cancel-in-progress: false @@ -30,15 +34,7 @@ jobs: uses: actions/checkout@v6 with: persist-credentials: false - - name: Validate potential conflicts and update advisory comments - continue-on-error: true + - name: Find conflicting PRs and update advisory comments env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - EVENT_ACTION: ${{ github.event.action }} - PR_NUMBER: ${{ github.event.pull_request.number }} - run: | - if [ "$EVENT_ACTION" = "closed" ]; then - .github/workflows/handle_potential_conflicts.py --pr-number "$PR_NUMBER" --cleanup-closed - else - .github/workflows/handle_potential_conflicts.py --pr-number "$PR_NUMBER" - fi + run: .github/workflows/handle_potential_conflicts.py diff --git a/.github/workflows/test_handle_potential_conflicts.py b/.github/workflows/test_handle_potential_conflicts.py index 2cf5fc04316a..098c06c6bf48 100755 --- a/.github/workflows/test_handle_potential_conflicts.py +++ b/.github/workflows/test_handle_potential_conflicts.py @@ -3,11 +3,15 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. +import argparse +import base64 +from datetime import datetime, timedelta, timezone import importlib.util +import json from pathlib import Path -import argparse import sys import unittest +from unittest import mock SCRIPT_PATH = Path(__file__).with_name("handle_potential_conflicts.py") @@ -29,372 +33,208 @@ def comment(comment_id, body, user=None): } -class TestPotentialConflictComments(unittest.TestCase): - def test_extract_state_from_managed_comment(self): - body = handle_potential_conflicts.render_comment_body({ - "outbound": [ - {"number": 2, "title": "two", "url": "https://example.test/2", "files": []}, - ], - "inbound": { - "3": {"number": 3, "title": "three", "url": "https://example.test/3", "files": ["src/a.cpp"]}, - }, - }) +def item(number, title="title", files=()): + return { + "number": number, + "title": title, + "url": f"https://github.com/dashpay/dash/pull/{number}", + "files": list(files), + } - state = handle_potential_conflicts.extract_state(body) - self.assertEqual(2, state["outbound"][0]["number"]) - self.assertEqual(3, state["inbound"]["3"]["number"]) +LONG_AGO = "2020-01-01T00:00:00Z" +RECENTLY = (datetime.now(timezone.utc) - timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ") - def test_invalid_state_is_treated_as_empty(self): - body = """ -## Potential PR merge conflicts -""" - state = handle_potential_conflicts.extract_state(body) +def pr(number, base="develop", draft=False, updated_at=LONG_AGO): + return { + "number": number, + "title": f"pr {number}", + "html_url": f"https://github.com/dashpay/dash/pull/{number}", + "base": {"ref": base}, + "head": {"label": f"contributor:branch-{number}"}, + "draft": draft, + "updated_at": updated_at, + } - self.assertEqual([], state["outbound"]) - self.assertEqual({}, state["inbound"]) - def test_render_comment_body_has_one_marker_and_both_sections(self): - state = { - "outbound": [ - { - "number": 12, - "title": "fix: target", - "url": "https://github.com/dashpay/dash/pull/12", - "files": ["src/net.cpp"], - } - ], - "inbound": { - "9": { - "number": 9, - "title": "refactor: source", - "url": "https://github.com/dashpay/dash/pull/9", - "files": ["src/net.cpp"], - } - }, - } +def legacy_body(state): + encoded = base64.b64encode(json.dumps(state).encode()).decode() + return f"## Potential PR merge conflicts\n\n{handle_potential_conflicts.COMMENT_START}\n{encoded}\n-->\n" - body = handle_potential_conflicts.render_comment_body(state) - self.assertEqual(1, body.count("dash-potential-conflicts:v1")) - self.assertIn("If this PR merges first", body) - self.assertIn("If these PRs merge first", body) - self.assertIn("[#12: fix: target](https://github.com/dashpay/dash/pull/12)", body) - self.assertIn("[#9: refactor: source](https://github.com/dashpay/dash/pull/9)", body) +def managed(comment_id, conflicts): + body = handle_potential_conflicts.render_comment_body(conflicts) + return handle_potential_conflicts.ManagedComment(comment_id, body, {int(entry["number"]) for entry in conflicts}) - def test_rendered_comment_state_round_trips(self): - state = { - "outbound": [ - { - "number": 12, - "title": f"title with {handle_potential_conflicts.COMMENT_START} --> marker-like text", - "url": "https://github.com/dashpay/dash/pull/12", - "files": ["src/net.cpp"], - } - ], - "inbound": {}, - } - body = handle_potential_conflicts.render_comment_body(state) - extracted = handle_potential_conflicts.extract_state(body) +def numbers(conflicts): + return {number: [entry["number"] for entry in items] for number, items in conflicts.items()} + + +class TestPotentialConflictComments(unittest.TestCase): + def test_rendered_comment_lists_prs_and_round_trips(self): + title = f"title with {handle_potential_conflicts.COMMENT_START} --> marker-like text" + body = handle_potential_conflicts.render_comment_body([item(12, title, ["src/net.cpp"]), item(9, "refactor: source")]) - self.assertEqual(state, extracted) + self.assertEqual({9, 12}, handle_potential_conflicts.extract_listed_prs(body)) self.assertTrue(body.startswith("## Potential PR merge conflicts")) self.assertTrue(body.rstrip("\n").endswith(handle_potential_conflicts.COMMENT_END)) + self.assertLess(body.index("[#9: refactor: source]"), body.index("[#12: title with")) - def test_extract_state_from_legacy_marker_first_comment(self): + def test_extract_listed_prs_from_legacy_comments(self): state = { - "outbound": [ - {"number": 2, "title": "two", "url": "https://example.test/2", "files": []}, - ], - "inbound": {}, + "outbound": [item(2), {"title": "missing number"}, "not-a-dict"], + "inbound": {"3": item(3), "bad": {"title": "missing number"}}, } - body = handle_potential_conflicts.render_comment_body(state) + body = legacy_body(state) marker_index = body.rfind(handle_potential_conflicts.COMMENT_START) - legacy_body = body[marker_index:] + body[:marker_index] + marker_first_body = body[marker_index:] + body[:marker_index] - self.assertTrue(legacy_body.startswith(handle_potential_conflicts.COMMENT_START)) - self.assertEqual(state, handle_potential_conflicts.extract_state(legacy_body)) + self.assertEqual({2, 3}, handle_potential_conflicts.extract_listed_prs(body)) + self.assertEqual({2, 3}, handle_potential_conflicts.extract_listed_prs(marker_first_body)) + + def test_invalid_state_is_treated_as_empty(self): + body = """ +## Potential PR merge conflicts +""" + + self.assertEqual(set(), handle_potential_conflicts.extract_listed_prs(body)) def test_rendered_comment_escapes_markdown_content(self): - body = handle_potential_conflicts.render_comment_body({ - "outbound": [ - { - "number": 12, - "title": "fix [link] title", - "url": "https://github.com/dashpay/dash/pull/12", - "files": ["src/`odd`.cpp"], - }, - ], - "inbound": {}, - }) + body = handle_potential_conflicts.render_comment_body([item(12, "fix [link] title", ["src/`odd`.cpp"])]) self.assertIn(r"[#12: fix \[link\] title](https://github.com/dashpay/dash/pull/12)", body) self.assertIn("src/`odd`.cpp", body) - def test_normalize_state_filters_malformed_entries(self): - state = handle_potential_conflicts.normalize_state({ - "outbound": [ - {"number": "12", "title": "valid", "url": "https://example.test/12", "files": ["src/a.cpp"]}, - {"title": "missing number"}, - "not-a-dict", - ], - "inbound": { - "bad": {"title": "missing number"}, - "13": {"number": "13", "title": "valid inbound", "url": "https://example.test/13", "files": []}, - }, - }) - - self.assertEqual([12], [item["number"] for item in state["outbound"]]) - self.assertEqual(["13"], list(state["inbound"].keys())) - - def test_state_is_empty_only_when_both_directions_are_empty(self): - self.assertTrue(handle_potential_conflicts.state_is_empty({"outbound": [], "inbound": {}})) - self.assertFalse(handle_potential_conflicts.state_is_empty({"outbound": [{"number": 1}], "inbound": {}})) - self.assertFalse(handle_potential_conflicts.state_is_empty({"outbound": [], "inbound": {"2": {"number": 2}}})) - def test_user_forged_managed_comment_is_ignored(self): - body = handle_potential_conflicts.render_comment_body({ - "outbound": [{"number": 12, "title": "forged", "url": "https://example.test/12", "files": []}], - "inbound": {}, - }) - comments = [comment(123, body, USER)] - original_list_issue_comments = handle_potential_conflicts.list_issue_comments - handle_potential_conflicts.list_issue_comments = lambda pr_num: comments - try: - managed_comments = handle_potential_conflicts.find_managed_comments(1) - finally: - handle_potential_conflicts.list_issue_comments = original_list_issue_comments - - self.assertEqual([], managed_comments) - - def test_update_comments_replaces_reciprocal_entries(self): - our_pr_json = { - "number": 10, - "title": "fix: source", - "html_url": "https://github.com/dashpay/dash/pull/10", - } - validated_conflicts = [ - { - "number": 12, - "title": "fix: new target", - "url": "https://github.com/dashpay/dash/pull/12", - "files": ["src/net.cpp"], - } - ] + body = handle_potential_conflicts.render_comment_body([item(12, "forged")]) + with mock.patch.object(handle_potential_conflicts, "list_issue_comments", return_value=[comment(123, body, USER)]): + self.assertEqual([], handle_potential_conflicts.find_managed_comments(1)) + + def save(self, comments, conflicts): + with mock.patch.object(handle_potential_conflicts, "github_patch") as patch, \ + mock.patch.object(handle_potential_conflicts, "github_delete") as delete, \ + mock.patch.object(handle_potential_conflicts, "github_post") as post: + handle_potential_conflicts.save_managed_comment(10, comments, conflicts) + return ([call.args[0] for call in post.call_args_list], + [call.args[0] for call in patch.call_args_list], + [call.args[0] for call in delete.call_args_list]) + + def test_save_managed_comment_writes_only_what_changed(self): + self.assertEqual((["issues/10/comments"], [], []), self.save([], [item(13)])) + self.assertEqual(([], ["issues/comments/3"], ["issues/comments/1"]), + self.save([managed(1, [item(11)]), managed(3, [item(12)])], [item(13)])) + self.assertEqual(([], [], []), self.save([managed(3, [item(13)])], [item(13)])) + self.assertEqual(([], [], ["issues/comments/1", "issues/comments/3"]), + self.save([managed(1, [item(11)]), managed(3, [item(12)])], [])) + + def find_conflicts(self, prs, files, mergeable_state, can_merge): + def github_get(path): + number = int(path.removeprefix("pulls/")) + if mergeable_state[number] is None: + raise OSError("boom") + return {"mergeable_state": mergeable_state[number]} + + with mock.patch.object(handle_potential_conflicts, "get_pr_files", side_effect=files.get), \ + mock.patch.object(handle_potential_conflicts, "github_get", side_effect=github_get), \ + mock.patch.object(handle_potential_conflicts, "branches_can_merge", + side_effect=lambda a, b: can_merge[(int(a.split("-")[-1]), int(b.split("-")[-1]))]) as check: + conflicts, unchecked = handle_potential_conflicts.find_conflicts(prs) + + checked = [tuple(int(label.split("-")[-1]) for label in call.args) for call in check.call_args_list] + return checked, numbers(conflicts), {number: sorted(items) for number, items in unchecked.items() if items} + + def test_find_conflicts_checks_each_overlapping_pair_once(self): + # 4 shares a file with 1 but targets another base; 5 shares nothing with 1-3. + prs = [pr(1), pr(2), pr(3), pr(4, base="v23.x"), pr(5)] + files = {1: {"a"}, 2: {"a", "b"}, 3: {"b"}, 4: {"a"}, 5: {"c"}} + mergeable_state = {1: "clean", 2: "blocked", 3: "unstable", 5: "clean"} + checked, conflicts, unchecked = self.find_conflicts(prs, files, mergeable_state, {(1, 2): False, (2, 3): True}) + + self.assertEqual([(1, 2), (2, 3)], checked) + self.assertEqual({1: [2], 2: [1], 3: [], 4: [], 5: []}, conflicts) + self.assertEqual({}, unchecked) + + def test_find_conflicts_leaves_pairs_it_cannot_check_unchecked(self): + # The merge check fails for 1 and 2; 3 needs a rebase, 4 could not be fetched, + # GitHub has not yet checked 5 against its base, and 8's files could not be listed, + # so their pairs are never checked. + prs = [pr(number) for number in range(1, 9)] + [pr(9, base="v23.x")] + files = {1: {"a"}, 2: {"a"}, 3: {"b"}, 4: {"b"}, 5: {"c"}, 6: {"c"}, 7: {"b"}, 8: None, 9: {"a"}} + mergeable_state = {1: "clean", 2: "clean", 3: "dirty", 4: None, 5: "unknown", 6: "clean", 7: "clean"} + checked, conflicts, unchecked = self.find_conflicts(prs, files, mergeable_state, {(1, 2): None}) + + self.assertEqual([(1, 2)], checked) + self.assertEqual({number: [] for number in range(1, 10)}, conflicts) + self.assertEqual({1: [2, 8], 2: [1, 8], 3: [4, 7, 8], 4: [3, 7, 8], 5: [6, 8], 6: [5, 8], 7: [3, 4, 8], + 8: [1, 2, 3, 4, 5, 6, 7]}, unchecked) + + with mock.patch.object(handle_potential_conflicts, "github_get_paginated", side_effect=OSError("boom")): + self.assertIsNone(handle_potential_conflicts.get_pr_files(8)) + + def run_main(self, open_prs, conflicts, unchecked, find_managed_comments, closed_prs=(), dry_run=False): + saved = [] + exit_code = None + with mock.patch.object(handle_potential_conflicts, "parse_args", return_value=argparse.Namespace(dry_run=dry_run)), \ + mock.patch.object(handle_potential_conflicts, "github_get_paginated", return_value=open_prs), \ + mock.patch.object(handle_potential_conflicts, "github_get", return_value=list(closed_prs)), \ + mock.patch.object(handle_potential_conflicts, "find_conflicts", return_value=(conflicts, unchecked)) as find, \ + mock.patch.object(handle_potential_conflicts, "find_managed_comments", side_effect=find_managed_comments), \ + mock.patch.object(handle_potential_conflicts, "save_managed_comment", + side_effect=lambda number, _comments, items: saved.append((number, [entry["number"] for entry in items]))), \ + mock.patch("sys.stdout"): + try: + handle_potential_conflicts.main() + except SystemExit as e: + exit_code = e.code + return [entry["number"] for entry in find.call_args.args[0]], saved, exit_code + + def test_main_rewrites_comments_and_clears_prs_no_longer_checked(self): + # 7 became a draft and 8 was closed since the comments were written. 1 and 3 could + # not be checked against 4 this run; only 1's comment already listed 4. 5 and 6 listed + # only each other and both closed recently, and 9 was made a draft recently, so nothing + # lists them any more; 10 was closed long ago. comments = { - 10: handle_potential_conflicts.ManagedComment( - comment_id=1, - body="", - state={ - "outbound": [ - { - "number": 11, - "title": "fix: stale target", - "url": "https://github.com/dashpay/dash/pull/11", - "files": ["src/old.cpp"], - } - ], - "inbound": { - "8": { - "number": 8, - "title": "fix: incoming", - "url": "https://github.com/dashpay/dash/pull/8", - "files": ["src/in.cpp"], - } - }, - }, - ), - 11: handle_potential_conflicts.ManagedComment(comment_id=2, body="", state={"outbound": [], "inbound": {"10": {"number": 10}}}), - 12: handle_potential_conflicts.ManagedComment(comment_id=3, body="", state={"outbound": [], "inbound": {}}), + 1: [managed(11, [item(2), item(4), item(7)])], + 2: [managed(12, [item(1), item(8)])], + 7: [managed(17, [item(1)])], + 8: [managed(18, [item(2)])], } - saved = {} - original_get_managed_comment = handle_potential_conflicts.get_managed_comment - original_save_managed_comment = handle_potential_conflicts.save_managed_comment - handle_potential_conflicts.get_managed_comment = lambda pr_num: comments[pr_num] - handle_potential_conflicts.save_managed_comment = lambda pr_num, state: saved.setdefault(pr_num, state) - try: - handle_potential_conflicts.update_comments(our_pr_json, validated_conflicts) - finally: - handle_potential_conflicts.get_managed_comment = original_get_managed_comment - handle_potential_conflicts.save_managed_comment = original_save_managed_comment - - self.assertEqual([12], [item["number"] for item in saved[10]["outbound"]]) - self.assertIn("8", saved[10]["inbound"]) - self.assertNotIn("10", saved[11]["inbound"]) - self.assertEqual(10, saved[12]["inbound"]["10"]["number"]) - - def test_cleanup_closed_pr_removes_bidirectional_state(self): - comments = { - 10: handle_potential_conflicts.ManagedComment( - comment_id=10, - body="managed", - state={ - "outbound": [ - { - "number": 12, - "title": "fix: target", - "url": "https://github.com/dashpay/dash/pull/12", - "files": ["src/net.cpp"], - } - ], - "inbound": { - "8": { - "number": 8, - "title": "fix: source", - "url": "https://github.com/dashpay/dash/pull/8", - "files": ["src/net.cpp"], - } - }, - }, - ), - 8: handle_potential_conflicts.ManagedComment( - comment_id=8, - body="managed", - state={ - "outbound": [ - {"number": 10, "title": "fix: closed target", "url": "https://github.com/dashpay/dash/pull/10", "files": []}, - {"number": 11, "title": "fix: other target", "url": "https://github.com/dashpay/dash/pull/11", "files": []}, - ], - "inbound": {}, - }, - ), - 12: handle_potential_conflicts.ManagedComment( - comment_id=12, - body="managed", - state={"outbound": [], "inbound": {"10": {"number": 10}}}, - ), - } - saved = {} - deleted = [] - original_get_managed_comment = handle_potential_conflicts.get_managed_comment - original_find_managed_comments = handle_potential_conflicts.find_managed_comments - original_save_managed_comment = handle_potential_conflicts.save_managed_comment - original_github_delete = handle_potential_conflicts.github_delete - handle_potential_conflicts.get_managed_comment = lambda pr_num: comments[pr_num] - handle_potential_conflicts.find_managed_comments = lambda pr_num: [comments[pr_num]] - handle_potential_conflicts.save_managed_comment = lambda pr_num, state: saved.setdefault(pr_num, state) - handle_potential_conflicts.github_delete = lambda path: deleted.append(path) - try: - handle_potential_conflicts.cleanup_closed_pr_comments(10) - finally: - handle_potential_conflicts.get_managed_comment = original_get_managed_comment - handle_potential_conflicts.find_managed_comments = original_find_managed_comments - handle_potential_conflicts.save_managed_comment = original_save_managed_comment - handle_potential_conflicts.github_delete = original_github_delete - - self.assertEqual([11], [item["number"] for item in saved[8]["outbound"]]) - self.assertNotIn("10", saved[12]["inbound"]) - self.assertEqual(["issues/comments/10"], deleted) - - def test_comment_fetch_failure_prevents_comment_creation(self): - posted = [] - original_list_issue_comments = handle_potential_conflicts.list_issue_comments - original_github_post = handle_potential_conflicts.github_post - handle_potential_conflicts.list_issue_comments = lambda pr_num: (_ for _ in ()).throw(OSError("boom")) - handle_potential_conflicts.github_post = lambda path, payload: posted.append((path, payload)) - try: - with self.assertRaises(OSError): - handle_potential_conflicts.save_managed_comment( - 10, - { - "outbound": [ - { - "number": 12, - "title": "fix: target", - "url": "https://github.com/dashpay/dash/pull/12", - "files": [], - } - ], - "inbound": {}, - }, - ) - finally: - handle_potential_conflicts.list_issue_comments = original_list_issue_comments - handle_potential_conflicts.github_post = original_github_post - - self.assertEqual([], posted) - - def test_main_cleans_current_draft_pr(self): - cleaned = [] - original_parse_args = handle_potential_conflicts.parse_args - original_get_pr_json = handle_potential_conflicts.get_pr_json - original_cleanup_closed_pr_comments = handle_potential_conflicts.cleanup_closed_pr_comments - handle_potential_conflicts.parse_args = lambda: argparse.Namespace( - pr_number=10, - dry_run=False, - cleanup_closed=False, + checked, saved, exit_code = self.run_main( + [pr(1), pr(2), pr(3), pr(4), pr(7, draft=True), pr(9, draft=True, updated_at=RECENTLY)], + {1: [item(2)], 2: [item(1)], 3: [], 4: []}, + {1: {4: item(4)}, 2: {}, 3: {4: item(4)}, 4: {1: item(1), 3: item(3)}}, + lambda number: comments.get(number, []), + closed_prs=[pr(5, updated_at=RECENTLY), pr(6, updated_at=RECENTLY), pr(10)], ) - handle_potential_conflicts.get_pr_json = lambda pr_num: { - "number": pr_num, - "draft": True, - "head": {"label": "contributor:branch"}, - } - handle_potential_conflicts.cleanup_closed_pr_comments = lambda pr_num: cleaned.append(pr_num) - try: - handle_potential_conflicts.main() - finally: - handle_potential_conflicts.parse_args = original_parse_args - handle_potential_conflicts.get_pr_json = original_get_pr_json - handle_potential_conflicts.cleanup_closed_pr_comments = original_cleanup_closed_pr_comments - - self.assertEqual([10], cleaned) - - def test_save_managed_comment_deletes_duplicate_managed_comments(self): - comments = [ - comment( - 1, - handle_potential_conflicts.render_comment_body({ - "outbound": [{"number": 11, "title": "old", "url": "https://example.test/11", "files": []}], - "inbound": {}, - }), - ), - comment( - 3, - handle_potential_conflicts.render_comment_body({ - "outbound": [{"number": 12, "title": "old", "url": "https://example.test/12", "files": []}], - "inbound": {}, - }), - ), - ] - patched = [] - deleted = [] - posted = [] - original_list_issue_comments = handle_potential_conflicts.list_issue_comments - original_github_patch = handle_potential_conflicts.github_patch - original_github_delete = handle_potential_conflicts.github_delete - original_github_post = handle_potential_conflicts.github_post - handle_potential_conflicts.list_issue_comments = lambda pr_num: comments - handle_potential_conflicts.github_patch = lambda path, payload: patched.append((path, payload)) - handle_potential_conflicts.github_delete = lambda path: deleted.append(path) - handle_potential_conflicts.github_post = lambda path, payload: posted.append((path, payload)) - try: - handle_potential_conflicts.save_managed_comment( - 10, - { - "outbound": [ - { - "number": 13, - "title": "new", - "url": "https://example.test/13", - "files": [], - } - ], - "inbound": {}, - }, - ) - finally: - handle_potential_conflicts.list_issue_comments = original_list_issue_comments - handle_potential_conflicts.github_patch = original_github_patch - handle_potential_conflicts.github_delete = original_github_delete - handle_potential_conflicts.github_post = original_github_post - - self.assertEqual(["issues/comments/3"], [path for path, _payload in patched]) - self.assertEqual(["issues/comments/1"], deleted) - self.assertEqual([], posted) + + self.assertEqual([1, 2, 3, 4], checked) + self.assertEqual([(1, [2, 4]), (2, [1]), (3, []), (4, []), (5, []), (6, []), (7, []), (8, []), (9, [])], saved) + self.assertIsNone(exit_code) + + def test_main_does_not_write_a_comment_it_could_not_read(self): + def find_managed_comments(number): + if number in (1, 5): + raise OSError("boom") + return [] + + _checked, saved, exit_code = self.run_main( + [pr(1), pr(2)], {1: [item(2)], 2: [item(1)]}, {1: {}, 2: {}}, find_managed_comments, + closed_prs=[pr(5, updated_at=RECENTLY)]) + + self.assertEqual([(2, [1])], saved) + self.assertIn("[1, 5]", exit_code) + + def test_dry_run_writes_nothing(self): + _checked, saved, exit_code = self.run_main( + [pr(1), pr(2)], {1: [item(2)], 2: [item(1)]}, {1: {}, 2: {}}, lambda number: [managed(number, [item(8)])], + closed_prs=[pr(5, updated_at=RECENTLY)], dry_run=True) + + self.assertEqual([], saved) + self.assertIsNone(exit_code) def test_format_files_limits_long_file_lists(self): files = [f"src/file_{i}.cpp" for i in range(handle_potential_conflicts.MAX_FILE_DETAILS + 2)]