Skip to content

test: make two tests portable to Python 3.10 and free of a startup race - #961

Merged
agentforce314 merged 1 commit into
agentforce314:mainfrom
fxinfo24:fix/portable-test-timeouts-and-signal-race
Oct 4, 2026
Merged

agentforce314 merged 1 commit into
agentforce314:mainfrom
fxinfo24:fix/portable-test-timeouts-and-signal-race

Conversation

@fxinfo24

@fxinfo24 fxinfo24 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Problem

requires-python = ">=3.10", but both tests below hard-coded 3.11-only behaviour, so neither could run on a supported interpreter. A second failure was a genuine race that could bite on any interpreter.

1. asyncio.timeout is 3.11+ (tests/bridge/test_repl_bridge.py)

test_pointer_mtime_task_fires_and_advances_updated_at_ms used async with asyncio.timeout(3). On 3.10 this is an AttributeError and the test cannot run at all.

Rewritten with asyncio.wait_for around the polling coroutine — the portable equivalent. The refreshed value is carried out of the closure through a dict holder; nonlocal cannot target a name that is only bound inside the enclosing try block.

2. SIGINT/SIGTERM startup race (tests/test_init_integration.py)

Three tests signalled their child process after a fixed 100–300 ms delay. That races interpreter startup: if the signal arrives before setup_graceful_shutdown() installs its handler, the default disposition terminates the child and it exits -SIGINT rather than the handled 130/143.

Measured directly:

delay before signal exit code
0.1 s -2 (default disposition)
0.3 s 130 (handler ran)

This reads as a product bug but is purely test timing — the child had not finished importing yet.

Fix

Replaced the guessed delay with a readiness handshake:

  • The child prints __READY__ once its handlers are installed.
  • _run_in_subprocess(..., wait_for_ready=True) blocks until that marker appears before signalling.

_drain_until_ready reads from the child's stdout using select under a deadline, so a child that dies during startup cannot hang the test. The new wait_for_ready parameter defaults to False, leaving existing callers unaffected.

Verification

Green on both interpreters, which is the point for a portability fix:

  • Python 3.11 (CI's pinned version): 61 passed
  • Python 3.10 (the declared floor): 61 passed

tests/test_init_integration.py was run three consecutive times to confirm the race is actually gone rather than merely unobserved.

`requires-python = ">=3.10"`, but both tests below hard-coded 3.11-only
behaviour, so neither could run on a supported interpreter.

`test_repl_bridge.py` used `asyncio.timeout`, added in 3.11. Rewritten with
`asyncio.wait_for` around the polling coroutine, which is the portable
equivalent. The `refreshed` binding is carried out of the closure through a
dict holder rather than `nonlocal`, which cannot target a name that only
exists in the enclosing function's try-block.

`test_init_integration.py` signalled its child after a fixed 100/200/300 ms.
That races interpreter startup: when the signal lands before
`setup_graceful_shutdown()` installs its handler, the default disposition
terminates the child and it exits `-SIGINT` instead of the handled `130`,
which reads as a product bug but is purely test timing. Measured directly —
a 0.1 s delay yields `rc=-2`, 0.3 s yields `rc=130`.

Replaced the guessed delay with a readiness handshake: the child prints
`__READY__` once its handlers are installed and `_run_in_subprocess` waits
for that marker before signalling. `_drain_until_ready` reads with
`select` under a deadline, so a child that dies during startup cannot hang
the test. `wait_for_ready` defaults to False, leaving existing callers
untouched.

Verified green on both interpreters, 61 passed on 3.11 and on 3.10.
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Test Results

     5 files   1 021 suites   20m 44s ⏱️
16 104 tests 16 082 ✅ 22 💤 0 ❌
32 178 runs  32 107 ✅ 71 💤 0 ❌

Results for commit 643ed9e.

@agentforce314
agentforce314 merged commit 4f31547 into agentforce314:main Oct 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants