test: cut slow suite runtimes and isolate experimental flag from host env#1595
test: cut slow suite runtimes and isolate experimental flag from host env#1595kermanx wants to merge 6 commits into
Conversation
… env - minidb compaction-race: drop the snapshot-phase stress from 150k to 10k records; writeSnapshot yields every 2000 records, so 10k still spans five yields and keeps the write-vs-compact race meaningful (~50s -> ~3s on CI) - image-compress / read-media (agent-core + agent-core-v2): shrink noise and solid-color fixtures to the smallest sizes that still exercise the ladder and downsample paths (60s/38s -> ~15s/11s, 23s/9s -> ~4s/3s) - goal-prompt, update/preflight: stub KIMI_CODE_EXPERIMENTAL_FLAG off in beforeEach so a host env leak cannot route tests down the unmocked v2 path (they hung until the 5s testTimeout) or bypass rollout batch holds - minidb compaction: give the concurrent-write recovery test a 15s timeout so full-suite CPU contention cannot trip the 5s default
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fc26c0499
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| it('reads a region crop at native resolution', async () => { | ||
| const big = await bigPng(2600, 2600); | ||
| const big = await bigPng(600, 600); |
There was a problem hiding this comment.
When this fixture is only 600×600, the region-read test no longer covers the documented follow-up flow for large images: a default read is downsampled at the 2000px edge cap, then a region read must still crop from the original coordinate space. A regression that accidentally crops from an already-downsampled copy would still pass here because this image never needs downsampling; keep the source just over the cap while requesting a small crop.
Useful? React with 👍 / 👎.
| it('reads image regions at native resolution', async () => { | ||
| const big = Buffer.from( | ||
| await new Jimp({ width: 2600, height: 2600, color: 0x3366ccff }).getBuffer('image/png'), | ||
| await new Jimp({ width: 600, height: 600, color: 0x3366ccff }).getBuffer('image/png'), |
There was a problem hiding this comment.
At 600×600 this test no longer exercises the large-image readback contract: after the default read downscales an image above the 2000px cap, a region read must still use original-image coordinates and full-fidelity source bytes. If the implementation regressed to crop from the downsampled delivery, this fixture would not catch it because no downsample occurs; keep the source just over the cap while using the same small crop.
Useful? React with 👍 / 👎.
The 600x600 fixtures could no longer distinguish cropping from the original coordinate space (the documented follow-up to a downsampled default read) from cropping the downsampled delivery: below the 2000px edge cap nothing is ever downsampled, so a regression in that path would pass silently. Same for the out-of-bounds error asserting the original size. Use 2100x2100 (just over the cap, still cheap solid PNGs) and note why the size must not shrink again.
…watch AC #2 - minidb: fuzz-model 10 seeds x 500 steps -> 6 x 250; compaction-race heavy-writes 20k/5k -> 10k/2k (still spans five writeSnapshot yields and a >64KiB WAL tail); crash-recovery runs 8/4 -> 5/3 (cost is subprocess spawn, not rounds); compaction concurrent 10k/2k -> 5k/1k - replace real-second waits with injected short timeouts or fake timers: hooks runner timeout 1s -> 0.05s (both packages), lifecycle-hooks printWaitCeilingS 1s -> 0.05s, task-tools block-timeout driven by fake timers, mcp connection-manager slow-startup child exits on stdin end, kap-server 429 test injects a token-only auth service (12x bcrypt ~250ms was the real cost, not the rate limiter) - fs-watch AC #2: the burst test required chokidar to deliver >500 events inside one 200ms window, which trickles under CI load. Add an fsWatcherOptions seam to ServerStartOptions (default behavior unchanged) and run the test with a 500ms window / 100-event threshold, which still proves truncation but tolerates 10x slower delivery
|
Follow-up commits:
|
fileParallelism: false serialized all 52 kap-server test files (~82s wall). Nothing actually requires it: servers bind port 0, homes and locks live in mkdtemp dirs. Two real blockers surfaced and are fixed: - v2 SessionFsWatchService hardcoded the 200ms/500 burst-truncation window, so the fs-watch e2e flaked whenever chokidar trickled events under CPU contention (same class of flake fixed earlier for the v1 server). Make the window env-overridable as a test seam (defaults unchanged when unset) and run the burst test at 500ms/100, which still proves truncation at 10x lower delivery rates. - dispatchSessionEvent could reject with 'InstantiationService has been disposed' when an event raced session teardown; the fire-and-forget caller made it an unhandled rejection (present in serial mode too). Extend the existing disposed-scope guard from the attach* block to the whole ensureState call. Parallel mode verified 5/5 runs green, 21.7-30.2s wall vs 82.3s serial.
|
|
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
The CI
testjob went from ~5.5–6 min to ~9 min 43 s right after #1441 landed (agent-core-v2 + kap-server added 328 test files, +44%). Profiling a full local run (vitest JSON reporter) showed the wall-clock is dominated by a handful of badly-scaled tests rather than by the file count itself:minidb/test/e2e/compaction-race.test.tswrote 150k records (~75 MB) just to build a "large enough" snapshot — 50.5 s for a single test on CI.image-compress.test.ts(agent-core 60 s, agent-core-v2 38 s) andread-media.test.ts(23 s / 9 s) ran jimp encode ladders over 1200x1200 pure-noise fixtures — the entropy upper bound — even where assertions only check byte budgets.goal-prompt.test.tsintegration tests each took exactly 5.00 s locally: a hostKIMI_CODE_EXPERIMENTAL_FLAG=1leaks into the vitest process, routesrunPromptdown the unmocked v2 path, and the test hangs until vitest's 5 stestTimeoutkills it (i.e. they were silently red in that environment). The same leak makesupdate/preflight.test.tsrollout-hold tests fail, becauserollout.tsskips batch gating when the flag is set.What changed
Measured before/after with the vitest JSON reporter: total test durations 655 s -> 394 s (-40%), full-run wall-clock 307 s -> 227 s (-26%).
writeSnapshotyields every 2000 records, so 10k still spans five event-loop yields and keeps the write-vs-compact race meaningful; timeout 60 s -> 15 s.vi.stubEnv('KIMI_CODE_EXPERIMENTAL_FLAG', '')inbeforeEach, following the existing pattern inrun-prompt.test.ts; tests that exercise the flag still opt in explicitly.Also lightens load for everything running in parallel with these files (~100 s of indirect savings across server/kap-server e2e files).
Checklist
gen-changesetsskill, or this PR needs no changeset. (Test-only changes — no changeset.)gen-docsskill, or this PR needs no doc update. (No user-facing behavior change.)