diff --git a/tests/unified/runner.py b/tests/unified/runner.py index 0e407d4f..8fa0374c 100644 --- a/tests/unified/runner.py +++ b/tests/unified/runner.py @@ -8,7 +8,7 @@ import time from dataclasses import dataclass from datetime import UTC, datetime from pathlib import Path -from typing import Any +from typing import Any, ClassVar import httpx from anthropic import AsyncAnthropic @@ -81,22 +81,35 @@ class TestExecutionError(Exception): DISCORD_MAX_CONTENT = 2000 -def truncate(text: str, limit: int) -> str: - """Clip `text` to `limit` characters, marking that it was clipped.""" - if len(text) <= limit: - return text - return text[: limit - 1].rstrip() + "…" +def clamp_lines(lines: list[str], limit: int) -> str: + """Join `lines` within `limit`, dropping the longest ones first if needed. + + Whole lines rather than characters: a presigned URL cut in half is useless + and renders as broken markdown. Longest-first rather than last-first because + the only lines that can blow the budget are presigned URLs — dropping one of + those keeps every short, always-valid link, the Actions run link above all, + instead of losing them to a long URL that merely came first. + """ + kept = list(range(len(lines))) + + def size() -> int: + return sum(len(lines[i]) for i in kept) + max(0, len(kept) - 1) + + while kept and size() > limit: + kept.remove(max(kept, key=lambda i: len(lines[i]))) + return "\n".join(lines[i] for i in sorted(kept)) -async def send_discord_message(webhook_url: str, message: str) -> None: - """Send a message to Discord via webhook. +async def send_discord_message(webhook_url: str, lines: list[str]) -> None: + """Send a report to Discord via webhook. - Clamped to Discord's content limit here rather than at the call site, so an - over-long report loses its tail instead of the whole notification. + Clamped to Discord's content limit here rather than at the call site: a + presigned URL carries an OIDC session token and can run past a thousand + characters on its own, and a 400 loses the whole notification. """ try: async with httpx.AsyncClient() as client: - content = truncate(message, DISCORD_MAX_CONTENT) + content = clamp_lines(lines, DISCORD_MAX_CONTENT) response = await client.post(webhook_url, json={"content": content}) response.raise_for_status() logger.info("Discord notification sent successfully") @@ -120,6 +133,9 @@ class StepFailure: class TestOutcome: """One test's result. `failure` carries the reason whenever status isn't PASS.""" + # Not a pytest case despite the name; keeps collection from warning on it. + __test__: ClassVar[bool] = False + status: str duration: float failure: StepFailure | None = None @@ -159,49 +175,47 @@ def presign(s3_client: Any, bucket: str, key: str) -> RunArtifact: return RunArtifact(key=key) -def artifact_lines(artifacts: RunArtifacts) -> list[str]: - """Render each uploaded artifact as one markdown line: link when presigned, key otherwise. +def artifact_line(label: str, artifact: RunArtifact | None) -> list[str]: + """One markdown line for an artifact: a link when presigned, the key otherwise.""" + if artifact is None: + return [] + if artifact.url: + return [f"[{label}]({artifact.url}) — `{artifact.key}`"] + return [f"{label}: `{artifact.key}`"] - Both artifacts are surfaced. The reasoning traces carry the full prompts and + +def artifact_lines(artifacts: RunArtifacts) -> list[str]: + """Both uploaded artifacts. The reasoning traces carry the full prompts and model outputs for the run, which is what a failure usually needs to diagnose. """ - labels = [ - ("Complete results", artifacts.results), - ("Reasoning traces", artifacts.traces), - ] - lines: list[str] = [] - for label, artifact in labels: - if artifact is None: - continue - if artifact.url: - lines.append(f"[{label}]({artifact.url}) — `{artifact.key}`") - else: - lines.append(f"{label}: `{artifact.key}`") - return lines + return artifact_line("View Complete Results", artifacts.results) + artifact_line( + "Reasoning traces", artifacts.traces + ) -def failure_lines( - results: dict[str, "TestOutcome"], - limit: int | None = None, - max_reason_chars: int | None = None, -) -> list[str]: - """One markdown bullet per failing test, naming the step and the reason. +def gha_run_lines() -> list[str]: + """Link to this run's Actions page, which hosts the job summary. - An LLM-judge verdict runs to several hundred characters, so callers with a - size budget pass `max_reason_chars`; the job summary and S3 keep them whole. + That summary carries the per-test failure reasons in full, so the Discord + message can stay short and point at it instead of restating them. """ + run_id = os.getenv("GITHUB_RUN_ID") + repository = os.getenv("GITHUB_REPOSITORY") + if not run_id or not repository: + return [] + server = os.getenv("GITHUB_SERVER_URL", "https://github.com") + return [f"[View GHA]({server}/{repository}/actions/runs/{run_id})"] + + +def failure_lines(results: dict[str, "TestOutcome"]) -> list[str]: + """One markdown bullet per failing test, naming the step and the reason.""" failed = [(name, o) for name, o in results.items() if o.status != "PASS"] if not failed: return [] - shown = failed if limit is None else failed[:limit] lines = ["", "**Failures**"] - for name, outcome in shown: + for name, outcome in failed: reason = outcome.failure.describe() if outcome.failure else outcome.status - if max_reason_chars is not None: - reason = truncate(reason, max_reason_chars) lines.append(f"- `{name}` — {reason}") - if len(shown) < len(failed): - lines.append(f"- …and {len(failed) - len(shown)} more") return lines @@ -948,12 +962,15 @@ class UnifiedTestRunner: f"{status_emoji} **Unified Test Results**", headline, f"Execution time: {total_suite_time:.2f}s", - *failure_lines(results, limit=10, max_reason_chars=160), - *artifact_lines(artifacts), + *artifact_line("View Complete Results", artifacts.results), + *gha_run_lines(), + *( + [f"Reasoning traces: `{artifacts.traces.key}`"] + if artifacts.traces + else [] + ), ] - await send_discord_message( - discord_webhook_url, "\n".join(message_lines) - ) + await send_discord_message(discord_webhook_url, message_lines) return failed_count diff --git a/tests/unified/test_reporting.py b/tests/unified/test_reporting.py new file mode 100644 index 00000000..48734232 --- /dev/null +++ b/tests/unified/test_reporting.py @@ -0,0 +1,143 @@ +"""Tests for how a unified run is reported to Discord and the job summary. + +Discord rejects an over-long payload with a 400, which loses the whole +notification, so the size behavior here is worth pinning down. +""" + +from __future__ import annotations + +import pytest + +from tests.unified.runner import ( + DISCORD_MAX_CONTENT, + RunArtifact, + RunArtifacts, + StepFailure, + TestOutcome, + artifact_line, + artifact_lines, + clamp_lines, + failure_lines, + gha_run_lines, +) + +_PREFIX = "unified-test-results/2026-09-03/1123-merge-abc1234-33779689337-1" + + +def _presigned(name: str, token_len: int) -> RunArtifact: + """A presigned URL of realistic shape; OIDC session tokens dominate its length.""" + url = ( + f"https://honcho-unified-tests.s3.amazonaws.com/{_PREFIX}/{name}" + "?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Expires=259200" + f"&X-Amz-Security-Token={'t' * token_len}&X-Amz-Signature={'0' * 64}" + ) + return RunArtifact(key=f"{_PREFIX}/{name}", url=url) + + +def _discord_lines(artifacts: RunArtifacts) -> list[str]: + """Mirror of the Discord report the runner assembles.""" + return [ + "⚠️ **Unified Test Results**", + "Results: 35/41 passed, 6/41 failed", + "Execution time: 1015.42s", + *artifact_line("View Complete Results", artifacts.results), + *gha_run_lines(), + *([f"Reasoning traces: `{artifacts.traces.key}`"] if artifacts.traces else []), + ] + + +@pytest.fixture +def in_actions(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GITHUB_RUN_ID", "33779689337") + monkeypatch.setenv("GITHUB_REPOSITORY", "plastic-labs/honcho") + + +def test_clamp_lines_leaves_a_short_report_alone() -> None: + lines = ["one", "two", "three"] + assert clamp_lines(lines, DISCORD_MAX_CONTENT) == "one\ntwo\nthree" + + +def test_clamp_lines_drops_the_longest_line_not_the_last() -> None: + """The Actions link is short and leads everywhere; a presigned URL is neither.""" + lines = ["head", "x" * 100, "[View GHA](url)"] + assert clamp_lines(lines, 40) == "head\n[View GHA](url)" + + +def test_clamp_lines_preserves_display_order() -> None: + lines = ["a", "y" * 50, "b", "c"] + assert clamp_lines(lines, 10) == "a\nb\nc" + + +@pytest.mark.usefixtures("in_actions") +@pytest.mark.parametrize("token_len", [0, 400, 900, 1400, 1800]) +def test_discord_report_never_exceeds_the_webhook_limit(token_len: int) -> None: + """Regression: six judge verdicts plus two presigned URLs returned a 400.""" + artifacts = RunArtifacts( + results=_presigned("results.json", token_len), + traces=_presigned("unified-reasoning-traces.jsonl", token_len), + ) + sent = clamp_lines(_discord_lines(artifacts), DISCORD_MAX_CONTENT) + assert len(sent) <= DISCORD_MAX_CONTENT + + +@pytest.mark.usefixtures("in_actions") +@pytest.mark.parametrize("token_len", [0, 400, 900, 1400, 1800]) +def test_actions_link_always_survives_clamping(token_len: int) -> None: + """However long the presigned URLs get, the run stays reachable.""" + artifacts = RunArtifacts( + results=_presigned("results.json", token_len), + traces=_presigned("unified-reasoning-traces.jsonl", token_len), + ) + sent = clamp_lines(_discord_lines(artifacts), DISCORD_MAX_CONTENT) + assert ( + "[View GHA](https://github.com/plastic-labs/honcho/actions/runs/33779689337)" + in sent + ) + + +@pytest.mark.usefixtures("in_actions") +def test_discord_report_omits_per_test_failures() -> None: + """Failure detail belongs in the job summary the Actions link points at.""" + artifacts = RunArtifacts(results=_presigned("results.json", 400)) + assert not any("**Failures**" in line for line in _discord_lines(artifacts)) + + +def test_gha_lines_are_empty_outside_actions(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("GITHUB_RUN_ID", raising=False) + monkeypatch.delenv("GITHUB_REPOSITORY", raising=False) + assert gha_run_lines() == [] + + +def test_artifact_line_falls_back_to_the_key_when_presigning_failed() -> None: + assert artifact_line("Traces", RunArtifact(key="k/x.jsonl")) == [ + "Traces: `k/x.jsonl`" + ] + assert artifact_line("Traces", None) == [] + + +def test_job_summary_keeps_both_signed_links() -> None: + artifacts = RunArtifacts( + results=_presigned("results.json", 900), + traces=_presigned("unified-reasoning-traces.jsonl", 900), + ) + lines = artifact_lines(artifacts) + assert len(lines) == 2 + assert all("https://" in line for line in lines) + + +def test_failure_lines_reports_every_failure_in_full() -> None: + reason = "LLM Judge failed: " + "the model did not recall the fact. " * 20 + results = { + "a.json": TestOutcome("FAIL", 1.0, StepFailure(4, "query", reason)), + "b.json": TestOutcome("PASS", 1.0), + "c.json": TestOutcome("INVALID SCHEMA", 0.1), + } + lines = failure_lines(results) + assert lines[:2] == ["", "**Failures**"] + assert len(lines) == 4 # blank, header, and one bullet per non-PASS + assert reason in lines[2] # untruncated + assert "INVALID SCHEMA" in lines[3] # falls back to status when no StepFailure + + +def test_failure_lines_empty_when_everything_passed() -> None: + assert failure_lines({"a.json": TestOutcome("PASS", 1.0)}) == []