Skip to content

Commit e509941

Browse files
committed
fix(cli): type the job reporter as a protocol so the suite type-checks
CI runs mypy over tests as well as src, and these tests were red: the recording fakes they pass to JobRegistry.start and JobLogTailer are not HandlerCallbacks. Neither collaborator wants the socket transport -- they want somewhere to send a result and somewhere to send logs -- so say that: JobReporter is a Protocol with those two methods, and HandlerCallback satisfies it structurally. The fakes then type-check as themselves rather than needing a cast at every call site. Worth noting the protocol immediately found a real gap: test_server_async's FakeCallback had no post_logs at all, so the tailer's calls into it were only ever working by accident of it never being exercised there. The rest is local: cast the _FakeRequest stand-ins to web.Request at the two handle_start call sites, and narrow web.Response.text (str | None) before json.loads and `in`.
1 parent 01c1fb1 commit e509941

3 files changed

Lines changed: 42 additions & 15 deletions

File tree

‎packages/uipath/src/uipath/_cli/_server_jobs.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import os
1717
import re
1818
import sys
19-
from typing import Any
19+
from typing import Any, Protocol
2020

2121
from aiohttp import ClientSession, ClientTimeout, UnixConnector
2222

@@ -90,6 +90,18 @@ class _PostOutcome(enum.Enum):
9090
UNREACHABLE = "unreachable" # transport failure or 5xx — worth retrying
9191

9292

93+
class JobReporter(Protocol):
94+
"""Where a job's logs and outcome go.
95+
96+
All the registry and the tailer need. Kept separate from HandlerCallback so a
97+
reporter can be substituted without inheriting the socket transport.
98+
"""
99+
100+
async def post_result(self, job_key: str, payload: dict[str, Any]) -> bool: ...
101+
102+
async def post_logs(self, job_key: str, lines: list[dict[str, Any]]) -> bool: ...
103+
104+
93105
class HandlerCallback:
94106
"""Posts job lifecycle events to the caller's Unix-socket HTTP API."""
95107

@@ -200,7 +212,7 @@ class JobLogTailer:
200212
still produced exactly as before, we just also forward it.
201213
"""
202214

203-
def __init__(self, job_key: str, path: str, callback: "HandlerCallback") -> None:
215+
def __init__(self, job_key: str, path: str, callback: "JobReporter") -> None:
204216
self.job_key = job_key
205217
self.path = path
206218
self.callback = callback
@@ -307,7 +319,7 @@ def start(
307319
args: list[str],
308320
env_vars: dict[str, str],
309321
working_dir: str | None,
310-
callback: HandlerCallback,
322+
callback: JobReporter,
311323
) -> bool:
312324
"""Register and schedule a job. False if one is already in flight for this key."""
313325
if self.is_active(job_key):
@@ -330,7 +342,7 @@ async def _run(
330342
args: list[str],
331343
env_vars: dict[str, str],
332344
working_dir: str | None,
333-
callback: HandlerCallback,
345+
callback: JobReporter,
334346
) -> None:
335347
tailer: JobLogTailer | None = None
336348
tail_task: "asyncio.Task[None] | None" = None

‎packages/uipath/tests/cli/test_server_async.py‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@
88
import asyncio
99
import json
1010
import os
11-
from typing import Any
11+
from typing import Any, cast
1212

1313
import click
1414
import pytest
15+
from aiohttp import web
1516

1617
from uipath._cli import _server_core, cli_server, cli_server_ipc
1718
from uipath._cli._server_core import (
@@ -44,6 +45,9 @@ async def post_result(self, job_key: str, payload: dict[str, Any]) -> bool:
4445
self.done.set()
4546
return True
4647

48+
async def post_logs(self, job_key: str, lines: list[dict[str, Any]]) -> bool:
49+
return True
50+
4751

4852
def _write_config(tmp_path, runtime: dict[str, Any] | None) -> str:
4953
config = {"runtime": runtime} if runtime is not None else {}
@@ -261,6 +265,16 @@ async def json(self) -> dict[str, Any]:
261265
return self._payload
262266

263267

268+
def _fake_request(job_key: str, payload: dict[str, Any]) -> web.Request:
269+
"""handle_start only touches match_info and json(); the cast keeps mypy honest."""
270+
return cast(web.Request, _FakeRequest(job_key, payload))
271+
272+
273+
def _body(response: web.Response) -> dict[str, Any]:
274+
assert response.text is not None
275+
return cast("dict[str, Any]", json.loads(response.text))
276+
277+
264278
class _RecordingRegistry:
265279
def __init__(self, accept: bool = True) -> None:
266280
self.accept = accept
@@ -276,14 +290,14 @@ async def test_http_start_with_callback_returns_accepted(monkeypatch):
276290
monkeypatch.setattr(cli_server, "get_registry", lambda: registry)
277291

278292
response = await cli_server.handle_start(
279-
_FakeRequest(
293+
_fake_request(
280294
"job-1",
281295
{"command": "run", "resultCallbackSocket": "/tmp/ack.sock"},
282296
)
283297
)
284298

285299
assert response.status == 202
286-
body = json.loads(response.text)
300+
body = _body(response)
287301
assert body["disposition"] == "accepted"
288302
assert body["contractVersion"] == CONTRACT_VERSION
289303
assert registry.started == ["job-1"]
@@ -295,13 +309,13 @@ async def test_http_start_duplicate_is_409(monkeypatch):
295309
)
296310

297311
response = await cli_server.handle_start(
298-
_FakeRequest(
312+
_fake_request(
299313
"job-1", {"command": "run", "resultCallbackSocket": "/tmp/ack.sock"}
300314
)
301315
)
302316

303317
assert response.status == 409
304-
assert json.loads(response.text)["success"] is False
318+
assert _body(response)["success"] is False
305319

306320

307321
async def test_http_start_without_callback_stays_synchronous(monkeypatch):
@@ -319,11 +333,11 @@ async def _fake_isolated(cmd, args, env_vars, working_dir):
319333
lambda: pytest.fail("registry must not be used without a callback socket"),
320334
)
321335

322-
response = await cli_server.handle_start(_FakeRequest("job-1", {"command": "run"}))
336+
response = await cli_server.handle_start(_fake_request("job-1", {"command": "run"}))
323337

324338
assert called.get("ran") is True
325339
assert response.status == 200
326-
body = json.loads(response.text)
340+
body = _body(response)
327341
assert body["success"] is True
328342
assert "disposition" not in body
329343

@@ -362,7 +376,7 @@ async def test_ipc_start_duplicate_reports_failure(monkeypatch):
362376

363377
assert result.ExitCode == 1
364378
assert result.Disposition is None
365-
assert "already in flight" in result.Error
379+
assert "already in flight" in (result.Error or "")
366380

367381

368382
async def test_ipc_start_without_callback_stays_synchronous(monkeypatch):

‎packages/uipath/tests/cli/test_server_result.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
rather than a stub, because it is the whole reason the exit code can be wrong.
88
"""
99

10-
from typing import Any
10+
from typing import Any, cast
1111

1212
import click
1313
import pytest
@@ -132,7 +132,8 @@ async def _fake_isolated(cmd, args, env_vars, working_dir):
132132
return command_result
133133

134134
monkeypatch.setattr(cli_server, "_run_command_isolated", _fake_isolated)
135-
return await cli_server.handle_start(_FakeRequest("job-1", {"command": "run"}))
135+
request = cast(web.Request, _FakeRequest("job-1", {"command": "run"}))
136+
return await cli_server.handle_start(request)
136137

137138

138139
async def test_success_body_carries_exit_code(monkeypatch):
@@ -157,7 +158,7 @@ async def test_failure_body_is_200_but_says_so_and_carries_exit_code(monkeypatch
157158
)
158159

159160
assert response.status == 200
160-
body = response.text
161+
body = response.text or ""
161162
assert '"success": false' in body
162163
assert '"exitCode": 1' in body
163164
assert "Exit code: 1" in body

0 commit comments

Comments
 (0)