refactor(cua-driver-testkit): shared integration-test harness + transport abstraction - #2039
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds a shared ChangesShared testkit crate and core API
Integration test migration
Sequence Diagram(s)sequenceDiagram
participant focus_check_test
participant McpDriver
participant ChildReaper
participant cua_driver as cua-driver
participant ToolResponse
focus_check_test->>McpDriver: McpDriver::spawn()
McpDriver->>ChildReaper: track spawned child
McpDriver->>cua_driver: initialize JSON-RPC over stdio
focus_check_test->>McpDriver: call(tool, args)
McpDriver->>cua_driver: tools/call request
cua_driver-->>McpDriver: JSON-RPC response
McpDriver->>ToolResponse: ToolResponse::from_mcp(raw)
McpDriver-->>focus_check_test: ToolResponse
Estimated review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
…ests
Introduces crates/cua-driver-testkit — the single home for machinery that was
copy-pasted across tests/ (workspace_root/driver_binary ×9, the JSON-RPC stdio
client ×10, the Windows kill-on-close Job Object reaper ×2, result accessors).
Key design: a Driver trait with two transports — McpDriver (long-lived server,
session-scoped state) and CliDriver (stateless cua-driver call) — that normalize
their differing payloads (the CLI prints structuredContent directly, MCP returns
the JSON-RPC envelope) into one ToolResponse {text(), structured(), is_error()}.
This makes the CLI-vs-MCP transport axis testable instead of MCP-only.
Proof-of-shape migrations (no behavior change):
- focus_check_test.rs (macOS) — verified compiles locally.
- harness_desktop_scope_test.rs (Windows) — drops its whole Job Object +
JSON-RPC block for the testkit; pending Windows compile-verify.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bb18f12 to
d19ff18
Compare
Stacks on #2038 (base is its branch so the diff is just this refactor). This is PR #1 of the test-suite cleanup — extract the duplicated harness machinery; renames/coverage come in follow-ups.
Why
tests/had no shared module — every file re-implemented the same code:workspace_root()/driver_binary()send/call/init)result_text/is_erroraccessorsWhat
New
crates/cua-driver-testkit(dev-dependency only, never shipped):Drivertrait + two transports —McpDriver(long-lived server, session-scoped state) andCliDriver(statelesscua-driver call). They normalize their different payloads (the CLI printsstructuredContentdirectly; MCP returns the JSON-RPC envelope) into oneToolResponse(text()/structured()/is_error()). This is what makes the CLI-vs-MCP transport axis testable instead of MCP-only.ChildReaper— cross-platform child kill-on-drop; on Windows assigns to aJOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSEjob (no orphaned windows / held ports on panic or SIGKILL).paths— onedriver_binary()(.exeviacfg!),workspace_root(),harness_app().Proof-of-shape migrations (no behavior change)
focus_check_test.rs(macOS) — ✅ compiles locally.harness_desktop_scope_test.rs(Windows) — drops its entire Job Object + JSON-RPC block for the testkit.Verification
cargo test --no-run -p cua-driver --test focus_check_test(testkit + test compile).cargo clean -p cua-driver-testkitthencargo test --no-run --test harness_desktop_scope_test→ recompiles the testkit Windows (Job Object) path,EXIT=0, binary produced.#[ignore](real desktop) — live--ignoredruns unchanged from before.Follow-ups (per the plan): migrate the remaining 11 files, then the 4-family rename, then the coverage matrix +
transport_config_persistence_test.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes