ci: add windows-latest to test matrix + fix 11 cross-platform test bugs (#271)
* ci: add windows-latest to test matrix + fix 11 cross-platform test bugs The test workflow ran on ``ubuntu-latest`` only. Per the issue's first bullet — the maintainer's explicit #1 priority — add ``windows-latest`` to the matrix so the manager and related modules are exercised on Windows on every PR. The matrix addition surfaces 18 pre-existing Windows-only test failures. Without fixes the new leg would be 18+ reds from day one and the matrix would just produce a wall of ``fail-fast`` noise. This PR fixes 11 of them; each fix is a real (cross-platform) bug, not a Windows-specific hack — most were already flagged by CodeRabbit on PR #236 but never acted on. The remaining 4 failures need code refactors (``os.killpg`` → ``psutil`` in ``background.py``, ``convert_virtual_paths_in_command`` Windows-aware quoting, tilde expansion) that are documented as out-of-scope follow-ups below. ## What changed * ``.github/workflows/test.yml`` - ``os: [ubuntu-latest, windows-latest]`` → 2 OS × 2 Python = 4 cells. - ``fail-fast: false`` so one bad cell doesn't cancel the rest while the Windows leg is being brought up. Removable in a future PR once the suite is fully green. * ``tests/test_backends.py`` - Hard-coded ``"python3"`` → ``{sys.executable}`` in 7 test commands. Windows has no ``python3`` on PATH; using ``sys.executable`` is portable and matches what CodeRabbit flagged on PR #236. - Strict string comparisons → ``shlex.split`` round-trip in 5 resolver tests. ``shlex.quote`` adds single quotes around backslash paths on Windows, which broke the direct ``==`` compare. - Cross-platform suffix checks in 2 path-resolution tests (``Path(resolved).parts[-2:]`` instead of ``str(resolved).endswith("src/main.py")``). - ``mkdir -p`` → ``sys.executable -c "import os; os.makedirs(...)"`` in the cwd-sanitization test. - ``skipif(sys.platform == "win32")`` on 3 e2e tests that hit the underlying ``shlex.quote`` + ``cmd.exe`` quoting bug (real, separate issue). * ``tests/test_sessions.py`` - ``test_uses_data_dir``: check ``.evoscientist`` in the long path form (via ``Path.resolve()``) rather than the short-path form ``get_db_path`` returns on Windows. * ``tests/test_mcp_client.py`` - ``endswith("python")`` → ``Path(result).stem.lower()`` so ``python.EXE`` matches on Windows. - ``endswith("npx")`` also accepts ``npx.cmd`` so the npm shim on Windows matches. ## Out of scope (follow-up issues to file) * ``os.killpg`` doesn't exist on Windows (``EvoScientist/background.py:248``) — 3 background tests fail. Real fix is the same ``psutil`` walk pattern PR #200 shipped in ``langgraph_dev/manager.py``. * Tilde expansion in file mentions. * Windows-aware shell quoting in ``convert_virtual_paths_in_command``. * Path conventions (``~/.config/evoscientist/`` vs ``%APPDATA%\EvoScientist``) — needs design discussion + ``platformdirs`` migration. * Cross-module audit of ``EvoScientist/tools/execute.py``, ``EvoScientist/ccproxy_manager.py``, ``EvoScientist/config/onboard.py``. Closes #207 (step 1 only — CI matrix + the easy test fixes; remaining bullets tracked separately). * fix: cross-platform compatibility for Windows CI runners - background.py: replace POSIX-only os.killpg/os.getpgid with cross-platform _kill_process_tree() helper. On Windows falls back to Popen.terminate()/Popen.kill() (TerminateProcess); on POSIX keeps existing os.killpg logic. - test_backends.py: replace mkdir -p shell execution in test_literal_workspace_path_replaced with preprocessing-boundary assertion (patch LocalShellBackend.execute, capture command, assert workspace path was rewritten to ./). Avoids POSIX-only mkdir -p on Windows runners. - test_file_mentions.py: monkeypatch USERPROFILE on Windows so ntpath.expanduser() resolves ~ to tmp_path even when HOME is unset on CI runners. * fix(test): cross-platform sleep/true commands for Windows CI Replace POSIX-only sleep/true with module-level helpers that use ping -n / cmd /c on Windows. Also fix python3 -> sys.executable in the non-timeout recovery test. - test_background.py: 7 sleep/true fixes - test_background_middleware.py: 6 sleep/true fixes - test_backends.py: 4 sleep fixes + 1 python3 fix 2318 passed, 0 failed on Windows. * fix(test): use shell-portable double quotes for python -c on Windows cmd.exe does not treat single quotes as string delimiters, so -c 'raise SystemExit(1)' was passed with literal quotes on Windows. Switch to double quotes which work on both cmd.exe and POSIX sh. * fix: use psutil for Windows process tree kill + avoid sys.executable under uv - background.py: replace Popen.terminate()/kill() with psutil-based process tree walking on Windows. TerminateProcess does NOT cascade to grandchildren; psutil.Process.children(recursive=True) ensures the entire tree is signaled. - test_backends.py: replace sys.executable with 'python' in sandbox execute() calls. Under uv, sys.executable is under the workspace and gets rewritten to ./ by prepare_sandbox_command, breaking Linux CI. The plain 'python' command resolves correctly in any activated venv. * fix: broaden try/except in _kill_process_tree to cover proc.children() If the process exits between Process(popen.pid) and children(recursive=True), the children call raises an uncaught exception escaping stop(). Move it inside the existing try/except block. * fix: narrow exception to ProcessLookupError in POSIX _kill_process_tree OSError is too broad — would silently swallow EPERM on SIGKILL, leaving the process alive when we report it as stopped. Match original behavior which only caught ProcessLookupError (process already gone). * style: ruff format test_backends.py * ci: trigger re-run for flaky prompt_toolkit test * style: fix ruff check (import order + RUF005 unpacking) --------- Co-authored-by: Xi Zhang <106144707+X-iZhang@users.noreply.github.com>
This commit is contained in:
houren Antony
+28
-12
@@ -1,5 +1,6 @@
|
||||
"""Tests for EvoScientist.background — the background-process manager."""
|
||||
|
||||
import sys
|
||||
import time
|
||||
|
||||
import pytest
|
||||
@@ -7,6 +8,21 @@ import pytest
|
||||
from EvoScientist import background as bg
|
||||
|
||||
|
||||
def _sleep_cmd(seconds: int) -> str:
|
||||
"""Cross-platform command that sleeps for *seconds* and exits 0."""
|
||||
if sys.platform == "win32":
|
||||
# ``ping -n N+1 127.0.0.1 > nul`` sleeps ~N seconds.
|
||||
return f"ping -n {seconds + 1} 127.0.0.1 > nul"
|
||||
return f"sleep {seconds}"
|
||||
|
||||
|
||||
def _true_cmd() -> str:
|
||||
"""Cross-platform command that exits 0 immediately."""
|
||||
if sys.platform == "win32":
|
||||
return "cmd /c exit /b 0"
|
||||
return "true"
|
||||
|
||||
|
||||
def _wait_until(predicate, timeout=4.0, interval=0.05):
|
||||
"""Poll ``predicate`` until true or ``timeout`` — avoids flaky fixed sleeps on slow CI."""
|
||||
deadline = time.time() + timeout
|
||||
@@ -37,7 +53,7 @@ def test_launch_returns_id_and_creates_log(tmp_path):
|
||||
|
||||
|
||||
def test_status_running_then_exited(tmp_path):
|
||||
pid = bg.launch("sleep 1", str(tmp_path))
|
||||
pid = bg.launch(_sleep_cmd(1), str(tmp_path))
|
||||
assert "RUNNING" in bg.status(pid)
|
||||
assert _wait_until(lambda: "EXITED" in bg.status(pid))
|
||||
out = bg.status(pid)
|
||||
@@ -52,7 +68,7 @@ def test_output_captured_in_status(tmp_path):
|
||||
|
||||
def test_large_log_returns_truncated_tail(tmp_path):
|
||||
"""status() preserves the truncation contract for a large log (output shape, not I/O)."""
|
||||
pid = bg.launch("true", str(tmp_path))
|
||||
pid = bg.launch(_true_cmd(), str(tmp_path))
|
||||
log_path = tmp_path / ".bg_processes" / f"{pid}.log"
|
||||
log_path.write_bytes(b"A" * 5000 + b"TAIL_MARKER")
|
||||
out = bg.status(pid, tail_bytes=64)
|
||||
@@ -62,7 +78,7 @@ def test_large_log_returns_truncated_tail(tmp_path):
|
||||
|
||||
|
||||
def test_stop_kills_running_process(tmp_path):
|
||||
pid = bg.launch("sleep 600", str(tmp_path))
|
||||
pid = bg.launch(_sleep_cmd(600), str(tmp_path))
|
||||
assert "RUNNING" in bg.status(pid)
|
||||
out = bg.stop(pid)
|
||||
assert "Stopped" in out
|
||||
@@ -70,14 +86,14 @@ def test_stop_kills_running_process(tmp_path):
|
||||
|
||||
|
||||
def test_stop_already_finished_is_graceful(tmp_path):
|
||||
pid = bg.launch("true", str(tmp_path))
|
||||
pid = bg.launch(_true_cmd(), str(tmp_path))
|
||||
assert _wait_until(lambda: bg._PROCESSES[pid].popen.poll() is not None)
|
||||
assert "already finished" in bg.stop(pid)
|
||||
|
||||
|
||||
def test_exited_elapsed_is_frozen(tmp_path):
|
||||
"""Elapsed for an exited process freezes at its runtime, it must not keep growing."""
|
||||
pid = bg.launch("true", str(tmp_path))
|
||||
pid = bg.launch(_true_cmd(), str(tmp_path))
|
||||
assert _wait_until(lambda: bg._PROCESSES[pid].finished_ts is not None)
|
||||
bg.status(pid) # observe exit -> records finished_ts
|
||||
proc = bg._PROCESSES[pid]
|
||||
@@ -89,7 +105,7 @@ def test_exited_elapsed_is_frozen(tmp_path):
|
||||
|
||||
def test_watcher_records_exit_without_polling(tmp_path):
|
||||
"""The daemon watcher records exit on its own (no status() call needed)."""
|
||||
pid = bg.launch("true", str(tmp_path))
|
||||
pid = bg.launch(_true_cmd(), str(tmp_path))
|
||||
assert _wait_until(lambda: bg._PROCESSES[pid].finished_ts is not None)
|
||||
proc = bg._PROCESSES[pid]
|
||||
assert proc.finished_ts is not None
|
||||
@@ -104,7 +120,7 @@ def test_on_exit_callback_fires(tmp_path):
|
||||
fired["pid"] = proc.process_id
|
||||
fired["rc"] = proc.returncode
|
||||
|
||||
pid = bg.launch("true", str(tmp_path), on_exit=cb)
|
||||
pid = bg.launch(_true_cmd(), str(tmp_path), on_exit=cb)
|
||||
assert _wait_until(lambda: fired.get("pid") == pid and fired.get("rc") == 0)
|
||||
assert fired.get("pid") == pid
|
||||
assert fired.get("rc") == 0
|
||||
@@ -117,7 +133,7 @@ def test_unknown_id_errors_gracefully():
|
||||
|
||||
def test_list_all(tmp_path):
|
||||
assert "No background processes" in bg.list_all()
|
||||
pid = bg.launch("sleep 1", str(tmp_path))
|
||||
pid = bg.launch(_sleep_cmd(1), str(tmp_path))
|
||||
listing = bg.list_all()
|
||||
assert pid in listing
|
||||
assert "RUNNING" in listing
|
||||
@@ -125,8 +141,8 @@ def test_list_all(tmp_path):
|
||||
|
||||
def test_list_all_scopes_to_origin_thread(tmp_path):
|
||||
"""list_all defaults to the launching session; include_all sees every session."""
|
||||
pid_a = bg.launch("sleep 1", str(tmp_path), origin_thread_id="A")
|
||||
pid_b = bg.launch("sleep 1", str(tmp_path), origin_thread_id="B")
|
||||
pid_a = bg.launch(_sleep_cmd(1), str(tmp_path), origin_thread_id="A")
|
||||
pid_b = bg.launch(_sleep_cmd(1), str(tmp_path), origin_thread_id="B")
|
||||
listing_a = bg.list_all("A")
|
||||
assert pid_a in listing_a
|
||||
assert pid_b not in listing_a # B's process is hidden from session A
|
||||
@@ -137,7 +153,7 @@ def test_list_all_scopes_to_origin_thread(tmp_path):
|
||||
|
||||
def test_list_all_hints_at_other_sessions(tmp_path):
|
||||
"""A session with no processes of its own is told others exist."""
|
||||
bg.launch("sleep 1", str(tmp_path), origin_thread_id="A")
|
||||
bg.launch(_sleep_cmd(1), str(tmp_path), origin_thread_id="A")
|
||||
out = bg.list_all("B") # a different session
|
||||
assert "other sessions" in out
|
||||
assert "all_threads=True" in out
|
||||
@@ -145,7 +161,7 @@ def test_list_all_hints_at_other_sessions(tmp_path):
|
||||
|
||||
def test_dedup_is_per_thread(tmp_path):
|
||||
"""A check from one session must not suppress another session's completion ping."""
|
||||
pid = bg.launch("true", str(tmp_path), origin_thread_id="A")
|
||||
pid = bg.launch(_true_cmd(), str(tmp_path), origin_thread_id="A")
|
||||
assert _wait_until(lambda: bg._PROCESSES[pid].finished_ts is not None)
|
||||
bg.status(pid, thread_id="B") # a DIFFERENT session inspects it
|
||||
assert bg.was_observed_done(pid, "B") is True # B saw it
|
||||
|
||||
Reference in New Issue
Block a user