-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(watch): don't indicate exit when no matching files #6968
fix(watch): don't indicate exit when no matching files #6968
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. |
4c7f1d5
to
b82e5ea
Compare
@@ -426,7 +426,10 @@ export class Vitest { | |||
|
|||
this.logger.printNoTestFound(filters) | |||
|
|||
if (!this.config.watch || !(this.config.changed || this.config.related?.length)) { | |||
if (this.config.watch) { | |||
this.report('onFinished', [], []) |
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 feel like this breaks the lifecycle. There is usually at least onPathsCollected
/onSpecsCollected
call and onCollected
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.
Yup, currently this is getting stuck in onInit
which also breaks the lifecycle.
What would be better approach here? Should we call reporters' onInit
after length of files
has been checked?
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 think that would be a big breaking change, the assumption now is that we always call onInit
. Maybe we shouldn't start timers in onInit
?
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.
Not sure what test reporters do with onInit
in cases where test run didn't start. If we started timers in next onPathsCollected
hook, we would get faster runs times in summary 😄
Looks like the internal junit reporter opens fs handle in onInit
and closes it in onFinish
. That logic sounds good to me. It's quite unexpected that onFinished
is never called after onInit
.
@@ -426,7 +426,10 @@ export class Vitest { | |||
|
|||
this.logger.printNoTestFound(filters) | |||
|
|||
if (!this.config.watch || !(this.config.changed || this.config.related?.length)) { | |||
if (this.config.watch) { |
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.
this.globTestFiles
can now also throw an error if you specify the :
incorrectly and the banner won't disappear:
vitest some-test:10
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.
It can also throw if git is not initialised and --changed
flag is used
Maybe after #7069 this could be solved |
Description
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:
.