0c1e3e7d39
* feat(create-pr): enforce Watch out and Pending headings * test(create-pr): cover Watch out and Pending headings * docs(handoff): add handoff and ADR for PR #207 --------- Co-authored-by: opencode-agent <agent@opencode.local>
265 lines
11 KiB
Python
265 lines
11 KiB
Python
"""Tests for .opencode/tools/create-pr.ts — the create-pr custom tool.
|
|
|
|
Mirrors tests/test_pipeline_status_tool.py / test_memory_setup_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/create-pr.ts``.
|
|
|
|
Modes used:
|
|
- ``load`` — sanity-check that the tool loads and declares title, body,
|
|
issue_number args.
|
|
- ``exec_stub_json`` — call execute with a stubbed spawnSync to verify:
|
|
(a) success path: valid title + body → "PR created: <url>",
|
|
(b) validation errors: missing scope, missing headings, Latin-only body,
|
|
(c) issue linkage: issue_number → body gets "Closes #N" appended.
|
|
|
|
create-pr.ts makes 1 spawnSync call (gh pr create) on the success path.
|
|
"""
|
|
|
|
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" / "create-pr.ts"
|
|
TS_FILE_REL = ".opencode/tools/create-pr.ts"
|
|
|
|
VALID_TITLE = "feat(tools): add create-pr tool validation"
|
|
VALID_BODY = (
|
|
"## Что сделано\nДобавлен tool\n\n"
|
|
"## Почему\nНужна валидация\n\n"
|
|
"## Watch out\n—\n\n"
|
|
"## Pending\n—"
|
|
)
|
|
PR_URL = "https://github.com/slaid098/opencode-config/pull/38"
|
|
PR_OK_RESPONSE = {"status": 0, "stdout": PR_URL + "\n", "stderr": ""}
|
|
|
|
|
|
def _run_loader(*args: str) -> dict:
|
|
"""Invoke the loader with TS_FILE env set to create-pr.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: create-pr.ts loads and declares title, body, issue_number args."""
|
|
if not TS_FILE.exists():
|
|
pytest.skip("create-pr.ts not present")
|
|
out = _run_loader("load")
|
|
assert "description" in out
|
|
args = out["args"]
|
|
assert "title" in args, f"missing title arg: {args}"
|
|
assert "body" in args, f"missing body arg: {args}"
|
|
assert "issue_number" in args, f"missing issue_number arg: {args}"
|
|
|
|
|
|
def test_valid_pr():
|
|
"""execute() with valid title + body returns 'PR created: <url>'."""
|
|
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [PR_OK_RESPONSE])
|
|
result = out["result"]
|
|
assert result == f"PR created: {PR_URL}", f"expected success, got: {result!r}"
|
|
|
|
|
|
def test_missing_scope():
|
|
"""execute() with title missing scope → error mentioning format."""
|
|
out = _run_exec({"title": "feat: no scope", "body": VALID_BODY}, [PR_OK_RESPONSE])
|
|
result = out["result"]
|
|
assert "must match format" in result, f"expected format error, got: {result!r}"
|
|
|
|
|
|
def test_missing_chto_sdelano():
|
|
"""execute() with body missing '## Что сделано' → error mentioning heading."""
|
|
out = _run_exec(
|
|
{"title": VALID_TITLE, "body": "## Почему\nПотому что"},
|
|
[PR_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert "## Что сделано" in result, f"expected heading error, got: {result!r}"
|
|
|
|
|
|
def test_missing_pochemu():
|
|
"""execute() with body missing '## Почему' → error mentioning heading."""
|
|
out = _run_exec(
|
|
{"title": VALID_TITLE, "body": "## Что сделано\nСделано"},
|
|
[PR_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert "## Почему" in result, f"expected heading error, got: {result!r}"
|
|
|
|
|
|
def test_missing_watch_out():
|
|
"""execute() with body missing '## Watch out' → error mentioning heading.
|
|
|
|
Body has the first two required headings (## Что сделано, ## Почему) so the
|
|
earlier checks pass; the third heading check (## Watch out) fires.
|
|
"""
|
|
body = "## Что сделано\nСделано\n\n## Почему\nПотому что\n\n## Pending\n—"
|
|
out = _run_exec({"title": VALID_TITLE, "body": body}, [PR_OK_RESPONSE])
|
|
result = out["result"]
|
|
assert "## Watch out" in result, f"expected heading error, got: {result!r}"
|
|
|
|
|
|
def test_missing_pending():
|
|
"""execute() with body missing '## Pending' → error mentioning heading.
|
|
|
|
Body has the first three required headings (## Что сделано, ## Почему,
|
|
## Watch out) so the earlier checks pass; the fourth heading check
|
|
(## Pending) fires.
|
|
"""
|
|
body = "## Что сделано\nСделано\n\n## Почему\nПотому что\n\n## Watch out\n—"
|
|
out = _run_exec({"title": VALID_TITLE, "body": body}, [PR_OK_RESPONSE])
|
|
result = out["result"]
|
|
assert "## Pending" in result, f"expected heading error, got: {result!r}"
|
|
|
|
|
|
def test_all_four_headings_success():
|
|
"""execute() with all 4 headings (incl. ## Watch out, ## Pending) succeeds.
|
|
|
|
A body with all four required headings (## Что сделано, ## Почему,
|
|
## Watch out, ## Pending) passes validation and returns the PR URL.
|
|
Uses em-dash placeholders for empty Watch out / Pending sections (ADR
|
|
pattern) — heading presence is sufficient, content may be '—'.
|
|
"""
|
|
body = (
|
|
"## Что сделано\nДобавлен enforcement\n\n"
|
|
"## Почему\nPipeline migration делает PR body single source of truth\n\n"
|
|
"## Watch out\n—\n\n"
|
|
"## Pending\n—"
|
|
)
|
|
out = _run_exec({"title": VALID_TITLE, "body": body}, [PR_OK_RESPONSE])
|
|
result = out["result"]
|
|
assert result == f"PR created: {PR_URL}", f"expected success, got: {result!r}"
|
|
|
|
|
|
def test_latin_only_body():
|
|
"""execute() with body containing no Cyrillic → error.
|
|
|
|
NOTE: spec validation order checks headings (## Что сделано, ## Почему)
|
|
BEFORE the Cyrillic check. Since the headings themselves are Cyrillic,
|
|
a body that passes the heading checks always passes the Cyrillic check.
|
|
Therefore a Latin-only body (no Cyrillic) also lacks the Russian headings
|
|
and fails on the heading check first. The Cyrillic check is effectively
|
|
dead code given the heading checks — documented as spec issue in handoff.
|
|
This test verifies the actual reachable behavior: heading check fires.
|
|
"""
|
|
out = _run_exec(
|
|
{"title": VALID_TITLE, "body": "## What done\nSomething\n\n## Why\nBecause"},
|
|
[PR_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
# Body lacks Russian headings → heading check fires (not Cyrillic check).
|
|
assert "## Что сделано" in result, f"expected heading error, got: {result!r}"
|
|
|
|
|
|
def test_issue_linkage():
|
|
"""execute() with issue_number → body gets 'Closes #N' appended.
|
|
|
|
The stub captures the spawnSync args; the --body value (index after
|
|
--body flag) must contain 'Closes #37'.
|
|
"""
|
|
out = _run_exec(
|
|
{"title": VALID_TITLE, "body": VALID_BODY, "issue_number": 37},
|
|
[PR_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert result == f"PR created: {PR_URL}", 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"]
|
|
body_idx = args.index("--body") + 1
|
|
body_val = args[body_idx]
|
|
assert "Closes #37" in body_val, f"expected 'Closes #37' in body, got: {body_val!r}"
|
|
|
|
|
|
def test_execute_uses_cwd_from_context():
|
|
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
|
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [PR_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_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(
|
|
{"title": VALID_TITLE, "body": VALID_BODY, "repo": "foo/bar"},
|
|
[PR_OK_RESPONSE],
|
|
)
|
|
result = out["result"]
|
|
assert result == f"PR created: {PR_URL}", 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).
|
|
"""
|
|
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [PR_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 gh pr create + exit code.
|
|
|
|
gh rejects an invalid owner/name with non-zero exit; the tool formats the
|
|
error via the shared formatResult helper (toolName='gh pr create').
|
|
"""
|
|
fail_response = {"status": 1, "stdout": "", "stderr": 'expected the "owner/repo" format'}
|
|
out = _run_exec(
|
|
{"title": VALID_TITLE, "body": VALID_BODY, "repo": "not-a-valid-repo"},
|
|
[fail_response],
|
|
)
|
|
result = out["result"]
|
|
assert "gh pr create 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}"
|