-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix(reporters): write buffered stdout/stderr on process exit #6932
fix(reporters): write buffered stdout/stderr on process exit #6932
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
27cb54e
to
d9f70ec
Compare
packages/vitest/src/node/reporters/renderers/windowedRenderer.ts
Outdated
Show resolved
Hide resolved
d9f70ec
to
416ef01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be happy if we can remove signal-exit
, but it's also fine to merge it as is
Do we want to implement our own |
I was hoping we would be able to drop it when |
If we need it for stdout handling, then sure let's keep it |
I can take a look at removing it completely once rest of But what we need is to be able to restore CLI cursor always, and make sure log buffer is flushed on exit. |
Looking at how I'll try in this PR to replace |
416ef01
to
97875c3
Compare
@@ -175,6 +174,38 @@ export class WindowRenderer { | |||
private write(message: string, type: 'output' | 'error' = 'output') { | |||
(this.streams[type] as Writable['write'])(message) | |||
} | |||
|
|||
private addProcessExitListeners() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point it's ok to have this here in windowedRenderer
. But later on it might make sense to move this to logger.ts
instead. 🤔
So that logger.ts
would handle cursor hiding and showing. It shoud also expose callback that reporters could hook into, so that windowedRenderer
could call flushBuffer
on exit too.
packages/vitest/src/node/reporters/renderers/windowedRenderer.ts
Outdated
Show resolved
Hide resolved
97875c3
to
ccba6a0
Compare
Description
logger.ts
writes errors for unhandled rejections, it exits immediatelly. The windowed-renderer intercepts allprocess.stdout/err
writes and buffers them. The buffered content must be written before exit.vitest/packages/vitest/src/node/logger.ts
Lines 306 to 317 in 78f07c0
process.exit
during tests. 🤔Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.