-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(ci): reduce Windows CLI test contention #13077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,9 +29,9 @@ if (argv.includes("--help") || argv.includes("-h")) { | |
| "", | ||
| "Options:", | ||
| " --ci Enable JUnit XML output to .artifacts/unit/junit.xml", | ||
| " --concurrency <N> Max parallel processes (default: min(4, CPU count))", | ||
| " --concurrency <N> Max parallel processes (default: min(4, CPU count), env: KILO_TEST_CONCURRENCY)", | ||
| " --timeout <ms> Per-test timeout passed to bun test (default: 60000)", | ||
| " --file-timeout <ms> Per-file process timeout (default: 300000)", | ||
| " --file-timeout <ms> Per-file process timeout (default: 300000, env: KILO_TEST_FILE_TIMEOUT)", | ||
| " --retries <N> Extra attempts for failing files (default: 1)", | ||
| " --profile <name> Run a curated test profile (env: KILO_TEST_PROFILE)", | ||
| " --shard <N/M> Run one balanced file shard (env: KILO_TEST_SHARD)", | ||
|
|
@@ -73,9 +73,39 @@ const dots = !verbose && (ci || argv.includes("--dots")) | |
| // Cap concurrency at 4 even on bigger runners: the bottleneck is shared | ||
| // resources (ports, global filesystem like ~/.local/share/kilo), not CPU. | ||
| // Eight parallel processes was triggering port/FS races, not going faster. | ||
| const concurrency = opt("concurrency", Math.min(4, os.cpus().length)) | ||
| // kilocode_change start - allow CI to lower concurrency via env. On the 4-vCPU | ||
| // Windows runner, the default (min(4, cpus)=4) oversubscribes: 4 heavy real-server | ||
| // test files share 4 vCPUs (~1 each) and blow their per-test timeouts. | ||
| // `KILO_TEST_CONCURRENCY` lets the workflow throttle Windows without affecting the | ||
| // local default. An explicit `--concurrency` flag wins. | ||
| const concurrencyEnv = (() => { | ||
| const raw = process.env.KILO_TEST_CONCURRENCY?.trim() | ||
| if (!raw) return undefined | ||
| const value = Number(raw) | ||
| if (!Number.isSafeInteger(value) || value < 1) { | ||
| console.error(`Invalid KILO_TEST_CONCURRENCY "${raw}"; expected a positive integer`) | ||
| process.exit(2) | ||
| } | ||
| return value | ||
| })() | ||
| const concurrency = opt("concurrency", concurrencyEnv ?? Math.min(4, os.cpus().length)) | ||
| // kilocode_change end | ||
| const timeout = opt("timeout", 60000) | ||
| const deadline = opt("file-timeout", 300000) | ||
| // kilocode_change start - allow CI to raise the per-file kill deadline via env. On Windows, | ||
| // heavy real-server files (e.g. config-overlay) legitimately run ~270s serially, only ~30s | ||
| // under the 300s default; raising it there prevents a slow-but-healthy run from being killed. | ||
| const fileTimeoutEnv = (() => { | ||
| const raw = process.env.KILO_TEST_FILE_TIMEOUT?.trim() | ||
| if (!raw) return undefined | ||
| const value = Number(raw) | ||
| if (!Number.isSafeInteger(value) || value < 1) { | ||
| console.error(`Invalid KILO_TEST_FILE_TIMEOUT "${raw}"; expected a positive integer (ms)`) | ||
| process.exit(2) | ||
| } | ||
| return value | ||
| })() | ||
| const deadline = opt("file-timeout", fileTimeoutEnv ?? 300000) | ||
| // kilocode_change end | ||
| const retries = opt("retries", 1) | ||
| const flag = text("profile") | ||
| const env = process.env.KILO_TEST_PROFILE?.trim() || undefined | ||
|
|
@@ -156,7 +186,28 @@ if (shard && shard.total > candidates.length) { | |
| console.error(`Test shard count ${shard.total} exceeds selected file count ${candidates.length}`) | ||
| process.exit(2) | ||
| } | ||
| const weight = (file: string) => Bun.file(path.join(root, "test", file)).size | ||
| // kilocode_change start - shard by estimated DURATION, not file size. File size is a poor | ||
| // proxy: run-process.test.ts is ~7 KB but ~230s, while config-overlay is the single slowest | ||
| // file — under size-weighting both landed in the same shard, stacking the two heaviest files. | ||
| // DURATION_HINTS are max observed per-file durations (ms) from real Windows CI runs; the LPT | ||
| // splitter places the highest-weight files first, so hinted heavy files get spread across | ||
| // distinct shards. Unhinted files fall back to size (a fine proxy among the fast majority); | ||
| // hint values (tens of thousands of ms) dominate byte sizes, so heavy files always sort first. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: The claim that "hint values ... dominate byte sizes, so heavy files always sort first" is only true for the largest hints. Several unhinted files outweigh the smaller hints by byte size today: Reply with |
||
| // Refresh these from observed CI durations when the suite changes materially. | ||
| const DURATION_HINTS: Record<string, number> = { | ||
| "kilocode/server/config-overlay.test.ts": 270_000, | ||
| "cli/run/run-process.test.ts": 233_000, | ||
| "snapshot/snapshot.test.ts": 165_000, | ||
| "session/prompt.test.ts": 128_000, | ||
| "tool/shell.test.ts": 95_000, | ||
| "kilocode/background-process.test.ts": 94_000, | ||
| "provider/provider.test.ts": 90_000, | ||
| "kilocode/indexing-startup.test.ts": 88_000, | ||
| "kilocode/daemon.test.ts": 65_000, | ||
| "tool/task.test.ts": 64_000, | ||
| } | ||
| const weight = (file: string) => DURATION_HINTS[file] ?? Bun.file(path.join(root, "test", file)).size | ||
| // kilocode_change end | ||
| const files = shard ? TestShard.split(candidates, weight, shard.total)[shard.index - 1] : candidates | ||
|
|
||
| if (files.length === 0) { | ||
|
|
@@ -189,9 +240,7 @@ const xmldir = ci ? path.join(os.tmpdir(), `opencode-junit-${process.pid}`) : "" | |
| if (ci) await fs.mkdir(xmldir, { recursive: true }) | ||
| // kilocode_change start | ||
| const supplied = process.env[TestCli.ENV] | ||
| const built = supplied | ||
| ? { binary: supplied, dir: undefined } | ||
| : { binary: await TestCli.build(root), dir: undefined } | ||
| const built = supplied ? { binary: supplied, dir: undefined } : { binary: await TestCli.build(root), dir: undefined } | ||
|
|
||
| async function cleanBinary() { | ||
| if (!built.dir) return | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: Comment says
run-process.test.tsis "~7 KB", but the file is ~19 KB (19,664 bytes) in the current tree.The rhetorical point (small file, ~230s runtime) still holds, but since the block asks future maintainers to refresh these from observed data, it's worth keeping the numbers accurate.
Reply with
@kilocode-bot fix itto have Kilo Code address this issue.