From 30c21e6681215877fbe64e6ae6ee6e3c0ba88cc1 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Mon, 17 Aug 2026 18:01:30 +0900 Subject: [PATCH] fix(ui): synchronize watch-run UI state on file removal (#10941) Co-authored-by: Hiroshi Ogawa <4232207+hi-ogawa@users.noreply.github.com> Co-authored-by: OpenCode (claude-opus-4-8) --- .../ui/client/composables/client/index.ts | 15 ++- .../ui/client/composables/client/state.ts | 72 ++++++++------ .../ui/client/composables/explorer/tree.ts | 30 ++++++ .../ui/client/composables/explorer/utils.ts | 35 +++++++ packages/vitest/src/api/setup.ts | 11 +++ packages/vitest/src/api/types.ts | 1 + test/ui/fixtures/watch-updates/basic.test.ts | 19 ++++ test/ui/fixtures/watch-updates/second.test.ts | 5 + test/ui/test/watch-updates.spec.ts | 99 +++++++++++++++++++ 9 files changed, 257 insertions(+), 30 deletions(-) create mode 100644 test/ui/fixtures/watch-updates/basic.test.ts create mode 100644 test/ui/fixtures/watch-updates/second.test.ts create mode 100644 test/ui/test/watch-updates.spec.ts diff --git a/packages/ui/client/composables/client/index.ts b/packages/ui/client/composables/client/index.ts index ba2f47152..ae90da9a0 100644 --- a/packages/ui/client/composables/client/index.ts +++ b/packages/ui/client/composables/client/index.ts @@ -39,19 +39,30 @@ function createVitestClient(): VitestClient { explorerTree.recordTestArtifact(testId, artifact) }, onSpecsCollected(specs, startTime) { - specs?.forEach(([config, file]) => { - client.state.clearFiles({ config }, [file]) + // Run-start specifications identify files before their task trees are collected. + specs?.forEach((spec) => { + client.state.clearFiles(spec) }) explorerTree.startTime = startTime || performance.now() }, onCollected(files) { + // Collection supplies complete task trees that supersede any file placeholders. client.state.collectFiles(files) + if (files) { + explorerTree.pruneStaleTasks(files) + } }, onTaskUpdate(packs: RunnerTaskResultPack[], events: RunnerTaskEventPack[]) { client.state.updateTasks(packs) explorerTree.resumeRun(packs, events) testRunState.value = 'running' }, + onTestRemoved(path?: string) { + if (path) { + client.state.removeFile(path) + explorerTree.removeFile(path) + } + }, onUserConsoleLog(log) { client.state.updateUserLog(log) }, diff --git a/packages/ui/client/composables/client/state.ts b/packages/ui/client/composables/client/state.ts index 3ae50a51c..13b573be3 100644 --- a/packages/ui/client/composables/client/state.ts +++ b/packages/ui/client/composables/client/state.ts @@ -3,6 +3,7 @@ import type { RunnerTask, RunnerTaskResultPack, RunnerTestFile, + SerializedTestSpecification, TestTagDefinition, UserConsoleLog, } from 'vitest' @@ -66,40 +67,55 @@ export class StateManager { } otherProject.push(file) this.filesMap.set(file.filepath, otherProject) + this.clearFileTaskIds(file.id) this.updateId(file) }) } - // this file is reused by ws-client, and should not rely on heavy dependencies like workspace - clearFiles( - _project: { config: { name: string | undefined; root: string } }, - paths: string[] = [], - ): void { - const project = _project - paths.forEach((path) => { - const files = this.filesMap.get(path) - const fileTask = createFileTask( - path, - project.config.root, - project.config.name || '', - ) - fileTask.local = true - this.idMap.set(fileTask.id, fileTask) - if (!files) { - this.filesMap.set(path, [fileTask]) - return - } - const filtered = files.filter( - file => file.projectName !== project.config.name, - ) - // always keep a File task, so we can associate logs with it - if (!filtered.length) { - this.filesMap.set(path, [fileTask]) + private clearFileTaskIds(fileId: string): void { + // this assumes each task id is prefixed with file id (see `TaskBase.id` jsdoc) + const prefix = `${fileId}_` + for (const id of this.idMap.keys()) { + if (id === fileId || id.startsWith(prefix)) { + this.idMap.delete(id) } - else { - this.filesMap.set(path, [...filtered, fileTask]) + } + } + + removeFile(filepath: string): void { + const files = this.filesMap.get(filepath) + if (files) { + for (const file of files) { + this.clearFileTaskIds(file.id) } - }) + } + this.filesMap.delete(filepath) + } + + /** Stage selected files as local placeholders for logs emitted during collection. */ + clearFiles([project, path]: SerializedTestSpecification): void { + const files = this.filesMap.get(path) + const fileTask = createFileTask( + path, + project.root, + project.name || '', + ) + fileTask.local = true + this.idMap.set(fileTask.id, fileTask) + if (!files) { + this.filesMap.set(path, [fileTask]) + return + } + const filtered = files.filter( + file => file.projectName !== project.name, + ) + // always keep a File task, so we can associate logs with it + if (!filtered.length) { + this.filesMap.set(path, [fileTask]) + } + else { + this.filesMap.set(path, [...filtered, fileTask]) + } } updateId(task: RunnerTask): void { diff --git a/packages/ui/client/composables/explorer/tree.ts b/packages/ui/client/composables/explorer/tree.ts index a53c87918..b8a8ae663 100644 --- a/packages/ui/client/composables/explorer/tree.ts +++ b/packages/ui/client/composables/explorer/tree.ts @@ -14,7 +14,9 @@ import { runFilter } from '~/composables/explorer/filter' import { filter, searchMatcher, + uiFiles, } from '~/composables/explorer/state' +import { isParentNode, pruneStaleChildren, removeNodeSubtree } from '~/composables/explorer/utils' export class ExplorerTree { private rafCollector: ReturnType @@ -100,6 +102,27 @@ export class ExplorerTree { } } + /** After recollection, trim nodes when a task list shrank or a position changed between suite and test. */ + pruneStaleTasks(files: File[]) { + for (let i = 0; i < files.length; i++) { + const file = files[i] + const fileNode = this.nodes.get(file.id) + if (fileNode && isParentNode(fileNode)) { + pruneStaleChildren(this.nodes, fileNode, file.tasks) + } + } + } + + /** Remove all file nodes matching a filepath across projects and recalculate the explorer view. */ + removeFile(filepath: string) { + for (const fileNode of this.root.tasks.filter(file => file.filepath === filepath)) { + removeNodeSubtree(this.nodes, fileNode) + this.root.tasks.splice(this.root.tasks.indexOf(fileNode), 1) + } + uiFiles.value = [...this.root.tasks] + this.collect(false, true) + } + endRun(executionTime = performance.now() - this.startTime) { this.executionTime = executionTime this.rafCollector.pause() @@ -111,6 +134,13 @@ export class ExplorerTree { this.collect(false, false) } + /** + * Synchronize task nodes, summary counts, and filtered entries with the runner state. + * + * @param start Reset summary counters before updates when true; skip the reset when false. + * @param end Traverse every file and finalize the run when true; process only pending files when false. + * @param task Invoke the collector in a microtask when true; invoke it immediately when false. + */ private collect(start: boolean, end: boolean, task = true) { if (task) { queueMicrotask(() => { diff --git a/packages/ui/client/composables/explorer/utils.ts b/packages/ui/client/composables/explorer/utils.ts index a414d5f7a..a5625942f 100644 --- a/packages/ui/client/composables/explorer/utils.ts +++ b/packages/ui/client/composables/explorer/utils.ts @@ -71,6 +71,12 @@ export function getSortedRootTasks(sort: SortUIType, tasks = explorerTree.root.t return sorted } +/** + * Create or update the explorer node that mirrors a runner file. + * + * @param file Runner file whose explorer node should be synchronized. + * @param collect Synchronize all descendant task nodes when true; update only the file node when false. + */ export function createOrUpdateFileNode( file: File, collect = false, @@ -232,3 +238,32 @@ export function createOrUpdateNode( } } } + +export function pruneStaleChildren(nodes: Map, parentNode: ParentTreeNode, tasks: Task[]) { + const taskById = new Map(tasks.map(task => [task.id, task] as const)) + for (const child of [...parentNode.tasks]) { + const task = taskById.get(child.id) + if (!task || task.type !== child.type) { + removeNodeSubtree(nodes, child) + } + else if (isParentNode(child) && task.type === 'suite') { + pruneStaleChildren(nodes, child, task.tasks) + } + } +} + +export function removeNodeSubtree(nodes: Map, node: UITaskTreeNode) { + if (isParentNode(node)) { + for (const child of [...node.tasks]) { + removeNodeSubtree(nodes, child) + } + } + nodes.delete(node.id) + const parent = nodes.get(node.parentId) + if (parent && isParentNode(parent) && parent.children.delete(node.id)) { + const index = parent.tasks.findIndex(task => task.id === node.id) + if (index !== -1) { + parent.tasks.splice(index, 1) + } + } +} diff --git a/packages/vitest/src/api/setup.ts b/packages/vitest/src/api/setup.ts index bf36743da..7f7441979 100644 --- a/packages/vitest/src/api/setup.ts +++ b/packages/vitest/src/api/setup.ts @@ -196,6 +196,7 @@ export function setup(ctx: Vitest, _server?: ViteDevServer): void { 'onFinishedReportCoverage', 'onCollected', 'onTaskUpdate', + 'onTestRemoved', ], serialize: (data: any) => stringify(data, stringifyReplace), deserialize: parse, @@ -275,6 +276,16 @@ export class WebSocketReporter implements Reporter { }) } + onTestRemoved(trigger?: string): void { + if (this.clients.size === 0) { + return + } + + this.clients.forEach((client) => { + client.onTestRemoved?.(trigger)?.catch?.(noop) + }) + } + private sum(items: T[], cb: (_next: T) => number | undefined) { return items.reduce((total, next) => { return total + Math.max(cb(next) || 0, 0) diff --git a/packages/vitest/src/api/types.ts b/packages/vitest/src/api/types.ts index a5808c531..1944a8ae4 100644 --- a/packages/vitest/src/api/types.ts +++ b/packages/vitest/src/api/types.ts @@ -77,6 +77,7 @@ export interface WebSocketEvents { onTestAnnotate?: (testId: string, annotation: TestAnnotation) => Awaitable onTestArtifactRecord?: (testId: string, artifact: TestArtifact) => Awaitable onTaskUpdate?: (packs: TaskResultPack[], events: TaskEventPack[]) => Awaitable + onTestRemoved?: (path?: string) => Awaitable onUserConsoleLog?: (log: UserConsoleLog) => Awaitable onPathsCollected?: (paths?: string[]) => Awaitable onSpecsCollected?: (specs?: SerializedTestSpecification[], startTime?: number) => Awaitable diff --git a/test/ui/fixtures/watch-updates/basic.test.ts b/test/ui/fixtures/watch-updates/basic.test.ts new file mode 100644 index 000000000..2c525dc2a --- /dev/null +++ b/test/ui/fixtures/watch-updates/basic.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, test } from 'vitest' + +test('reconcile-keep', () => { + expect(1 + 1).toBe(2) +}) + +// TEST TASK TYPE CHANGE START +describe('reconcile-type-suite', () => { + test('reconcile-type-child', () => { + expect(3 + 3).toBe(6) + }) +}) +// TEST TASK TYPE CHANGE END + +// TEST REMOVE START +test('reconcile-remove-me', () => { + expect(2 + 2).toBe(4) +}) +// TEST REMOVE END diff --git a/test/ui/fixtures/watch-updates/second.test.ts b/test/ui/fixtures/watch-updates/second.test.ts new file mode 100644 index 000000000..b4c9a4ab5 --- /dev/null +++ b/test/ui/fixtures/watch-updates/second.test.ts @@ -0,0 +1,5 @@ +import { expect, test } from 'vitest' + +test('reconcile-second-file', () => { + expect(true).toBe(true) +}) diff --git a/test/ui/test/watch-updates.spec.ts b/test/ui/test/watch-updates.spec.ts new file mode 100644 index 000000000..128ddd627 --- /dev/null +++ b/test/ui/test/watch-updates.spec.ts @@ -0,0 +1,99 @@ +import type { Vitest } from 'vitest/node' +import fs from 'node:fs' +import path from 'node:path' +import { expect, test } from '@playwright/test' +import { assertTestCounts, getExplorerItem, startVitestUi } from './helper' + +// Regression tests for explorer tree reconciliation on watch re-runs: +// - removing a test from a file must drop the stale test node (no ghost node) +// - changing a task type must replace the incompatible node with the reused id +// - deleting a test file must drop the stale file node (onTestRemoved forwarding) +test.describe('explorer watch updates', () => { + let vitest: Vitest | undefined + let baseURL: string + + const root = path.join(import.meta.dirname, '../fixtures/watch-updates') + const basicFile = path.join(root, 'basic.test.ts') + const secondFile = path.join(root, 'second.test.ts') + const basicContent = fs.readFileSync(basicFile, 'utf-8') + const secondContent = fs.readFileSync(secondFile, 'utf-8') + + test.beforeAll(async () => { + const server = await startVitestUi({ + root, + watch: true, + ui: true, + open: false, + reporters: [], + }) + vitest = server.vitest + baseURL = server.url + }) + + test.afterAll(async () => { + await vitest?.close() + fs.writeFileSync(basicFile, basicContent, 'utf-8') + fs.writeFileSync(secondFile, secondContent, 'utf-8') + }) + + test('prunes stale tasks and removes deleted files', async ({ page }) => { + await page.goto(baseURL) + + // initial: 3 tests in basic.test.ts + 1 test in second.test.ts + await assertTestCounts(page, { pass: 4, fail: 0 }) + await expect(getExplorerItem(page, 'reconcile-keep')).toBeVisible() + await expect(getExplorerItem(page, 'reconcile-type-suite')).toBeVisible() + await expect(getExplorerItem(page, 'reconcile-remove-me')).toBeVisible() + await expect(getExplorerItem(page, 'reconcile-second-file')).toBeVisible() + + // select then remove a single test from basic.test.ts and let watch mode re-run + await getExplorerItem(page, 'reconcile-remove-me').click() + await expect(page.getByTestId('file-detail')).toContainText('reconcile-remove-me') + fs.writeFileSync( + basicFile, + basicContent.replace( + /\/\/ TEST REMOVE START[\s\S]*?\/\/ TEST REMOVE END\n/, + '', + ), + 'utf-8', + ) + + // the removed test node must disappear (no ghost node), the kept one stays + await expect(getExplorerItem(page, 'reconcile-remove-me')).toHaveCount(0) + await expect(page.getByTestId('file-detail')).not.toContainText('reconcile-remove-me') + await expect(getExplorerItem(page, 'reconcile-keep')).toBeVisible() + await page.getByRole('button', { name: 'Show dashboard' }).click() + await assertTestCounts(page, { pass: 3, fail: 0 }) + + // replace a suite with a test at the same position-based id + fs.writeFileSync( + basicFile, + fs.readFileSync(basicFile, 'utf-8').replace( + /\/\/ TEST TASK TYPE CHANGE START[\s\S]*?\/\/ TEST TASK TYPE CHANGE END\n/, + `// TEST TASK TYPE CHANGE START +test('reconcile-type-test', () => { + expect(3 + 3).toBe(6) +}) +// TEST TASK TYPE CHANGE END +`, + ), + 'utf-8', + ) + + await expect(getExplorerItem(page, 'reconcile-type-suite')).toHaveCount(0) + await expect(getExplorerItem(page, 'reconcile-type-test')).toBeVisible() + await assertTestCounts(page, { pass: 3, fail: 0 }) + + // select then delete an entire test file and let the watcher emit onTestRemoved + await getExplorerItem(page, 'reconcile-second-file').click() + await expect(page.getByTestId('file-detail')).toContainText('reconcile-second-file') + fs.rmSync(secondFile) + + // the deleted file's test node must disappear (no ghost file node) + await expect(getExplorerItem(page, 'reconcile-second-file')).toHaveCount(0) + await expect(page.getByTestId('file-detail')).toHaveCount(0) + await expect(getExplorerItem(page, 'reconcile-keep')).toBeVisible() + await page.getByRole('button', { name: 'Show dashboard' }).click() + await assertTestCounts(page, { pass: 2, fail: 0 }) + }) +}) -- 2.51.2