From 91e8df5f2e19b50eefedf940dd82c03b0cbb00cb Mon Sep 17 00:00:00 2001 From: Piotr Korkus Date: Fri, 13 Mar 2026 14:27:06 +0100 Subject: [PATCH 1/4] add workflow to gather approvals on release branch --- .github/workflows/check_release_approvals.yml | 51 ++ .../cli/release/check_release_approvals.py | 456 ++++++++++++++++++ 2 files changed, 507 insertions(+) create mode 100644 .github/workflows/check_release_approvals.yml create mode 100755 scripts/tooling/cli/release/check_release_approvals.py diff --git a/.github/workflows/check_release_approvals.yml b/.github/workflows/check_release_approvals.yml new file mode 100644 index 00000000000..fa2e6a65e7c --- /dev/null +++ b/.github/workflows/check_release_approvals.yml @@ -0,0 +1,51 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +name: Check Release Branch Approvals +on: + pull_request_review: + types: [submitted, edited, dismissed] +jobs: + check-approvals: + name: Verify Required Approvals + if: startsWith(github.event.pull_request.base.ref, 'release/') + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout code + uses: actions/checkout@v4 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: 'pip' + cache-dependency-path: 'scripts/tooling/requirements.txt' + - name: Install dependencies + run: | + pip install -r scripts/tooling/requirements.txt + - name: Check Release Approvals + id: check-approvals + env: + REPO_OWNER: ${{ github.repository_owner }} + REPO_NAME: ${{ github.event.repository.name }} + PR_NUMBER: ${{ github.event.pull_request.number }} + BASE_BRANCH: ${{ github.base_ref }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + python3 scripts/tooling/cli/release/check_release_approvals.py + - name: Success + if: steps.check-approvals.outputs.all-approved == 'true' + run: | + echo "✅ All modules have required approvals" + echo "Approved modules: ${{ steps.check-approvals.outputs.approved-modules }}" diff --git a/scripts/tooling/cli/release/check_release_approvals.py b/scripts/tooling/cli/release/check_release_approvals.py new file mode 100755 index 00000000000..e572edbe14c --- /dev/null +++ b/scripts/tooling/cli/release/check_release_approvals.py @@ -0,0 +1,456 @@ +#!/usr/bin/env python3 +# ******************************************************************************* +# Copyright (c) 2025 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +"""Check release branch PR approvals against required maintainers.""" + +import argparse +import json +import os +import sys +from dataclasses import dataclass +from typing import Any +from urllib.error import URLError +from urllib.request import urlopen + +from github import Auth, Github # type: ignore[import-untyped] + + +@dataclass +class ModuleResult: + """Results of checking a module's reviews.""" + + maintainers: list[dict[str, Any]] + approved_maintainers: list[int] + approved_usernames: list[str] + disapproved_maintainers: list[int] + disapproved_usernames: list[str] + has_approval: bool + has_disapproval: bool + status: str # 'approved', 'disapproved', or 'pending' + + +def fetch_maintainers() -> dict[str, list[dict[str, Any]]]: + """Fetch maintainers from module metadata files. + + Returns: + Dictionary mapping module names to lists of maintainer information. + """ + modules = [ + "score_platform", + "score_baselibs", + "score_logging", + "score_baselibs_rust", + "score_communication", + "score_feo", + "score_kyron", + "score_lifecycle_health", + "score_persistency", + "score_orchestrator", + "score_itf", + "score_test_scenarios", + "score_docs_as_code", + ] + + modules_maintainers: dict[str, list[dict[str, Any]]] = {} + + for module_name in modules: + try: + url = f"https://raw.githubusercontent.com/eclipse-score/bazel_registry/main/modules/{module_name}/metadata.json" + with urlopen(url) as response: + data = json.loads(response.read()) + + if "maintainers" in data and isinstance(data["maintainers"], list): + modules_maintainers[module_name] = data["maintainers"] + print(f"{module_name} maintainers: {data['maintainers']}", file=sys.stderr) + else: + print(f"Warning: No maintainers found for {module_name}", file=sys.stderr) + modules_maintainers[module_name] = [] + except (URLError, json.JSONDecodeError) as error: + print(f"Error fetching {module_name}: {error}", file=sys.stderr) + modules_maintainers[module_name] = [] + + # Add extra maintainers + modules_maintainers["Testing"] = [ + { + "name": "Piotr Korkus", + "email": "piotr.korkus.ext@qorix.ai", + "github": "PiotrKorkus", + "github_user_id": 209438333, + } + ] + + modules_maintainers["Infrastructure"] = [ + { + "name": "Alexander Lanin", + "email": "alexander.lanin@etas.com", + "github": "AlexanderLanin", + "github_user_id": 5074553, + } + ] + + modules_maintainers["Technical_Leads"] = [ + { + "name": "Anton Krivoborodov", + "email": "anton.krivoborodov@bmw.de", + "github": "antonkri", + "github_user_id": 63401640, + }, + { + "name": "Frank Scholter Peres", + "email": "frank.scholter_peres@mercedes-benz.com", + "github": "FScholPer", + "github_user_id": 145544737, + }, + { + "name": "Lars Bauhofer", + "email": "lars.bauhofer@qorix.ai", + "github": "qor-lb", + "github_user_id": 155632781, + }, + ] + + return modules_maintainers + + +def check_pr_reviews( + repo_owner: str, + repo_name: str, + pr_number: int, + modules_maintainers: dict[str, list[dict[str, Any]]], + github_token: str, +) -> dict[str, Any]: + """Check PR reviews against required maintainers. + + Args: + repo_owner: Repository owner (organization or user) + repo_name: Repository name + pr_number: Pull request number + modules_maintainers: Dictionary mapping module names to maintainer lists + github_token: GitHub authentication token + + Returns: + Dictionary containing approval results for all modules + """ + # Initialize GitHub client + auth = Auth.Token(github_token) + github = Github(auth=auth) + + # Get repository and pull request + repo = github.get_repo(f"{repo_owner}/{repo_name}") + pr = repo.get_pull(pr_number) + + # Get all reviews for this PR + reviews = list(pr.get_reviews()) + + print(f"Modules and their maintainers: {json.dumps(modules_maintainers, indent=2)}", file=sys.stderr) + + # Get the latest review state from each user + latest_reviews_by_user: dict[int, Any] = {} + for review in reviews: + user_id = review.user.id + submitted_at = review.submitted_at + + if user_id not in latest_reviews_by_user or latest_reviews_by_user[user_id].submitted_at < submitted_at: + latest_reviews_by_user[user_id] = review + + review_summary = ", ".join([f"{user_id}: {review.state}" for user_id, review in latest_reviews_by_user.items()]) + print(f"Reviews by user ID: {review_summary}", file=sys.stderr) + + # Check which modules have at least one approval and no disapprovals + module_results: dict[str, ModuleResult] = {} + approved_modules: list[str] = [] + not_approved_modules: list[str] = [] + disapproved_modules: list[str] = [] + + for module_name, maintainers in modules_maintainers.items(): + approved_maintainer_ids: list[int] = [] + approved_usernames: list[str] = [] + disapproved_maintainer_ids: list[int] = [] + disapproved_usernames: list[str] = [] + + for maintainer in maintainers: + if isinstance(maintainer, dict): + maintainer_id = maintainer["github_user_id"] + maintainer_username = maintainer["github"] + else: + maintainer_id = maintainer + maintainer_username = str(maintainer) + + if maintainer_id in latest_reviews_by_user: + user_review = latest_reviews_by_user[maintainer_id] + if user_review.state == "APPROVED": + approved_maintainer_ids.append(maintainer_id) + approved_usernames.append(maintainer_username) + elif user_review.state == "CHANGES_REQUESTED": + disapproved_maintainer_ids.append(maintainer_id) + disapproved_usernames.append(maintainer_username) + + # If any maintainer disapproved, the module is disapproved + has_disapproval = len(disapproved_maintainer_ids) > 0 + has_approval = len(approved_maintainer_ids) > 0 + + if has_disapproval: + status = "disapproved" + elif has_approval: + status = "approved" + else: + status = "pending" + + module_results[module_name] = ModuleResult( + maintainers=maintainers, + approved_maintainers=approved_maintainer_ids, + approved_usernames=approved_usernames, + disapproved_maintainers=disapproved_maintainer_ids, + disapproved_usernames=disapproved_usernames, + has_approval=has_approval, + has_disapproval=has_disapproval, + status=status, + ) + + if has_disapproval: + disapproved_modules.append(module_name) + not_approved_modules.append(module_name) + print(f"🚫 {module_name}: Changes requested by {', '.join(disapproved_usernames)}", file=sys.stderr) + elif has_approval: + approved_modules.append(module_name) + print(f"✅ {module_name}: Approved by {', '.join(approved_usernames)}", file=sys.stderr) + else: + not_approved_modules.append(module_name) + maintainer_usernames = [m["github"] if isinstance(m, dict) else str(m) for m in maintainers] + required_str = f"requires one of: {', '.join(maintainer_usernames)}" + print(f"❌ {module_name}: No approvals ({required_str})", file=sys.stderr) + + all_approved = len(not_approved_modules) == 0 + + # Convert ModuleResult objects to dicts for JSON serialization + module_results_dict = { + name: { + "maintainers": result.maintainers, + "approvedMaintainers": result.approved_maintainers, + "approvedUsernames": result.approved_usernames, + "disapprovedMaintainers": result.disapproved_maintainers, + "disapprovedUsernames": result.disapproved_usernames, + "hasApproval": result.has_approval, + "hasDisapproval": result.has_disapproval, + "status": result.status, + } + for name, result in module_results.items() + } + + return { + "moduleResults": module_results_dict, + "approvedModules": approved_modules, + "notApprovedModules": not_approved_modules, + "disapprovedModules": disapproved_modules, + "allApproved": all_approved, + } + + +def generate_summary( + module_results: dict[str, dict[str, Any]], + all_approved: bool, + base_branch: str, + repo_owner: str | None = None, + repo_name: str | None = None, + pr_number: int | None = None, + github_token: str | None = None, +) -> str: + """Generate a markdown summary of approval results. + + Args: + module_results: Dictionary of module approval results + all_approved: Whether all modules are approved + base_branch: Target branch of the PR + repo_owner: Repository owner (for posting comments) + repo_name: Repository name (for posting comments) + pr_number: Pull request number (for posting comments) + github_token: GitHub authentication token (for posting comments) + + Returns: + Markdown-formatted summary string + """ + summary = "### Release Approval Check Results\n\n" + summary += f"**Target Branch:** {base_branch}\n\n" + + if all_approved: + summary += "✅ **Status:** All modules have required approvals\n\n" + else: + summary += "❌ **Status:** Some modules are missing required approvals\n\n" + + summary += "#### Modules:\n" + + for module_name, result in module_results.items(): + if result["status"] == "disapproved": + disapprovers = ", ".join(result["disapprovedUsernames"]) + approvers_text = "" + if result["approvedUsernames"]: + approvers_text = f" (approved by {', '.join(result['approvedUsernames'])})" + summary += f"- 🚫 **{module_name}**: Changes requested by {disapprovers}{approvers_text}\n" + elif result["status"] == "approved": + approvers = ", ".join(result["approvedUsernames"]) + summary += f"- ✅ **{module_name}**: Approved by {approvers}\n" + else: # pending + required_approvers = ", ".join( + [m["github"] if isinstance(m, dict) else str(m) for m in result["maintainers"]] + ) + summary += f"- ❌ **{module_name}**: Awaiting approval (requires one of: {required_approvers})\n" + + # Write to GitHub Actions step summary if available + if "GITHUB_STEP_SUMMARY" in os.environ: + with open(os.environ["GITHUB_STEP_SUMMARY"], "a") as f: + f.write(summary) + f.write("\n") + + # Post as PR comment if credentials provided + if all([repo_owner, repo_name, pr_number, github_token]): + try: + auth = Auth.Token(github_token) + github = Github(auth=auth) + repo = github.get_repo(f"{repo_owner}/{repo_name}") + pr = repo.get_pull(pr_number) + + # Check if there's already a comment from this workflow + comment_marker = "" + existing_comment = None + + for comment in pr.get_issue_comments(): + if comment_marker in comment.body: + existing_comment = comment + break + + full_comment = f"{comment_marker}\n{summary}" + + if existing_comment: + existing_comment.edit(full_comment) + print("Updated existing PR comment", file=sys.stderr) + else: + pr.create_issue_comment(full_comment) + print("Created new PR comment", file=sys.stderr) + except Exception as e: + print(f"Warning: Failed to post PR comment: {e}", file=sys.stderr) + + return summary + + +def cmd_fetch_maintainers() -> None: + """Command: Fetch maintainers.""" + modules_maintainers = fetch_maintainers() + print(json.dumps(modules_maintainers, indent=2)) + + +def cmd_check_all() -> None: + """Command: Run all steps (fetch, check, summarize).""" + # Get required parameters from environment variables + repo_owner = os.environ.get("REPO_OWNER", "") + repo_name = os.environ.get("REPO_NAME", "") + pr_number = int(os.environ.get("PR_NUMBER", "0")) + base_branch = os.environ.get("BASE_BRANCH", "unknown") + github_token = os.environ.get("GITHUB_TOKEN", "") + + if not all([repo_owner, repo_name, pr_number, github_token]): + print("Error: Missing required environment variables", file=sys.stderr) + print("Required: REPO_OWNER, REPO_NAME, PR_NUMBER, GITHUB_TOKEN", file=sys.stderr) + print("Optional: BASE_BRANCH (defaults to 'unknown')", file=sys.stderr) + sys.exit(1) + + # Step 1: Fetch maintainers + print("=== Fetching maintainers ===", file=sys.stderr) + modules_maintainers = fetch_maintainers() + + # Step 2: Check PR reviews + print("\n=== Checking PR reviews ===", file=sys.stderr) + results = check_pr_reviews( + repo_owner=repo_owner, + repo_name=repo_name, + pr_number=pr_number, + modules_maintainers=modules_maintainers, + github_token=github_token, + ) + + # Step 3: Generate summary + print("\n=== Generating summary ===", file=sys.stderr) + summary = generate_summary( + module_results=results["moduleResults"], + all_approved=results["allApproved"], + base_branch=base_branch, + repo_owner=repo_owner, + repo_name=repo_name, + pr_number=pr_number, + github_token=github_token, + ) + + # Set GitHub Actions outputs if running in GitHub Actions + if "GITHUB_OUTPUT" in os.environ: + with open(os.environ["GITHUB_OUTPUT"], "a") as f: + f.write(f"approved-modules={', '.join(results['approvedModules'])}\n") + f.write(f"not-approved-modules={', '.join(results['notApprovedModules'])}\n") + f.write(f"disapproved-modules={', '.join(results['disapprovedModules'])}\n") + f.write(f"all-approved={'true' if results['allApproved'] else 'false'}\n") + + # Exit with error if not all approved + if not results["allApproved"]: + print("\n" + summary, file=sys.stderr) + sys.exit(1) + else: + print("\n" + summary) + + +def main() -> None: + """Main entry point. + + Runs approval check using environment variables: + Required: + - REPO_OWNER: Repository owner (e.g., 'eclipse-score') + - REPO_NAME: Repository name (e.g., 'reference_integration') + - PR_NUMBER: Pull request number + - GITHUB_TOKEN: GitHub authentication token + + Optional: + - BASE_BRANCH: Target branch name (defaults to 'unknown') + """ + # Check if running in GitHub Actions (primary use case) + if all(var in os.environ for var in ["REPO_OWNER", "REPO_NAME", "PR_NUMBER", "GITHUB_TOKEN"]): + cmd_check_all() + else: + # Fallback for standalone/debug usage + parser = argparse.ArgumentParser( + description="Check release branch PR approvals", + epilog="Primary usage: Set environment variables and run without arguments", + ) + subparsers = parser.add_subparsers(dest="command", help="Debug commands") + + # fetch-maintainers command (for debugging) + subparsers.add_parser("fetch-maintainers", help="Fetch and print maintainers JSON") + + args = parser.parse_args() + + if args.command == "fetch-maintainers": + cmd_fetch_maintainers() + else: + parser.print_help() + print("\n" + "=" * 60) + print("ERROR: Missing required environment variables") + print("=" * 60) + print("Required environment variables:") + print(" - REPO_OWNER: Repository owner") + print(" - REPO_NAME: Repository name") + print(" - PR_NUMBER: Pull request number") + print(" - GITHUB_TOKEN: GitHub authentication token") + print("\nOptional:") + print(" - BASE_BRANCH: Target branch (defaults to 'unknown')") + sys.exit(1) + + +if __name__ == "__main__": + main() From 62eacf90dcaded737bad3c956328de77cc74952c Mon Sep 17 00:00:00 2001 From: Piotr Korkus Date: Mon, 16 Mar 2026 13:27:26 +0100 Subject: [PATCH 2/4] switch to bazel script execution --- .github/workflows/check_release_approvals.yml | 16 ++- scripts/tooling/cli/main.py | 2 + scripts/tooling/cli/release/__init__.py | 25 ++++ ...elease_approvals.py => check_approvals.py} | 116 +++++++++++++----- 4 files changed, 118 insertions(+), 41 deletions(-) create mode 100644 scripts/tooling/cli/release/__init__.py rename scripts/tooling/cli/release/{check_release_approvals.py => check_approvals.py} (84%) diff --git a/.github/workflows/check_release_approvals.yml b/.github/workflows/check_release_approvals.yml index fa2e6a65e7c..28f5856c975 100644 --- a/.github/workflows/check_release_approvals.yml +++ b/.github/workflows/check_release_approvals.yml @@ -25,15 +25,13 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v4 - - name: Set up Python - uses: actions/setup-python@v5 + - name: Setup Bazel + uses: bazel-contrib/setup-bazel@0.18.0 with: - python-version: '3.12' - cache: 'pip' - cache-dependency-path: 'scripts/tooling/requirements.txt' - - name: Install dependencies - run: | - pip install -r scripts/tooling/requirements.txt + bazelisk-cache: true + disk-cache: ${{ github.workflow }} + repository-cache: true + cache-save: ${{ github.event_name == 'push' }} - name: Check Release Approvals id: check-approvals env: @@ -43,7 +41,7 @@ jobs: BASE_BRANCH: ${{ github.base_ref }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - python3 scripts/tooling/cli/release/check_release_approvals.py + bazel run //scripts/tooling -- release check_approvals - name: Success if: steps.check-approvals.outputs.all-approved == 'true' run: | diff --git a/scripts/tooling/cli/main.py b/scripts/tooling/cli/main.py index f66e6a5c8cb..36b138ad729 100644 --- a/scripts/tooling/cli/main.py +++ b/scripts/tooling/cli/main.py @@ -20,8 +20,10 @@ def main() -> None: subparsers.required = True from scripts.tooling.cli.misc import register as _register_misc + from scripts.tooling.cli.release import register as _register_release _register_misc(subparsers) + _register_release(subparsers) args = parser.parse_args() sys.exit(args.func(args)) diff --git a/scripts/tooling/cli/release/__init__.py b/scripts/tooling/cli/release/__init__.py new file mode 100644 index 00000000000..8202ec40ae6 --- /dev/null +++ b/scripts/tooling/cli/release/__init__.py @@ -0,0 +1,25 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +import argparse + + +def register(subparsers: argparse._SubParsersAction) -> None: + release_parser = subparsers.add_parser("release", help="Release utilities") + release_sub = release_parser.add_subparsers(dest="command", metavar="COMMAND") + release_sub.required = True + + from scripts.tooling.cli.release.check_approvals import ( + register as _register_release_approvals, + ) + + _register_release_approvals(release_sub) diff --git a/scripts/tooling/cli/release/check_release_approvals.py b/scripts/tooling/cli/release/check_approvals.py similarity index 84% rename from scripts/tooling/cli/release/check_release_approvals.py rename to scripts/tooling/cli/release/check_approvals.py index e572edbe14c..6abad6e0348 100755 --- a/scripts/tooling/cli/release/check_release_approvals.py +++ b/scripts/tooling/cli/release/check_approvals.py @@ -19,12 +19,15 @@ import os import sys from dataclasses import dataclass +from pathlib import Path from typing import Any from urllib.error import URLError from urllib.request import urlopen from github import Auth, Github # type: ignore[import-untyped] +from scripts.tooling.lib.known_good import load_known_good + @dataclass class ModuleResult: @@ -40,26 +43,29 @@ class ModuleResult: status: str # 'approved', 'disapproved', or 'pending' -def fetch_maintainers() -> dict[str, list[dict[str, Any]]]: +def _find_repo_root() -> Path: + candidate = Path(__file__).resolve() + for parent in candidate.parents: + if (parent / "known_good.json").exists(): + return parent + return Path.cwd() + + +def fetch_maintainers(known_good_path: Path) -> dict[str, list[dict[str, Any]]]: """Fetch maintainers from module metadata files. Returns: Dictionary mapping module names to lists of maintainer information. """ - modules = [ + known_good = load_known_good(known_good_path) + sw_modules = known_good.modules.get("target_sw", {}).keys() + + # Add subset of tooling modules + modules = list(sw_modules) + [ + "score_docs_as_code", "score_platform", - "score_baselibs", - "score_logging", - "score_baselibs_rust", - "score_communication", - "score_feo", - "score_kyron", - "score_lifecycle_health", - "score_persistency", - "score_orchestrator", "score_itf", "score_test_scenarios", - "score_docs_as_code", ] modules_maintainers: dict[str, list[dict[str, Any]]] = {} @@ -81,6 +87,7 @@ def fetch_maintainers() -> dict[str, list[dict[str, Any]]]: modules_maintainers[module_name] = [] # Add extra maintainers + # TODO: move it to single config file modules_maintainers["Testing"] = [ { "name": "Piotr Korkus", @@ -343,14 +350,18 @@ def generate_summary( return summary -def cmd_fetch_maintainers() -> None: +def cmd_fetch_maintainers(known_good_path: Path) -> None: """Command: Fetch maintainers.""" - modules_maintainers = fetch_maintainers() + modules_maintainers = fetch_maintainers(known_good_path) print(json.dumps(modules_maintainers, indent=2)) -def cmd_check_all() -> None: - """Command: Run all steps (fetch, check, summarize).""" +def cmd_check_all(known_good_path: Path) -> int: + """Command: Run all steps (fetch, check, summarize). + + Returns: + Exit code: 0 if all approved, 1 otherwise + """ # Get required parameters from environment variables repo_owner = os.environ.get("REPO_OWNER", "") repo_name = os.environ.get("REPO_NAME", "") @@ -362,11 +373,11 @@ def cmd_check_all() -> None: print("Error: Missing required environment variables", file=sys.stderr) print("Required: REPO_OWNER, REPO_NAME, PR_NUMBER, GITHUB_TOKEN", file=sys.stderr) print("Optional: BASE_BRANCH (defaults to 'unknown')", file=sys.stderr) - sys.exit(1) + return 1 # Step 1: Fetch maintainers print("=== Fetching maintainers ===", file=sys.stderr) - modules_maintainers = fetch_maintainers() + modules_maintainers = fetch_maintainers(known_good_path) # Step 2: Check PR reviews print("\n=== Checking PR reviews ===", file=sys.stderr) @@ -401,9 +412,43 @@ def cmd_check_all() -> None: # Exit with error if not all approved if not results["allApproved"]: print("\n" + summary, file=sys.stderr) - sys.exit(1) + return 1 else: print("\n" + summary) + return 0 + + +def register(subparsers: argparse._SubParsersAction) -> None: + """Register this command as a subparser.""" + parser = subparsers.add_parser( + "check_approvals", + help="Check release branch PR approvals against required maintainers", + ) + parser.add_argument( + "--known_good", + metavar="PATH", + default=str(_find_repo_root()), + help="Directory containing known_good.json (default: repo root)", + ) + parser.set_defaults(func=_run) + + +def _run(args: argparse.Namespace) -> int: + """Run the command with parsed arguments. + + Returns: + Exit code: 0 on success, 1 on failure + """ + known_good_path = Path(args.known_good) / "known_good.json" + + # Check if running in GitHub Actions (primary use case) + if all(var in os.environ for var in ["REPO_OWNER", "REPO_NAME", "PR_NUMBER", "GITHUB_TOKEN"]): + return cmd_check_all(known_good_path) + else: + print("Error: Missing required environment variables", file=sys.stderr) + print("Required: REPO_OWNER, REPO_NAME, PR_NUMBER, GITHUB_TOKEN", file=sys.stderr) + print("Optional: BASE_BRANCH (defaults to 'unknown')", file=sys.stderr) + return 1 def main() -> None: @@ -419,24 +464,31 @@ def main() -> None: Optional: - BASE_BRANCH: Target branch name (defaults to 'unknown') """ + # Parse common arguments + parser = argparse.ArgumentParser( + description="Check release branch PR approvals", + epilog="Primary usage: Set environment variables and run without arguments", + ) + parser.add_argument( + "--known_good", + type=Path, + default=_find_repo_root() / "known_good.json", + help="Path to known_good.json file (default: repo_root/known_good.json)", + ) + subparsers = parser.add_subparsers(dest="command", help="Debug commands") + + # fetch-maintainers command (for debugging) + subparsers.add_parser("fetch-maintainers", help="Fetch and print maintainers JSON") + + args = parser.parse_args() + # Check if running in GitHub Actions (primary use case) if all(var in os.environ for var in ["REPO_OWNER", "REPO_NAME", "PR_NUMBER", "GITHUB_TOKEN"]): - cmd_check_all() + sys.exit(cmd_check_all(args.known_good)) else: # Fallback for standalone/debug usage - parser = argparse.ArgumentParser( - description="Check release branch PR approvals", - epilog="Primary usage: Set environment variables and run without arguments", - ) - subparsers = parser.add_subparsers(dest="command", help="Debug commands") - - # fetch-maintainers command (for debugging) - subparsers.add_parser("fetch-maintainers", help="Fetch and print maintainers JSON") - - args = parser.parse_args() - if args.command == "fetch-maintainers": - cmd_fetch_maintainers() + cmd_fetch_maintainers(args.known_good) else: parser.print_help() print("\n" + "=" * 60) From db5d9301f0f0f9adc8d08bec8c24689e572a5382 Mon Sep 17 00:00:00 2001 From: Piotr Korkus Date: Mon, 16 Mar 2026 13:42:26 +0100 Subject: [PATCH 3/4] use jinja for long text outputs --- .../tooling/cli/release/check_approvals.py | 190 ++++++++++++------ 1 file changed, 134 insertions(+), 56 deletions(-) diff --git a/scripts/tooling/cli/release/check_approvals.py b/scripts/tooling/cli/release/check_approvals.py index 6abad6e0348..3444ee398e5 100755 --- a/scripts/tooling/cli/release/check_approvals.py +++ b/scripts/tooling/cli/release/check_approvals.py @@ -25,9 +25,40 @@ from urllib.request import urlopen from github import Auth, Github # type: ignore[import-untyped] +from jinja2 import Template from scripts.tooling.lib.known_good import load_known_good +# Jinja2 templates for markdown generation +MODULE_STATUS_TEMPLATE = Template( + """{% if status == 'disapproved' -%} +- 🚫 **{{ module_name }}**: Changes requested by {{ disapproved_usernames|join(', ') }} +{%- if approved_usernames %} (approved by {{ approved_usernames|join(', ') }}){% endif %} +{% elif status == 'approved' -%} +- ✅ **{{ module_name }}**: Approved by {{ approved_usernames|join(', ') }} +{% else -%} +- ❌ **{{ module_name }}**: Awaiting approval (requires one of: {{ required_approvers|join(', ') }}) +{% endif -%} +""" +) + +SUMMARY_TEMPLATE = Template( + """### Release Approval Check Results + +**Target Branch:** {{ base_branch }} + +{% if all_approved -%} +✅ **Status:** All modules have required approvals +{% else -%} +❌ **Status:** Some modules are missing required approvals +{% endif %} +#### Modules: +{% for module in modules -%} +{{ module }} +{% endfor -%} +""" +) + @dataclass class ModuleResult: @@ -264,6 +295,106 @@ def check_pr_reviews( } +def _format_module_status(module_name: str, result: dict[str, Any]) -> str: + """Format a single module's approval status as markdown. + + Args: + module_name: Name of the module + result: Module result dictionary containing status and approver information + + Returns: + Markdown-formatted status line + """ + required_approvers = [m["github"] if isinstance(m, dict) else str(m) for m in result["maintainers"]] + + return MODULE_STATUS_TEMPLATE.render( + module_name=module_name, + status=result["status"], + approved_usernames=result["approvedUsernames"], + disapproved_usernames=result["disapprovedUsernames"], + required_approvers=required_approvers, + ) + + +def _write_github_actions_summary(summary: str) -> None: + """Write summary to GitHub Actions step summary if available. + + Args: + summary: Markdown-formatted summary text + """ + if "GITHUB_STEP_SUMMARY" in os.environ: + with open(os.environ["GITHUB_STEP_SUMMARY"], "a") as f: + f.write(summary) + f.write("\n") + + +def _post_pr_comment( + summary: str, + repo_owner: str, + repo_name: str, + pr_number: int, + github_token: str, +) -> None: + """Post or update PR comment with approval summary. + + Args: + summary: Markdown-formatted summary text + repo_owner: Repository owner + repo_name: Repository name + pr_number: Pull request number + github_token: GitHub authentication token + """ + try: + auth = Auth.Token(github_token) + github = Github(auth=auth) + repo = github.get_repo(f"{repo_owner}/{repo_name}") + pr = repo.get_pull(pr_number) + + comment_marker = "" + full_comment = f"{comment_marker}\n{summary}" + + # Find existing comment from this workflow + existing_comment = None + for comment in pr.get_issue_comments(): + if comment_marker in comment.body: + existing_comment = comment + break + + # Update existing or create new comment + if existing_comment: + existing_comment.edit(full_comment) + print("Updated existing PR comment", file=sys.stderr) + else: + pr.create_issue_comment(full_comment) + print("Created new PR comment", file=sys.stderr) + except Exception as e: + print(f"Warning: Failed to post PR comment: {e}", file=sys.stderr) + + +def _build_summary_markdown( + module_results: dict[str, dict[str, Any]], + all_approved: bool, + base_branch: str, +) -> str: + """Build the markdown summary text. + + Args: + module_results: Dictionary of module approval results + all_approved: Whether all modules are approved + base_branch: Target branch of the PR + + Returns: + Markdown-formatted summary string + """ + modules = [_format_module_status(module_name, result) for module_name, result in module_results.items()] + + return SUMMARY_TEMPLATE.render( + base_branch=base_branch, + all_approved=all_approved, + modules=modules, + ) + + def generate_summary( module_results: dict[str, dict[str, Any]], all_approved: bool, @@ -287,65 +418,12 @@ def generate_summary( Returns: Markdown-formatted summary string """ - summary = "### Release Approval Check Results\n\n" - summary += f"**Target Branch:** {base_branch}\n\n" + summary = _build_summary_markdown(module_results, all_approved, base_branch) - if all_approved: - summary += "✅ **Status:** All modules have required approvals\n\n" - else: - summary += "❌ **Status:** Some modules are missing required approvals\n\n" - - summary += "#### Modules:\n" - - for module_name, result in module_results.items(): - if result["status"] == "disapproved": - disapprovers = ", ".join(result["disapprovedUsernames"]) - approvers_text = "" - if result["approvedUsernames"]: - approvers_text = f" (approved by {', '.join(result['approvedUsernames'])})" - summary += f"- 🚫 **{module_name}**: Changes requested by {disapprovers}{approvers_text}\n" - elif result["status"] == "approved": - approvers = ", ".join(result["approvedUsernames"]) - summary += f"- ✅ **{module_name}**: Approved by {approvers}\n" - else: # pending - required_approvers = ", ".join( - [m["github"] if isinstance(m, dict) else str(m) for m in result["maintainers"]] - ) - summary += f"- ❌ **{module_name}**: Awaiting approval (requires one of: {required_approvers})\n" - - # Write to GitHub Actions step summary if available - if "GITHUB_STEP_SUMMARY" in os.environ: - with open(os.environ["GITHUB_STEP_SUMMARY"], "a") as f: - f.write(summary) - f.write("\n") + _write_github_actions_summary(summary) - # Post as PR comment if credentials provided if all([repo_owner, repo_name, pr_number, github_token]): - try: - auth = Auth.Token(github_token) - github = Github(auth=auth) - repo = github.get_repo(f"{repo_owner}/{repo_name}") - pr = repo.get_pull(pr_number) - - # Check if there's already a comment from this workflow - comment_marker = "" - existing_comment = None - - for comment in pr.get_issue_comments(): - if comment_marker in comment.body: - existing_comment = comment - break - - full_comment = f"{comment_marker}\n{summary}" - - if existing_comment: - existing_comment.edit(full_comment) - print("Updated existing PR comment", file=sys.stderr) - else: - pr.create_issue_comment(full_comment) - print("Created new PR comment", file=sys.stderr) - except Exception as e: - print(f"Warning: Failed to post PR comment: {e}", file=sys.stderr) + _post_pr_comment(summary, repo_owner, repo_name, pr_number, github_token) return summary From 564c8ae8ac0a74c584ffcea1d0ebb45c28ba341b Mon Sep 17 00:00:00 2001 From: Piotr Korkus Date: Tue, 17 Mar 2026 08:42:46 +0100 Subject: [PATCH 4/4] cleanup old code, add tests for check_pr logic --- BUILD | 1 + scripts/tooling/BUILD | 1 + .../tooling/cli/release/check_approvals.py | 51 +- scripts/tooling/tests/test_check_approvals.py | 463 ++++++++++++++++++ 4 files changed, 466 insertions(+), 50 deletions(-) create mode 100644 scripts/tooling/tests/test_check_approvals.py diff --git a/BUILD b/BUILD index 6ef3b7969df..7afff7dac7e 100644 --- a/BUILD +++ b/BUILD @@ -68,4 +68,5 @@ use_format_targets() exports_files([ "MODULE.bazel", "pyproject.toml", + "known_good.json", ]) diff --git a/scripts/tooling/BUILD b/scripts/tooling/BUILD index 40748469520..61466b85d8f 100644 --- a/scripts/tooling/BUILD +++ b/scripts/tooling/BUILD @@ -64,6 +64,7 @@ score_py_pytest( srcs = glob(["tests/**/*.py"]), data = [ ":cli/misc/assets/report_template.html", + "//:known_good.json", ], pytest_config = "//:pyproject.toml", deps = [ diff --git a/scripts/tooling/cli/release/check_approvals.py b/scripts/tooling/cli/release/check_approvals.py index 3444ee398e5..2a2a40a1f7b 100755 --- a/scripts/tooling/cli/release/check_approvals.py +++ b/scripts/tooling/cli/release/check_approvals.py @@ -530,56 +530,7 @@ def _run(args: argparse.Namespace) -> int: def main() -> None: - """Main entry point. - - Runs approval check using environment variables: - Required: - - REPO_OWNER: Repository owner (e.g., 'eclipse-score') - - REPO_NAME: Repository name (e.g., 'reference_integration') - - PR_NUMBER: Pull request number - - GITHUB_TOKEN: GitHub authentication token - - Optional: - - BASE_BRANCH: Target branch name (defaults to 'unknown') - """ - # Parse common arguments - parser = argparse.ArgumentParser( - description="Check release branch PR approvals", - epilog="Primary usage: Set environment variables and run without arguments", - ) - parser.add_argument( - "--known_good", - type=Path, - default=_find_repo_root() / "known_good.json", - help="Path to known_good.json file (default: repo_root/known_good.json)", - ) - subparsers = parser.add_subparsers(dest="command", help="Debug commands") - - # fetch-maintainers command (for debugging) - subparsers.add_parser("fetch-maintainers", help="Fetch and print maintainers JSON") - - args = parser.parse_args() - - # Check if running in GitHub Actions (primary use case) - if all(var in os.environ for var in ["REPO_OWNER", "REPO_NAME", "PR_NUMBER", "GITHUB_TOKEN"]): - sys.exit(cmd_check_all(args.known_good)) - else: - # Fallback for standalone/debug usage - if args.command == "fetch-maintainers": - cmd_fetch_maintainers(args.known_good) - else: - parser.print_help() - print("\n" + "=" * 60) - print("ERROR: Missing required environment variables") - print("=" * 60) - print("Required environment variables:") - print(" - REPO_OWNER: Repository owner") - print(" - REPO_NAME: Repository name") - print(" - PR_NUMBER: Pull request number") - print(" - GITHUB_TOKEN: GitHub authentication token") - print("\nOptional:") - print(" - BASE_BRANCH: Target branch (defaults to 'unknown')") - sys.exit(1) + print("This script should be run via `bazel` as part of CICD") if __name__ == "__main__": diff --git a/scripts/tooling/tests/test_check_approvals.py b/scripts/tooling/tests/test_check_approvals.py new file mode 100644 index 00000000000..93ad686d3a3 --- /dev/null +++ b/scripts/tooling/tests/test_check_approvals.py @@ -0,0 +1,463 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +from datetime import datetime +from unittest.mock import MagicMock, Mock, patch + +import pytest + +from scripts.tooling.cli.release.check_approvals import check_pr_reviews + + +@pytest.fixture +def modules_maintainers(): + """Sample modules and maintainers for testing.""" + return { + "module_a": [ + { + "name": "Alice Developer", + "email": "alice@example.com", + "github": "alice", + "github_user_id": 100, + }, + { + "name": "Bob Developer", + "email": "bob@example.com", + "github": "bob", + "github_user_id": 101, + }, + ], + "module_b": [ + { + "name": "Charlie Developer", + "email": "charlie@example.com", + "github": "charlie", + "github_user_id": 102, + } + ], + "module_c": [ + { + "name": "Diana Developer", + "email": "diana@example.com", + "github": "diana", + "github_user_id": 103, + }, + { + "name": "Eve Developer", + "email": "eve@example.com", + "github": "eve", + "github_user_id": 104, + }, + ], + } + + +def _create_mock_review(user_id: int, username: str, state: str, submitted_at: datetime) -> Mock: + """Create a mock GitHub review object. + + Args: + user_id: GitHub user ID + username: GitHub username + state: Review state (APPROVED, CHANGES_REQUESTED, etc.) + submitted_at: Timestamp of review submission + + Returns: + Mock review object + """ + review = Mock() + review.user = Mock() + review.user.id = user_id + review.user.login = username + review.state = state + review.submitted_at = submitted_at + return review + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_all_approved(mock_github_class, modules_maintainers): + """Test when all modules have required approvals.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # Create mock reviews - one approval per module + reviews = [ + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 10, 0)), # module_a + _create_mock_review(102, "charlie", "APPROVED", datetime(2026, 1, 1, 11, 0)), # module_b + _create_mock_review(103, "diana", "APPROVED", datetime(2026, 1, 1, 12, 0)), # module_c + ] + mock_pr.get_reviews.return_value = reviews + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results + assert result["allApproved"] is True + assert len(result["approvedModules"]) == 3 + assert "module_a" in result["approvedModules"] + assert "module_b" in result["approvedModules"] + assert "module_c" in result["approvedModules"] + assert len(result["notApprovedModules"]) == 0 + assert len(result["disapprovedModules"]) == 0 + + # Verify module results + assert result["moduleResults"]["module_a"]["status"] == "approved" + assert result["moduleResults"]["module_a"]["hasApproval"] is True + assert result["moduleResults"]["module_a"]["hasDisapproval"] is False + assert "alice" in result["moduleResults"]["module_a"]["approvedUsernames"] + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_no_approvals(mock_github_class, modules_maintainers): + """Test when no modules have approvals.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # No reviews + mock_pr.get_reviews.return_value = [] + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results + assert result["allApproved"] is False + assert len(result["approvedModules"]) == 0 + assert len(result["notApprovedModules"]) == 3 + assert "module_a" in result["notApprovedModules"] + assert "module_b" in result["notApprovedModules"] + assert "module_c" in result["notApprovedModules"] + + # Verify module results + for module_name in ["module_a", "module_b", "module_c"]: + assert result["moduleResults"][module_name]["status"] == "pending" + assert result["moduleResults"][module_name]["hasApproval"] is False + assert result["moduleResults"][module_name]["hasDisapproval"] is False + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_with_changes_requested(mock_github_class, modules_maintainers): + """Test when some modules have changes requested.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # Create mock reviews - mix of approved and changes requested + reviews = [ + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 10, 0)), # module_a approved + _create_mock_review(102, "charlie", "CHANGES_REQUESTED", datetime(2026, 1, 1, 11, 0)), # module_b blocked + _create_mock_review(103, "diana", "APPROVED", datetime(2026, 1, 1, 12, 0)), # module_c approved + ] + mock_pr.get_reviews.return_value = reviews + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results + assert result["allApproved"] is False + assert len(result["approvedModules"]) == 2 + assert "module_a" in result["approvedModules"] + assert "module_c" in result["approvedModules"] + assert len(result["disapprovedModules"]) == 1 + assert "module_b" in result["disapprovedModules"] + assert "module_b" in result["notApprovedModules"] + + # Verify module results + assert result["moduleResults"]["module_b"]["status"] == "disapproved" + assert result["moduleResults"]["module_b"]["hasDisapproval"] is True + assert "charlie" in result["moduleResults"]["module_b"]["disapprovedUsernames"] + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_latest_review_wins(mock_github_class, modules_maintainers): + """Test that the latest review from a user takes precedence.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # Create mock reviews - same user reviews multiple times + reviews = [ + # Alice reviews module_a multiple times - latest is APPROVED + _create_mock_review(100, "alice", "CHANGES_REQUESTED", datetime(2026, 1, 1, 10, 0)), + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 12, 0)), # Latest wins + # Charlie reviews module_b multiple times - latest is CHANGES_REQUESTED + _create_mock_review(102, "charlie", "APPROVED", datetime(2026, 1, 1, 11, 0)), + _create_mock_review(102, "charlie", "CHANGES_REQUESTED", datetime(2026, 1, 1, 13, 0)), # Latest wins + # Diana approves module_c + _create_mock_review(103, "diana", "APPROVED", datetime(2026, 1, 1, 12, 0)), + ] + mock_pr.get_reviews.return_value = reviews + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results - latest reviews should be used + assert result["moduleResults"]["module_a"]["status"] == "approved" + assert result["moduleResults"]["module_a"]["hasApproval"] is True + assert result["moduleResults"]["module_a"]["hasDisapproval"] is False + + assert result["moduleResults"]["module_b"]["status"] == "disapproved" + assert result["moduleResults"]["module_b"]["hasDisapproval"] is True + + assert result["moduleResults"]["module_c"]["status"] == "approved" + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_multiple_maintainers_one_approval(mock_github_class, modules_maintainers): + """Test that only one maintainer approval is needed per module.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # module_a has two maintainers (alice=100, bob=101), only alice approves + # module_c has two maintainers (diana=103, eve=104), only diana approves + reviews = [ + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 10, 0)), # module_a + _create_mock_review(102, "charlie", "APPROVED", datetime(2026, 1, 1, 11, 0)), # module_b + _create_mock_review(103, "diana", "APPROVED", datetime(2026, 1, 1, 12, 0)), # module_c + ] + mock_pr.get_reviews.return_value = reviews + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results - one approval per module is sufficient + assert result["allApproved"] is True + assert len(result["approvedModules"]) == 3 + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_approval_with_disapproval(mock_github_class, modules_maintainers): + """Test that a disapproval blocks approval even if another maintainer approved.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # module_a: alice approves, bob requests changes + # module_c: diana approves, eve requests changes + reviews = [ + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 10, 0)), + _create_mock_review(101, "bob", "CHANGES_REQUESTED", datetime(2026, 1, 1, 11, 0)), + _create_mock_review(102, "charlie", "APPROVED", datetime(2026, 1, 1, 11, 0)), + _create_mock_review(103, "diana", "APPROVED", datetime(2026, 1, 1, 12, 0)), + _create_mock_review(104, "eve", "CHANGES_REQUESTED", datetime(2026, 1, 1, 13, 0)), + ] + mock_pr.get_reviews.return_value = reviews + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results - disapprovals should block + assert result["allApproved"] is False + assert len(result["disapprovedModules"]) == 2 + assert "module_a" in result["disapprovedModules"] + assert "module_c" in result["disapprovedModules"] + + # Verify module states + assert result["moduleResults"]["module_a"]["status"] == "disapproved" + assert result["moduleResults"]["module_a"]["hasApproval"] is True + assert result["moduleResults"]["module_a"]["hasDisapproval"] is True + assert "alice" in result["moduleResults"]["module_a"]["approvedUsernames"] + assert "bob" in result["moduleResults"]["module_a"]["disapprovedUsernames"] + + # module_b should still be approved + assert result["moduleResults"]["module_b"]["status"] == "approved" + assert "module_b" in result["approvedModules"] + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_non_maintainer_reviews(mock_github_class, modules_maintainers): + """Test that reviews from non-maintainers are ignored.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # Reviews from non-maintainers (user_id 999, 998) + reviews = [ + _create_mock_review(999, "random-user", "APPROVED", datetime(2026, 1, 1, 10, 0)), + _create_mock_review(998, "another-user", "CHANGES_REQUESTED", datetime(2026, 1, 1, 11, 0)), + # Actual maintainer reviews + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 12, 0)), # module_a + _create_mock_review(102, "charlie", "APPROVED", datetime(2026, 1, 1, 13, 0)), # module_b + _create_mock_review(103, "diana", "APPROVED", datetime(2026, 1, 1, 14, 0)), # module_c + ] + mock_pr.get_reviews.return_value = reviews + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results - only maintainer reviews count + assert result["allApproved"] is True + assert len(result["approvedModules"]) == 3 + + # Verify that non-maintainer user IDs don't appear in results + for module_result in result["moduleResults"].values(): + assert 999 not in module_result["approvedMaintainers"] + assert 999 not in module_result["disapprovedMaintainers"] + assert 998 not in module_result["approvedMaintainers"] + assert 998 not in module_result["disapprovedMaintainers"] + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_empty_maintainers(mock_github_class): + """Test handling of modules with no maintainers.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # Some reviews + reviews = [ + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 10, 0)), + ] + mock_pr.get_reviews.return_value = reviews + + # Module with no maintainers + modules_maintainers = { + "module_no_maintainers": [], + } + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results - module with no maintainers should be pending + assert result["allApproved"] is False + assert "module_no_maintainers" in result["notApprovedModules"] + assert result["moduleResults"]["module_no_maintainers"]["status"] == "pending" + assert result["moduleResults"]["module_no_maintainers"]["hasApproval"] is False + + +@patch("scripts.tooling.cli.release.check_approvals.Github") +def test_check_pr_reviews_mixed_scenario(mock_github_class, modules_maintainers): + """Test a complex mixed scenario with various states.""" + # Setup mock GitHub API + mock_github = MagicMock() + mock_github_class.return_value = mock_github + mock_repo = MagicMock() + mock_github.get_repo.return_value = mock_repo + mock_pr = MagicMock() + mock_repo.get_pull.return_value = mock_pr + + # Complex scenario: + # module_a: approved by alice + # module_b: no reviews (pending) + # module_c: diana approved, eve requested changes (disapproved) + reviews = [ + _create_mock_review(100, "alice", "APPROVED", datetime(2026, 1, 1, 10, 0)), + _create_mock_review(103, "diana", "APPROVED", datetime(2026, 1, 1, 12, 0)), + _create_mock_review(104, "eve", "CHANGES_REQUESTED", datetime(2026, 1, 1, 13, 0)), + ] + mock_pr.get_reviews.return_value = reviews + + # Run the function + result = check_pr_reviews( + repo_owner="test-org", + repo_name="test-repo", + pr_number=123, + modules_maintainers=modules_maintainers, + github_token="test-token", + ) + + # Verify results + assert result["allApproved"] is False + + # module_a: approved + assert "module_a" in result["approvedModules"] + assert result["moduleResults"]["module_a"]["status"] == "approved" + + # module_b: pending + assert "module_b" in result["notApprovedModules"] + assert result["moduleResults"]["module_b"]["status"] == "pending" + + # module_c: disapproved + assert "module_c" in result["disapprovedModules"] + assert "module_c" in result["notApprovedModules"] + assert result["moduleResults"]["module_c"]["status"] == "disapproved"