Skip to content

Commit 5c5dbab

Browse files
committed
Fix fork-safety test stdout flush, ruff formatting, and mypy typing in test suite
1 parent 1a7582c commit 5c5dbab

3 files changed

Lines changed: 28 additions & 10 deletions

File tree

‎tests/test_cli_client.py‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,9 @@ def test_cli_client_default_env_fn_rejects_a_non_callable_value() -> None:
252252
@pytest.mark.parametrize("verb", _SYNC_VERBS)
253253
def test_cli_client_default_env_fn_raise_aborts_sync_verb_before_spawn(verb: str) -> None:
254254
runner = RecordingRunner.replying(Reply.ok("should-not-run"))
255-
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner)
255+
# A resolver that raises (its float-typed body never actually returns) — the
256+
# deliberate wrong shape is what exercises the fail-closed abort.
257+
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner) # type: ignore[dict-item, return-value]
256258
# The resolver's own exception reaches the caller unchanged (not swallowed,
257259
# not remapped) — its cause is preserved.
258260
with pytest.raises(ZeroDivisionError):
@@ -263,7 +265,9 @@ def test_cli_client_default_env_fn_raise_aborts_sync_verb_before_spawn(verb: str
263265
@pytest.mark.parametrize("verb", _ASYNC_VERBS)
264266
def test_cli_client_default_env_fn_raise_aborts_async_verb_before_spawn(verb: str) -> None:
265267
runner = RecordingRunner.replying(Reply.ok("should-not-run"))
266-
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner)
268+
# Deliberately-raising resolver (float-typed body, never returns); see the
269+
# sync twin above.
270+
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner) # type: ignore[dict-item, return-value]
267271

268272
async def scenario() -> None:
269273
with pytest.raises(ZeroDivisionError):
@@ -277,7 +281,8 @@ def test_cli_client_default_env_fn_raise_aborts_command_build() -> None:
277281
# `command()` applies the client's defaults (resolving every default_env_fn),
278282
# so it is on the failing path too — same fail-closed behaviour as the verbs.
279283
runner = RecordingRunner.replying(Reply.ok("should-not-run"))
280-
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner)
284+
# Deliberately-raising resolver (float-typed body, never returns).
285+
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner) # type: ignore[dict-item, return-value]
281286
with pytest.raises(ZeroDivisionError):
282287
client.command(["--version"])
283288
assert len(runner.calls()) == 0
@@ -287,7 +292,7 @@ def test_cli_client_default_env_fn_non_str_result_aborts_before_spawn() -> None:
287292
# A non-`str` result is just as fail-closed as a raise: the failed `str`
288293
# conversion surfaces as a `TypeError`, before the runner is reached.
289294
runner = RecordingRunner.replying(Reply.ok("should-not-run"))
290-
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1}, runner=runner) # type: ignore[dict-item] # deliberately non-str to exercise the fail-closed conversion
295+
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1}, runner=runner) # type: ignore[dict-item, return-value] # deliberately non-str to exercise the fail-closed conversion
291296
with pytest.raises(TypeError):
292297
client.run(["--version"])
293298
assert len(runner.calls()) == 0
@@ -333,7 +338,9 @@ def test_cli_client_default_env_fn_failure_is_skipped_when_key_already_set() ->
333338
# whose value it does not supply. The explicit value wins and the runner is
334339
# invoked normally.
335340
runner = RecordingRunner.replying(Reply.ok(""))
336-
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner)
341+
# Deliberately-raising resolver (float-typed body, never returns); here its
342+
# key is already set, so it must never even run.
343+
client = CliClient(NO_SUCH_PROGRAM, default_env_fn={"PK_TOKEN": lambda: 1 / 0}, runner=runner) # type: ignore[dict-item, return-value]
337344
cmd = Command(NO_SUCH_PROGRAM, ["--version"]).env("PK_TOKEN", "explicit")
338345
client.run(cmd)
339346
assert runner.only_call().env_is("PK_TOKEN", "explicit")

‎tests/test_command.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,9 +1679,7 @@ def test_process_result_pickle_refusal_is_config_independent() -> None:
16791679
# the contract ("ProcessResult is not picklable") is simple and total, not a
16801680
# per-instance guess that would surprise callers.
16811681
plain = Command(PY, ["-c", "print('plain')"]).output()
1682-
customized = (
1683-
Command(PY, ["-c", "import sys; sys.exit(3)"]).success_codes([0, 3]).output()
1684-
)
1682+
customized = Command(PY, ["-c", "import sys; sys.exit(3)"]).success_codes([0, 3]).output()
16851683
for result in (plain, customized):
16861684
with pytest.raises(TypeError, match="ProcessResult cannot be pickled"):
16871685
pickle.dumps(result)

‎tests/test_fork_safety.py‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,13 @@ async def _use_async():
126126
127127
128128
asyncio.run(_use_async())
129-
print("OK")
129+
130+
# `os._exit()` (used throughout this driver to avoid post-fork interpreter
131+
# cleanup) skips the atexit stdout flush, and under `capture_output=True`
132+
# this stdout is a pipe — block-buffered, not line-buffered — so the success
133+
# marker must be flushed explicitly or it is silently dropped while the exit
134+
# code still reads 0. Flush before exiting so the parent test observes "OK".
135+
print("OK", flush=True)
130136
os._exit(0)
131137
"""
132138

@@ -163,4 +169,11 @@ def test_use_fork_use_refuses_without_hanging_or_orphaning(tmp_path: pathlib.Pat
163169
f"fork-safety driver failed (code {result.returncode}); "
164170
f"stdout={result.stdout!r} stderr={result.stderr!r}"
165171
)
166-
assert result.stdout.strip().splitlines()[-1] == "OK"
172+
# Use a slice (`[-1:]`) rather than an index (`[-1]`) so an unexpectedly
173+
# empty stdout fails as a clear, informative assertion — not a bare
174+
# IndexError that hides what the driver actually did.
175+
tail = result.stdout.strip().splitlines()[-1:]
176+
assert tail == ["OK"], (
177+
f"fork-safety driver did not confirm success with a trailing 'OK'; "
178+
f"stdout={result.stdout!r} stderr={result.stderr!r}"
179+
)

0 commit comments

Comments
 (0)