diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1ded0ff541..48bb1f14061 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -196,47 +196,10 @@ jobs: COMMIT_SHA: ${{ github.event.pull_request.head.sha }} COMMIT_MESSAGE: ${{ github.event.pull_request.head.commit.message }} COMMIT_URL: ${{ github.server_url }}/${{ github.repository }}/pull/${{ github.event.pull_request.number }}/commits/${{ github.event.pull_request.head.sha }} - # Structured review statuses from workflow_call jobs. - # Each job outputs a JSON array of {source, results: [...]} objects - # that the assembler renders directly — no hardcoded job-name - # matching. We merge all available outputs into one array. - REVIEW_STATUSES: ${{ toJSON(needs.*.outputs.review_status) }} run: | - set -uo pipefail - - # REVIEW_STATUSES is a JSON array of strings (some may be empty - # when a job was skipped). Parse each string and merge into one - # flat array for the assembler. - python3 - <<'PYEOF' - import json, os, sys - - raw = os.environ.get("REVIEW_STATUSES", "") - merged = [] - if raw: - try: - arr = json.loads(raw) - except (json.JSONDecodeError, TypeError): - arr = [] - for item in arr: - if not item: - continue - try: - statuses = json.loads(item) - except (json.JSONDecodeError, TypeError): - continue - if isinstance(statuses, list): - merged.extend(statuses) - - # Write merged array to a temp file the poller reads. - with open("/tmp/review_statuses.json", "w") as f: - json.dump(merged, f) - print(f"Merged {len(merged)} review status entries") - PYEOF - python3 scripts/ci/live_comment.py \ --interval 15 \ - --timeout 2100 \ - --review-statuses-file /tmp/review_statuses.json + --timeout 2100 # ───────────────────────────────────────────────────────────────────── # Gate: runs after everything. ``if: always()`` ensures it reports a @@ -304,8 +267,9 @@ jobs: # report with a gantt chart + per-step breakdown. The report is uploaded # as an artifact and a markdown summary is written to $GITHUB_STEP_SUMMARY. # - # The live comment poller can read the standalone review-status artifact - # after the HTML report is uploaded, so its link points straight at that report. + # The live comment poller dynamically fetches all review-status-* artifacts + # across the orchestrator and sub-workflow runs every cycle, so its link + # points straight at that report. # ───────────────────────────────────────────────────────────────────── ci-timings: name: CI timing report @@ -364,7 +328,7 @@ jobs: continue-on-error: true uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: ci-timings-review-status + name: review-status-ci-timings path: review-status.json retention-days: 14 diff --git a/.github/workflows/contributor-check.yml b/.github/workflows/contributor-check.yml index 014e1e2ff93..fd8d7bdff83 100644 --- a/.github/workflows/contributor-check.yml +++ b/.github/workflows/contributor-check.yml @@ -33,6 +33,7 @@ jobs: if [ -z "$NEW_EMAILS" ]; then echo "No new commits to check." echo "review_status=[]" >> "$GITHUB_OUTPUT" + echo "review_status=[]" > review-status.json exit 0 fi @@ -84,8 +85,22 @@ jobs: --arg how_to_fix "$HOW_TO_FIX" \ '[{"source":"contributor attribution","results":[{"kind":"action_required","title":"Unmapped contributor email(s)","summary":"New contributor email(s) are not in AUTHOR_MAP.","detail":$detail,"how_to_fix":$how_to_fix}]}]') echo "review_status=$REVIEW_STATUS" >> "$GITHUB_OUTPUT" + echo "review_status=$REVIEW_STATUS" > review-status.json exit 1 else echo "✅ All contributor emails are mapped." + echo "review_status=[]" >> "$GITHUB_OUTPUT" + echo "review_status=[]" > review-status.json fi + + - name: Upload review status artifact + if: always() && steps.check-emails.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: review-status-contributor-check + path: review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore diff --git a/.github/workflows/e2e-desktop.yml b/.github/workflows/e2e-desktop.yml index e9131c72522..bfa45f88962 100644 --- a/.github/workflows/e2e-desktop.yml +++ b/.github/workflows/e2e-desktop.yml @@ -170,6 +170,18 @@ jobs: cat /tmp/e2e-review-status.json echo '__E2E_REVIEW_STATUS__' } >> "$GITHUB_OUTPUT" + cp /tmp/e2e-review-status.json review-status.json + + - name: Upload review status artifact + if: always() && steps.review-status.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: review-status-e2e-desktop + path: apps/desktop/review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore # The trusted workflow_run publisher consumes only this flat, bounded # artifact. It turns selected images into GitHub attachment URLs; it diff --git a/.github/workflows/history-check.yml b/.github/workflows/history-check.yml index 668f0f795dd..f679007810f 100644 --- a/.github/workflows/history-check.yml +++ b/.github/workflows/history-check.yml @@ -44,6 +44,7 @@ jobs: if ! BASE=$(git merge-base origin/main HEAD 2>/dev/null) || [ -z "$BASE" ]; then STATUS='[{"source":"unrelated histories","results":[{"kind":"action_required","title":"Unrelated histories","summary":"This PR has no common ancestor with main.","detail":"","how_to_fix":"Rebase your changes onto current main:\n```\ngit fetch origin main\ngit checkout -b fix-branch origin/main\n# re-apply your changes (cherry-pick, copy files, etc.)\ngit push -f origin fix-branch\n```\n"}]}]' echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT" + echo "review_status=${STATUS}" > review-status.json echo "" echo "::error::This PR has no common ancestor with main." echo "" @@ -66,3 +67,15 @@ jobs: fi echo "::notice::Common ancestor with main: $BASE" echo "review_status=[]" >> "$GITHUB_OUTPUT" + echo "review_status=[]" > review-status.json + + - name: Upload review status artifact + if: always() && steps.merge-base-check.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: review-status-history-check + path: review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore diff --git a/.github/workflows/lockfile-diff.yml b/.github/workflows/lockfile-diff.yml index 9d8d59f6da7..aa72e319045 100644 --- a/.github/workflows/lockfile-diff.yml +++ b/.github/workflows/lockfile-diff.yml @@ -79,12 +79,24 @@ jobs: if [ "$CHANGED" = "true" ]; then CONTENT=$(cat /tmp/lockfile-diff.md | python3 -c "import sys,json; print(json.dumps(sys.stdin.read()))") - STATUS="[{\"source\":\"lockfile-diff\",\"results\":[{\"kind\":\"action_required\",\"title\":\"package-lock.json\",\"summary\":\"Locked npm dependency versions changed.\",\"detail\":${CONTENT},\"how_to_fix\":\"Add the \`ci-reviewed\` label after verifying the version changes are expected.\"}]}" + STATUS="[{\"source\":\"lockfile-diff\",\"results\":[{\"kind\":\"action_required\",\"title\":\"package-lock.json\",\"summary\":\"Locked npm dependency versions changed.\",\"detail\":${CONTENT},\"how_to_fix\":\"Add the \`ci-reviewed\` label after verifying the version changes are expected.\"}]}]" else STATUS="[]" fi echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT" + echo "review_status=${STATUS}" > review-status.json + + - name: Upload review status artifact + if: always() && steps.emit-status.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: review-status-lockfile-diff + path: review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore - name: Upload diff artifact uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 diff --git a/.github/workflows/osv-scanner.yml b/.github/workflows/osv-scanner.yml index 455ede33dd5..541c9ee4830 100644 --- a/.github/workflows/osv-scanner.yml +++ b/.github/workflows/osv-scanner.yml @@ -125,3 +125,15 @@ jobs: fi echo "review_status=${STATUS}" >> "$GITHUB_OUTPUT" + echo "review_status=${STATUS}" > review-status.json + + - name: Upload review status artifact + if: always() && steps.emit.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: review-status-osv-scanner + path: review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore diff --git a/.github/workflows/review-labels.yml b/.github/workflows/review-labels.yml index c8ea37dbbca..8b40fcaad22 100644 --- a/.github/workflows/review-labels.yml +++ b/.github/workflows/review-labels.yml @@ -98,9 +98,23 @@ jobs: if [ "$SUPPLY_CHAIN" = "true" ]; then args+=(--supply-chain); fi if [ "$LABEL_PRESENT" = "true" ]; then args+=(--label-present); fi + # Write to both $GITHUB_OUTPUT and review-status.json for the + # live comment poller to pick up as an artifact. python3 scripts/ci/emit_review_status.py "${args[@]}" \ --repo-url "$REPO_URL" --base-sha "$BASE_SHA" --head-sha "$HEAD_SHA" \ --output "$GITHUB_OUTPUT" + grep '^review_status=' "$GITHUB_OUTPUT" > review-status.json + + - name: Upload review status artifact + if: always() && steps.build-status.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: review-status-review-labels + path: review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore - name: Fail on missing label if: steps.label-check.outputs.ci_reviewed != 'true' diff --git a/.github/workflows/supply-chain-audit.yml b/.github/workflows/supply-chain-audit.yml index cca61e03a45..d73b749ab48 100644 --- a/.github/workflows/supply-chain-audit.yml +++ b/.github/workflows/supply-chain-audit.yml @@ -290,4 +290,19 @@ jobs: with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f: f.write(f"review_status={json.dumps(merged)}\n") f.write("critical_findings=" + os.environ.get("CRITICAL_FINDINGS", "false") + "\n") + + # Write review-status.json for the live comment poller artifact. + with open("review-status.json", "w", encoding="utf-8") as f: + f.write(f"review_status={json.dumps(merged)}\n") PYEOF + + - name: Upload review status artifact + if: always() && steps.merge.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: review-status-supply-chain + path: review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore diff --git a/.github/workflows/uv-lockfile-check.yml b/.github/workflows/uv-lockfile-check.yml index aff4f0eb8cb..9fd3a1e4b0a 100644 --- a/.github/workflows/uv-lockfile-check.yml +++ b/.github/workflows/uv-lockfile-check.yml @@ -126,7 +126,20 @@ jobs: echo "::error title=uv.lock out of sync::Run \`uv lock\` locally and commit the result. If on a PR, sync with main first." review_status='[{"source":"uv.lock check","results":[{"kind":"action_required","title":"uv.lock out of sync","summary":"uv.lock is out of sync with pyproject.toml.","how_to_fix":"Run `uv lock` locally and commit the result. If on a PR, sync with main first:\n```\ngit fetch origin main\ngit rebase origin/main\nuv lock\ngit add uv.lock\ngit commit -m \"chore: refresh uv.lock\"\n```\n"}]}]' echo "review_status=${review_status}" >> "$GITHUB_OUTPUT" + echo "review_status=${review_status}" > review-status.json exit 1 fi review_status='[]' echo "review_status=${review_status}" >> "$GITHUB_OUTPUT" + echo "review_status=${review_status}" > review-status.json + + - name: Upload review status artifact + if: always() && steps.verify.outcome != 'skipped' + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: review-status-uv-lockfile + path: review-status.json + retention-days: 1 + overwrite: true + if-no-files-found: ignore diff --git a/scripts/ci/live_comment.py b/scripts/ci/live_comment.py index 71dceaebacd..6ef11818597 100644 --- a/scripts/ci/live_comment.py +++ b/scripts/ci/live_comment.py @@ -20,13 +20,17 @@ Architecture: - :func:`find_comment_id` / :func:`upsert_comment` — thin API wrappers. - - :func:`_fetch_timings_statuses` — downloads the ci-timings artifact - (if available) and parses the ``review_status=`` line from it, merging - the status objects into the review statuses array. + - :func:`fetch_all_review_statuses` — enumerates all + ``review-status-*`` artifacts across the orchestrator run and all + sub-workflow runs, downloads each, parses the ``review_status=`` + line from ``review-status.json``, and merges into one array. + Recomputed from source every poll cycle, so statuses appear as + soon as each job uploads its artifact. - - :func:`run` — the polling loop. Calls the API, classifies, assembles, - upserts, sleeps, repeats. Before its final exit, it gives downstream jobs - a short grace period to appear. + - :func:`run` — the polling loop. Calls the API, classifies, + fetches artifacts, assembles, upserts, sleeps, repeats. Before + its final exit, it gives downstream jobs a short grace period + to appear. The orchestrator job names (detect, all-checks-pass, comment-live, etc.) are excluded from the comment — they're infrastructure, not review signal. @@ -37,11 +41,12 @@ from __future__ import annotations import argparse import json import os -import subprocess +import shutil import sys import time import urllib.error import urllib.request +import zipfile from pathlib import Path API_BASE = "https://api.github.com" @@ -265,39 +270,95 @@ def upsert_comment( # --------------------------------------------------------------------------- -# Artifact fetching (ci-timings review_status) +# Artifact fetching (dynamic review-status artifacts) # --------------------------------------------------------------------------- +# Prefix for all review-status artifacts uploaded by status-producing jobs. +# Each job uploads a ``review-status-`` artifact containing a +# ``review-status.json`` file in GITHUB_OUTPUT format: +# review_status= +_REVIEW_STATUS_ARTIFACT_PREFIX = "review-status-" -def _fetch_artifact_statuses( - token: str, repo: str, run_id: str, artifact_name: str, -) -> list[dict]: - """Download a workflow artifact and extract review_status entries. - The ci-timings job writes a ``review-status.json`` file containing - ``review_status=`` (GITHUB_OUTPUT format) into its artifact. - This function downloads the artifact, parses the line, and returns - the parsed status array. Returns ``[]`` if the artifact doesn't exist - yet or can't be parsed. +def _list_run_ids(token: str, repo: str, run_id: str) -> list[str]: + """Return the orchestrator run ID + all sub-workflow run IDs. + + Sub-workflow runs (workflow_call) may not exist yet on the first few + polls — that's fine, they just won't be in the list. """ + owner, repo_name = repo.split("/") + run_info = _api_request( + f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs/{run_id}", token + ) + created_at = run_info.get("created_at", "") + head_sha = run_info.get("head_sha", "") + + run_ids = [run_id] + + sub_runs = _api_get_paginated( + f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs" + f"?head_sha={head_sha}&event=workflow_call&per_page=100", + token, list_key="workflow_runs", + ) + sub_runs = [r for r in sub_runs if r.get("created_at", "") >= created_at] + run_ids.extend(str(r["id"]) for r in sub_runs) + + return run_ids + + +def _list_artifacts(token: str, repo: str, run_id: str) -> list[dict]: + """List artifacts for a given run (paginated).""" + owner, repo_name = repo.split("/") + return _api_get_paginated( + f"{API_BASE}/repos/{owner}/{repo_name}/actions/runs/{run_id}/artifacts", + token, list_key="artifacts", + ) + + +def _download_artifact( + token: str, repo: str, artifact: dict, dest_dir: Path, +) -> Path | None: + """Download a single artifact zip via the API and extract it. + + Returns the path to ``review-status.json`` inside the extracted dir, + or ``None`` if the download or extraction failed. + """ + owner, repo_name = repo.split("/") + archive_download_url = artifact.get("archive_download_url", "") + if not archive_download_url: + return None + + # The archive_download_url is an API URL that redirects to a S3 URL. + # Build the request with our auth headers so the redirect works. + req = urllib.request.Request(archive_download_url, headers={ + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "ci-live-comment", + }) + zip_path = dest_dir / f"{artifact['name']}.zip" try: - result = subprocess.run( - ["gh", "run", "download", run_id, "--repo", repo, - "--name", artifact_name, "--dir", "/tmp/artifact-dl"], - capture_output=True, timeout=30, - ) - if result.returncode != 0: - return [] + with urllib.request.urlopen(req) as resp: + zip_path.write_bytes(resp.read()) except Exception: - return [] + return None - status_file = Path("/tmp/artifact-dl/review-status.json") - if not status_file.exists(): - return [] + extract_dir = dest_dir / artifact["name"] + extract_dir.mkdir(parents=True, exist_ok=True) + try: + with zipfile.ZipFile(zip_path) as zf: + zf.extractall(extract_dir) + except Exception: + return None + status_file = extract_dir / "review-status.json" + return status_file if status_file.exists() else None + + +def _parse_status_file(status_file: Path) -> list[dict]: + """Parse a review-status.json file in GITHUB_OUTPUT format.""" try: content = status_file.read_text(encoding="utf-8").strip() - # GITHUB_OUTPUT format: review_status= if content.startswith("review_status="): content = content[len("review_status="):] statuses = json.loads(content) @@ -305,10 +366,59 @@ def _fetch_artifact_statuses( return statuses except (json.JSONDecodeError, OSError): pass - return [] +def fetch_all_review_statuses( + token: str, repo: str, run_id: str, +) -> list[dict]: + """Fetch and merge all review-status artifacts across all runs. + + Enumerates artifacts with the ``review-status-`` prefix from the + orchestrator run and all sub-workflow runs (workflow_call). Downloads + each, parses the ``review-status.json`` inside, and merges into a + single flat array. + + Returns the merged list of ``{source, results: [...]}`` objects. + Artifacts that don't exist yet or fail to parse are silently skipped. + """ + all_statuses: list[dict] = [] + temp_base = Path("/tmp/review-status-artifacts") + + try: + run_ids = _list_run_ids(token, repo, run_id) + except Exception: + return all_statuses + + for rid in run_ids: + try: + artifacts = _list_artifacts(token, repo, rid) + except Exception: + continue + + rs_artifacts = [ + a for a in artifacts + if a.get("name", "").startswith(_REVIEW_STATUS_ARTIFACT_PREFIX) + ] + if not rs_artifacts: + continue + + # Clean temp dir for this run's artifacts. + run_dl_dir = temp_base / rid + if run_dl_dir.exists(): + shutil.rmtree(run_dl_dir) + run_dl_dir.mkdir(parents=True, exist_ok=True) + + for artifact in rs_artifacts: + status_file = _download_artifact(token, repo, artifact, run_dl_dir) + if status_file is None: + continue + statuses = _parse_status_file(status_file) + all_statuses.extend(statuses) + + return all_statuses + + # --------------------------------------------------------------------------- # Comment assembly # --------------------------------------------------------------------------- @@ -344,12 +454,9 @@ def build_comment_body( ) -def _merge_statuses( - base_statuses: list[dict], extra_statuses: list[dict] -) -> str: - """Merge two status arrays into one JSON string.""" - merged = list(base_statuses) + list(extra_statuses) - return json.dumps(merged) if merged else "" +def _merge_statuses(statuses: list[dict]) -> str: + """Merge a list of status arrays into one JSON string.""" + return json.dumps(statuses) if statuses else "" def _commit_info_for_state(commit_info: str, pending: list[str]) -> str: @@ -370,7 +477,6 @@ def run( run_id: str, pr_number: str, run_url: str, - review_statuses_json: str = "", commit_info: str = "", interval: int = 15, timeout: int = 1800, @@ -386,13 +492,7 @@ def run( quiet_grace_used = False prev_completed: dict[str, str] = {} prev_pending: list[str] = [] - - # Parse the base statuses once (from review-labels, lockfile-diff, etc.) - try: - base_statuses = json.loads(review_statuses_json) if review_statuses_json else [] - except (json.JSONDecodeError, TypeError): - base_statuses = [] - print(f" Loaded {len(base_statuses)} base review status entries") + prev_artifact_count = 0 while True: elapsed = time.time() - start @@ -426,14 +526,14 @@ def run( if gone_pending: print(f" → {len(gone_pending)} job(s) disappeared from pending: {', '.join(gone_pending)}") - # Try to fetch ci-timings artifact statuses (may not exist yet). - artifact_statuses = _fetch_artifact_statuses( - token, repo, run_id, "ci-timings-review-status", - ) - if artifact_statuses: - print(f" Found ci-timings artifact with {len(artifact_statuses)} status entries") + # Dynamically fetch all review-status artifacts from every run. + artifact_statuses = fetch_all_review_statuses(token, repo, run_id) + if len(artifact_statuses) != prev_artifact_count: + print(f" Found {len(artifact_statuses)} review status entries from artifacts " + f"(was {prev_artifact_count} last poll)") + prev_artifact_count = len(artifact_statuses) - merged_json = _merge_statuses(base_statuses, artifact_statuses) + merged_json = _merge_statuses(artifact_statuses) current_commit_info = _commit_info_for_state(commit_info, pending) body = build_comment_body( @@ -450,7 +550,7 @@ def run( change_reasons.append(f"{len(new_pending)} new pending job(s)") if gone_pending: change_reasons.append(f"{len(gone_pending)} job(s) left pending") - if artifact_statuses: + if len(artifact_statuses) != prev_artifact_count: change_reasons.append("artifact statuses updated") if not change_reasons: change_reasons.append("initial post") @@ -507,8 +607,6 @@ def main() -> int: help="Seconds between polls (default: 15).") parser.add_argument("--timeout", type=int, default=1800, help="Max seconds to poll before giving up (default: 1800).") - parser.add_argument("--review-statuses-file", type=Path, default=None, - help="Path to a JSON file with merged review statuses from workflow_call jobs.") parser.add_argument("--dry-run", action="store_true", help="Print comment body instead of posting to PR.") args = parser.parse_args() @@ -533,14 +631,6 @@ def main() -> int: print("PR_NUMBER is required", file=sys.stderr) return 1 - # Read merged review statuses from file (prepared by the ci.yml step). - review_statuses_json = "" - if args.review_statuses_file: - try: - review_statuses_json = args.review_statuses_file.read_text(encoding="utf-8") - except OSError as e: - print(f"Warning: could not read review statuses file: {e}", file=sys.stderr) - # Build commit info line from env vars (set by ci.yml). commit_sha = os.environ.get("COMMIT_SHA", "") commit_msg = os.environ.get("COMMIT_MESSAGE", "") @@ -566,7 +656,6 @@ def main() -> int: run_id=run_id, pr_number=pr_number, run_url=run_url, - review_statuses_json=review_statuses_json, commit_info=commit_info, interval=args.interval, timeout=args.timeout, diff --git a/tests/ci/test_live_comment.py b/tests/ci/test_live_comment.py index c4d48027058..5008d9be4a6 100644 --- a/tests/ci/test_live_comment.py +++ b/tests/ci/test_live_comment.py @@ -1,14 +1,17 @@ -"""Tests for scripts/ci/live_comment.py — classify_jobs(). +"""Tests for scripts/ci/live_comment.py — classify_jobs() + artifact helpers. The poller's core logic is a pure function: take raw GitHub API job dicts -and split them into (completed, pending). The API wrapper + polling loop -are tested via E2E in CI, not here. +and split them into (completed, pending). Artifact parsing helpers are also +pure and tested here. The API wrapper + polling loop are tested via E2E +in CI, not here. """ from __future__ import annotations import importlib.util +import json import sys +import tempfile from pathlib import Path _PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "live_comment.py" @@ -172,3 +175,85 @@ def test_commit_info_uses_past_tense_after_jobs_complete(): assert _mod._commit_info_for_state(info, []) == ( "ran on [abc1234](https://commit-url) — fix: thing" ) + + +# --------------------------------------------------------------------------- +# Artifact parsing helpers +# --------------------------------------------------------------------------- + +def test_parse_status_file_with_prefix(): + """GITHUB_OUTPUT format: review_status=""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + f.write('review_status=[{"source":"test","results":[]}]') + f.flush() + statuses = _mod._parse_status_file(Path(f.name)) + assert len(statuses) == 1 + assert statuses[0]["source"] == "test" + + +def test_parse_status_file_without_prefix(): + """Raw JSON (no review_status= prefix) is also accepted.""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + f.write('[{"source":"raw","results":[]}]') + f.flush() + statuses = _mod._parse_status_file(Path(f.name)) + assert len(statuses) == 1 + assert statuses[0]["source"] == "raw" + + +def test_parse_status_file_empty_array(): + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + f.write('review_status=[]') + f.flush() + statuses = _mod._parse_status_file(Path(f.name)) + assert statuses == [] + + +def test_parse_status_file_invalid_json(): + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + f.write('review_status=not json') + f.flush() + statuses = _mod._parse_status_file(Path(f.name)) + assert statuses == [] + + +def test_parse_status_file_nonexistent(): + assert _mod._parse_status_file(Path("/nonexistent/file.json")) == [] + + +def test_parse_status_file_not_a_list(): + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + f.write('review_status={"not":"a list"}') + f.flush() + statuses = _mod._parse_status_file(Path(f.name)) + assert statuses == [] + + +def test_merge_statuses_empty(): + assert _mod._merge_statuses([]) == "" + + +def test_merge_statuses_single(): + statuses = [{"source": "a", "results": []}] + result = _mod._merge_statuses(statuses) + assert json.loads(result) == statuses + + +def test_merge_statuses_multiple(): + statuses = [ + {"source": "a", "results": []}, + {"source": "b", "results": [{"kind": "warning"}]}, + ] + result = _mod._merge_statuses(statuses) + assert json.loads(result) == statuses + + +def test_review_status_artifact_prefix(): + """The prefix is used to filter artifacts from the API.""" + assert _mod._REVIEW_STATUS_ARTIFACT_PREFIX == "review-status-" + # Artifacts with this prefix should be picked up + assert "review-status-ci-timings".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX) + assert "review-status-review-labels".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX) + # Artifacts without it should not + assert not "ci-timings-report".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX) + assert not "playwright-report".startswith(_mod._REVIEW_STATUS_ARTIFACT_PREFIX)