fix(vitest): test deep dependencies change detection #4934
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.
|
blake-newman
force-pushed
the
blake.newman/fix-changed-detection-deep-deps
branch
from
27e50f0
to
4e5589a
Compare
hi-ogawa
reviewed
Jan 18, 2024
There was a problem hiding this comment.
Thanks for the PR! (also reproduction and your analysis are awesome!)
This looks like a bug to me since the same logic is also used for vitest related https://vitest.dev/guide/cli.html#vitest-related and it also misses deep dependency. I checked it based on your repro https://stackblitz.com/edit/github-ndav2w?file=package.json
For the sake of verifying this PR, I don't think setting up new tests for --changed is necessary (actually there seems none currently probably because it's tedious to setup git), but It would be great if you can expand existing tests in test/changed/fixtures/related to show that deep dependencies is covered.
sheremet-va
requested changes
Jan 19, 2024
blake-newman
force-pushed
the
blake.newman/fix-changed-detection-deep-deps
branch
4 times, most recently
from
0f98151
to
3bfbfc8
Compare
`--changed` flag is not correctly looking up deep dependencies, as such the changed flag does not trigger specs if a sub dependency is changed. Alters the logic to add the collected dependency after recursive `addImports` is called, otherwise the logic returns early because that dependency has already been added to the collected deps set. So it will not look deeply.
blake-newman
force-pushed
the
blake.newman/fix-changed-detection-deep-deps
branch
from
3bfbfc8
to
89678fc
Compare
|
@sheremet-va @hi-ogawa i've added the relevant tests (including imports / re-export syntax). I also tested locally to ensure that the specs fail if the changes are not applied. |
sheremet-va
approved these changes
Jan 19, 2024
renovate bot
referenced
this pull request
in fwouts/previewjs
Jan 26, 2024
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [vitest](https://togithub.com/vitest-dev/vitest) ([source](https://togithub.com/vitest-dev/vitest/tree/HEAD/packages/vitest)) | [`^1.2.1` -> `^1.2.2`](https://renovatebot.com/diffs/npm/vitest/1.2.1/1.2.2) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>vitest-dev/vitest (vitest)</summary> ### [`v1.2.2`](https://togithub.com/vitest-dev/vitest/releases/tag/v1.2.2) [Compare Source](https://togithub.com/vitest-dev/vitest/compare/v1.2.1...v1.2.2) ##### 🐞 Bug Fixes - **coverage**: - Remove `coverage/.tmp` files after run - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5008](https://togithub.com/vitest-dev/vitest/issues/5008) [<samp>(d53b8)</samp>](https://togithub.com/vitest-dev/vitest/commit/d53b8580) - Don't crash when re-run removes earlier run's reports - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5022](https://togithub.com/vitest-dev/vitest/issues/5022) [<samp>(66898)</samp>](https://togithub.com/vitest-dev/vitest/commit/6689856f) - **expect**: - Improve `toThrow(asymmetricMatcher)` failure message - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5000](https://togithub.com/vitest-dev/vitest/issues/5000) [<samp>(a199a)</samp>](https://togithub.com/vitest-dev/vitest/commit/a199ac2d) - **forks**: - Set correct `VITEST_POOL_ID` - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5002](https://togithub.com/vitest-dev/vitest/issues/5002) [<samp>(7d0a4)</samp>](https://togithub.com/vitest-dev/vitest/commit/7d0a4692) - **threads**: - Mention common work-around for the logged error - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5024](https://togithub.com/vitest-dev/vitest/issues/5024) [<samp>(915d6)</samp>](https://togithub.com/vitest-dev/vitest/commit/915d6c43) - **typecheck**: - Fix `ignoreSourceErrors` in run mode - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5044](https://togithub.com/vitest-dev/vitest/issues/5044) [<samp>(6dae3)</samp>](https://togithub.com/vitest-dev/vitest/commit/6dae3feb) - **vite-node**: - Provide import.meta.filename and dirname - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5011](https://togithub.com/vitest-dev/vitest/issues/5011) [<samp>(73148)</samp>](https://togithub.com/vitest-dev/vitest/commit/73148575) - **vitest**: - Expose getHooks & setHooks - by [@​adriencaccia](https://togithub.com/adriencaccia) in [https://github.com/vitest-dev/vitest/issues/5032](https://togithub.com/vitest-dev/vitest/issues/5032) [<samp>(73448)</samp>](https://togithub.com/vitest-dev/vitest/commit/73448706) - Test deep dependencies change detection - by [@​blake-newman](https://togithub.com/blake-newman) in [https://github.com/vitest-dev/vitest/issues/4934](https://togithub.com/vitest-dev/vitest/issues/4934) [<samp>(9c7c0)</samp>](https://togithub.com/vitest-dev/vitest/commit/9c7c0fc9) - Throw an error if vi.mock is exported - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5034](https://togithub.com/vitest-dev/vitest/issues/5034) [<samp>(253df)</samp>](https://togithub.com/vitest-dev/vitest/commit/253df1cc) - Allow `useFakeTimers` to fake `requestIdleCallback` on non browser - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5028](https://togithub.com/vitest-dev/vitest/issues/5028) [<samp>(a9a48)</samp>](https://togithub.com/vitest-dev/vitest/commit/a9a486f2) - Support older NodeJS with async `import.meta.resolve` - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5045](https://togithub.com/vitest-dev/vitest/issues/5045) [<samp>(cf564)</samp>](https://togithub.com/vitest-dev/vitest/commit/cf5641a9) - Don't throw an error if mocked file was already imported - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5050](https://togithub.com/vitest-dev/vitest/issues/5050) [<samp>(fff1a)</samp>](https://togithub.com/vitest-dev/vitest/commit/fff1a270) ##### [View changes on GitHub](https://togithub.com/vitest-dev/vitest/compare/v1.2.1...v1.2.2) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **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 [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/fwouts/previewjs). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
](https://renovatebot.com){kind=link}
renovate bot
referenced
this pull request
in Johannes-Andersen/Johannes
Jan 26, 2024
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@vitest/coverage-v8](https://togithub.com/vitest-dev/vitest/tree/main/packages/coverage-v8#readme) ([source](https://togithub.com/vitest-dev/vitest/tree/HEAD/packages/coverage-v8)) | [`1.2.1` -> `1.2.2`](https://renovatebot.com/diffs/npm/@vitest%2fcoverage-v8/1.2.1/1.2.2) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [vitest](https://togithub.com/vitest-dev/vitest) ([source](https://togithub.com/vitest-dev/vitest/tree/HEAD/packages/vitest)) | [`1.2.1` -> `1.2.2`](https://renovatebot.com/diffs/npm/vitest/1.2.1/1.2.2) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>vitest-dev/vitest (@​vitest/coverage-v8)</summary> ### [`v1.2.2`](https://togithub.com/vitest-dev/vitest/releases/tag/v1.2.2) [Compare Source](https://togithub.com/vitest-dev/vitest/compare/v1.2.1...v1.2.2) ##### 🐞 Bug Fixes - **coverage**: - Remove `coverage/.tmp` files after run - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5008](https://togithub.com/vitest-dev/vitest/issues/5008) [<samp>(d53b8)</samp>](https://togithub.com/vitest-dev/vitest/commit/d53b8580) - Don't crash when re-run removes earlier run's reports - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5022](https://togithub.com/vitest-dev/vitest/issues/5022) [<samp>(66898)</samp>](https://togithub.com/vitest-dev/vitest/commit/6689856f) - **expect**: - Improve `toThrow(asymmetricMatcher)` failure message - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5000](https://togithub.com/vitest-dev/vitest/issues/5000) [<samp>(a199a)</samp>](https://togithub.com/vitest-dev/vitest/commit/a199ac2d) - **forks**: - Set correct `VITEST_POOL_ID` - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5002](https://togithub.com/vitest-dev/vitest/issues/5002) [<samp>(7d0a4)</samp>](https://togithub.com/vitest-dev/vitest/commit/7d0a4692) - **threads**: - Mention common work-around for the logged error - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5024](https://togithub.com/vitest-dev/vitest/issues/5024) [<samp>(915d6)</samp>](https://togithub.com/vitest-dev/vitest/commit/915d6c43) - **typecheck**: - Fix `ignoreSourceErrors` in run mode - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5044](https://togithub.com/vitest-dev/vitest/issues/5044) [<samp>(6dae3)</samp>](https://togithub.com/vitest-dev/vitest/commit/6dae3feb) - **vite-node**: - Provide import.meta.filename and dirname - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5011](https://togithub.com/vitest-dev/vitest/issues/5011) [<samp>(73148)</samp>](https://togithub.com/vitest-dev/vitest/commit/73148575) - **vitest**: - Expose getHooks & setHooks - by [@​adriencaccia](https://togithub.com/adriencaccia) in [https://github.com/vitest-dev/vitest/issues/5032](https://togithub.com/vitest-dev/vitest/issues/5032) [<samp>(73448)</samp>](https://togithub.com/vitest-dev/vitest/commit/73448706) - Test deep dependencies change detection - by [@​blake-newman](https://togithub.com/blake-newman) in [https://github.com/vitest-dev/vitest/issues/4934](https://togithub.com/vitest-dev/vitest/issues/4934) [<samp>(9c7c0)</samp>](https://togithub.com/vitest-dev/vitest/commit/9c7c0fc9) - Throw an error if vi.mock is exported - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5034](https://togithub.com/vitest-dev/vitest/issues/5034) [<samp>(253df)</samp>](https://togithub.com/vitest-dev/vitest/commit/253df1cc) - Allow `useFakeTimers` to fake `requestIdleCallback` on non browser - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5028](https://togithub.com/vitest-dev/vitest/issues/5028) [<samp>(a9a48)</samp>](https://togithub.com/vitest-dev/vitest/commit/a9a486f2) - Support older NodeJS with async `import.meta.resolve` - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5045](https://togithub.com/vitest-dev/vitest/issues/5045) [<samp>(cf564)</samp>](https://togithub.com/vitest-dev/vitest/commit/cf5641a9) - Don't throw an error if mocked file was already imported - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5050](https://togithub.com/vitest-dev/vitest/issues/5050) [<samp>(fff1a)</samp>](https://togithub.com/vitest-dev/vitest/commit/fff1a270) ##### [View changes on GitHub](https://togithub.com/vitest-dev/vitest/compare/v1.2.1...v1.2.2) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/Johannes-Andersen/Johannes). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot
referenced
this pull request
in ariakit/ariakit
Jan 26, 2024
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [vitest](https://togithub.com/vitest-dev/vitest) ([source](https://togithub.com/vitest-dev/vitest/tree/HEAD/packages/vitest)) | [`1.2.1` -> `1.2.2`](https://renovatebot.com/diffs/npm/vitest/1.2.1/1.2.2) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>vitest-dev/vitest (vitest)</summary> ### [`v1.2.2`](https://togithub.com/vitest-dev/vitest/releases/tag/v1.2.2) [Compare Source](https://togithub.com/vitest-dev/vitest/compare/v1.2.1...v1.2.2) ##### 🐞 Bug Fixes - **coverage**: - Remove `coverage/.tmp` files after run - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5008](https://togithub.com/vitest-dev/vitest/issues/5008) [<samp>(d53b8)</samp>](https://togithub.com/vitest-dev/vitest/commit/d53b8580) - Don't crash when re-run removes earlier run's reports - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5022](https://togithub.com/vitest-dev/vitest/issues/5022) [<samp>(66898)</samp>](https://togithub.com/vitest-dev/vitest/commit/6689856f) - **expect**: - Improve `toThrow(asymmetricMatcher)` failure message - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5000](https://togithub.com/vitest-dev/vitest/issues/5000) [<samp>(a199a)</samp>](https://togithub.com/vitest-dev/vitest/commit/a199ac2d) - **forks**: - Set correct `VITEST_POOL_ID` - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5002](https://togithub.com/vitest-dev/vitest/issues/5002) [<samp>(7d0a4)</samp>](https://togithub.com/vitest-dev/vitest/commit/7d0a4692) - **threads**: - Mention common work-around for the logged error - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5024](https://togithub.com/vitest-dev/vitest/issues/5024) [<samp>(915d6)</samp>](https://togithub.com/vitest-dev/vitest/commit/915d6c43) - **typecheck**: - Fix `ignoreSourceErrors` in run mode - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5044](https://togithub.com/vitest-dev/vitest/issues/5044) [<samp>(6dae3)</samp>](https://togithub.com/vitest-dev/vitest/commit/6dae3feb) - **vite-node**: - Provide import.meta.filename and dirname - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5011](https://togithub.com/vitest-dev/vitest/issues/5011) [<samp>(73148)</samp>](https://togithub.com/vitest-dev/vitest/commit/73148575) - **vitest**: - Expose getHooks & setHooks - by [@​adriencaccia](https://togithub.com/adriencaccia) in [https://github.com/vitest-dev/vitest/issues/5032](https://togithub.com/vitest-dev/vitest/issues/5032) [<samp>(73448)</samp>](https://togithub.com/vitest-dev/vitest/commit/73448706) - Test deep dependencies change detection - by [@​blake-newman](https://togithub.com/blake-newman) in [https://github.com/vitest-dev/vitest/issues/4934](https://togithub.com/vitest-dev/vitest/issues/4934) [<samp>(9c7c0)</samp>](https://togithub.com/vitest-dev/vitest/commit/9c7c0fc9) - Throw an error if vi.mock is exported - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5034](https://togithub.com/vitest-dev/vitest/issues/5034) [<samp>(253df)</samp>](https://togithub.com/vitest-dev/vitest/commit/253df1cc) - Allow `useFakeTimers` to fake `requestIdleCallback` on non browser - by [@​hi-ogawa](https://togithub.com/hi-ogawa) in [https://github.com/vitest-dev/vitest/issues/5028](https://togithub.com/vitest-dev/vitest/issues/5028) [<samp>(a9a48)</samp>](https://togithub.com/vitest-dev/vitest/commit/a9a486f2) - Support older NodeJS with async `import.meta.resolve` - by [@​AriPerkkio](https://togithub.com/AriPerkkio) in [https://github.com/vitest-dev/vitest/issues/5045](https://togithub.com/vitest-dev/vitest/issues/5045) [<samp>(cf564)</samp>](https://togithub.com/vitest-dev/vitest/commit/cf5641a9) - Don't throw an error if mocked file was already imported - by [@​sheremet-va](https://togithub.com/sheremet-va) in [https://github.com/vitest-dev/vitest/issues/5050](https://togithub.com/vitest-dev/vitest/issues/5050) [<samp>(fff1a)</samp>](https://togithub.com/vitest-dev/vitest/commit/fff1a270) ##### [View changes on GitHub](https://togithub.com/vitest-dev/vitest/compare/v1.2.1...v1.2.2) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **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 [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/ariakit/ariakit). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
--changedflag is not correctly looking up deep dependencies, as such the changed flag does not trigger specs if a sub dependency is changed.Alters the logic to add the collected dependency after recursive
addImportsis called, otherwise the logic returns early because that dependency has already been added to the collected deps set. So it will not look deeply.Resolves #4933
(Also Resolves #5009)