test: run the unit suite without --isolate (TA-001 round 2) (#2346)

* test: run the unit suite without --isolate (TA-001 round 2)

* test: clean up leaks exposed by running without --isolate (TA-001 round 2)

* test: clean up remaining leaks in shared-process runs (TA-001 round 2)

* test: reset query engine trace state before each test (TA-001 round 2)
This commit is contained in:
JATMN
2026-10-22 23:32:21 -07:00
committed by GitHub
parent ada7f0fe38
commit 4772c395b2
9 changed files with 41 additions and 16 deletions
+2 -1
View File
@@ -160,6 +160,7 @@ Dependency changes need a clear project benefit — fixing a bug, addressing a s
- Preserve existing repo patterns unless the change is intentionally refactoring them.
- Add or update tests when the change affects behavior.
- For vendor/integration catalog tests, cover each vendor with one table-driven test of its registry contract (route resolution, default base URL and model, credential env vars, catalog-to-descriptor linkage, and capability/limit values the runtime uses), plus a small number of behavioral tests such as model-discovery keep/drop parsing or availability windows. Do not copy full curated model lists or cache/refresh settings from vendor definitions into tests.
- Test files share a process within each run or shard; restore `mock.module` registrations, environment variables, globals, and process state in `afterEach`/`afterAll`.
- When tests set or delete environment variables, import `snapshotEnv` from `src/test/envSnapshot.ts` using the appropriate relative `.js` path. In `beforeEach`, save the named variables before changing them with `restoreEnv = snapshotEnv(keys)`; pass `{ clear: true }` when setup needs those variables unset. Call `restoreEnv()` in `afterEach`, or use `finally` for changes scoped to one test. Restoration preserves previous values, including empty strings, and deletes variables that were previously unset. List only the variables the test changes or deliberately clears; do not replace `process.env` or blanket-clear unrelated variables. When using the shared mutation lock, snapshot after acquiring it and restore before releasing it.
- In timing-sensitive tests, drive timers with Bun's fake timers (`jest.useFakeTimers()` and `jest.advanceTimersByTime()`) instead of racing real delays, and restore real timers during cleanup, so the tests pass in a single-process whole-suite run as well as in the sharded runner.
- Await completion promises or explicit readiness signals before asserting asynchronous results. For tests that deliberately suppress readiness, use a short, bounded wait and assert the expected failure.
@@ -313,7 +314,7 @@ Notes on the local preflight:
- PR Checks caches Bun package downloads using OS-scoped keys derived from `bun.lock` for root installs and `web/bun.lock` for web installs, with separate restore prefixes. Dependency installs still run with `--frozen-lockfile` on cache hits. Keep the relevant lockfile in sync with dependency changes and run the required local preflight regardless of cache status.
- `bun run check` runs `check:static` (the any-usage budget, smoke build, and deadcode checks), then the full unit pass (`test:full`) — do not run those separately, or you execute work twice.
- `bun run test` isolates each test file's globals and requires Bun's completed JUnit report as well as a successful exit. `test:full` runs all eight native Bun shards serially to release memory between groups. Use `bun run scripts/run-tests.ts --shards=N --shard-index=K` with the same test flags to run only shard K of N. `test:full` then applies the same completion check to the conversation-arc pass and the context-efficiency pass with `HISTORY_SNIP` enabled. Every shard must produce a complete report; a failed shard does not prevent the remaining shards from running. A test that terminates the runner early fails the command, even if it exits with code zero. Each shard has a ten-minute deadline (override with `--shard-timeout-ms=<milliseconds>`), reports its start/end and elapsed time, and kills its process group on timeout or cancellation. A timeout stops the remaining shards and exits nonzero. On Linux, shard output is relayed from temporary regular files to avoid Bun’s isolated-stdio pipe registration bug; diagnostics remain visible during execution. Temporary reports and logs are removed after the check. CI runs `check:static` in `smoke-and-tests`, runs the eight unit shards as parallel `unit-tests` jobs, and runs each dedicated feature pass once in `dedicated-tests`. Each unit shard builds the CLI before testing. CI caps these jobs at 15 minutes each and the other jobs at five minutes.
- `bun run test` runs test files in a shared process and requires Bun's completed JUnit report as well as a successful exit. `test:full` runs all eight native Bun shards serially to release memory between groups. Use `bun run scripts/run-tests.ts --shards=N --shard-index=K` with the same test flags to run only shard K of N. `test:full` then applies the same completion check to the conversation-arc pass and the context-efficiency pass with `HISTORY_SNIP` enabled. Every shard must produce a complete report; a failed shard does not prevent the remaining shards from running. A test that terminates the runner early fails the command, even if it exits with code zero. Each shard has a ten-minute deadline (override with `--shard-timeout-ms=<milliseconds>`), reports its start/end and elapsed time, and kills its process group on timeout or cancellation. A timeout stops the remaining shards and exits nonzero. On Linux, shard output is relayed from temporary regular files; diagnostics remain visible during execution. Temporary reports and logs are removed after the check. CI runs `check:static` in `smoke-and-tests`, runs the eight unit shards as parallel `unit-tests` jobs, and runs each dedicated feature pass once in `dedicated-tests`. Each unit shard builds the CLI before testing. CI caps these jobs at 15 minutes each and the other jobs at five minutes.
- Fetching upstream `main` by URL avoids assuming that a fork checkout's `origin` points at Twigpine/openclaude. `FETCH_HEAD` is the fetched upstream tip. The scan deliberately uses local `HEAD` so it includes commits that have not been pushed yet; CI uses the pushed PR head SHA after the push.
- The web CI job runs on PRs that change `web/**`, root `package.json` or `bun.lock`, `.bun-version`, or `.github/workflows/pr-checks.yml`, and on every push to `main`. Contributors do not need to run the web suite locally for changes that cannot affect the site.
- This preflight covers the same command families as CI, but it does not reproduce CI exactly in one shell. When selected, CI runs static checks with Node 22 available for smoke checks, runs the unit shards and dedicated feature passes under Node 22, separately builds and checks launcher compatibility under Node 22 and 24.11.x, separately builds and launches under exact Node 22.0.0, and executes every job on a clean runner.
-1
View File
@@ -61,7 +61,6 @@ try {
const child = Bun.spawn([
process.execPath,
'test',
'--isolate',
...testArgs,
...(shards > 1 ? [`--shard=${shard}/${shards}`] : []),
'--reporter=junit',
@@ -15,6 +15,8 @@ const originalTrace = process.env.OPENCLAUDE_INTERRUPT_TRACE
beforeEach(async () => {
await acquireSharedMutationLock('QueryEngine.interruptionTrace.test.ts')
// Earlier files can leave trace entries behind in non-isolated runs.
__resetInterruptionTraceForTests()
})
afterEach(async () => {
@@ -43,15 +43,15 @@ async function importRealModules(): Promise<RealModules> {
const cacheKey = `${Date.now()}-${Math.random()}`
realModules = {
browser: (await import(
`../../utils/browser.ts?setup-actions-real-${cacheKey}`
)) as BrowserModule,
browser: {
...await import(`../../utils/browser.ts?setup-actions-real-${cacheKey}`),
} as BrowserModule,
config: {
...await import(`../../utils/config.js?real=${cacheKey}`),
} as ConfigModule,
execFileNoThrow: (await import(
`../../utils/execFileNoThrow.ts?setup-actions-real-${cacheKey}`
)) as ExecFileNoThrowModule,
execFileNoThrow: {
...await import(`../../utils/execFileNoThrow.ts?setup-actions-real-${cacheKey}`),
} as ExecFileNoThrowModule,
}
return realModules
}
+2
View File
@@ -146,6 +146,7 @@ describe('/replay command', () => {
loadSameRepoMessageLogs: loadSameRepoMessageLogsMock,
}))
mock.module('../../utils/replayIndex.js', () => ({
...realReplayIndex,
loadReplayIndex: loadReplayIndexMock,
}))
mock.module('../../hooks/useTerminalSize.js', () => ({
@@ -155,6 +156,7 @@ describe('/replay command', () => {
useIsInsideModal: () => false,
}))
mock.module('../../components/LogSelector.js', () => ({
...realLogSelector,
LogSelector: (props: LogSelectorProps) => {
lastLogSelectorProps = props
return <Text>{`LogSelector:${props.logs.length}`}</Text>
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'
import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'
import type { BetaToolUnion } from '@anthropic-ai/sdk/resources/beta/messages/messages.mjs'
import type { TextBlockParam } from '@anthropic-ai/sdk/resources/index.mjs'
import type { QuerySource } from '../../constants/querySource.js'
@@ -30,9 +30,23 @@ const logForDebuggingMock = mock(
debugCalls.push({ message, options })
},
)
const actualDebugModule = await import('../../utils/debug.js')
const nonce = `${Date.now()}-${Math.random()}`
const actualDebugModule = {
...await import(`../../utils/debug.js?real=${nonce}`),
}
const actualAnalyticsModule = {
...await import(`../analytics/index.js?real=${nonce}`),
}
afterAll(() => {
mock.restore()
mock.module('../analytics/index.js', () => actualAnalyticsModule)
mock.module('src/utils/debug.js', () => actualDebugModule)
mock.module('../../utils/debug.js', () => actualDebugModule)
})
mock.module('../analytics/index.js', () => ({
...actualAnalyticsModule,
logEvent: logEventMock,
}))
+6 -3
View File
@@ -70,9 +70,12 @@ afterEach(() => {
})
async function importActualProviders(): Promise<ProvidersModule> {
return import(
`../../utils/model/providers.ts?withRetryActual=${Date.now()}-${Math.random()}`
)
// Bun repoints module namespaces when mocked; keep the original exports.
return {
...await import(
`../../utils/model/providers.ts?withRetryActual=${Date.now()}-${Math.random()}`
),
}
}
async function importFreshWithRetryModule(
+1
View File
@@ -53,6 +53,7 @@ mock.module('../../utils/config.js', () => ({
}))
mock.module('./tipRegistry.js', () => ({
...actualTipRegistry,
getRelevantTips: async () => relevantTipsRef.value,
}))
@@ -71,9 +71,12 @@ function restoreEnv(key: keyof typeof originalEnv): void {
}
async function importActualModelAllowlist(): Promise<ModelAllowlistModule> {
return import(
`../../utils/model/modelAllowlist.ts?agentToolActual=${Date.now()}-${Math.random()}`
)
// Bun repoints module namespaces when mocked; keep the original exports.
return {
...await import(
`../../utils/model/modelAllowlist.ts?agentToolActual=${Date.now()}-${Math.random()}`
),
}
}
async function importActualSettings(): Promise<SettingsModule> {