diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 5d1ea1a328..27a847225a 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -58,8 +58,10 @@ Shared C++ fixtures (not executables) live under `tests/cpp/viz/support/` in-tree configure: standalone wheel builds use the example as `CMAKE_SOURCE_DIR`, not the repo root. - A stand-in child process must stay alive on its own (`python -m ` - exits at once), and a `/proc//cmdline` check made right after `Popen()` - must poll, because the cmdline can still read empty + exits at once), and `Popen()` can return while the child's + `/proc//cmdline` still reads empty or the parent's argv: wait for the + child's own argv before any check, including one that expects no match, + which either stale value passes without testing anything (`python/core/cloudxr/test_background.py` does both). ## Adding tests diff --git a/tests/python/core/cloudxr/test_background.py b/tests/python/core/cloudxr/test_background.py index 92fd0185fe..d1d28b6f15 100644 --- a/tests/python/core/cloudxr/test_background.py +++ b/tests/python/core/cloudxr/test_background.py @@ -8,6 +8,7 @@ import subprocess import sys import time +from pathlib import Path import pytest from unittest.mock import MagicMock, patch @@ -33,14 +34,20 @@ def stand_in_service(tmp_path, monkeypatch): monkeypatch.setattr(background, "_MODULE", "stand_in_service") -def _poll_read_pid(run_dir: str) -> int | None: - """read_pid(), polled: a new child's cmdline can be empty when Popen() returns.""" - deadline = time.monotonic() + 2.0 - pid = background.read_pid(run_dir) - while pid is None and time.monotonic() < deadline: - time.sleep(0.05) - pid = background.read_pid(run_dir) - return pid +def _await_exec(pid: int) -> None: + """Wait for *pid*'s own cmdline: Popen() can return mid-exec. + + Until exec finishes it reads empty or, on the posix_spawn path, briefly + this process's argv. Neither names the service, so a check made then fails + a positive test and passes a negative one without testing anything. + """ + cmdline = Path(f"/proc/{pid}/cmdline") + ours = Path("/proc/self/cmdline").read_bytes() + deadline = time.monotonic() + 5.0 + while cmdline.read_bytes() in (b"", ours): + if time.monotonic() > deadline: + pytest.fail(f"{cmdline} is still empty 5s after the spawn") + time.sleep(0.01) @_posix_only @@ -65,6 +72,7 @@ def test_none_when_pid_belongs_to_something_else(self, tmp_path): """Pid reuse: the recorded pid is alive, but is not our service.""" other = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(30)"]) try: + _await_exec(other.pid) background.pid_path(str(tmp_path)).write_text(f"{other.pid}\n") assert background.read_pid(str(tmp_path)) is None finally: @@ -81,8 +89,9 @@ def test_recognises_a_service_under_any_shipped_module_name(self, tmp_path, modu [sys.executable, "-c", "import time; time.sleep(30)", module, "run"] ) try: + _await_exec(proc.pid) background.pid_path(str(tmp_path)).write_text(f"{proc.pid}\n") - assert _poll_read_pid(str(tmp_path)) == proc.pid + assert background.read_pid(str(tmp_path)) == proc.pid finally: proc.kill() proc.wait() @@ -99,6 +108,7 @@ def test_a_process_that_merely_names_the_module_is_not_the_service(self, tmp_pat ] ) try: + _await_exec(proc.pid) background.pid_path(str(tmp_path)).write_text(f"{proc.pid}\n") assert background.read_pid(str(tmp_path)) is None finally: @@ -192,6 +202,7 @@ def test_reads_real_cmdline_without_a_trailing_empty(self, tmp_path): ] ) try: + _await_exec(proc.pid) background.pid_path(str(tmp_path)).write_text(f"{proc.pid}\n") flags = background.read_run_flags(str(tmp_path)) assert flags == ["--host-client"] @@ -219,7 +230,8 @@ def test_sigterm_and_clears_the_pid_file(self, tmp_path, stand_in_service): pid, _ = background.spawn(["30"], run_dir, logs_dir) try: - assert _poll_read_pid(run_dir) == pid + _await_exec(pid) + assert background.read_pid(run_dir) == pid assert background.terminate(run_dir, timeout_sec=10) is True assert not background.pid_path(run_dir).exists()