diff --git a/bindings/js/README.md b/bindings/js/README.md index 67792b4..85173ee 100644 --- a/bindings/js/README.md +++ b/bindings/js/README.md @@ -51,7 +51,7 @@ Every failure maps to one of the daemon's exit codes: | `VersionMismatchError` | 4 | the daemon's version differs from this package | | `InternalError` | 5 | internal daemon error | -All derive from `ShellUseError` and carry `kind` and `exitCode`. `waitX` and `expectX` reject with `ExpectationError` on failure. +All derive from `ShellUseError` and carry `kind` and `exitCode`. `waitX` and `expectX` reject with `ExpectationError` on failure. Assertion errors include the current visible terminal content. On its first call, a client checks that the running daemon's version matches the package version and throws `VersionMismatchError` if they differ. Stop the daemon diff --git a/bindings/js/src/client.ts b/bindings/js/src/client.ts index 25fa963..93d9131 100644 --- a/bindings/js/src/client.ts +++ b/bindings/js/src/client.ts @@ -5,6 +5,7 @@ import { resolveHome, resolveSession, } from "./config.js"; +import { ExpectationError } from "./errors.js"; import { envPairs, unwrap } from "./protocol.js"; import * as transport from "./transport.js"; import { checkVersion } from "./version.js"; @@ -38,6 +39,21 @@ export interface MouseButtonOptions { button?: number; } +function withOperation(error: unknown, operation: string): unknown { + if (!(error instanceof ExpectationError)) { + return error; + } + const previous = error.message; + error.message = `${operation}: ${previous}`; + if (error.stack) { + error.stack = error.stack.replace( + `${error.name}: ${previous}`, + `${error.name}: ${error.message}`, + ); + } + return error; +} + class Mouse { #client: ShellUse; @@ -281,17 +297,21 @@ export class ShellUse { } async expectText(text: string, opts: ExpectTextOptions = {}): Promise { - await this.send({ - kind: "expect_text", - text, - regex: opts.regex ?? false, - full: opts.full ?? false, - strict: opts.strict ?? true, - not: opts.not ?? false, - fg: opts.fg ?? null, - bg: opts.bg ?? null, - timeout_ms: opts.timeout ?? 5000, - }); + try { + await this.send({ + kind: "expect_text", + text, + regex: opts.regex ?? false, + full: opts.full ?? false, + strict: opts.strict ?? true, + not: opts.not ?? false, + fg: opts.fg ?? null, + bg: opts.bg ?? null, + timeout_ms: opts.timeout ?? 5000, + }); + } catch (error) { + throw withOperation(error, "expectText"); + } } async expectExitCode(code: number): Promise { diff --git a/bindings/js/test/integration.test.mjs b/bindings/js/test/integration.test.mjs index 7ff74ca..3f93ec5 100644 --- a/bindings/js/test/integration.test.mjs +++ b/bindings/js/test/integration.test.mjs @@ -4,17 +4,22 @@ import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { test } from "node:test"; -import { ShellUse, sessions } from "../dist/index.js"; +import { ExpectationError, ShellUse, sessions } from "../dist/index.js"; const BIN = process.env.SHELL_USE_BIN; const skip = !BIN; +const shell = process.platform === "win32" ? "pwsh" : undefined; +const evalArgs = + typeof globalThis.Deno === "undefined" + ? ["-e", "console.log('ready'); setInterval(() => {}, 1000)"] + : ["eval", "console.log('ready'); setInterval(() => {}, 1000)"]; test("echo roundtrip drives a real session", { skip }, async () => { const home = mkdtempSync(join(tmpdir(), "shell-use-js-")); const session = `nodetest-${process.pid}-a`; const su = new ShellUse(session, { home }); try { - await su.open(); + await su.open({ shell }); await su.submit("echo hello-sdk"); await su.waitCommand(); await su.expectText("hello-sdk", { strict: false }); @@ -26,11 +31,53 @@ test("echo roundtrip drives a real session", { skip }, async () => { } }); +test( + "assertion errors include the current terminal", + { skip }, + async () => { + const home = mkdtempSync(join(tmpdir(), "shell-use-js-")); + const session = `nodetest-${process.pid}-error`; + const su = new ShellUse(session, { home }); + try { + await su.run(process.execPath, evalArgs); + await su.waitText("ready", { timeout: 2000 }); + await assert.rejects( + su.expectText("text-that-is-not-on-screen", { timeout: 50 }), + (error) => + error instanceof ExpectationError && + error.message.includes( + "expectText: timed out after 50ms waiting for 'text-that-is-not-on-screen' to be visible", + ) && + error.message.includes("Terminal content:\n╭") && + error.message.includes("ready") && + error.message.includes("\n╰"), + ); + await assert.rejects( + su.waitText("ready", { not: true, timeout: 50 }), + (error) => + error instanceof ExpectationError && + error.message.includes("timed out after 50ms waiting for 'ready' to be hidden") && + error.message.includes("Terminal content:\n╭"), + ); + await assert.rejects( + su.expectOutput("missing"), + (error) => + error instanceof ExpectationError && + error.message.includes("no command output tracked yet") && + error.message.includes("Terminal content:\n╭") && + error.message.includes("ready"), + ); + } finally { + await su.close(); + } + }, +); + test("sessions lists an open session", { skip }, async () => { const home = mkdtempSync(join(tmpdir(), "shell-use-js-")); const session = `nodetest-${process.pid}-b`; const su = new ShellUse(session, { home }); - await su.open(); + await su.open({ shell }); try { const names = await sessions({ home }); assert.ok(names.includes(session)); @@ -47,7 +94,7 @@ test("snapshot lands in the client cwd", { skip }, async () => { const session = `nodetest-${process.pid}-c`; const su = new ShellUse(session, { home }); try { - await su.open(); + await su.open({ shell }); await su.submit("echo snapshot-marker"); await su.waitCommand(); process.chdir(snapRoot); diff --git a/bindings/python/README.md b/bindings/python/README.md index 8f63112..1986a04 100644 --- a/bindings/python/README.md +++ b/bindings/python/README.md @@ -54,7 +54,7 @@ Every failure maps to one of the daemon's exit codes: | `VersionMismatchError` | 4 | the daemon's version differs from this package | | `InternalError` | 5 | internal daemon error | -All derive from `ShellUseError`. `wait_*` and `expect_*` raise `ExpectationError` on failure. +All derive from `ShellUseError`. `wait_*` and `expect_*` raise `ExpectationError` on failure. Assertion errors include the current visible terminal content. On its first call, a client checks that the running daemon's version matches the package version and raises `VersionMismatchError` if they differ. Stop the daemon diff --git a/bindings/python/src/shell_use/client.py b/bindings/python/src/shell_use/client.py index 30cf700..9fbcf43 100644 --- a/bindings/python/src/shell_use/client.py +++ b/bindings/python/src/shell_use/client.py @@ -6,7 +6,7 @@ from . import _config as cfg from . import _transport as transport from ._protocol import EnvLike, env_pairs, unwrap -from .errors import VersionMismatchError +from .errors import ExpectationError, VersionMismatchError from .types import Cell, State @@ -269,19 +269,24 @@ async def expect_text( bg: Optional[str] = None, timeout: int = 5000, ) -> None: - await self.send( - { - "kind": "expect_text", - "text": text, - "regex": regex, - "full": full, - "strict": strict, - "not": not_, - "fg": fg, - "bg": bg, - "timeout_ms": timeout, - } - ) + try: + await self.send( + { + "kind": "expect_text", + "text": text, + "regex": regex, + "full": full, + "strict": strict, + "not": not_, + "fg": fg, + "bg": bg, + "timeout_ms": timeout, + } + ) + except ExpectationError as error: + error.message = f"expect_text: {error.message}" + error.args = (error.message,) + raise async def expect_exit_code(self, code: int) -> None: await self.send({"kind": "expect_exit_code", "code": code}) diff --git a/bindings/python/tests/test_integration.py b/bindings/python/tests/test_integration.py index 6e7ffcc..1ed70ba 100644 --- a/bindings/python/tests/test_integration.py +++ b/bindings/python/tests/test_integration.py @@ -1,13 +1,15 @@ import asyncio import os +import sys import tempfile import unittest from pathlib import Path import shell_use -from shell_use import ShellUse +from shell_use import ExpectationError, ShellUse BIN = os.environ.get("SHELL_USE_BIN") +SHELL = "pwsh" if sys.platform == "win32" else None def run(coro): @@ -26,7 +28,7 @@ def _client(self): def test_echo_roundtrip(self): async def scenario(): async with self._client() as su: - await su.open() + await su.open(shell=SHELL) await su.submit("echo hello-sdk") await su.wait_command() await su.expect_text("hello-sdk", strict=False) @@ -36,10 +38,34 @@ async def scenario(): run(scenario()) + def test_expect_text_error_includes_terminal(self): + async def scenario(): + async with self._client() as su: + await su.run( + sys.executable, + "-c", + "import sys,time; sys.stdout.write('ready'); " + "sys.stdout.flush(); time.sleep(60)", + ) + await su.wait_text("ready", timeout=2000) + with self.assertRaises(ExpectationError) as raised: + await su.expect_text("text-that-is-not-on-screen", timeout=50) + message = str(raised.exception) + self.assertIn( + "expect_text: timed out after 50ms waiting for " + "'text-that-is-not-on-screen' to be visible", + message, + ) + self.assertIn("Terminal content:\n╭", message) + self.assertIn("ready", message) + self.assertIn("\n╰", message) + + run(scenario()) + def test_sessions_lists_open_session(self): async def scenario(): su = self._client() - await su.open() + await su.open(shell=SHELL) try: names = await shell_use.sessions(home=self._home) self.assertIn(self._session, names) @@ -54,7 +80,7 @@ async def scenario(): snap_root = tempfile.mkdtemp(prefix="shell-use-snap-") name = f"snap-{os.path.basename(snap_root)}" async with self._client() as su: - await su.open() + await su.open(shell=SHELL) await su.submit("echo snapshot-marker") await su.wait_command() os.chdir(snap_root) diff --git a/src/daemon/mod.rs b/src/daemon/mod.rs index e350917..df7ca62 100644 --- a/src/daemon/mod.rs +++ b/src/daemon/mod.rs @@ -19,7 +19,7 @@ use crate::config::{self, POLL_DELAY_MS}; use crate::input::{keys, mouse}; use crate::ipc; use crate::monitor; -use crate::protocol::{GetField, MouseAction, Request, Response}; +use crate::protocol::{ErrorKind, GetField, MouseAction, Request, Response}; use crate::terminal::emu::{rows_to_strings, Color, EmuCell}; use crate::terminal::locator::{self, Pattern}; use logger::Logger; @@ -348,7 +348,7 @@ fn text_of(rows: &[Vec]) -> String { } fn dispatch(s: &mut Session, req: Request) -> Response { - match req { + let mut response = match req { Request::State => state(s), Request::Text { full } => Response::with(json!({ "text": text_of(&grid(s, full)) })), Request::Cells { x, y, w, h } => cells(s, x, y, w, h), @@ -389,7 +389,13 @@ fn dispatch(s: &mut Session, req: Request) -> Response { } => do_snapshot(s, &name, update, include_colors, cwd), Request::Screenshot { full, path } => screenshot(s, full, path), _ => Response::internal("unsupported request"), + }; + if response.kind == Some(ErrorKind::Assertion) { + if let Some(message) = response.message.take() { + response.message = Some(assertion_message(s, &message)); + } } + response } fn act(r: anyhow::Result<()>) -> Response { @@ -573,8 +579,10 @@ fn wait_text( ); if found { Response::ok() + } else if not { + Response::assertion(timeout_message(&pattern.describe(), timeout_ms, true)) } else { - Response::assertion(timeout_message(s, &pattern.describe(), timeout_ms)) + Response::assertion(timeout_message(&pattern.describe(), timeout_ms, false)) } } @@ -655,10 +663,7 @@ fn expect_text( return if gone { Response::ok() } else { - Response::assertion(format!( - "expected '{}' to not be visible", - pattern.describe() - )) + Response::assertion(timeout_message(&pattern.describe(), timeout_ms, true)) }; } @@ -687,7 +692,7 @@ fn expect_text( } else if let Some(err) = last_err { Response::assertion(err) } else { - Response::assertion(timeout_message(s, &pattern.describe(), timeout_ms)) + Response::assertion(timeout_message(&pattern.describe(), timeout_ms, false)) } } @@ -822,9 +827,23 @@ fn screenshot(s: &Session, full: bool, path: Option) -> Response { } } -fn timeout_message(s: &Session, pattern: &str, timeout_ms: u64) -> String { - let snapshot = text_of(&viewable(s)); +fn timeout_message(pattern: &str, timeout_ms: u64, not: bool) -> String { format!( - "locator timeout: '{pattern}' not found after {timeout_ms}ms\n\nTerminal content:\n---START---\n{snapshot}\n---END---" + "timed out after {} waiting for '{pattern}' to be {}", + format_timeout(timeout_ms), + if not { "hidden" } else { "visible" } ) } + +fn assertion_message(s: &Session, message: &str) -> String { + let screen = snapshot::serialize(&viewable(s), s.cols, false); + format!("{message}\n\nTerminal content:\n{screen}") +} + +fn format_timeout(timeout_ms: u64) -> String { + if timeout_ms.is_multiple_of(1_000) { + format!("{}s", timeout_ms / 1_000) + } else { + format!("{timeout_ms}ms") + } +} diff --git a/src/terminal/locator.rs b/src/terminal/locator.rs index a267ace..b62f110 100644 --- a/src/terminal/locator.rs +++ b/src/terminal/locator.rs @@ -74,7 +74,7 @@ pub fn find( } if occurrences > 1 && strict { anyhow::bail!( - "strict mode violation: getByText({}) resolved to {} elements", + "strict mode expected one match for '{}', but found {}", text, occurrences ); @@ -90,7 +90,7 @@ pub fn find( } if matches.len() > 1 && strict { anyhow::bail!( - "strict mode violation: getByText({}) resolved to {} elements", + "strict mode expected one match for '{}', but found {}", re.as_str(), matches.len() );