fix: give e2e cleanup hooks timeout headroom

docs/maintenance.md

@@ -97,8 +97,11 @@ request with no answer within `TSSERVER_RESPONSE_TIMEOUT_MS`
 (`test/e2e/tsserver-fixture/timeouts.ts`) fails its test and stops that
 tsserver at once, and each tsserver runs with a heap cap
 (`test/e2e/tsserver-fixture/server.ts`); each unit test worker runs with a heap
-cap (`vitest.config.ts`). Vitest's own `testTimeout` cannot stop a test stuck
-in a synchronous loop, since its timer runs on the same blocked thread, so
+cap (`vitest.config.ts`). The e2e hook timeout leaves enough headroom for the
+harness to report a process-exit timeout from an `onTestFinished` cleanup
+before Vitest's generic hook timeout. Vitest's own `testTimeout` cannot stop a
+test stuck in a synchronous loop, since its timer runs on the same blocked
+thread, so
 `yarn test:unit` and `yarn test:e2e:run` each run through
 `scripts/with-deadline.ts` instead of calling Vitest directly (the deadline
 each passes it is set where package.json invokes the wrapper). The wrapper

test/e2e/tsserver-fixture/timeouts.ts

@@ -2,8 +2,9 @@
  * Upper bound on each tsserver response and on the process exit after `close()`. A request runs
  * against a real TypeScript program in a separate process, so its wall time tracks host CPU
  * contention rather than the request's own work; the default leaves headroom for a busy host.
- * `vitest.config.ts` derives the e2e `testTimeout` from this value, so a slow response fails with
- * this harness's message naming the request instead of Vitest's generic "test timed out".
+ * `vitest.config.ts` derives the e2e test and hook timeouts from this value. The hook timeout
+ * leaves headroom for `close()` to report a stalled process before Vitest reports a generic hook
+ * timeout.
  */
 const DEFAULT_RESPONSE_TIMEOUT_MS = 30_000
 
@@ -12,19 +13,33 @@ const responseTimeoutVariable = 'TSSERVER_RESPONSE_TIMEOUT_MS'
 /** Node fires any timer delay above this after 1 ms instead, so a larger timeout would fail at once. */
 const MAX_TIMER_DELAY_MS = 2 ** 31 - 1
 
-export function parsePositiveInteger(name: string, value: string | undefined): number | undefined {
+/** Lets Vitest cleanup wait long enough for the harness's own process-exit timeout to win. */
+export const E2E_TIMEOUT_HEADROOM_MS = 15_000
+
+export const MAX_RESPONSE_TIMEOUT_MS = MAX_TIMER_DELAY_MS - E2E_TIMEOUT_HEADROOM_MS
+
+export function parsePositiveInteger(
+  name: string,
+  value: string | undefined,
+  maximum = MAX_TIMER_DELAY_MS,
+): number | undefined {
   if (value === undefined) {
     return undefined
   }
   const parsed = Number(value)
-  if (!/^[1-9]\d*$/.test(value) || parsed > MAX_TIMER_DELAY_MS) {
+  if (!/^[1-9]\d*$/.test(value) || parsed > maximum) {
     throw new Error(
-      `${name} is "${value}", which is not a whole number of milliseconds from 1 to ${MAX_TIMER_DELAY_MS}. Set it to a value such as 60000, or unset it to use ${DEFAULT_RESPONSE_TIMEOUT_MS}.`,
+      `${name} is "${value}", which is not a whole number of milliseconds from 1 to ${maximum}. Set it to a value such as 60000, or unset it to use ${DEFAULT_RESPONSE_TIMEOUT_MS}.`,
     )
   }
   return parsed
 }
 
-export const responseTimeoutMs =
-  parsePositiveInteger(responseTimeoutVariable, process.env[responseTimeoutVariable]) ??
-  DEFAULT_RESPONSE_TIMEOUT_MS
+export function getResponseTimeoutMs(value: string | undefined): number {
+  return (
+    parsePositiveInteger(responseTimeoutVariable, value, MAX_RESPONSE_TIMEOUT_MS) ??
+    DEFAULT_RESPONSE_TIMEOUT_MS
+  )
+}
+
+export const responseTimeoutMs = getResponseTimeoutMs(process.env[responseTimeoutVariable])

test/unit/tsserver-timeouts.test.ts

@@ -1,6 +1,10 @@
 import { assert, describe, it } from 'vitest'
 
-import { parsePositiveInteger } from '../e2e/tsserver-fixture/timeouts'
+import {
+  getResponseTimeoutMs,
+  MAX_RESPONSE_TIMEOUT_MS,
+  parsePositiveInteger,
+} from '../e2e/tsserver-fixture/timeouts'
 
 describe('parsePositiveInteger', () => {
   it('returns undefined for an unset variable, so the default applies', () => {
@@ -25,3 +29,24 @@ describe('parsePositiveInteger', () => {
     },
   )
 })
+
+describe('getResponseTimeoutMs', () => {
+  it('uses the default when the environment variable is unset', () => {
+    assert.strictEqual(getResponseTimeoutMs(undefined), 30_000)
+  })
+
+  it.each([
+    ['1', 1],
+    ['60000', 60_000],
+    [String(MAX_RESPONSE_TIMEOUT_MS), 2 ** 31 - 1 - 15_000],
+  ])('accepts %s without consuming cleanup headroom', (value, expected) => {
+    assert.strictEqual(getResponseTimeoutMs(value), expected)
+  })
+
+  it('reserves headroom when parsing the tsserver response timeout', () => {
+    assert.throws(
+      () => getResponseTimeoutMs(String(MAX_RESPONSE_TIMEOUT_MS + 1)),
+      `TSSERVER_RESPONSE_TIMEOUT_MS is "${MAX_RESPONSE_TIMEOUT_MS + 1}", which is not a whole number of milliseconds from 1 to ${MAX_RESPONSE_TIMEOUT_MS}. Set it to a value such as 60000, or unset it to use 30000.`,
+    )
+  })
+})

test/unit/vitest-config.test.ts

@@ -0,0 +1,18 @@
+import { assert, describe, it } from 'vitest'
+
+import { e2eProjectConfig, e2eTestConfig, e2eTimeoutMs, vitestConfig } from '../../vitest.config.ts'
+import { responseTimeoutMs } from '../e2e/tsserver-fixture/timeouts.ts'
+
+describe('Vitest configuration', () => {
+  it('lets tsserver cleanup report its process-exit timeout before Vitest does', () => {
+    const projects = vitestConfig.test?.projects
+    if (!Array.isArray(projects)) {
+      assert.fail('Expected Vitest projects to be configured.')
+    }
+
+    assert.include(projects, e2eProjectConfig)
+    assert.isAbove(e2eTimeoutMs, responseTimeoutMs)
+    assert.strictEqual(e2eTestConfig.hookTimeout, e2eTimeoutMs)
+    assert.strictEqual(e2eTestConfig.testTimeout, e2eTimeoutMs)
+  })
+})

vitest.config.ts

@@ -2,7 +2,7 @@ import { availableParallelism } from 'node:os'
 
 import { defineConfig } from 'vitest/config'
 
-import { responseTimeoutMs } from './test/e2e/tsserver-fixture/timeouts'
+import { E2E_TIMEOUT_HEADROOM_MS, responseTimeoutMs } from './test/e2e/tsserver-fixture/timeouts'
 
 /**
  * Two projects, so only e2e files run the e2e globalSetup: Vitest assigns each test file to a
@@ -17,6 +17,26 @@ import { responseTimeoutMs } from './test/e2e/tsserver-fixture/timeouts'
  */
 const e2eParallelism = Math.max(1, Math.min(8, availableParallelism() - 1))
 
+/** Keeps cleanup from preempting the harness's own process-exit timeout. */
+export const e2eTimeoutMs = responseTimeoutMs + E2E_TIMEOUT_HEADROOM_MS
+
+export const e2eTestConfig = {
+  environment: 'node',
+  globals: true,
+  globalSetup: [
+    'test/e2e/tsserver-fixture/check-plugin-link.ts',
+    'test/e2e/tsserver-fixture/clear-logs.ts',
+  ],
+  hookTimeout: e2eTimeoutMs,
+  include: ['test/e2e/scenarios/**/*.test.ts'],
+  maxConcurrency: e2eParallelism,
+  maxWorkers: e2eParallelism,
+  name: 'e2e',
+  testTimeout: e2eTimeoutMs,
+}
+
+export const e2eProjectConfig = { test: e2eTestConfig }
+
 /**
  * Unit tests run plugin code inside each test worker, a child process in Vitest's default `forks`
  * pool. The heap cap ends a worker whose test retains without bound within seconds, as a failed
@@ -28,7 +48,7 @@ const e2eParallelism = Math.max(1, Math.min(8, availableParallelism() - 1))
  */
 const UNIT_WORKER_HEAP_CAP_MB = 1024
 
-export default defineConfig({
+export const vitestConfig = defineConfig({
   test: {
     projects: [
       {
@@ -40,22 +60,9 @@ export default defineConfig({
           name: 'unit',
         },
       },
-      {
-        test: {
-          environment: 'node',
-          globals: true,
-          globalSetup: [
-            'test/e2e/tsserver-fixture/check-plugin-link.ts',
-            'test/e2e/tsserver-fixture/clear-logs.ts',
-          ],
-          include: ['test/e2e/scenarios/**/*.test.ts'],
-          maxConcurrency: e2eParallelism,
-          maxWorkers: e2eParallelism,
-          name: 'e2e',
-          /** Above the harness's own response timeout, so its message naming the request wins. */
-          testTimeout: responseTimeoutMs + 15_000,
-        },
-      },
+      e2eProjectConfig,
     ],
   },
 })
+
+export default vitestConfig