fix: close vitest if it failed to start (#9573)

Author: sheremet-va

packages/vitest/src/node/cli/cli-api.ts

@@ -106,6 +106,7 @@ export async function startVitest(
     else {
       await ctx.start(cliFilters)
     }
+    return ctx
   }
   catch (e) {
     if (e instanceof FilesNotFoundError) {
@@ -131,14 +132,12 @@ export async function startVitest(
     ctx.logger.error('\n\n')
     return ctx
   }
-
-  if (ctx.shouldKeepServer()) {
-    return ctx
+  finally {
+    if (!ctx?.shouldKeepServer()) {
+      stdinCleanup?.()
+      await ctx.close()
+    }
   }
-
-  stdinCleanup?.()
-  await ctx.close()
-  return ctx
 }
 
 export async function prepareVitest(

packages/vitest/src/node/core.ts

@@ -1392,9 +1392,12 @@ export class Vitest {
         if (this.coreWorkspaceProject && !teardownProjects.includes(this.coreWorkspaceProject)) {
           teardownProjects.push(this.coreWorkspaceProject)
         }
+        const teardownErrors: unknown[] = []
         // do teardown before closing the server
         for (const project of teardownProjects.reverse()) {
-          await project._teardownGlobalSetup()
+          await project._teardownGlobalSetup().catch((error) => {
+            teardownErrors.push(error)
+          })
         }
 
         const closePromises: unknown[] = this.projects.map(w => w.close())
@@ -1415,7 +1418,7 @@ export class Vitest {
         closePromises.push(...this._onClose.map(fn => fn()))
 
         await Promise.allSettled(closePromises).then((results) => {
-          results.forEach((r) => {
+          [...results, ...teardownErrors.map(r => ({ status: 'rejected', reason: r }))].forEach((r) => {
             if (r.status === 'rejected') {
               this.logger.error('error during close', r.reason)
             }

packages/vitest/src/node/create.ts

@@ -43,13 +43,21 @@ export async function createVitest(
     plugins: await VitestPlugin(restOptions, ctx),
   }
 
-  const server = await createViteServer(
-    mergeConfig(config, mergeConfig(viteOverrides, { root: options.root })),
-  )
+  try {
+    const server = await createViteServer(
+      mergeConfig(config, mergeConfig(viteOverrides, { root: options.root })),
+    )
 
-  if (ctx.config.api?.port) {
-    await server.listen()
-  }
+    if (ctx.config.api?.port) {
+      await server.listen()
+    }
 
-  return ctx
+    return ctx
+  }
+  // Vitest can fail at any point inside "setServer" or inside a custom plugin
+  // Then we need to make sure everything was properly closed (like the logger)
+  catch (error) {
+    await ctx.close()
+    throw error
+  }
 }

test/cli/test/bail-race.test.ts

@@ -1,5 +1,5 @@
 import { resolve } from 'pathe'
-import { expect, test } from 'vitest'
+import { expect, onTestFinished, test } from 'vitest'
 import { createVitest } from 'vitest/node'
 import { StableTestFileOrderSorter } from '../../test-utils'
 
@@ -21,6 +21,7 @@ test('cancels previous run before starting new one', async () => {
       },
     }],
   })
+  onTestFinished(() => vitest.close())
 
   for (let i = 0; i <= 4; i++) {
     await vitest.start()

test/cli/test/config-loader.test.ts

@@ -4,25 +4,39 @@ import { runVitest } from '../../test-utils'
 const isTypeStrippingSupported = !!process.features.typescript
 
 test.runIf(isTypeStrippingSupported)('configLoader native', async () => {
-  const { stderr, exitCode } = await runVitest({
+  const { stderr, exitCode, ctx } = await runVitest({
     root: 'fixtures/config-loader',
+    standalone: true,
+    watch: true,
     $cliOptions: {
       configLoader: 'native',
     },
   })
+  expect(ctx?.projects.map(p => p.name)).toMatchInlineSnapshot(`
+    [
+      "node",
+      "browser (chromium)",
+    ]
+  `)
   expect(stderr).toBe('')
   expect(exitCode).toBe(0)
 })
 
 test('configLoader runner', async () => {
-  const { vitest, exitCode } = await runVitest({
+  const { vitest, exitCode, ctx } = await runVitest({
     root: 'fixtures/config-loader',
+    standalone: true,
+    watch: true,
     $cliOptions: {
       configLoader: 'runner',
     },
   })
+  expect(ctx?.projects.map(p => p.name)).toMatchInlineSnapshot(`
+    [
+      "node",
+      "browser (chromium)",
+    ]
+  `)
   expect(vitest.stderr).toBe('')
-  expect(vitest.stdout).toContain('✓ |node|')
-  expect(vitest.stdout).toContain('✓ |browser (chromium)|')
   expect(exitCode).toBe(0)
 })

test/cli/test/create-vitest.test.ts

@@ -1,5 +1,5 @@
 import type { TestModule } from 'vitest/node'
-import { expect, it, vi } from 'vitest'
+import { expect, it, onTestFinished, vi } from 'vitest'
 import { createVitest } from 'vitest/node'
 
 it(createVitest, async () => {
@@ -13,6 +13,7 @@ it(createVitest, async () => {
       },
     ],
   })
+  onTestFinished(() => ctx.close())
   const testFiles = await ctx.globTestSpecifications()
   await ctx.runTestSpecifications(testFiles, false)
 

test/cli/test/network-imports.test.ts

@@ -16,11 +16,15 @@ it.runIf(Number(major) <= 20).each([
   'forks',
   'vmThreads',
 ])('importing from network in %s', async (pool) => {
-  const { ctx, exitCode } = await runVitest({
+  const { ctx, stderr, exitCode } = await runVitest({
     ...config,
     root: './fixtures/network-imports',
     pool,
-  })
+  }, [], { printExitCode: true })
+  expect([...ctx!.state.errorsSet]).toStrictEqual([])
+  expect(stderr.replace(/\(node:\d+\)/, '(node:\d+)')).toBe(`(node:d+) ExperimentalWarning: Network Imports is an experimental feature and might change at any time
+(Use \`node --trace-warnings ...\` to show where the warning was created)
+`)
   expect(ctx!.state.getTestModules()).toHaveLength(1)
   expect(ctx!.state.getTestModules()[0].state()).toBe('passed')
   expect(exitCode).toBe(0)

test/cli/test/public-api.test.ts

@@ -97,8 +97,9 @@ it.each([
 
 it('can modify the global test name pattern', async () => {
   const { ctx } = await runVitest({
+    standalone: true,
+    watch: true,
     testNamePattern: 'custom',
-    include: ['non-existing'],
   })
 
   expect(ctx?.getGlobalTestNamePattern()).toEqual(/custom/)

test/cli/test/static-collect.test.ts

@@ -1,7 +1,7 @@
 import type { CliOptions, TestCase, TestModule, TestSuite } from 'vitest/node'
 import { runVitest } from '#test-utils'
 import { resolve } from 'pathe'
-import { expect, test } from 'vitest'
+import { expect, onTestFinished, test } from 'vitest'
 import { createVitest, rolldownVersion } from 'vitest/node'
 
 test('correctly collects a simple test', async () => {
@@ -1078,6 +1078,7 @@ async function collectTestModule(code: string, options?: CliOptions) {
       ],
     },
   )
+  onTestFinished(() => vitest.close())
   return vitest.experimental_parseSpecification(
     vitest.getRootProject().createSpecification('simple.test.ts'),
   )

test/test-utils/index.ts

@@ -31,6 +31,7 @@ globalThis.__VITEST_GENERATE_UI_TOKEN__ = true
 export interface VitestRunnerCLIOptions {
   std?: 'inherit'
   fails?: boolean
+  printExitCode?: boolean
   preserveAnsi?: boolean
   tty?: boolean
   mode?: 'test' | 'benchmark'
@@ -41,6 +42,8 @@ export interface RunVitestConfig extends TestUserConfig {
   $cliOptions?: TestCliOptions
 }
 
+const process_ = process
+
 /**
  * The config is assumed to be the config on the fille system, not CLI options
  * (Note that CLI only options like "standalone" are passed as CLI options, not config options)
@@ -60,6 +63,18 @@ export async function runVitest(
   process.exitCode = 0
   let exitCode = process.exitCode
 
+  if (runnerOptions.printExitCode) {
+    globalThis.process = new Proxy(process_, {
+      set(target, p, newValue, receiver) {
+        if (p === 'exitCode') {
+          // eslint-disable-next-line no-console
+          console.trace('exitCode was set to', newValue)
+        }
+        return Reflect.set(target, p, newValue, receiver)
+      },
+    })
+  }
+
   // Prevent possible process.exit() calls, e.g. from --browser
   const exit = process.exit
   process.exit = (() => { }) as never
@@ -194,6 +209,9 @@ export async function runVitest(
     cli.stderr += inspect(e)
   }
   finally {
+    if (runnerOptions.printExitCode) {
+      globalThis.process = process_
+    }
     exitCode = process.exitCode
     process.exitCode = 0