* refactor(tools): extract shared module for gh spawnSync logic * feat(tools): add repo parameter to 5 GitHub tools * test(tools): add tunnel tool tests * test(tools): add repo parameter test cases for 5 tools * docs(handoff): scaffold handoff and ADR for PR * docs(handoff): set PR number * docs(project-map): update after PR#65 structural changes --------- Co-authored-by: opencode-agent <agent@opencode.local>
304 lines
12 KiB
Python
304 lines
12 KiB
Python
"""Tests for .opencode/tools/post-docs-review.ts — the post-docs-review custom tool.
|
|
|
|
Mirrors tests/test_commit_tool.py / test_create_pr_tool.py:
|
|
exercises the tool's ``execute()`` function via ``tests/_ts_loader.mjs``
|
|
using the ``exec_stub_json`` mode (multi-arg tools).
|
|
|
|
The loader is parameterized via the ``TS_FILE`` env var. These tests set
|
|
``TS_FILE=.opencode/tools/post-docs-review.ts``.
|
|
|
|
Modes used:
|
|
- ``load`` — sanity-check that the tool loads and declares pr_number, verdict,
|
|
body args.
|
|
- ``exec_stub_json`` — call execute with a stubbed spawnSync to verify:
|
|
(a) success path: valid verdict (APPROVE/FIXED/NO_CHANGES)
|
|
→ "Docs review posted on PR #N: verdict=<V>",
|
|
(b) comment heading: body passed to gh contains "## Docs Review Summary",
|
|
(c) comment verdict: body passed to gh contains "### Verdict: <V>",
|
|
(d) spawnSync args: gh pr comment N --body <comment> (no --repo; gh auto-detects
|
|
from context.worktree — ADR-019 superseding note, PR#60).
|
|
|
|
post-docs-review.ts makes 1 spawnSync call (gh pr comment) on all paths.
|
|
|
|
Note on enum validation: see test_post_review_tool.py — same delegation to
|
|
zod schema, execute() itself is permissive.
|
|
"""
|
|
|
|
import json
|
|
import os
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
|
LOADER = REPO_ROOT / "tests" / "_ts_loader.mjs"
|
|
TS_FILE = REPO_ROOT / ".opencode" / "tools" / "post-docs-review.ts"
|
|
TS_FILE_REL = ".opencode/tools/post-docs-review.ts"
|
|
|
|
COMMENT_URL = "https://github.com/slaid098/opencode-config/issues/45#issuecomment-1"
|
|
COMMENT_OK_RESPONSE = {"status": 0, "stdout": COMMENT_URL + "\n", "stderr": ""}
|
|
EXPECTED_APPROVE = "Docs review posted on PR #45: verdict=APPROVE"
|
|
EXPECTED_FIXED = "Docs review posted on PR #45: verdict=FIXED"
|
|
EXPECTED_NO_CHANGES = "Docs review posted on PR #45: verdict=NO_CHANGES"
|
|
EXPECTED_APROVE_TYPO = "Docs review posted on PR #45: verdict=APROVE"
|
|
|
|
|
|
def _run_loader(*args: str) -> dict:
|
|
"""Invoke the loader with TS_FILE env set to post-docs-review.ts and parse JSON stdout."""
|
|
env = {**os.environ, "TS_FILE": TS_FILE_REL}
|
|
proc = subprocess.run(
|
|
["node", str(LOADER), *args],
|
|
capture_output=True,
|
|
text=True,
|
|
check=False,
|
|
cwd=str(REPO_ROOT),
|
|
timeout=60,
|
|
env=env,
|
|
)
|
|
if proc.returncode != 0:
|
|
raise RuntimeError(
|
|
f"_ts_loader.mjs {' '.join(args)} failed (exit {proc.returncode}):\n"
|
|
f"stdout: {proc.stdout}\nstderr: {proc.stderr}"
|
|
)
|
|
return json.loads(proc.stdout)
|
|
|
|
|
|
def _run_exec(args: dict, responses: list[dict]) -> dict:
|
|
"""Helper: exec_stub_json mode with JSON args + sequential stub responses."""
|
|
return _run_loader("exec_stub_json", json.dumps(args), json.dumps(responses))
|
|
|
|
|
|
def test_loader_can_load_tool():
|
|
"""Sanity: post-docs-review.ts loads and declares pr_number, verdict, body args."""
|
|
if not TS_FILE.exists():
|
|
pytest.skip("post-docs-review.ts not present")
|
|
out = _run_loader("load")
|
|
assert "description" in out
|
|
args = out["args"]
|
|
assert "pr_number" in args, f"missing pr_number arg: {args}"
|
|
assert "verdict" in args, f"missing verdict arg: {args}"
|
|
assert "body" in args, f"missing body arg: {args}"
|
|
|
|
|
|
def test_valid_approve():
|
|
"""execute() with APPROVE verdict returns 'Docs review posted on PR #N: verdict=APPROVE'."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "- Handoff: valid"},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert result == EXPECTED_APPROVE, f"expected success, got: {result!r}"
|
|
|
|
|
|
def test_valid_fixed():
|
|
"""execute() with FIXED verdict returns success."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "FIXED", "body": "- Handoff: fixed: added section"},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert result == EXPECTED_FIXED, f"expected success, got: {result!r}"
|
|
|
|
|
|
def test_valid_no_changes():
|
|
"""execute() with NO_CHANGES verdict returns success."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "NO_CHANGES", "body": "- No structural changes"},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert result == EXPECTED_NO_CHANGES, f"expected success, got: {result!r}"
|
|
|
|
|
|
def test_invalid_verdict_not_validated_by_execute():
|
|
"""execute() does NOT validate verdict inline — zod enum does at opencode layer.
|
|
|
|
The _ts_loader.mjs shim does not validate (chainable builder without checks).
|
|
Passing a typo "APROVE" (missing a P) builds a comment with the bad verdict.
|
|
The real guard is tool.schema.enum(VERDICTS) in the opencode runtime.
|
|
This test documents that execute() itself is permissive — validation is
|
|
delegated to the zod schema, not duplicated inside execute().
|
|
"""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APROVE", "body": "typo verdict"},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert result == EXPECTED_APROVE_TYPO, f"expected permissive, got: {result!r}"
|
|
|
|
|
|
def test_comment_has_heading():
|
|
"""Comment body passed to gh contains '## Docs Review Summary' heading."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
calls = out["calls"]
|
|
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
|
args = calls[0]["args"]
|
|
body_idx = args.index("--body") + 1
|
|
comment = args[body_idx]
|
|
assert comment.startswith("## Docs Review Summary\n"), (
|
|
f"expected heading at start, got: {comment!r}"
|
|
)
|
|
|
|
|
|
def test_comment_has_verdict():
|
|
"""Comment body passed to gh contains '### Verdict: <verdict>'."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
calls = out["calls"]
|
|
args = calls[0]["args"]
|
|
body_idx = args.index("--body") + 1
|
|
comment = args[body_idx]
|
|
assert "### Verdict: APPROVE" in comment, f"expected verdict line, got: {comment!r}"
|
|
|
|
|
|
def test_comment_has_fixed_verdict():
|
|
"""Comment body with FIXED contains '### Verdict: FIXED'."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "FIXED", "body": "- Handoff: fixed"},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
calls = out["calls"]
|
|
args = calls[0]["args"]
|
|
body_idx = args.index("--body") + 1
|
|
comment = args[body_idx]
|
|
assert "### Verdict: FIXED" in comment, f"expected verdict line, got: {comment!r}"
|
|
|
|
|
|
def test_comment_has_no_changes_verdict():
|
|
"""Comment body with NO_CHANGES contains '### Verdict: NO_CHANGES'."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "NO_CHANGES", "body": "- No changes"},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
calls = out["calls"]
|
|
args = calls[0]["args"]
|
|
body_idx = args.index("--body") + 1
|
|
comment = args[body_idx]
|
|
assert "### Verdict: NO_CHANGES" in comment, f"expected verdict line, got: {comment!r}"
|
|
|
|
|
|
def test_spawnsync_args():
|
|
"""spawnSync called with gh pr comment <N> --body <comment> (no --repo).
|
|
|
|
--repo was hardcoded to slaid098/opencode-config (ADR-019) and caused silent
|
|
misroute to the wrong repo when working outside opencode-config. Removed in
|
|
PR#60 — gh now auto-detects the repo from context.worktree (cwd), matching
|
|
create-pr.ts/merge-pr.ts. Symmetric with the success-path assertions.
|
|
"""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
calls = out["calls"]
|
|
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
|
call = calls[0]
|
|
assert call["cmd"] == "gh", f"expected cmd 'gh', got: {call['cmd']!r}"
|
|
args = call["args"]
|
|
assert args[0] == "pr", f"expected first arg 'pr', got: {args[0]!r}"
|
|
assert args[1] == "comment", f"expected second arg 'comment', got: {args[1]!r}"
|
|
assert args[2] == "45", f"expected PR number '45', got: {args[2]!r}"
|
|
assert "--body" in args, "missing --body flag"
|
|
# --repo MUST NOT be present — gh auto-detects from cwd (context.worktree).
|
|
# Hardcoded --repo caused silent misroute (PR#60 root cause).
|
|
assert "--repo" not in args, (
|
|
f"--repo must not be hardcoded; gh auto-detects from cwd. args: {args!r}"
|
|
)
|
|
# args must end with --body <comment> (no trailing --repo slaid098/...).
|
|
assert args[-2] == "--body", f"expected args to end with --body <comment>, got: {args[-2:]!r}"
|
|
assert args[-1].startswith("## Docs Review Summary\n"), (
|
|
f"expected last arg to be the comment body, got: {args[-1]!r}"
|
|
)
|
|
|
|
|
|
def test_execute_uses_cwd_from_context():
|
|
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
calls = out["calls"]
|
|
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
|
opts = calls[0]["opts"]
|
|
assert opts is not None, "spawnSync called without opts — expected cwd kwarg"
|
|
assert "cwd" in opts, f"opts missing 'cwd' key — got: {opts}"
|
|
assert opts["cwd"] == str(REPO_ROOT), (
|
|
f"cwd must equal context.worktree ({REPO_ROOT}), got: {opts['cwd']!r}"
|
|
)
|
|
|
|
|
|
def test_gh_failure_returns_error():
|
|
"""execute() with gh exit non-zero returns error message with exit code."""
|
|
fail_response = {"status": 1, "stdout": "", "stderr": "gh: not authenticated"}
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
|
[fail_response],
|
|
)
|
|
result = out["result"]
|
|
assert "post-docs-review failed" in result, f"expected failure message, got: {result!r}"
|
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
|
|
|
|
|
def test_repo_explicit_passed_to_gh():
|
|
"""execute() with repo='foo/bar' → gh receives '--repo foo/bar' before the subcommand.
|
|
|
|
The shared runGh helper prepends ['--repo', <repo>] to the gh argv so an
|
|
explicit repo targets the right owner/name regardless of context.worktree.
|
|
"""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body.", "repo": "foo/bar"},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert result == EXPECTED_APPROVE, f"expected success, got: {result!r}"
|
|
calls = out["calls"]
|
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
|
args = calls[0]["args"]
|
|
assert args[0] == "--repo", f"expected --repo first, got: {args[0]!r}"
|
|
assert args[1] == "foo/bar", f"expected repo value, got: {args[1]!r}"
|
|
assert args[2] == "pr", f"expected 'pr' after --repo <name>, got: {args[2]!r}"
|
|
|
|
|
|
def test_repo_omitted_no_repo_flag():
|
|
"""execute() without repo → gh argv has NO --repo (auto-detect from cwd).
|
|
|
|
Backward-compatibility: when repo is omitted, runGh returns [] from
|
|
parseRepo, so gh auto-detects the repo from context.worktree (cwd) —
|
|
matching the pre-refactor behaviour (ADR-025 / PR#61). This complements
|
|
test_spawnsync_args which asserts the same on the success path.
|
|
"""
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
|
[COMMENT_OK_RESPONSE],
|
|
)
|
|
calls = out["calls"]
|
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
|
args = calls[0]["args"]
|
|
assert "--repo" not in args, (
|
|
f"--repo must NOT be added when repo arg omitted; gh auto-detects. args: {args!r}"
|
|
)
|
|
assert args[0] == "pr", f"expected 'pr' first, got: {args[0]!r}"
|
|
|
|
|
|
def test_repo_invalid_gh_error():
|
|
"""execute() with invalid repo + gh failure → error mentions post-docs-review + exit code.
|
|
|
|
gh rejects an invalid owner/name with non-zero exit; the tool formats the
|
|
error with its tool-specific message (post-docs-review failed for PR #N).
|
|
"""
|
|
fail_response = {"status": 1, "stdout": "", "stderr": 'expected the "owner/repo" format'}
|
|
out = _run_exec(
|
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body.", "repo": "not-a-valid-repo"},
|
|
[fail_response],
|
|
)
|
|
result = out["result"]
|
|
assert "post-docs-review failed" in result, f"expected tool failure, got: {result!r}"
|
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
|
calls = out["calls"]
|
|
args = calls[0]["args"]
|
|
assert "--repo" in args, f"expected --repo in args even on failure, got: {args!r}"
|