fix(pool): prevent test run hang on worker crash (#10543) [backport t… · vitest-dev/vitest@934b0f5 (original) (raw)
File tree
- packages/vitest/src/node/pools
- fixtures/pool-worker-exit
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -101,19 +101,21 @@ export class Pool { | ||
| 101 | 101 | } |
| 102 | 102 | |
| 103 | 103 | runner.off('message', onFinished) |
| 104 | +runner.off('error', onTaskError) | |
| 104 | 105 | resolver.resolve() |
| 105 | 106 | } |
| 106 | 107 | } |
| 107 | 108 | |
| 109 | +function onTaskError(error: unknown) { | |
| 110 | +runner.off('message', onFinished) | |
| 111 | +runner.off('error', onTaskError) | |
| 112 | +resolver.reject(new Error(`[vitest-pool]: Worker ${task.worker} emitted error.`, { cause: error })) | |
| 113 | +} | |
| 114 | + | |
| 108 | 115 | runner.on('message', onFinished) |
| 116 | +runner.on('error', onTaskError) | |
| 109 | 117 | |
| 110 | 118 | if (!runner.isStarted) { |
| 111 | -runner.on('error', (error) => { | |
| 112 | -resolver.reject( | |
| 113 | -new Error(`[vitest-pool]: Worker ${task.worker} emitted error.`, { cause: error }), | |
| 114 | -) | |
| 115 | -}) | |
| 116 | - | |
| 117 | 119 | const id = setTimeout( |
| 118 | 120 | () => resolver.reject(new Error(`[vitest-pool]: Timeout starting ${task.worker} runner.`)), |
| 119 | 121 | WORKER_START_TIMEOUT, |
| @@ -148,6 +150,7 @@ export class Pool { | ||
| 148 | 150 | |
| 149 | 151 | if ( |
| 150 | 152 | !task.isolate |
| 153 | +&& !runner.isTerminated | |
| 151 | 154 | && !isMemoryLimitReached |
| 152 | 155 | && this.queue[0]?.task.isolate === false |
| 153 | 156 | && isEqualRunner(runner, this.queue[0].task) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -368,6 +368,7 @@ export class PoolRunner { | ||
| 368 | 368 | |
| 369 | 369 | private emitUnexpectedExit = (): void => { |
| 370 | 370 | const error = new Error('Worker exited unexpectedly') |
| 371 | +this._state = RunnerState.STOPPED | |
| 371 | 372 | |
| 372 | 373 | this._eventEmitter.emit('error', error) |
| 373 | 374 | } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| 1 | +import { expect, test } from 'vitest' | |
| 2 | +import { covered } from './src' | |
| 3 | + | |
| 4 | +test('first test exercises src so it should appear in coverage', () => { | |
| 5 | +expect(covered()).toBe(42) | |
| 6 | +}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| 1 | +import { test } from 'vitest' | |
| 2 | + | |
| 3 | +test('the worker dies before sending testfileFinished', async () => { | |
| 4 | +// SIGKILL the worker process so it can't send testfileFinished back to main. | |
| 5 | +// Pre-fix this caused pool.run() to hang forever instead of rejecting. | |
| 6 | +queueMicrotask(() => process.kill(process.pid, 'SIGKILL')) | |
| 7 | +await new Promise(() => {}) | |
| 8 | +}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| 1 | +import { test } from 'vitest' | |
| 2 | + | |
| 3 | +test('the worker dies before sending testfileFinished', async () => { | |
| 4 | +// SIGKILL the worker process so it can't send testfileFinished back to main. | |
| 5 | +// Pre-fix this caused pool.run() to hang forever instead of rejecting. | |
| 6 | +queueMicrotask(() => process.kill(process.pid, 'SIGKILL')) | |
| 7 | +await new Promise(() => {}) | |
| 8 | +}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| 1 | +import { expect, test } from 'vitest' | |
| 2 | +import { covered } from './src.js' | |
| 3 | + | |
| 4 | +test('third test', () => { | |
| 5 | +expect(covered()).toBe(42) | |
| 6 | +}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| 1 | +export function covered() { | |
| 2 | +return 42 | |
| 3 | +} | |
| 4 | + | |
| 5 | +export function uncovered() { | |
| 6 | +return 42 | |
| 7 | +} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| 1 | +import { runVitest, StableTestFileOrderSorter } from '#test-utils' | |
| 2 | +import { resolve } from 'pathe' | |
| 3 | +import { expect, test } from 'vitest' | |
| 4 | +import { readCoverageMap } from '../../coverage-test/utils' | |
| 5 | + | |
| 6 | +test('worker death on a shared runner does not skip coverage finalization', async () => { | |
| 7 | +const root = './fixtures/pool-worker-exit' | |
| 8 | + | |
| 9 | +const { buildTree } = await runVitest({ | |
| 10 | + root, | |
| 11 | +pool: 'forks', | |
| 12 | + | |
| 13 | +// Disable isolation to make sure crashed worker doesn't hang whole test run | |
| 14 | +isolate: false, | |
| 15 | +maxWorkers: 2, | |
| 16 | + | |
| 17 | +sequence: { sequencer: StableTestFileOrderSorter }, | |
| 18 | +include: [ | |
| 19 | +'1-first.test.ts', | |
| 20 | +'2-crash.test.ts', | |
| 21 | +'3-crash.test.ts', | |
| 22 | +'4-third.test.ts', | |
| 23 | +], | |
| 24 | + | |
| 25 | +reporters: 'default', | |
| 26 | +coverage: { | |
| 27 | +enabled: true, | |
| 28 | +provider: 'v8', | |
| 29 | +reporter: ['json'], | |
| 30 | +reportOnFailure: true, | |
| 31 | +}, | |
| 32 | +}) | |
| 33 | + | |
| 34 | +expect(buildTree(t => ({ state: t.result().state }))).toMatchInlineSnapshot(` | |
| 35 | + { | |
| 36 | + "1-first.test.ts": { | |
| 37 | + "first test exercises src so it should appear in coverage": { | |
| 38 | + "state": "passed", | |
| 39 | + }, | |
| 40 | + }, | |
| 41 | + "2-crash.test.ts": { | |
| 42 | + "the worker dies before sending testfileFinished": { | |
| 43 | + "state": "pending", | |
| 44 | + }, | |
| 45 | + }, | |
| 46 | + "3-crash.test.ts": { | |
| 47 | + "the worker dies before sending testfileFinished": { | |
| 48 | + "state": "pending", | |
| 49 | + }, | |
| 50 | + }, | |
| 51 | + "4-third.test.ts": { | |
| 52 | + "third test": { | |
| 53 | + "state": "passed", | |
| 54 | + }, | |
| 55 | + }, | |
| 56 | + } | |
| 57 | + `) | |
| 58 | + | |
| 59 | +// Crashing worker should not interfere with other test-run, coverage should be reported: | |
| 60 | +const coverageMap = await readCoverageMap(resolve(root, 'coverage/coverage-final.json')) | |
| 61 | +const fileCoverage = coverageMap.fileCoverageFor('/fixtures/pool-worker-exit/src.ts') | |
| 62 | + | |
| 63 | +expect(fileCoverage.toSummary().functions).toMatchInlineSnapshot(` | |
| 64 | + { | |
| 65 | + "covered": 1, | |
| 66 | + "pct": 50, | |
| 67 | + "skipped": 0, | |
| 68 | + "total": 2, | |
| 69 | + } | |
| 70 | + `) | |
| 71 | +}) |