Skip to content

interactiveui: make getchar() testable - #1412

Open
anushamukka-dev wants to merge 1 commit into
facebook:mainfrom
anushamukka-dev:export-D115572436
Open

interactiveui: make getchar() testable#1412
anushamukka-dev wants to merge 1 commit into
facebook:mainfrom
anushamukka-dev:export-D115572436

Conversation

@anushamukka-dev

Copy link
Copy Markdown

Summary:
interactiveui.getchar() had a # TODO: figure out tests on its non-tty
early return, and no test coverage at all - there is no existing test file
for interactiveui anywhere in eden/scm.

This extracts the raw terminal read into a module-level _readraw(fd) helper
and gives getchar() two optional injectable seams:

  • stdin - defaults to sys.stdin
  • readraw - defaults to _readraw

Real terminal behavior is unchanged: production callers pass neither argument,
_readraw contains the identical tcgetattr / setraw / os.read /
tcsetattr sequence, and the os.isatty(fd) guard and ctrl-c / ctrl-d
handling are byte-for-byte the same.

Initializing ch = None before the try also resolves the three
pyre-fixme[61] suppressions. Those flagged a real latent bug: if
tcgetattr raised termios.error, the except branch read ch before it
was ever assigned and would have raised NameError instead of falling back
to "".

Adds tests/test-interactiveui.py covering the non-tty early return (backed
by a real pipe fd), a single normal keypress, ctrl-c / ctrl-d, and the escape
sequence branch for arrow keys - including a run of several arrow keys plus a
normal key delivered in one read, checked through _splitkeypresses. The tty
cases use a real pty fd so the os.isatty guard is exercised for real rather
than mocked, with only the blocking read faked. The suite follows the existing
silenttestrunner convention used by neighboring tests such as
test-extutil.py and needs no BUCK change - unittestify.py globs
test-*.py.

Differential Revision: D115572436

Summary:
`interactiveui.getchar()` had a `# TODO: figure out tests` on its non-tty
early return, and no test coverage at all - there is no existing test file
for `interactiveui` anywhere in `eden/scm`.

This extracts the raw terminal read into a module-level `_readraw(fd)` helper
and gives `getchar()` two optional injectable seams:

- `stdin` - defaults to `sys.stdin`
- `readraw` - defaults to `_readraw`

Real terminal behavior is unchanged: production callers pass neither argument,
`_readraw` contains the identical `tcgetattr` / `setraw` / `os.read` /
`tcsetattr` sequence, and the `os.isatty(fd)` guard and ctrl-c / ctrl-d
handling are byte-for-byte the same.

Initializing `ch = None` before the `try` also resolves the three
`pyre-fixme[61]` suppressions. Those flagged a real latent bug: if
`tcgetattr` raised `termios.error`, the `except` branch read `ch` before it
was ever assigned and would have raised `NameError` instead of falling back
to `""`.

Adds `tests/test-interactiveui.py` covering the non-tty early return (backed
by a real pipe fd), a single normal keypress, ctrl-c / ctrl-d, and the escape
sequence branch for arrow keys - including a run of several arrow keys plus a
normal key delivered in one read, checked through `_splitkeypresses`. The tty
cases use a real pty fd so the `os.isatty` guard is exercised for real rather
than mocked, with only the blocking read faked. The suite follows the existing
`silenttestrunner` convention used by neighboring tests such as
`test-extutil.py` and needs no BUCK change - `unittestify.py` globs
`test-*.py`.

Differential Revision: D115572436
@meta-cla meta-cla Bot added the CLA Signed label Aug 12, 2026
@meta-codesync

meta-codesync Bot commented Aug 12, 2026

Copy link
Copy Markdown

@anushamukka-dev has exported this pull request. If you are a Meta employee, you can view the originating Diff in D115572436.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant