opensource/kilocode
3
0
Fork 0
mirror of https://github.com/Kilo-Org/kilocode.git synced 2026-08-28 11:05:31 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(test): second review round — measurement integrity and hang budget

Browse Source 
--update-durations now disables batching so every file is measured
per-file (batched members' entries otherwise rot), and only passing
runs record durations (a timed-out file would write the kill deadline
and permanently skew LPT).

Timed-out work items are no longer retried: the deadline already
proves a hang, and a second 600s attempt could push a shard past the
45-minute job budget.

A flaky batch now annotates the member files that failed the first
attempt instead of the pseudo-file. Defender exclusions apply per
path instead of aborting on the first policy-locked one.
This commit is contained in:
Yury Zialionka
2026-08-13 21:24:42 -06:00
parent f1bb992ff6
commit 031ae4a7a3
2 changed files with 89 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/test.yml
+9 -6
View File
@@ -105,14 +105,17 @@ jobs:
continue-on-error: true
shell: pwsh
run: |
try {
foreach ($dir in @($env:GITHUB_WORKSPACE, $env:RUNNER_TEMP, $env:TEMP, "$env:USERPROFILE\.bun")) {
if ($dir) { Add-MpPreference -ExclusionPath $dir -ErrorAction Stop }
$applied = 0
foreach ($dir in @($env:GITHUB_WORKSPACE, $env:RUNNER_TEMP, $env:TEMP, "$env:USERPROFILE\.bun")) {
if (-not $dir) { continue }
try {
Add-MpPreference -ExclusionPath $dir -ErrorAction Stop
$applied++
} catch {
Write-Host "Defender exclusion failed for ${dir}: $($_.Exception.Message)"
}
Write-Host "Defender path exclusions applied."
} catch {
Write-Host "Defender exclusions not applied: $($_.Exception.Message)"
}
Write-Host "Defender path exclusions applied: $applied"
# kilocode_change end
- name: Checkout repository
packages/opencode/script/test-runner.ts
+80 -51
View File
@@ -235,8 +235,9 @@ const weight = (file: string) => {
// cleanly in a single pass (empirically: all pass in one process). This trades ~1s of process
// boot + TS compile per file for a single boot, the dominant cost for these small fast files.
// The batch enters shard splitting as one pseudo-file with a duration-scale weight, so LPT
// places it like any other heavy file and exactly one shard executes it. Extend the list only
// with files proven to pass in a shared process (run: bun test <dirs> locally).
// places it like any other heavy file and exactly one shard executes it. Directories are
// batched by default; files that cannot share a process are excluded by BATCH_EXCLUDES or
// demoted automatically by the unsafe-marker scan below.
// Deliberately excludes directories whose runtime is real I/O work (git/, server/, session/,
// project/, provider/, ...): those parallelize well across the per-file worker pool, while a
// batch runs its members sequentially. The tier is for files where boot cost dominates.
@@ -245,55 +246,40 @@ const weight = (file: string) => {
// Batch weights are computed from member durations, never hand-maintained.
const FAST_TIERS: Record<string, string[]> = {
"fast-tier-core": [
"account/",
"config/",
"effect/",
"event-manifest.test.ts",
"format/",
"image/",
"installation/",
"patch/",
"provider/model-status.test.ts",
"provider/transform.test.ts",
"question/",
"share/",
"suggestion/",
"util/",
"account/",
"config/",
"effect/",
"event-manifest.test.ts",
"format/",
"image/",
"installation/",
"patch/",
"provider/model-status.test.ts",
"provider/transform.test.ts",
"question/",
"share/",
"suggestion/",
"util/",
],
"fast-tier-kilocode": [
"kilocode/config/",
"kilocode/memory/",
// kilocode/permission/ stays per-file: permission-origins asserts the merged config has
// no global-scope keys, so it cannot share an XDG root with tests that write global config.
"kilocode/presence/",
"kilocode/project/",
"kilocode/provider/",
"kilocode/skills/",
"kilocode/storage/",
"kilocode/suggestion/",
"kilocode/tui/",
"kilocode/util/",
"kilocode/config/",
"kilocode/memory/",
// kilocode/permission/ stays per-file: permission-origins asserts the merged config has
// no global-scope keys, so it cannot share an XDG root with tests that write global config.
"kilocode/presence/",
"kilocode/project/",
"kilocode/provider/",
"kilocode/skills/",
"kilocode/storage/",
"kilocode/suggestion/",
"kilocode/tui/",
"kilocode/util/",
],
"fast-tier-kilocode-sessions": ["kilocode/session-export/", "kilocode/session/", "kilocode/sessions/"],
},
"fast-tier-kilocode-tools": ["kilocode/anaconda-desktop/", "kilocode/cloud/", "kilocode/tool/"],
},
"fast-tier-cli": ["cli/"],
},
"fast-tier-misc": [
"acp/",
"auth/",
"bun/",
"filesystem/",
"ide/",
"lsp/",
"mcp/",
"plugin/",
"storage/",
"v2/",
],
"fast-tier-misc": ["acp/", "auth/", "bun/", "filesystem/", "ide/", "lsp/", "mcp/", "plugin/", "storage/", "v2/"],
"fast-tier-tool": ["tool/"],
},
}
// Files that must run alone, never in a shared batch, for reasons a source scan cannot
// detect: real subprocesses, fs watchers, and wall-clock stall simulations are all
@@ -313,8 +299,7 @@ const BATCH_EXCLUDES = [
"tool/task.test.ts",
]
// Entry semantics shared by FAST_TIERS and BATCH_EXCLUDES: ".ts" = exact file, else prefix.
const matchesEntry = (file: string, entry: string) =>
entry.endsWith(".ts") ? file === entry : file.startsWith(entry)
const matchesEntry = (file: string, entry: string) => (entry.endsWith(".ts") ? file === entry : file.startsWith(entry))
const isBatchExcluded = (file: string) => BATCH_EXCLUDES.some((entry) => matchesEntry(file, entry))
// 8 batches (~24 files each) keep the heaviest single work item small enough for
// LPT to pack shards evenly; fewer, bigger batches set a floor under the slowest shard.
@@ -334,7 +319,9 @@ const tierOf = (file: string) => {
return kilocodeRootTier(file)
}
const batches = new Map<string, string[]>()
if (patterns.length === 0 && !profile) {
// --update-durations disables batching so every file runs (and is measured) individually;
// batched members otherwise never appear in results and their entries would rot.
if (patterns.length === 0 && !profile && !updateDurations) {
for (const file of candidates) {
const tier = tierOf(file)
if (!tier) continue
@@ -348,6 +335,8 @@ if (patterns.length === 0 && !profile) {
// bun's mock.module is process-wide and permanent, AppRuntime.dispose() kills the shared
// runtime, and global-fetch spies observe batch-mates' traffic. A developer adding such
// a test anywhere keeps a green suite; the file just does not share a process.
// Limitation: only the test file's own source is scanned — a marker hidden in an
// imported helper is invisible; batch-only failures that vanish per-file point there.
// Markers: bun module mocks, disposal of the shared app runtime, spies on true globals,
// and module-scope env writes (column 0 — env set inside a test body is indented and
// typically restored; a load-time write leaks into every batch-mate's import snapshot).
@@ -374,7 +363,11 @@ if (patterns.length === 0 && !profile) {
`Fast tier: ${demoted.size} file(s) use process-wide mocks/disposal/spies and run per-file instead:\n` +
[...demoted].map((file) => `- ${file}`).join("\n"),
)
for (const [name, members] of batches) batches.set(name, members.filter((member) => !demoted.has(member)))
for (const [name, members] of batches)
batches.set(
name,
members.filter((member) => !demoted.has(member)),
)
}
for (const [name, members] of batches) if (members.length < 2) batches.delete(name)
}
@@ -688,6 +681,26 @@ const results: Result[] = []
// after everything else finished. Heaviest-first keeps the tail short. kilocode_change
const queue = TestShard.order(files, shardWeight)
// kilocode_change start - a flaky batch names only its pseudo-file; pull the members that
// failed on the earlier attempt out of that attempt's output so annotations can attribute
// the flake to real files. bun prints a "test/<file>:" heading before each file's tests.
const flakyMembers = new Map<string, string[]>()
const failedMembersOf = (stdout: string, members: string[]) => {
const failed = new Set<string>()
let current: string | undefined
for (const line of stdout.split("\n")) {
const heading = line.match(/^(?:.*[\\/])?test[\\/](.+\.test\.tsx?):\s*$/)
if (heading) {
const name = heading[1].replaceAll("\\", "/")
current = members.includes(name) ? name : undefined
continue
}
if (current && /^\(fail\)/.test(line.trim())) failed.add(current)
}
return [...failed]
}
// kilocode_change end
const workers = Array.from({ length: Math.min(concurrency, files.length) }, async () => {
while (queue.length > 0 && !stopped.value) {
const file = queue.shift()!
@@ -696,7 +709,12 @@ const workers = Array.from({ length: Math.min(concurrency, files.length) }, asyn
// attempt; contention-based flakes (port races, slow FS, slow spawn) recover.
// Preserve the last attempt's stdout/stderr/duration so a truly broken file
// still shows a useful diagnostic.
while (!result.passed && result.attempts <= retries && !stopped.value) {
// A timed-out item already burned the full kill deadline; retrying doubles a
// pathological hang (2x600s) and can push a shard past the 45-minute job budget.
// Contention flakes fail fast and still get their retry.
while (!result.passed && !result.timedout && result.attempts <= retries && !stopped.value) {
const members = batches.get(file) // kilocode_change
if (members) flakyMembers.set(file, failedMembersOf(result.stdout, members)) // kilocode_change
const retry = await run(file)
retry.attempts = result.attempts + 1
result = retry
@@ -765,7 +783,16 @@ if (flaky.length > 0) {
// the bottom of the job page and in the workflow summary email.
if (process.env.GITHUB_ACTIONS === "true") {
for (const r of sorted) {
if (batches.has(r.file)) continue // kilocode_change - pseudo-file, no source path to annotate
// kilocode_change start - annotate a flaky batch's failing members, not the pseudo-file
if (batches.has(r.file)) {
for (const member of flakyMembers.get(r.file) ?? []) {
console.log(
`::warning file=packages/opencode/test/${member},title=Flaky test file (in ${r.file})::passed on attempt ${r.attempts} of ${retries + 1}`,
)
}
continue
}
// kilocode_change end
const repo = `packages/opencode/test/${r.file}`
console.log(`::warning file=${repo},title=Flaky test file::passed on attempt ${r.attempts} of ${retries + 1}`)
}
@@ -810,7 +837,9 @@ if (updateDurations) {
// durations still feed the computed batch weights — replacing would erase them.
const fresh = Object.fromEntries(
results
.filter((result) => !batches.has(result.file))
// Only passing runs measure real duration: a timed-out file would record the kill
// deadline (~600s) and a failing file records contention noise, skewing LPT.
.filter((result) => result.passed && !batches.has(result.file))
.map((result) => [result.file, Math.round(result.duration)] as const),
)
const files = new Set(candidates)