Addon Test: Always run Vitest in watch mode internally #29749

JReinhold · 2024-11-29T12:09:11Z

What I did

Changed the Vitest logic to always run in watch mode, and then internally disable triggering tests on file changes when SB watch mode is disabled. This ensures that coverage is always re-intrumented on file changes, and Vitest module caches, even when SB watch mode is disabled.

Basically, when we want the flow where we're re-using the Vitest instance, we want to always enable watch mode too, as that is how Vitest expects to behave. Disabled watch-mode are only supported for single-use Vitest instances.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
- bug: Internal changes that fixes incorrect behavior.
- maintenance: User-facing maintenance tasks.
- dependencies: Upgrading (sometimes downgrading) dependencies.
- build: Internal-facing build tooling & test updates. Will not show up in release changelog.
- cleanup: Minor cleanup style change. Will not show up in release changelog.
- documentation: Documentation only changes. Will not show up in release changelog.
- feature request: Introducing a new feature.
- BREAKING CHANGE: Changes that break compatibility in some way with current major version.
- other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name	before	after	diff	z	%
createSize	0 B	0 B	0 B	-	-
generateSize	77.7 MB	77.7 MB	1.48 kB	-0.3	0%
initSize	133 MB	133 MB	1.5 kB	2.97	0%
diffSize	55.1 MB	55.1 MB	24 B	2.95	0%
buildSize	6.87 MB	6.87 MB	0 B	1.16	0%
buildSbAddonsSize	1.51 MB	1.51 MB	0 B	3	0%
buildSbCommonSize	195 kB	195 kB	0 B	-	0%
buildSbManagerSize	1.86 MB	1.86 MB	0 B	2.95	0%
buildSbPreviewSize	0 B	0 B	0 B	-	-
buildStaticSize	0 B	0 B	0 B	-	-
buildPrebuildSize	3.57 MB	3.57 MB	0 B	3	0%
buildPreviewSize	3.3 MB	3.3 MB	0 B	1.13	0%
testBuildSize	0 B	0 B	0 B	-	-
testBuildSbAddonsSize	0 B	0 B	0 B	-	-
testBuildSbCommonSize	0 B	0 B	0 B	-	-
testBuildSbManagerSize	0 B	0 B	0 B	-	-
testBuildSbPreviewSize	0 B	0 B	0 B	-	-
testBuildStaticSize	0 B	0 B	0 B	-	-
testBuildPrebuildSize	0 B	0 B	0 B	-	-
testBuildPreviewSize	0 B	0 B	0 B	-	-

name	before	after	diff	z	%
createTime	7.4s	7.3s	-122ms	-0.54	-1.7%
generateTime	21.5s	21.2s	-317ms	0.36	-1.5%
initTime	13.5s	15.5s	1.9s	1.62	🔺12.6%
buildTime	8.3s	12.2s	3.8s	5.99	🔺31.4%
testBuildTime	0ms	0ms	0ms	-	-
devPreviewResponsive	5.7s	5.4s	-268ms	0.38	-4.9%
devManagerResponsive	4.2s	4s	-234ms	0.61	-5.8%
devManagerHeaderVisible	576ms	692ms	116ms	1.76	🔺16.8%
devManagerIndexVisible	677ms	707ms	30ms	0.54	4.2%
devStoryVisibleUncached	1.9s	1.9s	12ms	0.63	0.6%
devStoryVisible	673ms	731ms	58ms	1.1	7.9%
devAutodocsVisible	686ms	462ms	-224ms	-0.9	-48.5%
devMDXVisible	562ms	677ms	115ms	2.28	🔺17%
buildManagerHeaderVisible	748ms	590ms	-158ms	0.02	-26.8%
buildManagerIndexVisible	757ms	735ms	-22ms	0.66	-3%
buildStoryVisible	596ms	518ms	-78ms	-0.16	-15.1%
buildAutodocsVisible	426ms	472ms	46ms	0.32	9.7%
buildMDXVisible	510ms	431ms	-79ms	-0.22	-18.3%

Greptile Summary

Modified Vitest test runner to always operate in watch mode internally while using Storybook's watch mode flag to control test execution, improving cache invalidation and coverage instrumentation.

Changed VitestManager to always use watch: true in Vitest configuration
Added guard in runAffectedTestsAfterChange to only trigger tests when Storybook watch mode is enabled
Set cleanOnRerun: false for coverage options to preserve instrumentation state
Updated TestManager to handle coverage mode changes without affecting watch mode state
Modified test suite to verify new watch mode and coverage behavior

nx-cloud · 2024-11-29T12:13:57Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 12179c1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

🟥 Failed Commands
`nx run-many -t build --parallel=3`

Sent with 💌 from NxCloud.

code/addons/test/src/node/vitest-manager.ts

greptile-apps

_{2 file(s) reviewed, 2 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

code/addons/test/src/node/test-manager.ts

…vitest

ghengeveld

Needs a bit of work, I think.

code/addons/test/src/node/test-manager.ts

code/addons/test/src/node/vitest-manager.ts

storybook-pr-benchmarking · 2024-12-02T14:49:27Z

Package Benchmarks

^{Commit: 12179c1, ran on 10 December 2024 at 09:35:23 UTC}

No significant changes detected, all good. 👏

…ways-watch-vitest

greptile-apps

_{3 file(s) reviewed, 3 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-12-10T09:25:16Z

code/addons/test/src/node/vitest-manager.ts

            clean: false,
-            cleanOnRerun: !watchMode,
+            cleanOnRerun: false,


style: cleanOnRerun:false could potentially lead to stale coverage data if files are deleted or renamed

greptile-apps · 2024-12-10T09:25:21Z

code/addons/test/src/node/test-manager.test.ts

+    expect(createVitest).toHaveBeenCalledTimes(4);
+    expect(vitest.runFiles).toHaveBeenCalledWith([], true);


logic: test expects 4 createVitest calls but only verifies empty test run. Should verify both coverage-disabled and coverage-enabled runs

greptile-apps · 2024-12-10T09:25:24Z

code/addons/test/src/node/test-manager.ts

+      if (payload.config) {
+        this.handleConfigChange({
+          providerId: payload.providerId,
+          config: payload.config,
+        });
      }


style: handleConfigChange is called before checking if the config actually changed, which could trigger unnecessary restarts

always run vitest in watch mode

86e4c96

JReinhold self-assigned this Nov 29, 2024

JReinhold requested review from valentinpalkovic and ghengeveld November 29, 2024 12:12

JReinhold changed the title ~~always run vitest in watch mode~~ Test: Always run Vitest in watch mode internally Nov 29, 2024

JReinhold commented Nov 29, 2024

View reviewed changes

code/addons/test/src/node/vitest-manager.ts Show resolved Hide resolved

JReinhold added maintenance User-facing maintenance tasks addon: test ci:normal labels Nov 29, 2024

JReinhold marked this pull request as ready for review November 29, 2024 12:14

greptile-apps bot reviewed Nov 29, 2024

View reviewed changes

code/addons/test/src/node/test-manager.ts Outdated Show resolved Hide resolved

code/addons/test/src/node/test-manager.ts Outdated Show resolved Hide resolved

valentinpalkovic approved these changes Nov 29, 2024

View reviewed changes

Merge branch 'jeppe/vitest-coverage-backend' into jeppe/always-watch-…

92ef7bf

…vitest

Base automatically changed from jeppe/vitest-coverage-backend to next November 29, 2024 14:27

ghengeveld reviewed Nov 29, 2024

View reviewed changes

fix coverage mode toggling

e5fb87d

ghengeveld approved these changes Dec 2, 2024

View reviewed changes

JReinhold and others added 2 commits December 6, 2024 11:27

add coverage unit tests

47101c6

Merge branch 'next' into jeppe/always-watch-vitest

50bfe4a

JReinhold marked this pull request as draft December 10, 2024 08:01

JReinhold added 3 commits December 10, 2024 09:08

Merge branch 'next' of github.com:storybookjs/storybook into jeppe/al…

685a3e8

…ways-watch-vitest

update yarn.lock

57cb78b

Merge branch 'next' of github.com:storybookjs/storybook into jeppe/al…

12179c1

…ways-watch-vitest

JReinhold marked this pull request as ready for review December 10, 2024 09:24

greptile-apps bot reviewed Dec 10, 2024

View reviewed changes

JReinhold merged commit 76ce9db into next Dec 10, 2024
58 of 59 checks passed

JReinhold deleted the jeppe/always-watch-vitest branch December 10, 2024 12:47

github-actions bot mentioned this pull request Dec 10, 2024

Release: Prerelease 8.5.0-alpha.20 #29999

Merged

11 tasks

shilman changed the title ~~Test: Always run Vitest in watch mode internally~~ Addon Test: Always run Vitest in watch mode internally Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addon Test: Always run Vitest in watch mode internally #29749

Addon Test: Always run Vitest in watch mode internally #29749

JReinhold commented Nov 29, 2024 •

edited by storybook-pr-benchmarking bot

Loading

nx-cloud bot commented Nov 29, 2024 •

edited

Loading

greptile-apps bot left a comment

ghengeveld left a comment

storybook-pr-benchmarking bot commented Dec 2, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Dec 10, 2024

greptile-apps bot Dec 10, 2024

greptile-apps bot Dec 10, 2024

		expect(createVitest).toHaveBeenCalledTimes(4);
		expect(vitest.runFiles).toHaveBeenCalledWith([], true);

Addon Test: Always run Vitest in watch mode internally #29749

Addon Test: Always run Vitest in watch mode internally #29749

Conversation

JReinhold commented Nov 29, 2024 • edited by storybook-pr-benchmarking bot Loading

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

nx-cloud bot commented Nov 29, 2024 • edited Loading

☁️ Nx Cloud Report

greptile-apps bot left a comment

Choose a reason for hiding this comment

ghengeveld left a comment

Choose a reason for hiding this comment

storybook-pr-benchmarking bot commented Dec 2, 2024 • edited Loading

Package Benchmarks

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot Dec 10, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 10, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 10, 2024

Choose a reason for hiding this comment

JReinhold commented Nov 29, 2024 •

edited by storybook-pr-benchmarking bot

Loading

nx-cloud bot commented Nov 29, 2024 •

edited

Loading

storybook-pr-benchmarking bot commented Dec 2, 2024 •

edited

Loading