fix: stop the runner before restarting, restart on workspace config change (#6859)

sheremet-va · web-flow · commit b01df47da0a3 · 2024-11-08T10:02:23.000+01:00
diff --git a/packages/vitest/src/node/cli/cli-api.ts b/packages/vitest/src/node/cli/cli-api.ts
@@ -73,10 +73,6 @@ export async function startVitest(
     stdinCleanup = registerConsoleShortcuts(ctx, stdin, stdout)
   }
 
-  ctx.onServerRestart((reason) => {
-    ctx.report('onServerRestart', reason)
-  })
-
   ctx.onAfterSetServer(() => {
     if (ctx.config.standalone) {
       ctx.init()
diff --git a/packages/vitest/src/node/core.ts b/packages/vitest/src/node/core.ts
@@ -83,6 +83,7 @@ export class Vitest {
   public distPath = distDir
 
   private _cachedSpecs = new Map<string, WorkspaceSpec[]>()
+  private _workspaceConfigPath?: string
 
   /** @deprecated use `_cachedSpecs` */
   projectTestFiles = this._cachedSpecs
@@ -110,6 +111,10 @@ export class Vitest {
     this._browserLastPort = defaultBrowserPort
     this.pool?.close?.()
     this.pool = undefined
+    this.closingPromise = undefined
+    this.projects = []
+    this.resolvedProjects = []
+    this._workspaceConfigPath = undefined
     this.coverageProvider = undefined
     this.runningPromise = undefined
     this._cachedSpecs.clear()
@@ -145,22 +150,22 @@ export class Vitest {
       const serverRestart = server.restart
       server.restart = async (...args) => {
         await Promise.all(this._onRestartListeners.map(fn => fn()))
+        this.report('onServerRestart')
+        await this.close()
         await serverRestart(...args)
-        // watcher is recreated on restart
-        this.unregisterWatcher()
-        this.registerWatcher()
       }
 
       // since we set `server.hmr: false`, Vite does not auto restart itself
       server.watcher.on('change', async (file) => {
         file = normalize(file)
         const isConfig = file === server.config.configFile
+          || this.resolvedProjects.some(p => p.server.config.configFile === file)
+          || file === this._workspaceConfigPath
         if (isConfig) {
           await Promise.all(this._onRestartListeners.map(fn => fn('config')))
+          this.report('onServerRestart', 'config')
+          await this.close()
           await serverRestart()
-          // watcher is recreated on restart
-          this.unregisterWatcher()
-          this.registerWatcher()
         }
       })
     }
@@ -175,8 +180,6 @@ export class Vitest {
     }
     catch { }
 
-    await Promise.all(this._onSetServer.map(fn => fn()))
-
     const projects = await this.resolveWorkspace(cliOptions)
     this.resolvedProjects = projects
     this.projects = projects
@@ -193,6 +196,8 @@ export class Vitest {
     if (this.config.testNamePattern) {
       this.configOverride.testNamePattern = this.config.testNamePattern
     }
+
+    await Promise.all(this._onSetServer.map(fn => fn()))
   }
 
   public provide<T extends keyof ProvidedContext & string>(key: T, value: ProvidedContext[T]) {
@@ -235,7 +240,7 @@ export class Vitest {
       || this.projects[0]
   }
 
-  private async getWorkspaceConfigPath(): Promise<string | null> {
+  private async getWorkspaceConfigPath(): Promise<string | undefined> {
     if (this.config.workspace) {
       return this.config.workspace
     }
@@ -251,7 +256,7 @@ export class Vitest {
     })
 
     if (!workspaceConfigName) {
-      return null
+      return undefined
     }
 
     return join(configDir, workspaceConfigName)
@@ -260,6 +265,8 @@ export class Vitest {
   private async resolveWorkspace(cliOptions: UserConfig) {
     const workspaceConfigPath = await this.getWorkspaceConfigPath()
 
+    this._workspaceConfigPath = workspaceConfigPath
+
     if (!workspaceConfigPath) {
       return [await this._createCoreProject()]
     }
diff --git a/packages/vitest/src/node/workspace.ts b/packages/vitest/src/node/workspace.ts
@@ -432,6 +432,7 @@ export class WorkspaceProject {
       )
     }
 
+    this.closingPromise = undefined
     this.testProject = new TestProject(this)
 
     this.server = server
@@ -476,7 +477,7 @@ export class WorkspaceProject {
     if (!this.closingPromise) {
       this.closingPromise = Promise.all(
         [
-          this.server.close(),
+          this.server?.close(),
           this.typechecker?.stop(),
           this.browser?.close(),
           this.clearTmpDir(),
diff --git a/test/test-utils/index.ts b/test/test-utils/index.ts
@@ -1,5 +1,6 @@
 import type { Options } from 'tinyexec'
 import type { UserConfig as ViteUserConfig } from 'vite'
+import type { WorkspaceProjectConfiguration } from 'vitest/config'
 import type { UserConfig, Vitest, VitestRunMode } from 'vitest/node'
 import fs from 'node:fs'
 import { Readable, Writable } from 'node:stream'
@@ -234,3 +235,61 @@ export function resolvePath(baseUrl: string, path: string) {
   const filename = fileURLToPath(baseUrl)
   return resolve(dirname(filename), path)
 }
+
+export function useFS(root: string, structure: Record<string, string | ViteUserConfig | WorkspaceProjectConfiguration[]>) {
+  const files = new Set<string>()
+  const hasConfig = Object.keys(structure).some(file => file.includes('.config.'))
+  if (!hasConfig) {
+    structure['./vitest.config.js'] = {}
+  }
+  for (const file in structure) {
+    const filepath = resolve(root, file)
+    files.add(filepath)
+    const content = typeof structure[file] === 'string'
+      ? structure[file]
+      : `export default ${JSON.stringify(structure[file])}`
+    fs.mkdirSync(dirname(filepath), { recursive: true })
+    fs.writeFileSync(filepath, String(content), 'utf-8')
+  }
+  onTestFinished(() => {
+    if (process.env.VITEST_FS_CLEANUP !== 'false') {
+      fs.rmSync(root, { recursive: true, force: true })
+    }
+  })
+  return {
+    editFile: (file: string, callback: (content: string) => string) => {
+      const filepath = resolve(root, file)
+      if (!files.has(filepath)) {
+        throw new Error(`file ${file} is outside of the test file system`)
+      }
+      const content = fs.readFileSync(filepath, 'utf-8')
+      fs.writeFileSync(filepath, callback(content))
+    },
+    createFile: (file: string, content: string) => {
+      if (file.startsWith('..')) {
+        throw new Error(`file ${file} is outside of the test file system`)
+      }
+      const filepath = resolve(root, file)
+      if (!files.has(filepath)) {
+        throw new Error(`file ${file} already exists in the test file system`)
+      }
+      createFile(filepath, content)
+    },
+  }
+}
+
+export async function runInlineTests(
+  structure: Record<string, string | ViteUserConfig | WorkspaceProjectConfiguration[]>,
+  config?: UserConfig,
+) {
+  const root = resolve(process.cwd(), `vitest-test-${crypto.randomUUID()}`)
+  const fs = useFS(root, structure)
+  const vitest = await runVitest({
+    root,
+    ...config,
+  })
+  return {
+    fs,
+    ...vitest,
+  }
+}
diff --git a/test/watch/test/config-watching.test.ts b/test/watch/test/config-watching.test.ts
@@ -0,0 +1,93 @@
+import { expect, test } from 'vitest'
+import { runInlineTests } from '../../test-utils'
+
+const ts = String.raw
+
+test('reruns tests when configs change', async () => {
+  const { fs, vitest } = await runInlineTests({
+    'vitest.workspace.ts': [
+      './project-1',
+      './project-2',
+    ],
+    'vitest.config.ts': {},
+    'project-1/vitest.config.ts': {},
+    'project-1/basic.test.ts': ts`
+      import { test } from 'vitest'
+      test('basic test 1', () => {})
+    `,
+    'project-2/vitest.config.ts': {},
+    'project-2/basic.test.ts': ts`
+      import { test } from 'vitest'
+      test('basic test 2', () => {})
+    `,
+  }, { watch: true })
+
+  await vitest.waitForStdout('Waiting for file changes')
+  vitest.resetOutput()
+
+  // editing the project config should trigger a restart
+  fs.editFile('./project-1/vitest.config.ts', c => `\n${c}`)
+
+  await vitest.waitForStdout('Restarting due to config changes...')
+  await vitest.waitForStdout('Waiting for file changes')
+  vitest.resetOutput()
+
+  // editing the root config should trigger a restart
+  fs.editFile('./vitest.config.ts', c => `\n${c}`)
+
+  await vitest.waitForStdout('Restarting due to config changes...')
+  await vitest.waitForStdout('Waiting for file changes')
+  vitest.resetOutput()
+
+  // editing the workspace config should trigger a restart
+  fs.editFile('./vitest.workspace.ts', c => `\n${c}`)
+
+  await vitest.waitForStdout('Restarting due to config changes...')
+  await vitest.waitForStdout('Waiting for file changes')
+})
+
+test('rerun stops the previous browser server and restarts multiple times without port mismatch', async () => {
+  const { fs, vitest } = await runInlineTests({
+    'vitest.workspace.ts': [
+      './project-1',
+    ],
+    'vitest.config.ts': {},
+    'project-1/vitest.config.ts': {
+      test: {
+        browser: {
+          enabled: true,
+          name: 'chromium',
+          provider: 'playwright',
+          headless: true,
+        },
+      },
+    },
+    'project-1/basic.test.ts': ts`
+      import { test } from 'vitest'
+      test('basic test 1', () => {})
+    `,
+  }, { watch: true })
+
+  await vitest.waitForStdout('Waiting for file changes')
+  vitest.resetOutput()
+
+  // editing the project config the first time restarts the browser server
+  fs.editFile('./project-1/vitest.config.ts', c => `\n${c}`)
+
+  await vitest.waitForStdout('Restarting due to config changes...')
+  await vitest.waitForStdout('Waiting for file changes')
+
+  expect(vitest.stdout).not.toContain('is in use, trying another one...')
+  expect(vitest.stderr).not.toContain('is in use, trying another one...')
+  vitest.resetOutput()
+
+  // editing the project the second time also restarts the server
+  fs.editFile('./project-1/vitest.config.ts', c => `\n${c}`)
+
+  await vitest.waitForStdout('Restarting due to config changes...')
+  await vitest.waitForStdout('Waiting for file changes')
+
+  expect(vitest.stdout).not.toContain('is in use, trying another one...')
+  expect(vitest.stderr).not.toContain('is in use, trying another one...')
+  vitest.resetOutput()
+})
diff --git a/test/watch/vitest.config.ts b/test/watch/vitest.config.ts
@@ -1,6 +1,11 @@
 import { defineConfig } from 'vitest/config'
 
 export default defineConfig({
+  server: {
+    watch: {
+      ignored: ['**/fixtures/**'],
+    },
+  },
   test: {
     reporters: 'verbose',
     include: ['test/**/*.test.*'],
diff --git a/test/workspaces-browser/space_browser/vitest.config.ts b/test/workspaces-browser/space_browser/vitest.config.ts
@@ -4,9 +4,9 @@ export default defineProject({
   test: {
     browser: {
       enabled: true,
-      name: process.env.BROWSER || 'chrome',
+      name: process.env.BROWSER || 'chromium',
       headless: true,
-      provider: process.env.PROVIDER || 'webdriverio',
+      provider: process.env.PROVIDER || 'playwright',
     },
   },
 })
diff --git a/test/workspaces-browser/vitest.workspace.ts b/test/workspaces-browser/vitest.workspace.ts
@@ -8,9 +8,9 @@ export default defineWorkspace([
       root: './space_browser_inline',
       browser: {
         enabled: true,
-        name: process.env.BROWSER || 'chrome',
+        name: process.env.BROWSER || 'chromium',
         headless: true,
-        provider: process.env.PROVIDER || 'webdriverio',
+        provider: process.env.PROVIDER || 'playwright',
       },
       alias: {
         'test-alias-from-vitest': new URL('./space_browser_inline/test-alias-to.ts', import.meta.url).pathname,

Original file line number	Diff line number	Diff line change
`@@ -432,6 +432,7 @@ export class WorkspaceProject {`
`432`	`432`	`)`
`433`	`433`	`}`
`434`	`434`
	`435`	`+ this.closingPromise = undefined`
`435`	`436`	`this.testProject = new TestProject(this)`
`436`	`437`
`437`	`438`	`this.server = server`
`@@ -476,7 +477,7 @@ export class WorkspaceProject {`
`476`	`477`	`if (!this.closingPromise) {`
`477`	`478`	`this.closingPromise = Promise.all(`
`478`	`479`	`[`
`479`		`- this.server.close(),`
	`480`	`+ this.server?.close(),`
`480`	`481`	`this.typechecker?.stop(),`
`481`	`482`	`this.browser?.close(),`
`482`	`483`	`this.clearTmpDir(),`