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

fix(reporters): rewrite dot reporter without log-update #6943

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

AriPerkkio
Copy link
Member

@AriPerkkio AriPerkkio commented Nov 21, 2024

Description

  • Fixes issues where log-update hides process.stdout calls that come outside Vitest, e.g. NodeJS deprecation warnings. This is visible for example in Vite repo's test-serve command.
  • Fixes issues where log-update got confused of row count and started to duplicate rows
  • Improves performance as now we don't render finished tests all the time. There's some logic in DotSummary's onTestFileFinished that optimizes this.
  • Removes usage of log-update from dot reporter.

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:.

@AriPerkkio AriPerkkio changed the title fix(reporters): rewrite dot renderer without log-update fix(reporters): rewrite dot reporter without log-update Nov 21, 2024
Copy link

netlify bot commented Nov 21, 2024

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 967104c
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/674570ae1717360009119997
😎 Deploy Preview https://deploy-preview-6943--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 site configuration.

@AriPerkkio AriPerkkio force-pushed the fix/dot-reporter-rewrite branch 2 times, most recently from 967104c to d7a87a0 Compare November 26, 2024 08:14
Copy link

pkg-pr-new bot commented Nov 26, 2024

@vitest/browser

npm i https://pkg.pr.new/@vitest/browser@6943

@vitest/coverage-istanbul

npm i https://pkg.pr.new/@vitest/coverage-istanbul@6943

@vitest/mocker

npm i https://pkg.pr.new/@vitest/mocker@6943

@vitest/expect

npm i https://pkg.pr.new/@vitest/expect@6943

@vitest/pretty-format

npm i https://pkg.pr.new/@vitest/pretty-format@6943

@vitest/coverage-v8

npm i https://pkg.pr.new/@vitest/coverage-v8@6943

@vitest/runner

npm i https://pkg.pr.new/@vitest/runner@6943

@vitest/snapshot

npm i https://pkg.pr.new/@vitest/snapshot@6943

@vitest/spy

npm i https://pkg.pr.new/@vitest/spy@6943

@vitest/ui

npm i https://pkg.pr.new/@vitest/ui@6943

@vitest/utils

npm i https://pkg.pr.new/@vitest/utils@6943

vite-node

npm i https://pkg.pr.new/vite-node@6943

vitest

npm i https://pkg.pr.new/vitest@6943

@vitest/web-worker

npm i https://pkg.pr.new/@vitest/web-worker@6943

@vitest/ws-client

npm i https://pkg.pr.new/@vitest/ws-client@6943

commit: 1eb3bfb

@AriPerkkio AriPerkkio force-pushed the fix/dot-reporter-rewrite branch 4 times, most recently from 82e32e9 to 287433e Compare November 26, 2024 09:24
@AriPerkkio AriPerkkio force-pushed the fix/dot-reporter-rewrite branch from 287433e to 1eb3bfb Compare November 26, 2024 13:26
@AriPerkkio AriPerkkio marked this pull request as ready for review November 26, 2024 13:33
async onWatcherRerun(files: string[], trigger?: string) {
await this.stopListRender()
super.onWatcherRerun(files, trigger)
onTestFilePrepare(file: File): void {
Copy link
Member

Choose a reason for hiding this comment

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

I just noticed we are still using the old API here... We need to experiment with the new one to see what's missing. Otherwise, it will never be out of the experimental

I don't know if we should do it here, but keep in mind that it needs to be rewritten before 2.2 (I can do it myself, if no one has resources)

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean the File part, as in we should use state.getReportedEntity instead?

When building the summary reporter I had at first that one in use and also some new Reporter API hooks. But as nothing was working well I started to reduce amount of changes. First fix all reporter flickering and scrolling issues and then look into new APIs.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the File part. Everything else looks nice 👍🏻

@AriPerkkio AriPerkkio merged commit be969cf into vitest-dev:main Nov 26, 2024
18 checks passed
@AriPerkkio AriPerkkio deleted the fix/dot-reporter-rewrite branch November 26, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants