fix(tests): keep the Discord report short and link to the Actions run
The Discord message restated every failure, which pushed it past Discord's 2000-character limit and returned a 400 — run 33779689337 sent no notification at all. The report is now the headline, the results link, an Actions run link, and the traces S3 key. Per-test failure reasons stay in the job summary that the Actions link points at, along with both presigned URLs, so nothing is lost by not repeating them in chat. Restating failures was not the only size risk. A presigned URL carries an OIDC session token and can run past a thousand characters by itself, so two of them exceeded the limit unaided — which is why the traces go in as their S3 key, the `aws s3 cp` path, at ~90 characters instead of ~1500. `clamp_lines` drops whole lines rather than characters, since half a presigned URL is useless and renders as broken markdown, and drops the longest line first so an overlong URL cannot evict the short Actions link that leads to everything else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Eugene Eisenstein
parent
4ba8d2ec04
commit
568c4c6967
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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)}) == []
|
||||
Loading…
Reference in New Issue