vitest
vitest copied to clipboard
vitest-dev
chore: replace fast-glob with tinyglobby
Description
tinyglobby is meant to be a drop-in replacement to globby and fast-glob, and it only uses two subdependencies - fdir and picomatch. as discussed in the discord server, this PR switches to it
expandDirectories is explicitly disabled for better fast-glob compatibility
funny enough the vitest monorepo already uses it in some way, due to unocss and vite-plugin-pwa using it
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.yamlunless 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 docscommand.
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:, orchore:.
Deploy Preview for vitest-dev ready!
| Name | Link |
|---|---|
| Latest commit | ab3aded4a8323a48e84d41bf0b223790c4512f86 |
| Latest deploy log | https://app.netlify.com/sites/vitest-dev/deploys/66daf80c3975a8000868d95b |
| Deploy Preview | https://deploy-preview-6274--vitest-dev.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
the tests seem to fail some snapshots, but only because they have a different order. should i update the snapshots or do a manual .sort?
the tests seem to fail some snapshots, but only because they have a different order. should i update the snapshots or do a manual .sort?
Why is the order different?
unsure why until i debug a bit more, but it looks like it's alphabetically sorted now, maybe it's the opposite in fast-glob? ideally vitest should report errors in the same order regardless of the order the glob results came in. from earlier tests i did weeks ago tinyglobby's globs aren't guaranteed to be sorted. iirc globs in general follow a non-deterministic order even if fast-glob in particular doesn't
unsure why until i debug a bit more, but it looks like it's alphabetically sorted now, maybe it's the opposite in fast-glob? ideally vitest should report errors in the same order regardless of the order the glob results came in. from earlier tests i did weeks ago tinyglobby's globs aren't guaranteed to be sorted. iirc globs in general follow a non-deterministic order even if fast-glob in particular doesn't
Vitest has a built in sequencer that does sorting: https://vitest.dev/config/#sequence-sequencer
i'm having a bit of trouble debugging due to vitest running tests using itself
Hm, looks like we sort by names only when sharding. Normally, we use fs.stat and fallback to the existing sorting
okay, so after debugging i can confirm that on that particular test case, fast-glob returns the two results reverse sorted, while tinyglobby does sort them correctly. should i just change the snapshots?
pushed the snapshot sorting update, i don't see why it should keep the unsorted order it had, and if anything that can always be changed in a followup pr
okay, so after debugging i can confirm that on that particular test case, fast-glob returns the two results reverse sorted, while tinyglobby does sort them correctly. should i just change the snapshots?
What determines “correctly”? Why the previous sorting is incorrect and what are the consequences of that change? Is this flaky? Will test order be non-deterministic with tinyglobby?
from my testing, the array of two elements produced from globbing in that test is not alphabetically sorted, and it is in tinyglobby, every single time i've ran these tests. by correct i meant it being sorted at all. despite that, i don't have evidence that globbing is deterministic with tinyglobby. the proper fix would be to just let vitest sort the reports itself
by correct i meant it being sorted at all.
You said it returns them in reversed order, so that's sorted, no? Just in reverse.
i have no idea why the tests are failing now, the only thing changed after force pushing was that snapshot in particular.
Tests bail out early, so if there are more errors, you won't see them unless all tests have run
Sorry for being picky here. I just remember that the sorting is important for filtering for example and while I appreciate the smaller size and dependencies count (and I agree that we should keep making it smaller), we should keep it as compatible as possible.
that array of two elements in particular seems to be reverse sorted, not because fast-glob returns globs reverse sorted but because an array of two can only ever be either sorted or reverse sorted :P weird wording on my part
it seems like the config in that test in particular breaks tinyglobby, and it'd be complex and hacky enough to add. thankfully it's the only remaining failing test
https://github.com/vitest-dev/vitest/blob/eae3bf4a72c1ab0b0b8ea0e13a3ed8a415ebfc66/test/reporters/reportTest2/custom-reporter-path.vitest.config.ts
it looks like a very hacky config? if the root is __dirname, it makes no sense to include tests outside that root. do users actually do that instead of defining a broader root (like path.join(__dirname, '..'))?
it seems like the config in that test in particular breaks tinyglobby, and it'd be complex and hacky enough to add. thankfully it's the only remaining failing test
eae3bf4/test/reporters/reportTest2/custom-reporter-path.vitest.config.tsit looks like a very hacky config? if the root is
__dirname, it makes no sense to include tests outside that root. do users actually do that instead of defining a broader root (likepath.join(__dirname, '..'))?
It does seem weird, but if it worked before then someone might've used this in the wild. But I haven't seen any usage of this pattern to be honest. We'll discuss with the team how we can proceed here in the next meeting. Thank you for the PR!
after thinking a lot about the problem, it is something important to have regardless of its use in vitest. i have a not that hacky fix almost ready, but it makes using patterns with leading ../ slower in some cases (cases that can be made a lot faster using the ignore option). ill publish a new version with the fix very soon
opened a fix SuperchupuDev/tinyglobby#18, will wait until you have the meeting you mentioned though
after refactoring out the ../ in that one test tests seem to pass now, except one in macos that's failing in other prs too
ended up adding ../ support in 0.2.3 anyways, some use cases from an unrelated package were breaking without it
Hey @SuperchupuDev unfortunately this does not seem to be a drop-in replacement, and ended up causing a breaking change.
At the Storybook repo we have extensive CI tests that depend on symlinks. Turns out that Vitest 2.1 stopped detecting the symlinked files and as a result they're not being tested anymore. Seems like it is related to this PR, given the context of this issue: https://github.com/SuperchupuDev/tinyglobby/issues/10
Do you think it's doable to add support to symlinks in the short term?
support for symlinks should be very straightforward, but unfortunately it needs thecodrr/fdir#114 to be merged first. support should arrive in a new tinyglobby version almost immediately once that gets released. @thecodrr any chance you can take a look at the pr?
hi @yannbf, i've just released a new tinyglobby version that comes with symlinks resolution by default. this should fix your issue as soon as vitest upgrades to it and releases a new version