Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 22 additions & 38 deletions packages/agents/run-bun-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ import {
writeFileSync,
} from 'node:fs';
import { join, relative } from 'node:path';
import { availableParallelism, tmpdir } from 'node:os';
import { tmpdir } from 'node:os';
import {
DEFAULT_PER_FILE_TIMEOUT_MS,
DEFAULT_PER_TEST_TIMEOUT_MS,
MAX_TEST_CONCURRENCY,
resolveTestConcurrency,
} from '../../scripts/lib/bun-test-policy.js';

/**
* Every path this runner touches — discovery, the child's working directory
Expand All @@ -51,46 +57,24 @@ const JUNIT_PATH = join(WORKSPACE_ROOT, 'junit.xml');

const TEST_ROOTS = ['src'] as const;

/** Upper bound on file concurrency, regardless of how many cores are present. */
const MAX_CONCURRENCY = 4;

/** Lower bound: small CI runners need one free core for each Bun child. */
const MIN_CONCURRENCY = 1;
/**
* Upper bound on file concurrency, regardless of how many cores are present.
*
* Kept as a named constant because the JUnit summary reports it.
*/
const MAX_CONCURRENCY = MAX_TEST_CONCURRENCY;

/**
* Number of test files executed at once.
*
* Deliberately half the core count, clamped to [1, 4]. Each file is a fresh
* `bun test` process that re-executes the whole agents module graph, and many
* suites under `src/api/__tests__/` additionally build a real Agent (tool
* registry, provider bootstrap, settings) per test. Saturating every core with
* that work starves individual tests past the 30s budget, which surfaces as a
* different file failing on each run rather than as an honestly slow run.
* Leaving headroom matters most on small CI runners, where the core count is
* roughly the concurrency an unclamped default would pick. macOS CI runs one
* file at a time because its virtual cores repeatedly starve one process past
* the test budget even at half-concurrency.
*
* `LLXPRT_AGENTS_TEST_CONCURRENCY` overrides this.
* The half-the-cores policy this workspace arrived at by measurement now lives
* in scripts/lib/bun-test-policy.ts, so the other runners inherit it instead of
* rediscovering it (issue #3139). `LLXPRT_AGENTS_TEST_CONCURRENCY` overrides.
*/
function resolveConcurrency(): number {
const override = process.env.LLXPRT_AGENTS_TEST_CONCURRENCY;
if (override !== undefined) {
if (!/^[1-9][0-9]*$/.test(override.trim())) {
throw new Error(
`LLXPRT_AGENTS_TEST_CONCURRENCY must be a positive integer, got: ${override}`,
);
}
return Number.parseInt(override.trim(), 10);
}
if (process.platform === 'darwin' && process.env.CI === 'true') {
return MIN_CONCURRENCY;
}
const half = Math.floor(availableParallelism() / 2);
return Math.min(MAX_CONCURRENCY, Math.max(MIN_CONCURRENCY, half));
}

const CONCURRENCY = resolveConcurrency();
const CONCURRENCY = resolveTestConcurrency({
envVar: 'LLXPRT_AGENTS_TEST_CONCURRENCY',
maxConcurrency: MAX_CONCURRENCY,
});

/**
* Per-test timeout, mirroring the `testTimeout: 30000` this workspace ran under
Expand All @@ -112,7 +96,7 @@ const CONCURRENCY = resolveConcurrency();
* by PER_FILE_TIMEOUT_MS below, which is what should happen - a raised
* per-test bound must not turn a hang into a longer hang.
*/
const PER_TEST_TIMEOUT_MS = 180_000;
const PER_TEST_TIMEOUT_MS = DEFAULT_PER_TEST_TIMEOUT_MS;

/**
* Per-file wall-clock budget. The slowest agents files (streaming chat-session
Expand All @@ -123,7 +107,7 @@ const PER_TEST_TIMEOUT_MS = 180_000;
// subagentOrchestrator-loadBalancer was measured at 99.4s under load, leaving
// almost no headroom under the previous 120s cap. This remains the backstop
// for a suite that genuinely hangs.
const PER_FILE_TIMEOUT_MS = 300_000;
const PER_FILE_TIMEOUT_MS = DEFAULT_PER_FILE_TIMEOUT_MS;

/**
* Directories that are pruned during discovery.
Expand Down
67 changes: 46 additions & 21 deletions packages/auth/run-bun-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
import { spawn } from 'node:child_process';
import { readdirSync, statSync, writeFileSync } from 'node:fs';
import { join, relative } from 'node:path';
import { availableParallelism } from 'node:os';
import {
DEFAULT_PER_FILE_TIMEOUT_MS,
DEFAULT_PER_TEST_TIMEOUT_MS,
resolveTestConcurrency,
} from '../../scripts/lib/bun-test-policy.js';

/**
* Every path this runner touches — discovery, the child's working directory,
Expand All @@ -25,8 +29,10 @@ import { availableParallelism } from 'node:os';
const WORKSPACE_ROOT = import.meta.dir;
const PRELOAD = join(WORKSPACE_ROOT, 'bun-preload.ts');
const JUNIT_PATH = join(WORKSPACE_ROOT, 'junit.xml');
const CONCURRENCY = Math.min(8, availableParallelism());
const PER_FILE_TIMEOUT_MS = 60_000;
const CONCURRENCY = resolveTestConcurrency({
envVar: 'LLXPRT_AUTH_TEST_CONCURRENCY',
});
const PER_FILE_TIMEOUT_MS = DEFAULT_PER_FILE_TIMEOUT_MS;

const TEST_ROOTS = ['src'] as const;

Expand Down Expand Up @@ -88,25 +94,35 @@ export function runTestFile(file: string): Promise<TestResult> {
let resolved = false;
const child = spawn(
process.execPath,
['test', '--preload', PRELOAD, file],
[
'test',
// Bun 1.3.14 ignores a `[test] timeout` key in bunfig.toml and falls
// back to 5s, so this flag is the only thing that sets the budget.
'--timeout',
String(DEFAULT_PER_TEST_TIMEOUT_MS),
'--preload',
PRELOAD,
file,
],
{
cwd: WORKSPACE_ROOT,
stdio: 'inherit',
env: process.env,
},
);

// Set by the timer so the exit handler can report the real reason. The
// result is only produced once the process has actually been reaped:
// `kill()` only sends a signal and returns, so resolving from the timer
// would free the worker slot while the killed process was still running,
// letting the pool exceed its concurrency cap exactly when the machine is
// already struggling. This mattered less under the old fixed batches; with
// a worker pool the freed slot is filled immediately.
let killedByTimeout = false;

const timer = setTimeout(() => {
if (resolved) return;
resolved = true;
killedByTimeout = true;
child.kill('SIGKILL');
resolve({
file,
passed: false,
exitCode: null,
timedOut: true,
signal: null,
});
}, PER_FILE_TIMEOUT_MS);

child.on('exit', (code, signal) => {
Expand All @@ -115,10 +131,10 @@ export function runTestFile(file: string): Promise<TestResult> {
clearTimeout(timer);
resolve({
file,
passed: code === 0,
exitCode: code,
timedOut: false,
signal: signal ?? null,
passed: !killedByTimeout && code === 0,
exitCode: killedByTimeout ? null : code,
timedOut: killedByTimeout,
signal: killedByTimeout ? null : (signal ?? null),
});
});

Expand Down Expand Up @@ -198,14 +214,23 @@ async function main(): Promise<void> {
`Running ${testFiles.length} test files with concurrency ${CONCURRENCY}`,
);

// A worker pool rather than fixed batches: a batch only advances when its
// slowest file finishes, so one slow file leaves CONCURRENCY - 1 slots idle
// exactly when the machine has work queued behind it.
const results: TestResult[] = [];
let nextIndex = 0;

for (let i = 0; i < testFiles.length; i += CONCURRENCY) {
const batch = testFiles.slice(i, i + CONCURRENCY);
const batchResults = await Promise.all(batch.map(runTestFile));
results.push(...batchResults);
async function worker(): Promise<void> {
while (nextIndex < testFiles.length) {
const file = testFiles[nextIndex++];
results.push(await runTestFile(file));
}
}

await Promise.all(
Array.from({ length: Math.min(CONCURRENCY, testFiles.length) }, worker),
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const passed = results.filter((r) => r.passed).length;
const failed = results.filter((r) => !r.passed);

Expand Down
12 changes: 10 additions & 2 deletions packages/auth/src/__tests__/run-bun-tests.behavior.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ import {
runTestFile,
type TestResult,
} from '../../run-bun-tests.js';
import { DEFAULT_PER_FILE_TIMEOUT_MS } from '../../../../scripts/lib/bun-test-policy.js';

/**
* Derived from the shared policy rather than hardcoded: the assertion is that
* the reason names the budget that was actually applied, not that the budget
* has one particular value (issue #3139).
*/
const EXPECTED_TIMEOUT_REASON = `Timed out after ${DEFAULT_PER_FILE_TIMEOUT_MS / 1000}s`;

describe('auth run-bun-tests JUnit failure reporting', () => {
const baseResult: TestResult = {
Expand All @@ -38,7 +46,7 @@ describe('auth run-bun-tests JUnit failure reporting', () => {

it('reports a timeout', () => {
const xml = generateJUnit([{ ...baseResult, timedOut: true }], 1, 1);
expect(xml).toContain('Timed out after 60s');
expect(xml).toContain(EXPECTED_TIMEOUT_REASON);
});

it('falls back to an exit code when neither signal nor timeout is present', () => {
Expand Down Expand Up @@ -67,7 +75,7 @@ describe('auth run-bun-tests failure reason formatting', () => {
signal: 'SIGTERM',
exitCode: 1,
});
expect(reason).toBe('Timed out after 60s');
expect(reason).toBe(EXPECTED_TIMEOUT_REASON);
});

it('reports a signal before an exit code', () => {
Expand Down
49 changes: 34 additions & 15 deletions packages/cli/run-bun-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,38 @@
import { spawn } from 'node:child_process';
import { readdirSync, realpathSync, statSync, writeFileSync } from 'node:fs';
import { join, relative } from 'node:path';
import { availableParallelism } from 'node:os';
import {
DEFAULT_PER_FILE_TIMEOUT_MS,
DEFAULT_PER_TEST_TIMEOUT_MS,
resolveTestConcurrency,
} from '../../scripts/lib/bun-test-policy.js';

const PER_FILE_TIMEOUT_MS = 120_000;
const PER_FILE_TIMEOUT_MS = DEFAULT_PER_FILE_TIMEOUT_MS;
const PER_INTEGRATION_FILE_TIMEOUT_MS = 900_000;
/**
* Per-test timeout, matching the testTimeout the removed vitest.config.ts set.
* Bun defaults to 5s, which the tests that spawn the real CLI exceed once the
* suite runs with concurrency. Passed as a flag because the bunfig.toml key is
* not picked up for a single-file invocation.
* Per-test timeout. Bun defaults to 5s, which the tests that spawn the real
* CLI exceed once the suite runs with concurrency. Passed as a flag because
* the bunfig.toml key is not picked up for a single-file invocation.
*
* Raised from 30s to the shared budget for issue #3139: this workspace was the
* worst-failing CI shard (5/15 first attempts) because it combined the tight
* bound with a pool that saturated every core.
*/
const PER_TEST_TIMEOUT_MS = 30_000;
const PER_TEST_TIMEOUT_MS = DEFAULT_PER_TEST_TIMEOUT_MS;

/**
* Integration tests spawn the built CLI, which cold-starts from TypeScript
* source and is far slower than an in-process test — especially on a loaded CI
* runner. They get a larger per-test budget so a slow boot is not reported as a
* failure.
*
* Expressed as a multiple of the shared budget rather than a fixed 120s: once
* the unit budget rose to 180s for issue #3139 a fixed value silently became
* the *smaller* of the two, which would have given the slowest tests in the
* workspace the tightest bound. It stays well inside
* PER_INTEGRATION_FILE_TIMEOUT_MS, which remains the hang backstop.
*/
const PER_INTEGRATION_TEST_TIMEOUT_MS = 120_000;
const PER_INTEGRATION_TEST_TIMEOUT_MS = DEFAULT_PER_TEST_TIMEOUT_MS * 2;

const SKIPPED_DIRECTORIES = new Set([
'node_modules',
Expand Down Expand Up @@ -104,7 +117,7 @@ function parseConcurrency(): number {
return parsed;
}
}
return Math.max(1, Math.min(8, availableParallelism()));
return resolveTestConcurrency({ envVar: 'LLXPRT_CLI_TEST_CONCURRENCY' });
}

export function isTestFile(fileName: string): boolean {
Expand Down Expand Up @@ -186,11 +199,17 @@ function runTestFile(file: string): Promise<TestResult> {
output += chunk.toString();
});

// Set by the timer so the exit handler can report the real reason. The
// result is only produced once the process has actually exited: killing a
// tree only signals it, so resolving from the timer would free this worker
// slot while the tree was still winding down. The pool would then exceed
// its concurrency cap exactly when the machine is already struggling —
// which is how a timeout on one file turns into timeouts on others.
let killedByTimeout = false;

const timer = setTimeout(() => {
if (settled) return;
settled = true;
killedByTimeout = true;
killProcessTree(child);
resolve({ file, passed: false, exitCode: null, timedOut: true, output });
}, fileTimeoutForFile(file));

child.on('exit', (code) => {
Expand All @@ -199,9 +218,9 @@ function runTestFile(file: string): Promise<TestResult> {
clearTimeout(timer);
resolve({
file,
passed: code === 0,
exitCode: code,
timedOut: false,
passed: !killedByTimeout && code === 0,
exitCode: killedByTimeout ? null : code,
timedOut: killedByTimeout,
output,
});
});
Expand Down
24 changes: 17 additions & 7 deletions packages/core/run-bun-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,24 @@
* file as a separate `bun test <file>` invocation avoids the multi-file
* process management that triggers the hang.
*
* Each child process has a per-file timeout (60s on POSIX, 180s on Windows).
* If a file takes longer, the process is killed to prevent a single
* slow/hanging file from blocking the entire suite.
* Concurrency and both timeout budgets come from
* scripts/lib/bun-test-policy.ts, shared with the other runners (issue #3139).
* This workspace keeps its own lower concurrency cap because its files are
* unusually heavy; the budgets are the shared ones. If a file exceeds the
* per-file budget the process is killed, so a single hanging file cannot block
* the suite.
*
* Exit code is 0 if all files pass, 1 if any file fails.
*/

import { spawn, type ChildProcess } from 'node:child_process';
import { readdirSync, statSync, writeFileSync } from 'node:fs';
import { join, relative } from 'node:path';
import { availableParallelism } from 'node:os';
import {
DEFAULT_PER_FILE_TIMEOUT_MS,
DEFAULT_PER_TEST_TIMEOUT_MS,
resolveTestConcurrency,
} from '../../scripts/lib/bun-test-policy.js';

/**
* Every path this runner touches — discovery, the child's working directory,
Expand All @@ -38,9 +45,12 @@ const JUNIT_PATH = join(WORKSPACE_ROOT, 'junit.xml');
// children overlap. POSIX retains bounded parallelism without saturating shared
// CI runners, where event-loop starvation can trip otherwise healthy test files.
const MAX_CONCURRENCY = process.platform === 'win32' ? 1 : 2;
const CONCURRENCY = Math.min(MAX_CONCURRENCY, availableParallelism());
const PER_TEST_TIMEOUT_MS = 30_000;
const PER_FILE_TIMEOUT_MS = process.platform === 'win32' ? 180_000 : 60_000;
const CONCURRENCY = resolveTestConcurrency({
envVar: 'LLXPRT_CORE_TEST_CONCURRENCY',
maxConcurrency: MAX_CONCURRENCY,
});
const PER_TEST_TIMEOUT_MS = DEFAULT_PER_TEST_TIMEOUT_MS;
const PER_FILE_TIMEOUT_MS = DEFAULT_PER_FILE_TIMEOUT_MS;
Comment thread
acoliver marked this conversation as resolved.

const TEST_ROOTS = ['src', 'test'] as const;

Expand Down
6 changes: 5 additions & 1 deletion packages/core/tsconfig.runner.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,9 @@
"@vybestack/llxprt-code-core/*": ["./src/*"]
}
},
"include": ["run-bun-tests.ts", "test/run-bun-tests.test.ts"]
"include": [
"run-bun-tests.ts",
"test/run-bun-tests.test.ts",
"../../scripts/lib/bun-test-policy.ts"
]
}
Loading
Loading