Skip to content

test: e2e tests with interactive pty#149

Merged
branchseer merged 28 commits intomainfrom
feat_vite_pty
Feb 10, 2026
Merged

test: e2e tests with interactive pty#149
branchseer merged 28 commits intomainfrom
feat_vite_pty

Conversation

@branchseer
Copy link
Copy Markdown
Member

@branchseer branchseer commented Feb 10, 2026 •
edited
Loading

feat: use vite_pty for e2e snapshot tests

Replace piped stdin/stdout with PTY-based process spawning in e2e tests.
Child processes now run in a real terminal, closer to actual user experience.

This was referenced Feb 10, 2026
Merged
Merged
Merged
@branchseer Graphite App
Copy link
Copy Markdown
Member Author

branchseer commented Feb 10, 2026 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@branchseer branchseer mentioned this pull request Feb 10, 2026
Merged
@branchseer branchseer changed the title refactor: rename fspy_test_utils to subprocess_test and command_executing to command_for_fn test: e2e tests with interactive pty Feb 10, 2026
@branchseer branchseer marked this pull request as ready for review February 10, 2026 10:42
@branchseer branchseer requested a review from fengmk2 February 10, 2026 12:07
@branchseer branchseer changed the base branch from 02-09-chore_fix_clippy_issues to graphite-base/149 February 10, 2026 12:10
branchseer and others added 20 commits February 10, 2026 12:15
@branchseer @claude
refactor: rename fspy_test_utils to subprocess_test and command_execu…
ae75320 
…ting to command_for_fn

Renamed the crate to use no prefix since it's shared by both fspy and vite_* crates.
The macro name now more clearly conveys that it creates a command for executing a function.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer @claude
feat: add Command struct to subprocess_test with explicit fields
2dae705 
Added a new Command struct that captures all subprocess configuration:
- program: OsString
- args: Vec<OsString>
- envs: HashMap<OsString, OsString> (all inherited envs)
- cwd: PathBuf (always explicit, no defaults)

The command_for_fn macro now returns this Command struct instead of
std::process::Command, enabling better control and inspection of subprocess
configuration.

Implemented conversions:
- From<Command> for std::process::Command
- From<Command> for fspy::Command (under fspy feature flag)

Updated all usage sites:
- fspy_shared tests: convert Command before calling .status()/.output()
- fspy test utils: renamed spawn_std -> spawn_command, accepts Command directly
- Renamed track_child! macro -> track_fn! for clarity

Fixed circular dependency by moving subprocess_test to dev-dependencies in fspy.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer
feat: add portable-pty support to subprocess_test
67bb2a3
@branchseer
add new crate vite_pty
8424231
@branchseer
init test
d9af9c4
@branchseer @claude
fix: add ctor as dev-dependency for command_for_fn macro users
bd7e82d 
The command_for_fn macro uses the ctor attribute which requires the ctor
crate to be available in the calling crate's dependency graph. This is a
limitation of Rust's proc-macro system - attribute macros cannot be
re-exported through module paths.

Added ctor as dev-dependency to vite_pty and updated README to document
this requirement for users of the command_for_fn macro.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer
add is_terminal test
98df43d
@branchseer @claude
feat: improve read_until with buffer management
0119142 
- Store buffer in Terminal struct to persist data between calls
- Only process data up to and including expected string
- Keep remaining data for next call
- Create reader before spawning child for better Windows compatibility
- Update read_to_end to process buffered data first
- Add comprehensive tests with 5-second timeouts

Note: Windows tests for read_until are currently timing out,
while read_to_end works correctly. Further investigation needed.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer @claude
fix: process data immediately through parser in read_until
aff750f 
The key difference between read_until and read_to_end was that
read_to_end processes data through the parser immediately, while
read_until was delaying processing until after finding the expected
string.

On Windows, the vt100 parser with Vt100Callbacks (which can write
responses back to the PTY) needs to process data as it arrives.
Delaying this processing caused the reader to block indefinitely.

Now read_until processes each chunk through the parser immediately
after reading, just like read_to_end, while still maintaining the
buffer for pattern matching and keeping unprocessed remainder for
subsequent calls.

All tests now pass on both macOS and Windows.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer @claude
fix: handle expected string spanning read boundaries
16f8562 
Previously, if the expected string was split across the buffer
and newly read data, read_until would fail to find it. For example,
if buffer contained "hel" and we read "lo world", we wouldn't find
"hello".

This is fixed by:
1. Creating a unified read() method that handles reading from the
   reader and appending to buffer
2. Checking the entire accumulated buffer after each read, ensuring
   we can find patterns that span boundaries
3. Unifying read_to_end() to use the same read() method

Added tests:
- read_until_boundary_spanning: Tests finding pattern across chunks
- read_until_exact_boundary: Tests finding pattern after a boundary

All 7 tests now pass on both macOS and Windows.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer @claude
fix: preserve buffer after read_to_end for subsequent searches
a7a383d 
Previously, read_to_end cleared the buffer after reading all data.
This prevented calling read_until after read_to_end to search in
the already-read data.

Now the buffer is preserved, allowing use cases like:
1. Call read_to_end() to read everything
2. Call read_until() to search in the buffered data
3. Since EOF was reached, no more data can be read, but searches
   in existing buffer still work

Added test: read_until_after_read_to_end
- Verifies buffer is preserved after read_to_end
- Verifies read_until can search in buffered data
- Verifies EOF prevents reading more data

All 8 tests pass on both macOS and Windows.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer
wip
639cb56
@branchseer @claude
refactor: separate read and consume responsibilities
66627c2 
Refactored the read logic into two clear methods:

- read_to_buffer(): Reads data from PTY into buffer (no processing)
- consume(n): Processes first n bytes through parser and removes them

Key insight: After read_to_buffer(), we track the old buffer length
and process only the newly read portion. This ensures:
1. Data is processed immediately (critical for Windows PTY)
2. Data is processed exactly once (no double-processing)
3. Buffer accumulates all data for pattern searching
4. When pattern found, consume() processes and removes consumed portion

This clean separation makes the code more maintainable while
preserving Windows compatibility and buffer search functionality.

All 8 tests pass on both macOS and Windows.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer @claude
feat(vite_pty): improve read_until with buffer management
619e30c 
- Store buffer in Terminal struct to preserve data between calls
- Only process data up to and including the expected string
- Implement prefix matching for boundary spanning cases
- Check existing buffer before reading to avoid unnecessary EOF errors

All tests pass on macOS (8/8). On Windows, one test (read_until_not_found)
times out - this appears to be a Windows PTY EOF signaling issue.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer
update
99845d1
@branchseer
a
64538a0
@branchseer @claude
fix(vite_pty): add EOF handling to read_until
fe05060 
Add missing EOF check when n == 0 to prevent infinite loop.
All tests now pass on both macOS (8/8) and Windows (8/8).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer @claude
refactor(vite_pty): separate read_to_end and screen_contents
5d3e50f 
- Change read_to_end to return Result<()> instead of Result<String>
- Users now call screen_contents() separately to get screen contents
- This separates concerns: reading/processing vs retrieving state
- Update all tests to use the new API

All tests pass on macOS (8/8) and Windows (8/8).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@branchseer @claude @happy-otter
feat(vite_pty): add resize and write methods to Terminal
a65974f 
- Add resize() method to dynamically change terminal dimensions
  - Resizes PTY via portable-pty's MasterPty::resize()
  - Updates vt100 parser screen buffer dimensions
  - Cross-platform: Unix sends SIGWINCH, Windows uses ConPTY resize

- Add write() method for sending input to terminal
  - Handles Windows CRLF conversion automatically
  - Thread-safe with proper locking
  - Returns error if child process has exited

- Add comprehensive tests for both features
  - resize_terminal: verifies SIGWINCH delivery on Unix, ConPTY on Windows
  - write_basic_echo, write_multiple_lines, write_interactive_prompt
  - write_after_exit: validates error handling
  - All tests pass on macOS and Windows (via cross-compilation)

- Add test dependencies:
  - terminal_size 0.4 for cross-platform size queries
  - signal-hook 0.3 (Unix-only) for SIGWINCH handling

Generated with [Claude Code](https://claude.com/claude-code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@branchseer @claude @happy-otter
docs: add cross-platform testing requirements to CLAUDE.md
36dd95d 
- Document critical requirement: no platform skipping
- Add cargo xtest command for Windows cross-compilation testing
- Include cross-platform test design patterns
- Reference vite_pty::resize_terminal as example implementation
- Add cross-platform requirements to Code Constraints section

This establishes clear guidelines that all features must work on both
Unix and Windows, with proper testing on both platforms.

Generated with [Claude Code](https://claude.com/claude-code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
branchseer and others added 7 commits February 10, 2026 12:15
@branchseer @claude @happy-otter
feat(vite_pty): add send_ctrl_c method to Terminal
ac72110 
Add Terminal::send_ctrl_c() to send Ctrl+C (SIGINT) to child processes
in a cross-platform way. The method sends ASCII 0x03 which is interpreted
by both Unix PTYs (converted to SIGINT) and Windows ConPTY (generates
CTRL_C_EVENT).

Includes comprehensive cross-platform test following the pattern from
resize_terminal - both Unix and Windows execute the test without skipping,
with platform-specific verification code using #[cfg] attributes.

Tests pass on both macOS (local) and Windows (aarch64-pc-windows-msvc via
cargo xtest cross-compilation).

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@branchseer @claude @happy-otter
feat(vite_pty): return ExitStatus from read_to_end
7af3c68 
Update Terminal::read_to_end() to return the child process exit status
instead of (). Uses OnceLock to synchronize between the background
thread (which waits for the child and sets the status) and read_to_end()
(which waits on the OnceLock after reading all output).

Changes:
- Add exit_status: Arc<OnceLock<ExitStatus>> field to Terminal
- Background thread now captures and stores exit status before closing writer
- read_to_end() returns ExitStatus after reading all output
- Re-export ExitStatus from vite_pty crate
- Add tests for successful (0) and non-zero (42) exit codes

All 16 tests pass on both macOS and Windows (via cargo xtest).

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@branchseer
fix lint issues
b01f8a4
@branchseer @claude
feat(vite_task_bin): use vite_pty for e2e snapshot tests
a9b91ed 
Replace piped stdin/stdout with PTY-based process spawning in e2e tests.
Child processes now run in a real terminal, closer to actual user experience.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@branchseer @claude
fix(ci): exclude vite_pty from zigbuild tests
24fc9c2 
The ctor crate used by subprocess_test is incompatible with zig linker,
causing all vite_pty PTY tests to timeout. vite_pty is tested natively
on macOS and Windows CI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@branchseer
Revert "fix(ci): exclude vite_pty from zigbuild tests"
a9b2257 
This reverts commit fa919df.
@branchseer @claude
fix(e2e): remove stdin passthrough test, use channel for timeout
c585eb6 
- Remove stdin-passthrough fixture (PTY stdin not needed for e2e tests)
- Replace busy-loop timeout with mpsc channel recv_timeout
- Simplify Step type to transparent newtype

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@graphite-app graphite-app bot changed the base branch from graphite-base/149 to main February 10, 2026 12:16
@branchseer @claude
fix(vite_pty): drop slave after spawn to signal EOF on child exit
6af87ab 
Drop the PTY slave handle immediately after spawning the child process.
This ensures EOF is signaled on the reader when the child exits, which
is critical for the zig linker builds where the slave handle was keeping
the PTY open.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@branchseer branchseer merged commit 043c86e into main Feb 10, 2026
7 checks passed
@branchseer branchseer deleted the feat_vite_pty branch February 10, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fengmk2 fengmk2 fengmk2 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@branchseer @fengmk2