Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(vm-threads): tests not cancelled on key press, cancelled tests shown twice #4781

Merged
merged 2 commits into from
Dec 28, 2023
Merged

fix(vm-threads): tests not cancelled on key press, cancelled tests shown twice #4781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9baad15
fix(vm-threads): tests not cancelled on key press
AriPerkkio Dec 19, 2023
4298d79
fix(watch): cancelled tests are shown twice in reporter
AriPerkkio Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/vitest/src/node/pools/browser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function createBrowserPool(ctx: Vitest): ProcessPool {
if (project.config.browser.isolate) {
for (const path of paths) {
if (isCancelled) {
ctx.state.cancelFiles(files.slice(paths.indexOf(path)), ctx.config.root, project.getName())
ctx.state.cancelFiles(files.slice(paths.indexOf(path)), ctx.config.root, project.config.name)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to match with the collected files:

vitest/packages/runner/src/collect.ts

Line 28 in 9497f48

projectName: config.name,

This feels a bit hacky. The WorkspaceProject.getSerializableConfig() always returns undefined for .name instead of '' when there's no name set for the project (no workspaces used).

The state.clearFiles uses the projectName when comparing files. When they don't match, there can be duplicate entries where one is from collection and one from cancelling:

'/x/vitest-example-project/vitest/test/sum.2.test.js' => [
  {
    filepath: '/x/vitest-example-project/vitest/test/sum.2.test.js',
    id: '/x/vitest-example-project/vitest/test/sum.2.test.js',
    mode: 'skip',   // <-- From test cancel
    name: 'test/sum.2.test.js',
    projectName: '',
    type: 'suite',
    ...
  },
  {
    filepath: '/x/vitest-example-project/vitest/test/sum.2.test.js',
    mode: 'run',     // <-- From test collect
    name: 'test/sum.2.test.js',
    type: 'suite',
    ...
  }
],

These show up in reports like below:

test-cancel.mp4

break
}

Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/pools/child.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export function createChildProcessPool(ctx: Vitest, { execArgv, env, forksPath }

// Intentionally cancelled
else if (ctx.isCancelling && error instanceof Error && /The task has been cancelled/.test(error.message))
ctx.state.cancelFiles(files, ctx.config.root, project.getName())
ctx.state.cancelFiles(files, ctx.config.root, project.config.name)

else
throw error
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/pools/threads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function createThreadsPool(ctx: Vitest, { execArgv, env, workerPath }: Po

// Intentionally cancelled
else if (ctx.isCancelling && error instanceof Error && /The task has been cancelled/.test(error.message))
ctx.state.cancelFiles(files, ctx.config.root, project.getName())
ctx.state.cancelFiles(files, ctx.config.root, project.config.name)

else
throw error
Expand Down
5 changes: 4 additions & 1 deletion packages/vitest/src/node/pools/vm-threads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export function createVmThreadsPool(ctx: Vitest, { execArgv, env, vmPath }: Pool

// Intentionally cancelled
else if (ctx.isCancelling && error instanceof Error && /The task has been cancelled/.test(error.message))
ctx.state.cancelFiles(files, ctx.config.root, project.getName())
ctx.state.cancelFiles(files, ctx.config.root, project.config.name)

else
throw error
Expand All @@ -123,6 +123,9 @@ export function createVmThreadsPool(ctx: Vitest, { execArgv, env, vmPath }: Pool
}

return async (specs, invalidates) => {
// Cancel pending tasks from pool when possible
ctx.onCancel(() => pool.cancelPendingTasks())

const configs = new Map<WorkspaceProject, ResolvedConfig>()
const getConfig = (project: WorkspaceProject): ResolvedConfig => {
if (configs.has(project))
Expand Down