tests: add Escape-cancel coverage for confirm, text, and path prompts#1170
Conversation
Greptile SummaryThis PR adds three new unit tests covering the Escape-to-cancel behavior for Confidence Score: 5/5Safe to merge — test-only change with no production code impact. All three new tests faithfully mirror the existing select-escape test, are logically correct, and the only remaining feedback is a P2 style suggestion about parameterization and missing coverage for checkbox/password. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant T as Test
participant PS as prompt_support
participant Q as questionary.<prompt>
participant App as Application
T->>PS: install_questionary_escape_cancel()
PS-->>T: (idempotent if already installed)
T->>Q: create prompt(input=pipe_input)
Q->>App: build Application with merged key bindings
T->>App: pipe_input.send_bytes(b"\x1b")
T->>App: app.run()
App->>App: Escape binding fires → exit(result=None)
App-->>T: returns None
T->>T: assert result is None ✓
Reviews (1): Last reviewed commit: "tests: add Escape-cancel coverage for co..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def test_stock_questionary_confirm_escape_cancels() -> None: | ||
| """Verify that pressing Escape cancels a confirm prompt (Issue #1117). | ||
|
|
||
| Sends the Escape byte (\\x1b) to a questionary.confirm application | ||
| and asserts that it returns None instead of hanging. | ||
| """ | ||
| install_questionary_escape_cancel() | ||
| with create_pipe_input() as pipe_input: | ||
| q = questionary.confirm( | ||
| "Are you sure?", | ||
| input=pipe_input, | ||
| output=DummyOutput(), | ||
| ) | ||
| pipe_input.send_bytes(b"\x1b") | ||
| app = q.application | ||
| app.input = pipe_input | ||
| app.output = DummyOutput() | ||
| assert app.run() is None | ||
|
|
||
|
|
||
| def test_stock_questionary_text_escape_cancels() -> None: | ||
| """Verify that pressing Escape cancels a text input prompt (Issue #1117). | ||
|
|
||
| Sends the Escape byte (\\x1b) to a questionary.text application | ||
| and asserts that it returns None. | ||
| """ | ||
| install_questionary_escape_cancel() | ||
| with create_pipe_input() as pipe_input: | ||
| q = questionary.text( | ||
| "Name", | ||
| input=pipe_input, | ||
| output=DummyOutput(), | ||
| ) | ||
| pipe_input.send_bytes(b"\x1b") | ||
| app = q.application | ||
| app.input = pipe_input | ||
| app.output = DummyOutput() | ||
| assert app.run() is None | ||
|
|
||
|
|
||
| def test_stock_questionary_path_escape_cancels() -> None: | ||
| """Verify that pressing Escape cancels a path selection prompt (Issue #1117). | ||
|
|
||
| Sends the Escape byte (\\x1b) to a questionary.path application | ||
| and asserts that it returns None. | ||
| """ | ||
| install_questionary_escape_cancel() | ||
| with create_pipe_input() as pipe_input: | ||
| q = questionary.path( | ||
| "Path", | ||
| input=pipe_input, | ||
| output=DummyOutput(), | ||
| ) | ||
| pipe_input.send_bytes(b"\x1b") | ||
| app = q.application | ||
| app.input = pipe_input | ||
| app.output = DummyOutput() | ||
| assert app.run() is None |
There was a problem hiding this comment.
checkbox/password coverage
The three new tests are structurally identical to each other and to test_stock_questionary_select_escape_cancels. Both checkbox and password are also patched by install_questionary_escape_cancel() but have no escape-cancel test yet. Parameterizing all five prompt types with @pytest.mark.parametrize would reduce duplication and make it easier to add new prompt types in the future without repeating the boilerplate setup.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
@DsThakurRawat pls check review comments |
|
@DsThakurRawat merging this for now, but going forward pls take care of review comments |
|
🚀 Houston, we have a merge. @DsThakurRawat your PR is in orbit. Thanks for launching this one!
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
2 participants
Describe the changes you have made in this PR -
Added unit test coverage for the "Escape To cancel" functionality in the CLI. Specifically ,added tests for confirm,text,and path prompts to ensure they return None when the Escape key is pressed,Preventing the CLI from hanging
Demo/Screenshot for feature changes and bug fixes -
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The problem was missing test coverage for the Escape-cancel feature across all CLI prompt types — only
selectwas covered.I followed the existing
test_stock_questionary_select_escape_cancelspattern already present in the file:install_questionary_escape_cancel()to activate the Escape-cancel patch before each test.create_pipe_input()fromprompt_toolkitas a context manager to simulate a real terminal input.confirm,text, orpath) with the pipe as its input.b"\x1b") to simulate the user pressing Escape.app.run()returnsNone— confirming the prompt was cancelled cleanly.No alternative approaches were considered since the existing pattern in the file was clear and correct. AI assistance was used only to add the docstrings/comments to the test functions.
Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.