-
-
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
feat: global setup #372
feat: global setup #372
Conversation
✔️ Deploy Preview for vitest-dev ready! 🔨 Explore the source changes: 62d95d5 🔍 Inspect the deploy log: https://app.netlify.com/sites/vitest-dev/deploys/61dbf28b8a7c0500079e63bd 😎 Browse the preview: https://deploy-preview-372--vitest-dev.netlify.app |
Great! About the API, is there a reason we want to use a file-based API instead of accepting the setup function in the config?
Is the idea that it could be less verbose because you normally will need to import this function from a file anyways? Maybe we could accept a function (or array of functions), but still, allow to pass a file name as a shortcut? (with the same API as you propose, but it looks better as an optional shortcut than as the API for this feature to me... but maybe everyone is very used to using files for this purpose) |
i did take a cue from jests feature which uses files (likely because their config can also be json) and it aligns with vitest's own Directly using a function in config would simplify the internal implementation which i like a lot. But then you'd have to use regular imports for all the things you want/need in setup and that could bloat the config itself. I'm leaning towards the file approach due to prior art and dx result. But if using the function approach enables ts support without extra work and the bloat is not an issue i'm all for it. in the end jest users can write a simple wrapper around their existing files and import that |
What about providing both ways? (both in |
The |
implementation as function here: https://github.com/dominikg/vitest/tree/feat/global-setup-function the complexity in vitest is greatly reduced, except for this little "hack": https://github.com/dominikg/vitest/blob/22c5ec2ef05c4001ffaff43d581914c2328e2e62/packages/vitest/src/node/pool.ts#L80 Unfortunately stacktraces don't look great when globalSetup gets bundled into the config: 20:26 $ pnpm test
> @vitest/test-globalSetup@ test /home/dominikg/develop/vitest/test/global-setup
> vitest
file:///home/dominikg/develop/vitest/test/global-setup/vitest.config.ts.js?t=1640892430013:9
throw new Error("ooops");
^
Error: ooops
at vitest_global_setup_default (file:///home/dominikg/develop/vitest/test/global-setup/vitest.config.ts.js?t=1640892430013:9:9) this is not good and i'd rather have a stacktrace referencing the correct files and lines. Is that something that can be improved in vite's config bundling? |
543a61a
to
cb2710a
Compare
docs/config/index.md
Outdated
- **Type:** `string | string[]` | ||
|
||
Path to global setup files, relative to project root | ||
```js |
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.
The setup file entry doesn't have an example of how to use the setting. Might consider removing it here (or we might end up needing to put examples for each rule.
} | ||
``` | ||
|
||
Multiple globalSetup files are possible. setup and teardown are executed sequentially with teardown in reverse order. |
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.
Maybe adding a callout to this will do it a little more justice 🙂
docs/config/index.md
Outdated
} | ||
``` | ||
|
||
Files can either export named functions |
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.
Files can either export named functions | |
Example |
docs/config/index.md
Outdated
``` | ||
|
||
or a setup function as default that returns the teardown | ||
```js |
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.
Could be all in one example (divised by comments to show the different approaches
3ac952a
to
d372de5
Compare
6d56a1b
to
46d2eed
Compare
…tead of showing examples inline
46d2eed
to
0f48694
Compare
Let's ship it for now, and we can see how we could improve the stacktrace later. |
The stacktraces in this PR should be fine. I'm still a bit worried about the intermittent test failures. Havn't been able to reliably reproduce/debug it unfortunately as it only happens on windows and trying to get to the bottom of a possible v8 issue inside a virtual machine running windows hasn't been a great experience at all. maybe @userquin has an idea? Steps to try to reproduce:
sometimes it fails with an error like this (different testcases) examples/vue test: FATAL ERROR: v8::FromJust Maybe value is Nothing.
examples/vue test: 1: 00007FF63D8E30AF v8::internal::CodeObjectRegistry::~CodeObjectRegistry+112511
examples/vue test: 2: 00007FF63D872216 DSA_meth_get_flags+65542
examples/vue test: 3: 00007FF63D8730CD node::OnFatalError+301
examples/vue test: 4: 00007FF63E18F4C5 v8::V8::FromJustIsNothing+53
examples/vue test: 5: 00007FF63D85B203 v8::base::CPU::has_sse+31491
examples/vue test: 6: 00007FF63D945BF7 uv_timer_stop+1207
examples/vue test: 7: 00007FF63D9421CB uv_async_send+331
examples/vue test: 8: 00007FF63D94195C uv_loop_init+1292
examples/vue test: 9: 00007FF63D941AFA uv_run+202
examples/vue test: 10: 00007FF63D910C05 node::SpinEventLoop+309
examples/vue test: 11: 00007FF63D7AB4D0 v8::internal::wasm::SignatureMap::Freeze+35776
examples/vue test: 12: 00007FF63D7A6B28 v8::internal::wasm::SignatureMap::Freeze+16920
examples/vue test: 13: 00007FF63D9322DD uv_poll_stop+557
examples/vue test: 14: 00007FF63E743DF0 v8::internal::compiler::RepresentationChanger::Uint32OverflowOperatorFor+146416
examples/vue test: 15: 00007FFC64087034 BaseThreadInitThunk+20
examples/vue test: 16: 00007FFC64E82651 RtlUserThreadStart+33
examples/vue test: Failed
undefined |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [vitest](https://github.com/vitest-dev/vitest) | devDependencies | patch | [`0.0.139` -> `0.0.140`](https://renovatebot.com/diffs/npm/vitest/0.0.139/0.0.140) | --- ### Release Notes <details> <summary>vitest-dev/vitest</summary> ### [`v0.0.140`](https://github.com/vitest-dev/vitest/releases/v0.0.140) [Compare Source](vitest-dev/vitest@v0.0.139...v0.0.140) ##### Bug Fixes - inline snapshot if not called inside suite ([6d743c5](vitest-dev/vitest@6d743c5)), closes [#​484](vitest-dev/vitest#484) - **ui:** flex / percentage based layout ([#​492](vitest-dev/vitest#492)) ([c43ebaf](vitest-dev/vitest@c43ebaf)) - correctly inline shapshot with properties ([4603ffd](vitest-dev/vitest@4603ffd)) - mocking is lost with threads: false ([28b97d8](vitest-dev/vitest@28b97d8)), closes [#​482](vitest-dev/vitest#482) - Reflect.get called on non-object ([3c9073a](vitest-dev/vitest@3c9073a)), closes [#​479](vitest-dev/vitest#479) - snapshot ignores indentation ([aff1481](vitest-dev/vitest@aff1481)) - **ui:** reduce graph container size ([#​478](vitest-dev/vitest#478)) ([23e1e62](vitest-dev/vitest@23e1e62)) ##### Features - global setup ([#​372](vitest-dev/vitest#372)) ([eaa119f](vitest-dev/vitest@eaa119f)) - **ui:** tasks state group ([7782e7d](vitest-dev/vitest@7782e7d)) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Reviewed-on: https://kolaente.dev/vikunja/frontend/pulls/1348 Co-authored-by: renovate <renovatebot@kolaente.de> Co-committed-by: renovate <renovatebot@kolaente.de>
I was trying out this feature and came across this peculiar issue. I am using an isomorphic library supabase-js. When I import it in a file that is sourced by
When I import the same in a normal test file it works fine. Here's the playground where the error is reproduced. |
@bhvngt could you create an issue with this info so it is easier to track it? |
fixes #300
ideas for tests, documentation and api improvements welcome
Questions:
enforce: 'post'
plugin