test(extensions): pin what a capped provider run returns and stops - #4924
Conversation
run_capped_process guards both the bytes a provider can push back and the process group it leaves behind, but the only two tests here covered failure paths. Nothing pinned the success contract, that the capture really stays bounded against an endless writer, or which stream overflowed. Signed-off-by: hahahahahayesyeseys <95999512+hahahahahayesyeseys@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
评审精确 head f5db9ad464398c8ca873afa92a6d7b277c87e537。扩展 provider 的 run_capped_process 是已上线的输出上限和子进程终止边界,但原有两项测试主要覆盖 overflow/timeout 时后代进程退出,没有固定正常响应会原样返回、精确上限可接受、非零 provider 退出码与 runtime failure 的区别,以及 stderr 溢出的归因。
改动思路
只在现有模块所属的 tests/extensions/test_process_runtime.py 增加五项真实子进程测试,断言调用者实际读到的 returncode、stdout 与 failure_kind,同时用持续输出的子进程检验上限和及时停止;不新增产品代码、测试框架或公开示例。
具体改动
新增 UTF-8/换行原样回显、provider exit(3)、恰好达到 byte limit、无界 stdout、stderr flood 五个场景。无界输出场景要求只保留 limit + 1 字节并在 20 秒内结束;stderr 场景要求 stderr_too_large 而不是 stdout failure。调用链检查确认 extension runtime 本来就分别处理 failure_kind 与退出码;作者提供的移除切片上限 mutation 反例与断言直接对应。同作者近期 #4900 守护的是 Lark manager context,非此边界的重复测试批次。
对主干的风险
只改一份测试文件(+95 行),不改变生产行为、状态或权限。当前 head 的本模块 7 项、相邻 extension runtime 63 项、Ruff 与 diff 检查通过;PR base 与当前主干在被测产品文件及原测试文件上的 blob 相同,旧基线未引入额外差异。Windows taskkill 分支和两路同时溢出的非确定性优先级仍未验证,不能从本次 POSIX 测试推断它们通过。
我的整体评价
APPROVE。 这是对真实 extension provider 资源边界有持续价值的薄回归,不是只复述现有行为的独立 smoke,也没有把一个场景拆成多 PR。现有测试文件是合适的维护位置;未见需要伴随的产品重构。
English verdict: APPROVE - exact head f5db9ad adds durable real-subprocess coverage for bounded output, exact bytes, exit semantics and stderr attribution; 70 focused/adjacent tests, Ruff and diff checks passed. Windows-only behavior remains untested.
|
Exact-head readiness update for Please update the branch against |
Goal And Delivered Outcome
loopx/extensions/process_runtime.py::run_capped_processis the guard that keeps anextension provider from (a) handing back an unbounded response and (b) leaving its process group running.
The file's only two tests both drove the overflow/timeout path and asserted that descendants died. Nothing
pinned what a caller actually reads back: that a normal response survives intact, that the capture really
stays bounded against a writer that never stops, that a provider exiting nonzero is not a runtime failure,
and that a stderr flood is not reported as a stdout flood.
(
destination.extend(chunk[:remaining])→destination.extend(chunk)) and the whole existing suite staysgreen; with this PR the same mutation is caught by exactly one test. Same for the other three mutations
below. Validation row
regression_paritycarries the table.Related to PR test(lark): pin the manager context window's bounds and order #4900 (same shape: pin a bounded surface that had no success-path test).
Scope And Continuation
changed. Deliberately not covered here, and named so the next person does not re-derive them: the
Windows
taskkilltree-kill branch and the unsupported-platform fallback (both already marked# pragma: no cover - exercised on Windows hosts),env/cwdpass-through, and whichfailure_kindwinswhen stdout and stderr overflow at the same time —
record_limitkeeps the first kind under a lock, and therace is not deterministic enough to pin honestly from a unit test.
file, which needs a runner rather than a test change.
Validation
f5db9ad46unitpassedpython -m pytest tests/extensions/test_process_runtime.py→ 7 passed in 2.6s, repeated 3× for flake;python -W error::SyntaxWarningclean. The five additions cover: response returned byte-for-byte (including a trailing newline and non-ASCII payload via stdin echo), providerexit(3)reported asreturncode=3withfailure_kind=None, a response of exactlyoutput_limit_bytesaccepted, a never-ending 64 KiB-per-round writer refused withlen(stdout) == limit + 1and under a wall-clock guard, and a 256 KiB stderr flood reported asstderr_too_largewithstdout == b"".regression_paritypassedloopx/extensions/process_runtime.py, each verified to have modified the file, then reverted: (A) swap the twooverflow_kindvalues → 3 failed (test_a_spewing_provider_on_stderr_is_its_own_failure,test_a_spewing_provider_is_stopped_instead_of_drained, pre-existingtest_output_overflow_terminates_provider_descendants); (B) extend with the whole chunk → 1 failed, the new spew test only; (D) report nonzero provider exit as a failure → 1 failed, the new declining-provider test only; (E) drop the+1in the remaining-bytes arithmetic → 3 failed; (C) remove thereturnafterrecord_limit→ 7 passed, not caught. (C) was then read through and is an equivalent mutant: the reader keeps draining a closed pipe until EOF, the captured length stays bounded and the main loop still kills on the same event, so no caller-visible outcome changes. Reported as uncaught rather than papered over.unit(regression around the change)passedpython -m pytest tests/extensions tests/canary→ 978 passed in 80s.staticpassedruff check tests/extensions/test_process_runtime.pyclean; the file wasruff format --checkclean before this change and stays clean (one of my own long lines was reformatted, no other file touched);git diff --checkclean;loopx check --scan-path tests/extensions/test_process_runtime.py→ public boundary scan clean.real_backend/integrationnot_applicablerun_capped_processagainst a real child process, so thepins land on the caller-observable tuple (
returncode,stdout,failure_kind) rather than on internals.Remaining untested paths are the two platform branches and the concurrent-overflow race named above; the
POSIX process-group behavior itself was already covered and is unchanged.
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist
or verifier output, internal links, or local machine paths (including
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).Why this branch is based on an older
mainThe branch root is
04ba65ac1and bothloopx/extensions/process_runtime.pyandtests/extensions/test_process_runtime.pyare byte-identical between that commit and currentmain(verified with
git rev-parse <rev>:<path>), so the diff carries only the new tests and the merge review readsthe same either way. Environment used for the runs above: CPython 3.12 with the project installed as
pip install -e ".[test]", Node 22.23.2 onPATHfor the sibling suites that shell out.