Enable cltr-c tests on windows #2412
Conversation
|
|
|
Turns out Ctrl-C doesn't actually work on the windows client in many cases, since synchronicity (though actually the underlying concurrent.futures.Future) doesn't support aborting a long running blocking call. Attempt at fix in modal-labs/synchronicity#179 |
| raise KeyboardInterrupt() | ||
|
|
||
| original_sigint_handler = None | ||
| def run_async_gen( |
There was a problem hiding this comment.
Just pulled this out of the Runner here and made it use the identical Runner implementation from synchronicity itself instead
| @@ -27,7 +27,7 @@ install_requires = | |||
| grpclib==0.4.7 | |||
| protobuf>=3.19,<5.0,!=4.24.0 | |||
| rich>=12.0.0 | |||
| synchronicity~=0.8.3 | |||
| synchronicity~=0.9.1 | |||
There was a problem hiding this comment.
This version upgrade the actual windows issues
| @@ -54,3 +55,25 @@ def deploy_app_externally( | |||
| print(f"Deploying app failed!\n### stdout ###\n{stdout_s}\n### stderr ###\n{stderr_s}") | |||
| raise Exception("Test helper failed to deploy app") | |||
| return stdout_s | |||
|
|
|||
|
|
|||
| class PopenWithCtrlC(subprocess.Popen): | |||
There was a problem hiding this comment.
We have an identical helper in the tests of synchronicity, but it's not part of the synchronicity package, so we can't reuse it here
|
@prbot approve |
![modal-pr-review-automation[bot]](https://avatars.githubusercontent.com/in/335990?s=60&v=4){kind=small}
We were previously not testing Ctrl-C behavior on windows since SIGINT doesn't work and the python standard library makes it tricky to send CTRL_C_EVENT to a subprocess.
This was worked around in recent #2322 and this patch generalizes that fix and reactivates our CLI tests for windows