Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,20 @@ jobs:
artifact_path: reports/
secrets: inherit

test-windows:
name: Run tests on Windows
uses: ./.github/workflows/python-job.yml
with:
runner: windows-latest
run: |
# tests/installer.py is a symlink to ../dk-installer.py. A Windows checkout
# materializes it as a plain file holding the target path unless core.symlinks is
# enabled, which turns `from tests.installer import ...` into a SyntaxError. Copy
# over it when that is what we got; a real symlink is left alone.
if [ ! -L tests/installer.py ]; then cp dk-installer.py tests/installer.py; fi
pytest
secrets: inherit

report-coverage:
name: Report tests coverage
runs-on: ubuntu-latest
Expand Down
7 changes: 6 additions & 1 deletion dk-installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,12 @@ def stream_iterator(proc: subprocess.Popen, stream_name: str, file_path: pathlib
"stdout": (0, "output"),
"stderr": (1, "stderr"),
}[stream_name]
buffer = io.TextIOWrapper(io.BytesIO())
# Explicit utf-8: the default is ``locale.getpreferredencoding()``, which is cp1252 on
# US Windows. That mangles every non-ASCII byte a command writes (docker compose and uv
# both draw progress with box-drawing and check marks) and, worse, silently defeats the
# partial-character handling below -- cp1252 maps almost any byte to *something*, so an
# incomplete utf-8 sequence decodes to garbage instead of raising UnicodeDecodeError.
buffer = io.TextIOWrapper(io.BytesIO(), encoding="utf-8")

def _iter():
proc_exited = False
Expand Down
7 changes: 7 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import os
from argparse import Namespace
from contextlib import contextmanager
from pathlib import Path
Expand All @@ -25,7 +26,13 @@ def _no_real_process_group_signals():
pgid 1 (init) and ``os.killpg(1, SIGTERM)`` from a root container actually
signals init → CI runner shutdown. Tests that need to assert on these
explicitly override the patches inside their own ``with patch(...)``.

Nothing to guard where process groups do not exist: Windows has no ``os.killpg`` to
call by accident, and the tests that would reach it are skipped there.
"""
if not hasattr(os, "killpg"):
yield Mock()
return
with (
patch("tests.installer.os.killpg") as killpg_mock,
patch("tests.installer.os.getpgid", return_value=99999),
Expand Down
3 changes: 2 additions & 1 deletion tests/test_obs_install.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
from functools import partial
from itertools import count
from pathlib import Path
Expand Down Expand Up @@ -66,7 +67,7 @@ def _stdout_side_effect():
@pytest.mark.integration
def test_obs_existing_install_abort(obs_install_action, compose_path, stdout_mock):
stdout_mock.side_effect = [
[f'[{{"Name":"test-project","Status":"running(4)","ConfigFiles":"{compose_path}"}}]'],
[json.dumps([{"Name": "test-project", "Status": "running(4)", "ConfigFiles": str(compose_path)}])],
[],
]
with patch.object(obs_install_action, "steps", new=[ComposeVerifyExistingInstallStep]):
Expand Down
19 changes: 11 additions & 8 deletions tests/test_tg_install.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
from functools import partial
from pathlib import Path
from unittest.mock import call, patch
Expand Down Expand Up @@ -50,24 +51,26 @@ def test_tg_install(tg_install_action, start_cmd_mock, stdout_mock, tmp_data_fol
assert Path(tmp_data_folder).joinpath("dk-tg-credentials.txt").stat().st_size > 0
marker = Path(tmp_data_folder).joinpath("dk-tg-install.json")
assert marker.exists()
import json as _json

assert _json.loads(marker.read_text())["install_mode"] == "docker"
assert json.loads(marker.read_text())["install_mode"] == "docker"


@pytest.mark.integration
@pytest.mark.parametrize(
"stdout_effect",
(
[['[{"Name":"test-project","Status":"running(2)","ConfigFiles":"<COMPOSE>"}]'], []],
[[], ['{"Labels":"com.docker.compose.project=test-project,", "Status":"N/A"}']],
lambda compose_path: [
[json.dumps([{"Name": "test-project", "Status": "running(2)", "ConfigFiles": str(compose_path)}])],
[],
],
lambda compose_path: [
[],
[json.dumps({"Labels": "com.docker.compose.project=test-project,", "Status": "N/A"})],
],
),
ids=("container", "volume"),
)
def test_tg_existing_install_abort(stdout_effect, tg_install_action, stdout_mock, compose_path):
stdout_mock.side_effect = [
[line.replace("<COMPOSE>", str(compose_path)) for line in output] for output in stdout_effect
]
stdout_mock.side_effect = stdout_effect(compose_path)
compose_path.touch()

with patch.object(tg_install_action, "steps", new=[ComposeVerifyExistingInstallStep]):
Expand Down
5 changes: 4 additions & 1 deletion tests/test_tg_pip_delete.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import platform
from functools import partial
from pathlib import Path
from unittest.mock import MagicMock, patch
Expand Down Expand Up @@ -72,7 +73,9 @@ def test_pip_delete_removes_uv_tool_and_home(pip_delete_action, start_cmd_mock,
def test_pip_delete_removes_installer_local_uv(pip_delete_action, start_cmd_mock, tmp_data_folder, tmp_path):
local_bin = Path(tmp_data_folder) / "bin"
local_bin.mkdir()
local_uv = local_bin / "uv"
# ``resolve_uv_path`` looks for uv.exe on Windows: create the name this platform's
# installer will actually look for, so the test exercises the real branch on both.
local_uv = local_bin / ("uv.exe" if platform.system() == "Windows" else "uv")
local_uv.write_bytes(b"#!/bin/sh\n")

fake_tg_home = tmp_path / ".testgen"
Expand Down
33 changes: 32 additions & 1 deletion tests/test_tg_start.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import signal
from pathlib import Path
from unittest.mock import MagicMock, patch
Expand All @@ -18,6 +19,14 @@
)


# The POSIX stop path signals a process group, which Windows has no equivalent of --
# ``supports_graceful_stop`` keeps the installer off that branch there entirely. These are
# skipped rather than shimmed: synthesizing ``os.killpg``/``signal.SIGKILL`` would also make
# a future capability-based ``supports_graceful_stop`` report True on Windows and quietly
# send the Windows tests down the wrong branch.
posix_only = pytest.mark.skipif(not hasattr(os, "killpg"), reason="needs POSIX process groups")


# --- start_testgen_app helper -------------------------------------------------


Expand Down Expand Up @@ -55,6 +64,10 @@ def test_start_testgen_app_happy_path(app_action, args_mock, proc_running_then_s
patch("tests.installer.resolve_testgen_path", return_value="/bin/testgen"),
patch("tests.installer.subprocess.Popen", return_value=proc_running_then_stops) as popen_mock,
patch("tests.installer.wait_for_tcp_port", return_value=True) as port_mock,
# The ``finally`` cleanup is not this test's subject, and on Windows it shells out
# via ``subprocess.run`` -- which goes through the Popen patched right above and
# would count as a second app launch.
patch("tests.installer.stop_app_tree"),
):
start_testgen_app(app_action, args_mock)

Expand Down Expand Up @@ -120,6 +133,9 @@ def test_start_testgen_app_handles_keyboard_interrupt(app_action, args_mock, con
patch("tests.installer.resolve_testgen_path", return_value="/bin/testgen"),
patch("tests.installer.subprocess.Popen", return_value=proc),
patch("tests.installer.wait_for_tcp_port", return_value=True),
# Pinned: the grace-period messages below are POSIX-only behaviour, and the
# Windows counterpart is test_start_testgen_app_makes_no_grace_promise_on_windows.
patch("tests.installer.supports_graceful_stop", return_value=True),
patch("tests.installer.stop_app_tree", return_value=True) as stop_mock,
):
start_testgen_app(app_action, args_mock)
Expand Down Expand Up @@ -148,6 +164,8 @@ def test_start_testgen_app_warns_when_grace_period_expires(app_action, args_mock
patch("tests.installer.resolve_testgen_path", return_value="/bin/testgen"),
patch("tests.installer.subprocess.Popen", return_value=proc),
patch("tests.installer.wait_for_tcp_port", return_value=True),
# Pinned: Windows never promises a grace period, so it never warns one expired.
patch("tests.installer.supports_graceful_stop", return_value=True),
patch("tests.installer.stop_app_tree", return_value=False),
):
start_testgen_app(app_action, args_mock)
Expand Down Expand Up @@ -185,10 +203,16 @@ def test_start_testgen_app_makes_no_grace_promise_on_windows(app_action, args_mo

@pytest.mark.unit
def test_stop_app_tree_no_op_when_proc_already_exited():
"""POSIX only. Windows deliberately does not take this shortcut: a console Ctrl+C
kills the parent along with the installer, so a dead parent there says nothing about
its children -- see test_stop_app_tree_windows_uses_taskkill_tree."""
proc = MagicMock()
proc.poll.return_value = 0 # already exited

with patch("tests.installer.subprocess.run") as run_mock:
with (
patch("tests.installer.supports_graceful_stop", return_value=True),
patch("tests.installer.subprocess.run") as run_mock,
):
stop_app_tree(proc)

run_mock.assert_not_called()
Expand All @@ -215,6 +239,7 @@ def test_stop_app_tree_windows_uses_taskkill_tree():


@pytest.mark.unit
@posix_only
def test_stop_app_tree_posix_signals_process_group():
proc = MagicMock()
proc.poll.return_value = None
Expand All @@ -234,6 +259,7 @@ def test_stop_app_tree_posix_signals_process_group():


@pytest.mark.unit
@posix_only
def test_stop_app_tree_falls_through_to_kill_on_timeout():
"""If SIGTERM doesn't take, escalate to SIGKILL / proc.kill()."""
import subprocess as sp
Expand All @@ -255,6 +281,7 @@ def test_stop_app_tree_falls_through_to_kill_on_timeout():


@pytest.mark.unit
@posix_only
def test_stop_app_tree_reports_graceful_stop():
"""Exiting within the grace period is the signal the caller uses to decide
whether a running job got to checkpoint."""
Expand All @@ -272,6 +299,7 @@ def test_stop_app_tree_reports_graceful_stop():


@pytest.mark.unit
@posix_only
def test_stop_app_tree_swallows_second_interrupt():
"""A second Ctrl+C while we wait means 'stop now'. It must force-kill here rather
than escaping to the caller's ``finally``, which would re-signal the tree — TestGen
Expand Down Expand Up @@ -311,6 +339,7 @@ def test_stop_app_tree_windows_stop_is_always_forced():


@pytest.mark.unit
@posix_only
def test_force_kill_sweeps_orphans_outside_the_process_group():
"""``run-app all`` starts its ui/scheduler children in their own sessions, so killpg
on the parent leaves them holding the port and the data dir — breaking the next
Expand All @@ -331,6 +360,7 @@ def test_force_kill_sweeps_orphans_outside_the_process_group():


@pytest.mark.unit
@posix_only
def test_second_interrupt_still_sweeps_orphans():
"""The second-Ctrl+C path force-kills, so it owes the same cleanup."""
proc = MagicMock()
Expand All @@ -350,6 +380,7 @@ def test_second_interrupt_still_sweeps_orphans():


@pytest.mark.unit
@posix_only
def test_graceful_stop_does_not_sweep_orphans():
"""A tree that stopped on its own has nothing left behind — don't reach for pkill."""
proc = MagicMock()
Expand Down
Loading