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.

Sign up for GitHub

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

Jump to bottom

fix: stop the runner before restarting, restart on workspace config change #6859

Merged
merged 10 commits into from
Nov 8, 2024
Merged

fix: stop the runner before restarting, restart on workspace config change #6859

merged 10 commits into from
Nov 8, 2024

Conversation

sheremet-va
Copy link
Member

Description

Fixes #6738
FIxes #5327

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

@netlify Netlify
Copy link

netlify bot commented Nov 5, 2024

✅ Deploy Preview for vitest-dev ready!

Name Link
🔨 Latest commit 9641b6d
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/672a204e243cb40008f0926f
😎 Deploy Preview https://deploy-preview-6859--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.

hi-ogawa
hi-ogawa reviewed Nov 6, 2024
View reviewed changes
Copy link
Contributor

@hi-ogawa hi-ogawa 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 to me 👍 Tested locally with pnpm -C examples/workspace test and pnpm -C test/browser test-fixtures.
Can you add relevant tests?

@sheremet-va
Copy link
Member Author

@hi-ogawa @AriPerkkio added the test. interested to know your opinion on the useFS helper

@AriPerkkio
Copy link
Member

Looks handy and easy to read. Sometimes it's nicer to have static fixtures directory but in cases like this where they are not shared between multiple tests, re-creating fixtures on test run makes sense.

The reruns tests when configs change test case does a lot of file writing so it might be slow on CI. Looks like it did pass though.

@sheremet-va
Copy link
Member Author

The reruns tests when configs change test case does a lot of file writing so it might be slow on CI. Looks like it did pass though.

Maybe we can emit the event manually instead of waiting then 🤔

@sheremet-va
Copy link
Member Author

It takes 1200-1300ms in CI, the same locally for me. Takes the longest on Windows - 1700ms, but changing to use watcher.emit didn't improve times significantly 🤷🏻

hi-ogawa
hi-ogawa previously approved these changes Nov 7, 2024
View reviewed changes
Copy link
Contributor

@hi-ogawa hi-ogawa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

interested to know your opinion on the useFS helper

It looks nice for small things and I like it 👍
But for complicated setup, I prefer having files committed in the repo as it allows vitest --root (some-fixture-dir) to debug and iterate easily from cli.

Btw, currently root fixtures/config-watching nor any directories inside are not removed. Should we cleanup root entirely? (though I might even want a mode with env var where it leaves files without being deleted to debug later)

Just for future references, sharing a few projects I know doing something similar:

From my experience, I wasn't fond of Remix's huge fixture as it was often hard to iterate and debug. But, looking at playwright's ones, they make use of a lot of small runInlineTest and it looks nice.

@sheremet-va
Copy link
Member Author

Should we cleanup root entirely? (though I might even want a mode with env var where it leaves files without being deleted to debug later)

Removing the root now.

But, looking at playwright's ones, they make use of a lot of small runInlineTest and it looks nice.

Added a similar helper!

hi-ogawa
hi-ogawa approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

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

👍

@sheremet-va sheremet-va merged commit b01df47 into vitest-dev:main Nov 8, 2024
15 of 17 checks passed
@sheremet-va sheremet-va deleted the fix/close-process-correctly branch November 8, 2024 09:02
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Nov 13, 2024
@renovate
chore(deps): update dependency vitest to v2.1.5
916c364 
##### [v2.1.5](https://github.com/vitest-dev/vitest/releases/tag/v2.1.5)

#####    🚀 Features

-   **experimental**: Expose vite on the test project as a separate field  -  by [@sheremet-va](https://github.com/sheremet-va) [<samp>(bcc59)</samp>](vitest-dev/vitest@bcc59088)

#####    🐞 Bug Fixes

-   `dangerouslyIgnoreUnhandledErrors` without base reporter  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6808 [<samp>(0bf0a)</samp>](vitest-dev/vitest@0bf0ab00)
-   Capture `unhandledRejection` even when base reporter is not used  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6812 [<samp>(8878b)</samp>](vitest-dev/vitest@8878b04d)
-   Don't change the working directory when loading workspace projects  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6811 [<samp>(f0aea)</samp>](vitest-dev/vitest@f0aeaca8)
-   Remove `sequence.concurrent` from the `RuntimeConfig` type  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6880 [<samp>(6af73)</samp>](vitest-dev/vitest@6af73d93)
-   Stop the runner before restarting, restart on workspace config change  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6859 [<samp>(b01df)</samp>](vitest-dev/vitest@b01df47d)
-   Don't rerun on Esc or Ctrl-C during watch filter  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6895 [<samp>(98f76)</samp>](vitest-dev/vitest@98f76ea7)
-   Print ssrTransform error  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6885 [<samp>(4c96c)</samp>](vitest-dev/vitest@4c96cce7)
-   Throw an error and a warning if `.poll`, `.element`, `.rejects`/`.resolves`, and `locator.*` weren't awaited  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6877 [<samp>(93b67)</samp>](vitest-dev/vitest@93b67c24)
-   **browser**:
    -   Don't process the default css styles  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6861 [<samp>(0d67f)</samp>](vitest-dev/vitest@0d67f04b)
    -   Support non US key input  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6873 [<samp>(5969d)</samp>](vitest-dev/vitest@5969d8da)
    -   Stop the browser rpc when the pool is closed  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6858 [<samp>(9a0c9)</samp>](vitest-dev/vitest@9a0c93d7)
    -   Init browsers eagerly when tests are running  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6876 [<samp>(417bd)</samp>](vitest-dev/vitest@417bdb42)
-   **coverage**:
    -   Report uncovered files when re-run by `enter` or `'a'`  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6848 [<samp>(487c8)</samp>](vitest-dev/vitest@487c80ae)
-   **ui**:
    -   Remove crossorigin attributes for same origin assets  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6883 [<samp>(6e793)</samp>](vitest-dev/vitest@6e793c64)
-   **vite-node**:
    -   Improve esm check to decide external  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6816 [<samp>(7e1fa)</samp>](vitest-dev/vitest@7e1faf3d)
    -   Top-level throw in module is not reported properly  -  by [@vanaigr](https://github.com/vanaigr) and [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6840 [<samp>(cf0cb)</samp>](vitest-dev/vitest@cf0cbf6a)
-   **vitest**:
    -   Use fetch() implementation from happy-dom  -  by [@capricorn86](https://github.com/capricorn86) in vitest-dev/vitest#6879 [<samp>(3c56f)</samp>](vitest-dev/vitest@3c56feb4)

#####    🏎 Performance

-   Limit workspace project concurrency to available cores  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6869 [<samp>(f4c04)</samp>](vitest-dev/vitest@f4c04ab6)

#####     [View changes on GitHub](vitest-dev/vitest@v2.1.4...v2.1.5)
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Nov 13, 2024
@renovate
chore(deps): update dependency vitest to v2.1.5
58ba87f 
##### [v2.1.5](https://github.com/vitest-dev/vitest/releases/tag/v2.1.5)

#####    🚀 Features

-   **experimental**: Expose vite on the test project as a separate field  -  by [@sheremet-va](https://github.com/sheremet-va) [<samp>(bcc59)</samp>](vitest-dev/vitest@bcc59088)

#####    🐞 Bug Fixes

-   `dangerouslyIgnoreUnhandledErrors` without base reporter  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6808 [<samp>(0bf0a)</samp>](vitest-dev/vitest@0bf0ab00)
-   Capture `unhandledRejection` even when base reporter is not used  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6812 [<samp>(8878b)</samp>](vitest-dev/vitest@8878b04d)
-   Don't change the working directory when loading workspace projects  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6811 [<samp>(f0aea)</samp>](vitest-dev/vitest@f0aeaca8)
-   Remove `sequence.concurrent` from the `RuntimeConfig` type  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6880 [<samp>(6af73)</samp>](vitest-dev/vitest@6af73d93)
-   Stop the runner before restarting, restart on workspace config change  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6859 [<samp>(b01df)</samp>](vitest-dev/vitest@b01df47d)
-   Don't rerun on Esc or Ctrl-C during watch filter  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6895 [<samp>(98f76)</samp>](vitest-dev/vitest@98f76ea7)
-   Print ssrTransform error  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6885 [<samp>(4c96c)</samp>](vitest-dev/vitest@4c96cce7)
-   Throw an error and a warning if `.poll`, `.element`, `.rejects`/`.resolves`, and `locator.*` weren't awaited  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6877 [<samp>(93b67)</samp>](vitest-dev/vitest@93b67c24)
-   **browser**:
    -   Don't process the default css styles  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6861 [<samp>(0d67f)</samp>](vitest-dev/vitest@0d67f04b)
    -   Support non US key input  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6873 [<samp>(5969d)</samp>](vitest-dev/vitest@5969d8da)
    -   Stop the browser rpc when the pool is closed  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6858 [<samp>(9a0c9)</samp>](vitest-dev/vitest@9a0c93d7)
    -   Init browsers eagerly when tests are running  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6876 [<samp>(417bd)</samp>](vitest-dev/vitest@417bdb42)
-   **coverage**:
    -   Report uncovered files when re-run by `enter` or `'a'`  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6848 [<samp>(487c8)</samp>](vitest-dev/vitest@487c80ae)
-   **ui**:
    -   Remove crossorigin attributes for same origin assets  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6883 [<samp>(6e793)</samp>](vitest-dev/vitest@6e793c64)
-   **vite-node**:
    -   Improve esm check to decide external  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6816 [<samp>(7e1fa)</samp>](vitest-dev/vitest@7e1faf3d)
    -   Top-level throw in module is not reported properly  -  by [@vanaigr](https://github.com/vanaigr) and [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6840 [<samp>(cf0cb)</samp>](vitest-dev/vitest@cf0cbf6a)
-   **vitest**:
    -   Use fetch() implementation from happy-dom  -  by [@capricorn86](https://github.com/capricorn86) in vitest-dev/vitest#6879 [<samp>(3c56f)</samp>](vitest-dev/vitest@3c56feb4)

#####    🏎 Performance

-   Limit workspace project concurrency to available cores  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6869 [<samp>(f4c04)</samp>](vitest-dev/vitest@f4c04ab6)

#####     [View changes on GitHub](vitest-dev/vitest@v2.1.4...v2.1.5)
@renovate renovate bot mentioned this pull request Nov 18, 2024
Open
1 task
@renovate renovate bot mentioned this pull request Nov 25, 2024
Merged
1 task
@renovate renovate bot mentioned this pull request Dec 5, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AriPerkkio AriPerkkio AriPerkkio left review comments

@hi-ogawa hi-ogawa hi-ogawa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@sheremet-va @AriPerkkio @hi-ogawa