feat(test runner): allow stopping testrun with escape #32584
Conversation
This comment has been minimized.
This comment has been minimized.
|
What do you think about handling the first Ctrl+C (Cmd+C) to stop? It might be a bit more discoverable. Esc is designed to be used for escape sequences in the terminal environments, so there is an expectation that something follows Esc. |
|
We're already doing that, see https://github.com/microsoft/playwright/pull/32584/files#diff-6a03ad981aab4989f4c00a92fbeb80df93f97bbd33e1e66bd292cfbffcf3a943R273. We're listening to both Ctrl+C, Escape and their Control sequence equivalents. Do you think we should be listening exclusively to Ctrl+C? |
I misunderstood the intention behind the patch. |
|
|
||
| process.stdin.on('keypress', handler); | ||
|
|
||
| return { |
There was a problem hiding this comment.
returning the callback effect-style would also work. We only use Disposable when we interoperate with vscode.
| }); | ||
|
|
||
| void result.finally(reader.dispose); |
There was a problem hiding this comment.
I think readKeyPress should have this code, not readCommand
There was a problem hiding this comment.
i've moved readKeyPress to return a promise in f0de6cc. That changes quite a bit about the code, but makes it easier to reason about in general - and it also moves the disposal into readKeyPress 👍
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Test results for "tests 1"2 failed 35488 passed, 659 skipped Merge workflow run. |
|
Unsure why this is failing on Windows. Need to investigate |
Closes #32579