Skip to content
Open
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
60 changes: 51 additions & 9 deletions .github/scripts/preflight_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@

POLL_SECONDS = 0.2
STATUS_INTERVAL_SECONDS = 5.0
FORWARD_SIGNAL_NAMES = ("SIGINT", "SIGTERM", "SIGHUP")


def supported_forward_signals() -> tuple[int, ...]:
return tuple(signum for name in FORWARD_SIGNAL_NAMES if isinstance((signum := getattr(signal, name, None)), int))


def forward_signal_to_child(child: subprocess.Popen[str], signum: int) -> None:
if child.poll() is not None:
return
try:
kill_process_group = getattr(os, "killpg", None)
if callable(kill_process_group):
kill_process_group(child.pid, signum)
else:
child.send_signal(signum)
except ProcessLookupError:
pass


def atomic_json(path: Path, value: dict) -> None:
Expand All @@ -32,9 +50,38 @@ def read_json(path: Path) -> dict:
return {}


def windows_process_exists(pid: int) -> bool:
import ctypes
from ctypes import wintypes

process_query_limited_information = 0x1000
error_access_denied = 5
still_active = 259
kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
kernel32.OpenProcess.argtypes = (wintypes.DWORD, wintypes.BOOL, wintypes.DWORD)
kernel32.OpenProcess.restype = wintypes.HANDLE
kernel32.GetExitCodeProcess.argtypes = (wintypes.HANDLE, ctypes.POINTER(wintypes.DWORD))
kernel32.GetExitCodeProcess.restype = wintypes.BOOL
kernel32.CloseHandle.argtypes = (wintypes.HANDLE,)
kernel32.CloseHandle.restype = wintypes.BOOL

handle = kernel32.OpenProcess(process_query_limited_information, False, pid)
if not handle:
return ctypes.get_last_error() == error_access_denied
try:
exit_code = wintypes.DWORD()
if not kernel32.GetExitCodeProcess(handle, ctypes.byref(exit_code)):
return True
return exit_code.value == still_active
finally:
kernel32.CloseHandle(handle)


def process_exists(pid: int) -> bool:
if pid <= 0:
return False
if os.name == "nt":
return windows_process_exists(pid)
try:
os.kill(pid, 0)
return True
Expand Down Expand Up @@ -173,15 +220,10 @@ def write_status() -> None:
)

def forward_signal(signum: int, _frame: object) -> None:
if child is not None and child.poll() is None:
try:
os.killpg(child.pid, signum)
except ProcessLookupError:
pass

previous_handlers = {
signum: signal.signal(signum, forward_signal) for signum in (signal.SIGINT, signal.SIGTERM, signal.SIGHUP)
}
if child is not None:
forward_signal_to_child(child, signum)

previous_handlers = {signum: signal.signal(signum, forward_signal) for signum in supported_forward_signals()}
exit_code = 1
try:
print(f"Pre-push single-flight log: {log_path}")
Expand Down
80 changes: 80 additions & 0 deletions .github/scripts/test_pr_preflight.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,24 @@
import io
import json
import os
import signal
import subprocess
import sys
import tempfile
import time
import unittest
from unittest import mock
from pathlib import Path

import preflight_runner
from pr_metadata import load_from_api
from pr_preflight import select_checks

SCRIPT_DIR = Path(__file__).resolve().parent
RUNNER = SCRIPT_DIR / "preflight_runner.py"
REPO_ROOT = SCRIPT_DIR.parents[1]
PRE_PUSH_SINGLEFLIGHT = REPO_ROOT / "scripts" / "pre-push-singleflight"
PRE_PUSH = REPO_ROOT / "scripts" / "pre-push"


class FakeResponse(io.BytesIO):
Expand Down Expand Up @@ -59,6 +64,19 @@ def opener(request: object, timeout: int) -> FakeResponse:


class SelectionTests(unittest.TestCase):
def test_pre_push_wrapper_reuses_current_bash_interpreter(self) -> None:
wrapper = PRE_PUSH_SINGLEFLIGHT.read_text(encoding="utf-8")
self.assertIn(' -- "$BASH" scripts/pre-push "$@"', wrapper)

def test_pre_push_accepts_and_propagates_windows_backend_python(self) -> None:
pre_push = PRE_PUSH.read_text(encoding="utf-8")
setup_prefix = pre_push[: pre_push.index("require_backend_python()")]

self.assertIn('BACKEND_PYTHON="${BACKEND_PYTHON:-}"', setup_prefix)
self.assertIn('"$PWD/backend/.venv/bin/python"', setup_prefix)
self.assertIn('"$PWD/backend/.venv/Scripts/python.exe"', setup_prefix)
self.assertIn('PYRIGHT_PYTHON="$BACKEND_PYTHON" bash scripts/typecheck.sh', pre_push)

def test_make_preflight_resolves_pr_metadata_before_running_checks(self) -> None:
result = subprocess.run(
["make", "-n", "preflight"],
Expand Down Expand Up @@ -196,6 +214,68 @@ def test_pr_body_file_env_is_honored(self) -> None:
self.assertIn(str(body.resolve()), result.stdout)


class SignalCompatibilityTests(unittest.TestCase):
@unittest.skipUnless(os.name == "nt", "Windows-specific process probe")
def test_windows_process_probe_finds_current_process(self) -> None:
self.assertTrue(preflight_runner.windows_process_exists(os.getpid()))

@unittest.skipUnless(os.name == "nt", "Windows-specific process probe")
def test_windows_process_probe_rejects_exited_process(self) -> None:
child = subprocess.Popen([sys.executable, "-c", "pass"])
child.wait(timeout=5)

self.assertFalse(preflight_runner.windows_process_exists(child.pid))

def test_process_exists_uses_windows_probe_without_os_kill(self) -> None:
with (
mock.patch.object(preflight_runner.os, "name", "nt"),
mock.patch.object(preflight_runner, "windows_process_exists", return_value=True) as windows_probe,
mock.patch.object(preflight_runner.os, "kill") as kill,
):
self.assertTrue(preflight_runner.process_exists(4321))

windows_probe.assert_called_once_with(4321)
kill.assert_not_called()

def test_supported_forward_signals_skip_unavailable_members(self) -> None:
with mock.patch.object(preflight_runner.signal, "SIGHUP", None, create=True):
self.assertEqual(
preflight_runner.supported_forward_signals(),
(signal.SIGINT, signal.SIGTERM),
)

def test_forward_signal_uses_child_api_without_killpg(self) -> None:
child = mock.Mock(pid=4321)
child.poll.return_value = None

with mock.patch.object(preflight_runner.os, "killpg", None, create=True):
preflight_runner.forward_signal_to_child(child, signal.SIGTERM)

child.send_signal.assert_called_once_with(signal.SIGTERM)

@unittest.skipUnless(os.name == "nt", "Windows-specific signal forwarding")
def test_forward_signal_terminates_windows_child(self) -> None:
child = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(30)"])
try:
preflight_runner.forward_signal_to_child(child, signal.SIGTERM)
child.wait(timeout=5)
finally:
if child.poll() is None:
child.kill()
child.wait(timeout=5)

def test_forward_signal_keeps_posix_process_group_behavior(self) -> None:
child = mock.Mock(pid=4321)
child.poll.return_value = None
killpg = mock.Mock()

with mock.patch.object(preflight_runner.os, "killpg", killpg, create=True):
preflight_runner.forward_signal_to_child(child, signal.SIGTERM)

killpg.assert_called_once_with(4321, signal.SIGTERM)
child.send_signal.assert_not_called()


class SingleFlightTests(unittest.TestCase):
def run_runner(
self,
Expand Down
18 changes: 13 additions & 5 deletions scripts/pre-push
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,22 @@ SELECTED_BACKEND_TESTS_REASON=$(mktemp "${TMPDIR:-/tmp}/omi-pre-push-backend-tes
FAST_BACKEND_TESTS=$(mktemp "${TMPDIR:-/tmp}/omi-pre-push-fast-backend-tests.XXXXXX")
trap 'rm -f "$CHANGED_FILES_LIST" "$SELECTED_BACKEND_TESTS" "$SELECTED_BACKEND_TESTS_REASON" "$FAST_BACKEND_TESTS"' EXIT
printf '%s\n' "${CHANGED_FILES[@]}" > "$CHANGED_FILES_LIST"
BACKEND_PYTHON="$PWD/backend/.venv/bin/python"
BACKEND_PYTHON="${BACKEND_PYTHON:-}"
if [[ -z "$BACKEND_PYTHON" ]]; then
for backend_python_candidate in "$PWD/backend/.venv/bin/python" "$PWD/backend/.venv/Scripts/python.exe"; do
if [[ -x "$backend_python_candidate" ]]; then
BACKEND_PYTHON="$backend_python_candidate"
break
fi
done
fi

require_backend_python() {
if [[ -x "$BACKEND_PYTHON" ]]; then
if [[ -n "$BACKEND_PYTHON" && -x "$BACKEND_PYTHON" ]]; then
return 0
fi
echo "FAIL: a selected backend check requires backend/.venv/bin/python, but it was not found." >&2
echo " Create it with: cd backend && bash scripts/sync-python-deps.sh" >&2
echo "FAIL: a selected backend check requires a usable backend Python." >&2
echo " Set BACKEND_PYTHON or run: cd backend && bash scripts/sync-python-deps.sh" >&2
return 1
}

Expand Down Expand Up @@ -275,7 +283,7 @@ check_backend_typecheck_if_needed() {
require_backend_python
(
cd backend
bash scripts/typecheck.sh
PYRIGHT_PYTHON="$BACKEND_PYTHON" bash scripts/typecheck.sh
)
}

Expand Down
2 changes: 1 addition & 1 deletion scripts/pre-push-singleflight
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ set -euo pipefail

ROOT="$(git rev-parse --show-toplevel)"
cd "$ROOT"
exec python3 .github/scripts/preflight_runner.py --name pre-push -- scripts/pre-push "$@"
exec python3 .github/scripts/preflight_runner.py --name pre-push -- "$BASH" scripts/pre-push "$@"
Loading