Left over fixes for Windows Papercut PR (#781)
<!-- CURSOR_SUMMARY --> > [!NOTE] > **Low Risk** > Mostly CI/test and CLI output tweaks, plus a small refactor to reuse existing subprocess termination logic; low risk with minor potential for CI environment/version compatibility issues. > > **Overview** > Expands CI coverage by adding Python `3.13` and `3.14` to the GitHub Actions matrices (main tests, install test, and no-auth CLI integration), and removes a redundant editable install step in the no-auth workflow. > > Cleans up Windows subprocess handling by dropping `arcade_cli.deploy._graceful_terminate` and calling the shared `arcade_core.subprocess_utils.graceful_terminate_process` directly, with corresponding test updates. > > Improves `arcade new` scaffolding guidance by printing numbered “Next steps” with explicit stdio/HTTP run options, and adds/updates CLI tests to assert this output. Also bumps package version to `1.11.2` and tightens pre-commit `ruff` excludes (no longer excluding `_scratch`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 55c2ae106f13e5657acdbebf63e00d74c171181f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This commit is contained in:
parent
4a737b9710
commit
bcee0f556f
10 changed files with 69 additions and 49 deletions
|
|
@ -12,13 +12,11 @@ jobs:
|
|||
name: CLI Integration No-Auth (${{ matrix.os }}, Python ${{ matrix.python-version }})
|
||||
runs-on: ${{ matrix.os }}
|
||||
timeout-minutes: 35
|
||||
env:
|
||||
ARCADE_USAGE_TRACKING: "0"
|
||||
|
||||
strategy:
|
||||
matrix:
|
||||
os: [ubuntu-latest, windows-latest, macos-latest]
|
||||
python-version: ["3.10", "3.11", "3.12"]
|
||||
python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"]
|
||||
fail-fast: false
|
||||
|
||||
steps:
|
||||
|
|
@ -30,10 +28,6 @@ jobs:
|
|||
with:
|
||||
python-version: ${{ matrix.python-version }}
|
||||
|
||||
- name: Install arcade-mcp from source
|
||||
run: |
|
||||
uv pip install -e .
|
||||
|
||||
- name: Run no-auth CLI smoke checks
|
||||
run: |
|
||||
uv run python tests/integration/no_auth_cli_smoke.py
|
||||
|
|
|
|||
2
.github/workflows/main.yml
vendored
2
.github/workflows/main.yml
vendored
|
|
@ -31,7 +31,7 @@ jobs:
|
|||
strategy:
|
||||
matrix:
|
||||
os: [ubuntu-latest, windows-latest, macos-latest]
|
||||
python-version: ["3.10", "3.11", "3.12"]
|
||||
python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"]
|
||||
fail-fast: false
|
||||
steps:
|
||||
- name: Check out
|
||||
|
|
|
|||
2
.github/workflows/test-install.yml
vendored
2
.github/workflows/test-install.yml
vendored
|
|
@ -15,7 +15,7 @@ jobs:
|
|||
fail-fast: false
|
||||
matrix:
|
||||
os: [ubuntu-latest, windows-latest, macos-latest]
|
||||
python-version: ["3.10", "3.12"]
|
||||
python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"]
|
||||
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
|
|
|
|||
|
|
@ -17,6 +17,6 @@ repos:
|
|||
hooks:
|
||||
- id: ruff
|
||||
args: [--fix]
|
||||
exclude: "(.*/templates/.*|libs/tests/.*|_scratch/.*)"
|
||||
exclude: "(.*/templates/.*|libs/tests/.*)"
|
||||
- id: ruff-format
|
||||
exclude: "(.*/templates/.*|libs/tests/.*|_scratch/.*)"
|
||||
exclude: "(.*/templates/.*|libs/tests/.*)"
|
||||
|
|
|
|||
|
|
@ -370,11 +370,6 @@ def create_package_archive(package_dir: Path) -> str:
|
|||
return package_bytes_b64
|
||||
|
||||
|
||||
def _graceful_terminate(process: subprocess.Popen) -> None:
|
||||
"""Terminate a subprocess using shared graceful shutdown semantics."""
|
||||
graceful_terminate_process(process)
|
||||
|
||||
|
||||
def _resolve_server_process_stdio(debug: bool) -> tuple[int | None, int | None]:
|
||||
"""Choose stdout/stderr targets for the temporary validation server process.
|
||||
|
||||
|
|
@ -496,7 +491,7 @@ def wait_for_health(
|
|||
time.sleep(0.5)
|
||||
|
||||
if not is_healthy:
|
||||
_graceful_terminate(process)
|
||||
graceful_terminate_process(process)
|
||||
try:
|
||||
process.communicate(timeout=2)
|
||||
except subprocess.TimeoutExpired:
|
||||
|
|
@ -651,7 +646,7 @@ def verify_server_and_get_metadata(
|
|||
|
||||
finally:
|
||||
# Always stop the server
|
||||
_graceful_terminate(process)
|
||||
graceful_terminate_process(process)
|
||||
try:
|
||||
process.wait(timeout=5)
|
||||
except subprocess.TimeoutExpired:
|
||||
|
|
|
|||
|
|
@ -227,13 +227,11 @@ def create_new_toolkit(output_directory: str, toolkit_name: str) -> None:
|
|||
f"[green]Toolkit '{toolkit_name}' created successfully at '{toolkit_directory}'.[/green]"
|
||||
)
|
||||
console.print("\nNext steps:", style="bold")
|
||||
console.print(f" cd {toolkit_directory / toolkit_name}")
|
||||
console.print(f" 1. cd {toolkit_directory / toolkit_name}")
|
||||
console.print("")
|
||||
console.print(" Run with stdio transport (for MCP clients):", style="dim")
|
||||
console.print(" uv run server.py")
|
||||
console.print("")
|
||||
console.print(" Run with HTTP transport (for development/testing):", style="dim")
|
||||
console.print(" uv run server.py --transport http --port 8000")
|
||||
console.print(" 2. Run the server (choose one transport):", style="dim")
|
||||
console.print(" - stdio: uv run server.py")
|
||||
console.print(" - http: uv run server.py --transport http --port 8000")
|
||||
console.print("")
|
||||
create_deployment(toolkit_directory, toolkit_name)
|
||||
except Exception:
|
||||
|
|
@ -288,13 +286,11 @@ def create_new_toolkit_minimal(output_directory: str, toolkit_name: str) -> None
|
|||
)
|
||||
server_dir = toolkit_directory / toolkit_name / "src" / toolkit_name
|
||||
console.print("\nNext steps:", style="bold")
|
||||
console.print(f" cd {server_dir}")
|
||||
console.print(f" 1. cd {server_dir}")
|
||||
console.print("")
|
||||
console.print(" Run with stdio transport (for MCP clients):", style="dim")
|
||||
console.print(" uv run server.py")
|
||||
console.print("")
|
||||
console.print(" Run with HTTP transport (for development/testing):", style="dim")
|
||||
console.print(" uv run server.py --transport http --port 8000")
|
||||
console.print(" 2. Run the server (choose one transport):", style="dim")
|
||||
console.print(" - stdio: uv run server.py")
|
||||
console.print(" - http: uv run server.py --transport http --port 8000")
|
||||
console.print("")
|
||||
except Exception:
|
||||
remove_toolkit(toolkit_directory, toolkit_name)
|
||||
|
|
|
|||
|
|
@ -1,11 +1,45 @@
|
|||
from io import StringIO
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from arcade_cli.new import create_new_toolkit_minimal
|
||||
from arcade_cli.new import create_new_toolkit, create_new_toolkit_minimal
|
||||
from rich.console import Console
|
||||
|
||||
|
||||
def test_create_new_toolkit_prints_next_steps(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""create_new_toolkit (full template) should print numbered next steps."""
|
||||
output_dir = tmp_path / "full_test"
|
||||
output_dir.mkdir()
|
||||
|
||||
# Use a cwd that does not trigger community/official toolkit prompts
|
||||
fake_cwd = tmp_path / "cwd"
|
||||
fake_cwd.mkdir()
|
||||
monkeypatch.chdir(fake_cwd)
|
||||
|
||||
# Mock prompts: description, author, email, evals (yes)
|
||||
with patch("arcade_cli.new.typer.prompt", side_effect=["", "", "", "y"]):
|
||||
buf = StringIO()
|
||||
test_console = Console(file=buf, force_terminal=False)
|
||||
import arcade_cli.new as new_mod
|
||||
|
||||
orig = new_mod.console
|
||||
new_mod.console = test_console
|
||||
try:
|
||||
create_new_toolkit(str(output_dir), "my_server")
|
||||
finally:
|
||||
new_mod.console = orig
|
||||
|
||||
output = buf.getvalue()
|
||||
assert "Next steps:" in output
|
||||
assert "1. cd " in output
|
||||
assert "2. Run the server (choose one transport):" in output
|
||||
assert "- stdio: uv run server.py" in output
|
||||
assert "- http: uv run server.py --transport http --port 8000" in output
|
||||
assert "uv run server.py" in output
|
||||
assert "my_server" in output
|
||||
|
||||
|
||||
def test_create_new_toolkit_minimal_with_spaces(tmp_path: Path) -> None:
|
||||
output_dir = tmp_path / "dir with spaces"
|
||||
output_dir.mkdir()
|
||||
|
|
@ -38,6 +72,14 @@ def test_create_new_toolkit_minimal_prints_next_steps(tmp_path: Path) -> None:
|
|||
|
||||
output = buf.getvalue()
|
||||
assert "Next steps:" in output, f"Expected 'Next steps:' in output:\n{output}"
|
||||
assert "1. cd " in output, f"Expected numbered step 1 in output:\n{output}"
|
||||
assert "2. Run the server (choose one transport):" in output, (
|
||||
f"Expected numbered step 2 in output:\n{output}"
|
||||
)
|
||||
assert "- stdio: uv run server.py" in output, f"Expected stdio option in output:\n{output}"
|
||||
assert "- http: uv run server.py --transport http --port 8000" in output, (
|
||||
f"Expected HTTP option in output:\n{output}"
|
||||
)
|
||||
assert "uv run server.py" in output, f"Expected 'uv run server.py' in output:\n{output}"
|
||||
assert "demo_srv" in output, f"Expected toolkit name in output:\n{output}"
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
Verifies that:
|
||||
- Background subprocess calls set CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP on Windows.
|
||||
- _graceful_terminate sends CTRL_BREAK_EVENT on Windows, falls back to terminate().
|
||||
- graceful_terminate_process sends CTRL_BREAK_EVENT on Windows, falls back to terminate().
|
||||
- MCPApp._run_with_reload shutdown uses CTRL_BREAK_EVENT on Windows.
|
||||
- stdio transport registers a stdlib signal.signal fallback on Windows.
|
||||
|
||||
|
|
@ -138,22 +138,22 @@ class TestDeployCreateNoWindow:
|
|||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# deploy.py — _graceful_terminate()
|
||||
# subprocess_utils.py — graceful_terminate_process()
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestGracefulTerminate:
|
||||
"""Verify _graceful_terminate uses CTRL_BREAK_EVENT on Windows."""
|
||||
"""Verify graceful_terminate_process uses CTRL_BREAK_EVENT on Windows."""
|
||||
|
||||
def test_sends_ctrl_break_on_win32(self) -> None:
|
||||
"""On Windows, _graceful_terminate should send CTRL_BREAK_EVENT."""
|
||||
from arcade_cli.deploy import _graceful_terminate
|
||||
"""On Windows, graceful_terminate_process should send CTRL_BREAK_EVENT."""
|
||||
from arcade_core.subprocess_utils import graceful_terminate_process
|
||||
|
||||
mock_proc = MagicMock()
|
||||
|
||||
# sys.platform mock: verifies CTRL_BREAK_EVENT dispatch with mocked process.
|
||||
with _patch_win32_ctrl_break():
|
||||
_graceful_terminate(mock_proc)
|
||||
graceful_terminate_process(mock_proc)
|
||||
|
||||
# Should try send_signal with CTRL_BREAK_EVENT (not terminate)
|
||||
mock_proc.send_signal.assert_called_once_with(WIN_CTRL_BREAK_EVENT)
|
||||
|
|
@ -161,25 +161,25 @@ class TestGracefulTerminate:
|
|||
|
||||
def test_falls_back_to_terminate_on_win32_oserror(self) -> None:
|
||||
"""If send_signal fails on Windows, fall back to terminate."""
|
||||
from arcade_cli.deploy import _graceful_terminate
|
||||
from arcade_core.subprocess_utils import graceful_terminate_process
|
||||
|
||||
mock_proc = MagicMock()
|
||||
mock_proc.send_signal.side_effect = OSError("Process exited")
|
||||
|
||||
# sys.platform mock: exercises OSError fallback with mocked process.
|
||||
with _patch_win32_ctrl_break():
|
||||
_graceful_terminate(mock_proc)
|
||||
graceful_terminate_process(mock_proc)
|
||||
|
||||
mock_proc.terminate.assert_called_once()
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="Non-Windows terminate() path")
|
||||
def test_calls_terminate_on_linux(self) -> None:
|
||||
"""On Linux/macOS, _graceful_terminate should call terminate() directly."""
|
||||
from arcade_cli.deploy import _graceful_terminate
|
||||
"""On Linux/macOS, graceful_terminate_process should call terminate() directly."""
|
||||
from arcade_core.subprocess_utils import graceful_terminate_process
|
||||
|
||||
mock_proc = MagicMock()
|
||||
|
||||
_graceful_terminate(mock_proc)
|
||||
graceful_terminate_process(mock_proc)
|
||||
|
||||
mock_proc.terminate.assert_called_once()
|
||||
mock_proc.send_signal.assert_not_called()
|
||||
|
|
|
|||
|
|
@ -124,9 +124,6 @@ async def test_call_tool_success(base_worker_no_auth):
|
|||
assert response.output.value == 8
|
||||
assert response.output.error is None
|
||||
assert response.execution_id == "test_exec_id"
|
||||
assert response.duration > 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_tool_execution_error(base_worker_no_auth):
|
||||
# Tool is now defined at module level
|
||||
|
|
|
|||
|
|
@ -57,10 +57,6 @@ evals = [
|
|||
"pytz>=2024.1",
|
||||
]
|
||||
|
||||
windows = [
|
||||
"platformdirs>=4.3.6",
|
||||
]
|
||||
|
||||
dev = [
|
||||
# Test framework
|
||||
"pytest>=8.1.2",
|
||||
|
|
|
|||
Loading…
Reference in a new issue