fix(browser): server shutdown and its tests on Windows, Python < 3.13 and CI
Three CI failures on ed809f7:
- Windows: server.py exited 1 on stdin EOF because create_app works with
the temp root as its cwd, and Windows cannot remove a process's cwd.
main() now restores the original cwd before the root is removed.
- Python 3.10 to 3.12: the lifetime test's cleanup called communicate()
after closing stdin, which flushes the closed pipe (ValueError); 3.13
tolerates it. It now waits and closes the pipes itself, and the exit
assertion shows the server's stderr. Under 3.12: errors=2 before, OK
after.
- Tooling (CI container): the SIGKILLed-worker tests treated an exited
server as alive, because the container's PID 1 does not reap orphans
and kill(pid, 0) succeeds on a zombie. Reproduced locally with docker
without --init (2 failed); a zombie now counts as exited (2 passed).
This commit is contained in:
@@ -583,8 +583,13 @@ def main(argv: list[str] | None = None) -> None:
|
||||
loop = asyncio.new_event_loop()
|
||||
if args.exit_on_stdin_eof:
|
||||
exit_when_stdin_closes(loop)
|
||||
original_cwd = os.getcwd()
|
||||
with tempfile.TemporaryDirectory(prefix="pm-browser-") as directory:
|
||||
web.run_app(create_app(Path(directory).resolve()), sock=listener, loop=loop)
|
||||
try:
|
||||
web.run_app(create_app(Path(directory).resolve()), sock=listener, loop=loop)
|
||||
finally:
|
||||
# create_app works inside the root; Windows cannot remove a process's cwd.
|
||||
os.chdir(original_cwd)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -39,7 +39,10 @@ class TestBrowserServerLifetime(unittest.TestCase):
|
||||
def stop():
|
||||
if process.poll() is None:
|
||||
process.kill()
|
||||
process.communicate(timeout=SERVER_TIMEOUT_S)
|
||||
process.wait(timeout=SERVER_TIMEOUT_S)
|
||||
# communicate() would flush a closed stdin, which Python < 3.13 refuses.
|
||||
for stream in (process.stdin, process.stdout, process.stderr):
|
||||
stream.close()
|
||||
|
||||
self.addCleanup(stop)
|
||||
line = process.stdout.readline().decode()
|
||||
@@ -63,7 +66,10 @@ class TestBrowserServerLifetime(unittest.TestCase):
|
||||
process = self.start("--exit-on-stdin-eof")
|
||||
self.assertEqual(len(self.roots()), 1, "the server works in its own temp root")
|
||||
process.stdin.close()
|
||||
self.assertEqual(process.wait(timeout=SERVER_TIMEOUT_S), 0)
|
||||
code = process.wait(timeout=SERVER_TIMEOUT_S)
|
||||
self.assertEqual(
|
||||
code, 0, process.stderr.read().decode(errors="replace")[-4000:]
|
||||
)
|
||||
self.assertEqual(self.roots(), [], "the temp root and its database are removed")
|
||||
|
||||
def test_server_keeps_running_with_stdin_closed_without_the_flag(self):
|
||||
|
||||
+6
-1
@@ -3,6 +3,7 @@ import assert from 'node:assert/strict';
|
||||
import { createServer } from 'node:net';
|
||||
import { spawn } from 'node:child_process';
|
||||
import { once } from 'node:events';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { fileURLToPath, pathToFileURL } from 'node:url';
|
||||
import { setTimeout as delay } from 'node:timers/promises';
|
||||
import { startBrowserServer, createServerSupervisor, attachServerStderr } from '../../tools/vendor/start-browser-server.mjs';
|
||||
@@ -27,7 +28,11 @@ async function writeStderr(server, text) {
|
||||
await waitFor(() => server.stderrTail().endsWith(text.trimEnd()), `stderr ${JSON.stringify(text)}`);
|
||||
}
|
||||
|
||||
const alive = pid => { try { process.kill(pid, 0); return true; } catch { return false; } };
|
||||
/** A process that exited counts as gone even while a non-reaping PID 1 (CI's container) leaves it a zombie. */
|
||||
function alive(pid) {
|
||||
try { process.kill(pid, 0); } catch { return false; }
|
||||
try { return readFileSync(`/proc/${pid}/stat`, 'utf8').replace(/^.*\) /s, '')[0] !== 'Z'; } catch { return true; }
|
||||
}
|
||||
|
||||
/** Starts a server from a separate Node worker process, SIGKILLs that worker and returns the server's pid. */
|
||||
async function serverOfKilledWorker(options) {
|
||||
|
||||
Reference in New Issue
Block a user