issue_comments: 1357084279

You were super-close on the python version of the test here, changing http to https on 8b73fc6b47dffd8836f5c58aae1e57c1f66a5754 is enough to pass the test:

diff diff --git a/tests/conftest.py b/tests/conftest.py index 69dee68b4a3f..ba07a11d37f6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -207,7 +207,7 @@ def ds_localhost_https_server(tmp_path_factory): stderr=subprocess.STDOUT, cwd=tempfile.gettempdir(), ) - wait_until_responds("http://localhost:8042/", verify=client_cert) + wait_until_responds("https://localhost:8042/", verify=client_cert) # Check it started successfully assert not ds_proc.poll(), ds_proc.stdout.read().decode("utf-8") yield ds_proc, client_cert

My speculation about what was happening here: when wait_until_responds() would time out due to SSL connection problems, because .terminate() isn’t in a finally, the datasette process wouldn’t get killed. That could (1) hang CI and (2) cause all your future local test runs to mysteriously fail because they’d be secretly talking to that old datasette process still hanging around from a past test run with an old temporary server certificate, and that old server cert wouldn’t validate against your newly-created ca cert.

A finally for .terminate() would help; a fancier version could be a context manager for running the external datasette process that could: - ensure the process always exited when no longer needed - if you want to be fancy, call terminate(), wait() for a short timeout for the process to exit, then try kill() and wait() again; raise an exception complaining about the seemingly-unkillable process if all that fails - raise an error if the process exited with a non-zero error code; here it’s likely that some datasette executions were secretly failing with [Errno 48] error while attempting to bind on address ('127.0.0.1', 8042): address already in use