Skip to content

fix(reporter): allow dot reporter to work in non interactive terminals #7994

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

Merged
merged 5 commits into from
Jun 4, 2025

Conversation

bstephen1
Copy link
Contributor

Description

Updates dot reporter to support non-TTY terminals instead of silently switching to basic reporter. Closes #3932

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • Run the tests with pnpm test:ci.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

Copy link

netlify bot commented May 17, 2025

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 8aeb372
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/683fe09e7fbbd80008665c29
😎 Deploy Preview https://deploy-preview-7994--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@AriPerkkio AriPerkkio changed the title fix(dot): allow dot reporter to work in non interactive terminals fix(reporter): allow dot reporter to work in non interactive terminals May 18, 2025
Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

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

We definitely do not want to make changes to logger.ts just for dot reporter.

Instead, the dot reporter should maybe print multiple test results at once instead of one-by-one, when in non-TTY.

@bstephen1
Copy link
Contributor Author

I don't think there's a way to ensure the next console log will print with a prepending newline unless there's a wrapper like that in the logger to determine if the last write was a dot. If you print all the tests in a module at once then the next console log will still print on the same line since writing dots will never include a newline. The only way to avoid it completely would be to print all the tests at once at the end, where you could explicitly add the newline since the tests are done.

I would assume if you're running the dot reporter in CI you probably are suppressing console logs to have minimal output so I'm not sure if it would really be an issue to print them on the same line. I would think the typical output would just be the dots if everything passed. I tried it with the playwright dot reporter and it prints console logs inline too.
playwright no html

@AriPerkkio
Copy link
Member

We should not try to intercept the console logs at all here. Instead, let's not write test results one-by-one on onTestCaseResult - let's instead write test cases in batches at onTestModuleEnd.

Removing this isTTY condition should make the reporter print tests as soon as there are enough finished tests to fill a single terminal row:

onTestModuleEnd(testModule: TestModule): void {
super.onTestModuleEnd(testModule)
if (!this.isTTY) {
return
}
const columns = this.ctx.logger.getColumns()

@bstephen1
Copy link
Contributor Author

bstephen1 commented Jun 3, 2025

Is that worth the tradeoff of not printing tests as they come in? You won't be able to check in on the test runner as it's running. It can take a long time to get through a whole row of dots, especially if you're testing browser code. You might not even have enough tests for a full row of dots.

I did some demos with other dot reporters and I couldn't find any that work like that. They all print tests as they finish and either print console logs in line or don't print them at all.

node -- ignores all console logs
node dot test

mocha -- prints inline
mocha dot test

cypress -- ignores all console logs
cypress dot reporter

playwright -- prints inline
playwright no html

japa -- prints inline
japa dot reporter

I would propose the behavior from the first commit that doesn't touch the logger, prints tests as they finish, and has no extra handling for console logs.

@AriPerkkio
Copy link
Member

I would propose the behavior from the first commit that doesn't touch the logger, prints tests as they finish, and has no extra handling for console logs.

Yeah, you are right. There's no easy way to make this work perfectly. When we rewrote Vitest's reporters, this part was something we spent a lot of time optimizing in. But that was just for the TTY mode where we can control the terminal completely.

Let's do this how it was in the first commit. It seems to be inline with other test runners. Thanks for testing other tools too! 💯

@bstephen1 bstephen1 force-pushed the fix-dot-reporter-ci branch from 93a0021 to c46ac09 Compare June 4, 2025 03:00
@AriPerkkio AriPerkkio force-pushed the fix-dot-reporter-ci branch from d6e1f0f to 5cff550 Compare June 4, 2025 06:01
Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the PR @bstephen1! 🎉

@AriPerkkio AriPerkkio merged commit 6db9f52 into vitest-dev:main Jun 4, 2025
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vitest dot reporter is not used in CI (jenkins)
2 participants