Repository navigation
[Bug]: test:byok main case has ~1.5x headroom (58.6s of a 90s budget over 42 serial CLI spawns) and false-negatives on any loaded machine #395
Description
Product or interface
Source build or repository tooling
中文摘要:
test/byok.test.mjs第一个用例BYOK runs without managed login and resumes its saved conversation的预算是{ timeout: 90000 }(第 28 行),但它在本机空载实测就要 58.6 秒——余量只有约 1.5×。这个用例串行拉起 42 次真实 CLI(provider add×6、provider list×2、7–10 组代理变量下的provider test×10、exec×12),总耗时是所有子步骤之和。机器一忙(并行构建、CI 共享 runner),就撞穿 90 秒被testTimeoutFailure判死,报出来的是产品失败,实际上链路是好的。建议把预算提到有意义的余量,或按平台缩放(仓库里smoke.test.mjs:17已有runtimeTimeoutMs这个惯例)。
Version
Source at main 1e136bf. Node.js v22.22.2, pnpm 10.x, macOS 15 arm64. Reproduced with pnpm test:byok / node --test test/byok.test.mjs.
Platform
macOS observed. The cause is a wall-clock budget, so any slower or more contended runner shows it — the same applies to Linux and to shared CI runners, and it is worst on Windows where process spawn is slower still.
Issue area
Build / CI / source distribution
Steps to reproduce
Baseline, on an otherwise idle machine:
node --test test/byok.test.mjs
# ok 1 - BYOK runs without managed login and resumes its saved conversation
# duration_ms: 58580.762417Then put load on the machine — a pnpm build or any parallel test run is enough:
pnpm build & # or any second CPU/IO-heavy job
node --test test/byok.test.mjsExpected and actual behavior
Expected: the test reports the BYOK transport result it is written to check. The file's own comment at test/byok.test.mjs:25 says exactly that: "This fixture validates BYOK transport and real Runtime persistence, not model quality."
Actual: it dies on the clock, and no assertion about the transport was ever evaluated:
not ok 1 - BYOK runs without managed login and resumes its saved conversation
---
duration_ms: 90705.44475
type: 'test'
failureType: 'testTimeoutFailure'
error: 'test timed out after 90000ms'
code: 'ERR_TEST_FAILURE'
...
# tests 3
# pass 2
# fail 0
# cancelled 1
Re-running it with nothing else on the machine gives 3/3 passing. So the failure carries no information about the product — it is a false negative whose only symptom is "this machine is busy."
Analysis
test/byok.test.mjs:26-28 gives the test a 90 s budget:
test(
"BYOK runs without managed login and resumes its saved conversation",
{ timeout: 90000 },That single test performs 42 sequential real CLI invocations — provider add ×6, provider list ×2, provider test ×10 (the ambient-proxy matrix, one per proxy variable present in the environment), plus 12 exec runs. Every one is a fresh Node process that boots the whole runtime. The duration is the sum of all of them, and the 90 s cap is a global budget over the whole set, not a per-step allowance — measured from the passing run's own diagnostics, that is ≈1.4 s per invocation.
Measured headroom: 58.6 s used of 90 s, ≈1.5×. A transient 1.5× slowdown is ordinary — a parallel build, a laptop on power, a shared CI runner, antivirus on Windows.
Two things make this more likely to bite than a plain tight timeout:
- The file already carries a workaround for a real test-runner defect:
test/byok.test.mjs:283-285keeps the shared-fixture phase sequential becauset.test()returnsundefinedon Node 24.0–24.2 (nodejs/node#58227). The phase that had to be un-parallelised is exactly the one with the proxy-matrix runs, so the cost is unavoidably serial. - The two sibling tests use
{ timeout: 45000 }(:602,:608), so 90 s is the outlier in this file rather than a considered ceiling.
Suggested fix
Either is fine; the second is the more durable:
- Raise the budget so a normal transient slowdown cannot trip it — e.g. 180 s, or scale it like
smoke.test.mjs:17does:const runtimeTimeoutMs = process.platform === "win32" ? 30000 : 15000is the existing convention in this repository for a machine-speed-dependent budget. - Or cut the serial cost: the proxy matrix is the bulk of it, and the sub-checks are independent of each other. The blocker is only that they share one fixture and must run before
rejectConnectionassertions. Splitting the proxy matrix into its own test with its own fixture would let it run in parallel and would also make a future timeout localise to one phase instead of failing the whole 58 s block.
Not asking for a specific approach — the point is that a 1.5× margin on a 42-process serial test is below what the suite can rely on.
Redacted error summary
# idle: # tests 3 | # pass 3 | # fail 0 | # cancelled 0 (first test 58.6s)
# under load: # tests 3 | # pass 2 | # fail 0 | # cancelled 1 (first test >90s)
# failureType: 'testTimeoutFailure' — no assertion evaluatedSearched the tracker first: #171 and #175 are closed and cover BYOK proxy-variable handling and a provider-test proxy timeout, not a test-harness wall-clock budget. #322 is a Windows verification report with different root causes.
Activity
hetaoBackend commentedon Sep 29, 2026
@minmax — verified again, and this one is also already fixed on
main. It no longer reproduces, so I'm closing it as already-resolved. The reasoning in your report is spot-on and the fix follows it exactly.Verification (against current
main)test/byok.test.mjsno longer uses the flat 90 s budget. The serial case is now budgeted the way you suggested — a base allowance plus a Windows multiplier for slower process spawn:That is 180 s on POSIX and 360 s on Windows (vs the old flat
90_000), i.e. ~3× your 58.6 s idle baseline on POSIX and ample headroom on Windows where process spawn is slower. The comment above it states the rationale in the same terms you used, and explicitly names thesmoke.test.mjsconvention you pointed at:And
smoke.test.mjs:17does carry that convention (runtimeTimeoutMs = process.platform === "win32" ? 30000 : 15000), which the byok case now mirrors.Classification
bug(as filed)main→ closing. The change lives intest/byok.test.mjs(byokSerialTimeoutMs); no.out-of-scope/entry is needed because this was implemented, not rejected.Your core point — that the symptom "this machine is busy" is a false negative carrying no information about the product, because the file's own comment says the fixture validates BYOK transport and not model quality — is exactly the right framing, and the fix removes that failure mode.
No credentials or sensitive data echoed.
test: pin the srt-macos probe PATH and scale the BYOK serial timeout …
merge: upstream 5f3240b (scrollback natif, erreurs terminal, volume W…
merge: upstream e77c763 (sync 0.5.9/0.5.10, scrollback baseline rever…
merge: upstream 564e916 (MiniMax-AI#423 side-session Esc and run time…
merge upstream 56221c1 (MiniMax-AI#429 rebuild différé après shrink, M…
merge: upstream a5b56b7 (MiniMax-AI#444 system prompt, MiniMax-AI#443 …