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.

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

Print errors when test runner is interrupted #3373

Closed
4 tasks done
gajus opened this issue May 14, 2023 · 3 comments · Fixed by #3407
Closed
4 tasks done

Print errors when test runner is interrupted #3373

gajus opened this issue May 14, 2023 · 3 comments · Fixed by #3407
Labels
enhancement New feature or request

Comments

@gajus
Copy link

gajus commented May 14, 2023

Clear and concise description of the problem

Currently, if test runner is interrupted with sigint, it just exits without producing useful output.

Suggested solution

If test runner is interrupted with sigint, then the test runner should print errors before exiting.

❯ |@contra/contra-api| src/foo.test.ts (15) 370ms
✓ foo
× bar
· baz
^C

After ^C was received, I would expect runner to provide context about × bar failure.

Alternative

No response

Additional context

No response

Validations

@AriPerkkio
Copy link
Member

I think this would be useful improvement.

Maybe we should intercept the SIGINT signal sent from CTRL+C and gracefully stop the test execution and print the final report - exactly as we do in --bail and watch-mode test interruption. But this would mean that CTRL+C wouldn't stop tests immediately. Instead it would wait for currently running tests to finish and then stop execution. If we don't stop test execution gracefully, the final test report will include errors from forcefully terminated workers.

For vitest run it would be something as simple as:

if (process.stdin.isTTY && ctx.config.watch)
registerConsoleShortcuts(ctx)

  if (process.stdin.isTTY && ctx.config.watch) {
    registerConsoleShortcuts(ctx)
  }
+  else {
+    process.on('SIGINT', async () => {
+      await ctx.cancelCurrentRun('keyboard-input')
+    })
+  }

And same for vitest --watch:

// ctrl-c or esc
if (str === '\x03' || str === '\x1B' || (key && key.ctrl && key.name === 'c'))
return ctx.exit(true)

    // ctrl-c or esc
    if (str === '\x03' || str === '\x1B' || (key && key.ctrl && key.name === 'c')) {
+     await ctx.cancelCurrentRun('keyboard-input')
      return ctx.exit(true)
    }

@gajus
Copy link
Author

gajus commented May 17, 2023

But this would mean that CTRL+C wouldn't stop tests immediately.

Do it so that the first SIGINT starts graceful shutdown, but a second SIGINT is received it is forced terminated.

@AriPerkkio
Copy link
Member

That's great idea, I think it would work perfectly.

After looking at this a while it seems that there are couple of process.on("SIGINT") listeners added by dependencies. We may need to overwrite stdin.on("keypress") even when not in watch mode for this. Otherwise the listeners set by dependencies will race with the new listener.

@AriPerkkio AriPerkkio added the enhancement New feature or request label May 19, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants