-
-
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
feat(runner): add "queued" state #6931
feat(runner): add "queued" state #6931
Conversation
✅ Deploy Preview for vitest-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@vitest/browser
@vitest/coverage-istanbul
@vitest/expect
@vitest/coverage-v8
@vitest/mocker
@vitest/pretty-format
@vitest/runner
@vitest/snapshot
@vitest/spy
@vitest/ui
@vitest/utils
vite-node
vitest
@vitest/web-worker
@vitest/ws-client
commit: |
Note: doesn't seem like it's working as I expected in a real project - https://github.com/zammad/zammad |
b47f145
to
138a163
Compare
138a163
to
753456b
Compare
Tested and it now looks very nice 👌🏻 |
@AriPerkkio it now shows (0/0) - should we print |
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.
@AriPerkkio it now shows (0/0) - should we print
[queued]
instead? 🤔
We should probably hide test count when testFile.total
is 0
:
vitest/packages/vitest/src/node/reporters/summary.ts
Lines 306 to 311 in 753456b
summary.push( | |
c.bold(c.yellow(` ${F_POINTER} `)) | |
+ formatProjectName(testFile.projectName) | |
+ testFile.filename | |
+ c.dim(` ${testFile.completed}/${testFile.total}`), | |
) |
Do you want to just remove it without having any labels?
|
@AriPerkkio any feedback? 👀 |
I will look into it. Any feedback on #6931 (comment) ? |
If label doesn't cause too much flickering, sure. Maybe the icon could also be different, though I'm not sure what color and shape would work best. |
Should be fixed |
Added another workaround. I agree would be nice to have this statistics in test suites themselves :D Maybe we can track it in the |
I can't really reproduce it - I've run it on the vite repo several times. |
Does it help reproducing if you add this in // after all other code
test('temp', async () => {
await new Promise((r) => setTimeout(r, 10_000))
}) E: Oh wait, no. Maybe it's related to the |
Adding this in if (testFile.completed > testFile.total) {
this.ctx.logger.log(`Error: ${testFile.completed} > ${testFile.total} ${testFile.filename}`)
} vitest/packages/vitest/src/node/reporters/summary.ts Lines 269 to 271 in 4e60333
We need might need to extend the check to - if (!stats) {
+ if (!stats || stats.total === 0) { This is all related to the fact that test runners can skip test and test file phases, e.g. test can be reported as finished even though it was never reported to start. I'll take closer look at this later today. |
Verified fix from f91a5f7 in Vite's During #7069 we'll be fixing this "prevent reporting tests finished if they haven't even been reported as starting" properly. We'll need some state logic in TaskParser (or in it's successor). Then we can remove some complex logic from |
Description
Fixes #6915
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:
.