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.

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

Addon Test: Fix environment variable for Vitest Storybook integration #30054

Merged

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Dec 13, 2024

Closes #

What I did

The process.env.VITEST_STORYBOOK variable wasn't set properly by the vitest process. This was causing component tests failures as soon as a11y tests were failing.

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 pull request has been released as version 0.0.0-pr-30054-sha-fb986dc5. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-30054-sha-fb986dc5
Triggered by @valentinpalkovic
Repository storybookjs/storybook
Branch valentin/fix-environment-detection-for-vitest
Commit fb986dc5
Datetime Fri Dec 13 08:54:23 UTC 2024 (1734080063)
Workflow run 12312532319

To request a new release of this pull request, mention the @storybookjs/core team.

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

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.7 MB 77.7 MB 0 B 1.22 0%
initSize 133 MB 133 MB 8.18 kB 1.23 0%
diffSize 55.3 MB 55.3 MB 8.18 kB 1.22 0%
buildSize 6.87 MB 6.87 MB 0 B -0.91 0%
buildSbAddonsSize 1.51 MB 1.51 MB 0 B -0.9 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.86 MB 1.86 MB 0 B -1 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.57 MB 3.57 MB 0 B -0.91 0%
buildPreviewSize 3.3 MB 3.3 MB 0 B -1 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 23.9s 22s -1s -936ms 1.11 -8.8%
generateTime 18.8s 29.8s 11s 1.15 37%
initTime 12.3s 17.5s 5.2s 0.52 29.6%
buildTime 8.8s 9s 196ms -0.36 2.2%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5.9s 4.4s -1s -451ms -1.09 -32.6%
devManagerResponsive 4.2s 3.4s -812ms -0.84 -23.6%
devManagerHeaderVisible 582ms 521ms -61ms -0.7 -11.7%
devManagerIndexVisible 657ms 554ms -103ms -0.97 -18.6%
devStoryVisibleUncached 2s 1.7s -246ms -0.32 -13.8%
devStoryVisible 655ms 559ms -96ms -0.82 -17.2%
devAutodocsVisible 593ms 490ms -103ms -0.38 -21%
devMDXVisible 665ms 451ms -214ms -0.81 -47.5%
buildManagerHeaderVisible 620ms 562ms -58ms -0.36 -10.3%
buildManagerIndexVisible 734ms 642ms -92ms -0.44 -14.3%
buildStoryVisible 556ms 503ms -53ms -0.4 -10.5%
buildAutodocsVisible 442ms 408ms -34ms -0.58 -8.3%
buildMDXVisible 458ms 402ms -56ms -0.64 -13.9%

Greptile Summary

Based on the provided information, I'll create a concise summary of the changes:

Sets the VITEST_STORYBOOK environment variable directly at module level to simplify Vitest integration with Storybook's testing environment.

  • Modified code/addons/test/src/node/vitest-manager.ts to set VITEST_STORYBOOK environment variable at module level
  • Removed redundant environment variable definitions from browser context (define)
  • Removed redundant environment variable definitions from test runner context (configOverride.env)
  • Simplified configuration by consolidating environment variable declaration to a single location

The changes make the code cleaner and more maintainable while preserving the same functionality for Vitest integration in Storybook.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

nx-cloud bot commented Dec 13, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit fb986dc. 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.

@storybook-pr-benchmarking
Copy link

Package Benchmarks

Commit: fb986dc, ran on 13 December 2024 at 09:04:56 UTC

The following packages have significant changes to their size or dependencies:

@storybook/core

Before After Difference
Dependency count 49 50 🚨 +1 🚨
Self size 21.44 MB 21.44 MB 0 B
Dependency size 14.34 MB 14.36 MB 🚨 +16 KB 🚨
Bundle Size Analyzer Link Link

storybook

Before After Difference
Dependency count 50 51 🚨 +1 🚨
Self size 22 KB 22 KB 0 B
Dependency size 35.78 MB 35.79 MB 🚨 +16 KB 🚨
Bundle Size Analyzer Link Link

sb

Before After Difference
Dependency count 51 52 🚨 +1 🚨
Self size 1 KB 1 KB 0 B
Dependency size 35.80 MB 35.82 MB 🚨 +16 KB 🚨
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 392 393 🚨 +1 🚨
Self size 493 KB 493 KB 0 B
Dependency size 77.38 MB 77.40 MB 🚨 +16 KB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 272 273 🚨 +1 🚨
Self size 612 KB 612 KB 0 B
Dependency size 67.38 MB 67.39 MB 🚨 +16 KB 🚨
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 108 109 🚨 +1 🚨
Self size 1.11 MB 1.11 MB 0 B
Dependency size 44.91 MB 44.93 MB 🚨 +16 KB 🚨
Bundle Size Analyzer Link Link

@valentinpalkovic valentinpalkovic merged commit c20e364 into next Dec 13, 2024
65 of 69 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/fix-environment-detection-for-vitest branch December 13, 2024 10:13
@github-actions github-actions bot mentioned this pull request Dec 13, 2024
17 tasks
@shilman shilman changed the title Addon Test: Set environment variable for Vitest to indicate Storybook integration Addon Test: Fix environment variable to indicate Vitest Storybook integration Dec 13, 2024
@shilman shilman changed the title Addon Test: Fix environment variable to indicate Vitest Storybook integration Addon Test: Fix environment var to indicate Vitest Storybook integration Dec 13, 2024
@shilman shilman changed the title Addon Test: Fix environment var to indicate Vitest Storybook integration Addon Test: Fix environment var for Vitest Storybook integration Dec 13, 2024
@shilman shilman changed the title Addon Test: Fix environment var for Vitest Storybook integration Addon Test: Fix environment variable for Vitest Storybook integration Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant