-
Notifications
You must be signed in to change notification settings - Fork 12
Add end-to-end tests and improve unit test quality #84
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
Conversation
🦋 Changeset detectedLatest commit: 4d1c0c2 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR 💥 An error occurred when fetching the changed packages and changesets in this PR |
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.
Pull Request Overview
This PR introduces comprehensive end-to-end tests for the MCP addon and improves unit test quality across the codebase. The E2E tests validate the MCP endpoint functionality using a real Storybook instance and include version validation to ensure compatibility with the latest Storybook prereleases.
Key changes:
- Added E2E test infrastructure in
apps/internal-storybook/testswith MCP protocol tests and dependency version validation - Improved unit test coverage and quality in
packages/mcpandpackages/addon-mcpwith better mocking and test organization - Updated build configuration to support E2E tests and HTML file loading
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Added HTML loader plugin and expanded test projects to include apps |
| turbo.json | Added build dependencies for test tasks to ensure proper execution order |
| pnpm-workspace.yaml | Updated Storybook catalog versions from 10.1.0-alpha.10 to 10.1.0-alpha.11 |
| packages/mcp/src/index.test.ts | New integration tests for MCP handler with client-side testing |
| packages/addon-mcp tests | Refactored telemetry mocking to use Storybook internal mock |
| packages/addon-mcp/src/preset.ts | Removed duplicate POST handler registration |
| packages/addon-mcp/src/mcp-handler.ts | Moved telemetry initialization logic into server initialization |
| apps/internal-storybook/tests/*.e2e.test.ts | New E2E tests for MCP endpoint and dependency validation |
| apps/internal-storybook/vitest.config.ts | E2E project configuration with extended timeouts |
| .github/copilot-instructions.md | Added documentation for E2E test suite |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #84 +/- ##
===========================================
+ Coverage 38.57% 87.72% +49.15%
===========================================
Files 23 17 -6
Lines 687 334 -353
Branches 169 94 -75
===========================================
+ Hits 265 293 +28
+ Misses 390 5 -385
- Partials 32 36 +4 ☔ View full report in Codecov by Sentry. |
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.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/addon-mcp/src/preset.ts:17
- [nitpick] The
shouldRedirectvalue is calculated before the POST endpoint is registered. IfisManifestAvailableis slow or the manifest availability changes between the time this is calculated and when the GET endpoint is accessed, the redirect logic could be stale. Consider either:
- Moving this calculation inside the GET handler to ensure it's always up-to-date, or
- Adding a comment explaining why this is calculated once at startup
app!.post('/mcp', (req, res) =>
Bundle ReportChanges will increase total bundle size by 50 bytes (0.13%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: @storybook/addon-mcp-esmAssets Changed:
Files in
|
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.
Pull Request Overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.
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.
Pull Request Overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.
kasperpeulen
left a comment
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.
LGTM
* enter prereelase mode on next branch * Handle HTML in a separate file (#56) * extract human-readable /mcp to maintainable html file * upgrade tsdown, remove json treeshaking workaround * add changeset * fix tsdown types * add changeset release branches to checks * commit releases with gh api. see https://github.com/changesets/action#inputs * Version Packages (next) (#57) Co-authored-by: storybook-app-bot[bot] <175111413+storybook-app-bot[bot]@users.noreply.github.com> * Replace Storybook canary versions with 10.1.0 prereleases (#59) * upgrade to storybook 10.1.0-alpha.2 * changesets * Version Packages (next) (#60) Co-authored-by: storybook-app-bot[bot] <175111413+storybook-app-bot[bot]@users.noreply.github.com> * Rename "examples" to "stories" in component manifest format (#61) * Initial plan * Rename "examples" to "stories" in component manifest format - Updated type definitions in types.ts files to rename Example to Story - Updated format-manifest.ts to use story terminology (story, story_name, story_description, story_code) - Updated all fixture JSON files to use "stories" instead of "examples" - Updated test files and descriptions to use "stories" terminology - Updated test snapshots to reflect the new XML output format - All tests passing, build and typecheck successful --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Jeppe Reinhold <[email protected]> * Support name in manifest errors (#55) * add name to manifest errors * add fixtures with errors * add changeset * improve test reports in ci * update fixtures to use stories instead of examples * more example -> story renaming * Improve code quality and development setup (#64) * replace prettier with oxfmt * add oxlint for linting * update actions using npx actions-up * add publint * fix types * add check-everything script * add build-storybook to check all * split GH Workflows * explain lint disables * Version Packages (next) (#63) Co-authored-by: storybook-app-bot[bot] <175111413+storybook-app-bot[bot]@users.noreply.github.com> * Replace oxfmt with Prettier (#68) * replace oxfmt with prettier * typo * Update to the latest SB alpha so the internal storybook version works with the server (#71) Co-authored-by: Jeppe Reinhold <[email protected]> * Revert Embed demo image from storybook.js.org#21 (#75) * Evals (#69) * add initial eval setup * well, a lot happened here... * add clack * Add interactive prompts and styled output to eval CLI (#65) * Initial plan * Add interactive prompts and prettier output to eval CLI Co-authored-by: JReinhold <[email protected]> * Use tasks API for parallel evaluation steps Co-authored-by: JReinhold <[email protected]> * Apply oxfmt formatting to eval.ts --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JReinhold <[email protected]> * improve terminal experience * save environment * improve terminal experience * only allow one eval at a time * add support for custom context * format * add support for eval hooks, add log about how to rerun experiments * prompt to start storybook at the end of the evaluation * add message about getting into the experiment * improve experiment dir name * take screenshots of failed stories too * cleanup * improve reshaped stories, improve test+a11y summary, improve mcp server config arg * support --[no-]storybook flag * collect experiment description and branch name * save result summary to google sheets * improve plain prompt * prompt for google sheets upload * fix google sheets upload * support "Storybook MCP" context, which starts up the docs-only @storybook/mcp server with a given component manifest * Add basic Radix eval (#66) * Add Radix eval * Add Rsuite eval (#67) --------- Co-authored-by: Jeppe Reinhold <[email protected]> * format * fix typechecking * add reshaped component manifest * add conversation-viewer.html with approximate token count * cleanup * add documentation, fixups * format * fix stories not having imports anymore * fix plain and radix experiments * experiments will have unique package names * more eval test fixing * more story fixes * fix typecheck and lint summary * improve conversation viewer * simplify viewer content * simplify viewer content * result visualisations is via storybook * upload to chromatic * update google sheet row order * add Chromatic link to CLI log * add note about public results * remove description arg from evals * Evals: Add Radix UI website prompt (#74) --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: JReinhold <[email protected]> Co-authored-by: Michael Shilman <[email protected]> * Review Kasper (#70) * Start review * Fix * More comments * Fix config files and restructure * Resolve conflicts * Fix github actions * Fix coverage * Fix type error * Fix * Fix * Dedupe * Update packages/mcp/src/index.ts Co-authored-by: Jeppe Reinhold <[email protected]> * Update .github/workflows/check.yml Co-authored-by: Jeppe Reinhold <[email protected]> * Update .github/workflows/check.yml Co-authored-by: Jeppe Reinhold <[email protected]> * Improve get/post handling * Dedupe vite * lock file * test perf of check-everything in CI * rename * rename * Add turbo caching * check cache invalidation * refactor * refactor * refactor * refactor * Use node version file * description * refactor * rollback * use turbo for artifacts * install node * optimize * install offline for faster symlinking * optimize * Check ci * Only upload test results on failure * Check github reporter * Fix command * Fix test * Remove check everything * test corepack enable * test corepack enable * test corepack enable * fix * Check if this is faster * Check if this is faster * no cache * rollback * Change nothing * Fix prettier * Modify changeset for MCP server GET responses Updated the changeset to handle GET responses in the MCP server. * Prettier * use docker * debug * use node 24 * Try own caching * Prune it * Don't format pnpm lock * Fix * again * use composite * change * Revert "change" This reverts commit 8031a63. * Revert "use composite" This reverts commit 7f26a54. * Revert "again" This reverts commit 7fdccdf. * Revert "Fix" This reverts commit f4dd004. * Revert "Don't format pnpm lock" This reverts commit c11c4ec. * Revert "Prune it" This reverts commit 1009ad5. * Revert "Try own caching" This reverts commit 82eb804. * Revert "use node 24" This reverts commit c63f9ee. * Revert "debug" This reverts commit d647a91. * Revert "use docker" This reverts commit 766462e. * Address feedback * Initial plan * Update README and Copilot instructions for script changes Co-authored-by: JReinhold <[email protected]> * Address feedback * Make it loose * Watch storybook by default * Fix command * Fix * Add pnpm to ignore * Fix dev command * Cleanup * get CI green --------- Co-authored-by: Jeppe Reinhold <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JReinhold <[email protected]> * Make `get-component-documentation` tool only accept a single component ID instead of multiple (#79) * cleanup * get-component-documentation only accepts a single component id * Fix evals (#81) * cleanup * get-component-documentation only accepts a single component id * fix versions * use vitest cli instead of node for evals * prefix experiment scripts so they are not picked up by turborepo * Add toolset property to telemetry payloads in addon-mcp (#78) * Initial plan * Add toolset property to all telemetry payloads in addon-mcp Co-authored-by: JReinhold <[email protected]> * add changeset --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JReinhold <[email protected]> Co-authored-by: Jeppe Reinhold <[email protected]> Co-authored-by: Jeppe Reinhold <[email protected]> * remove source API and use the request instead (#54) * remove source API and use the request instead * cleanup * add changesets * add path argument to manifestProvider * cleanup * update changeset * fix serve.ts * cleanup * Fix internal stdio-based MCP server (#85) * allow undefined requests when using custom manifestProvider * changeset * add tests for internal stdio transport * cleanup * Add end-to-end tests and improve unit test quality (#84) * add e2e tests * improve e2e scripting * add tests for mcp index * add preset tests * add telemetry tests * simplify tool test mocks * simplify mcp-handler tests, improve disableTelemetry handling * add tests for manifest availability * exclude evals from coverage * cleanup * changeset * fix preset registering handlers instead of middlewares * update tests to match changes in base branch * cleanup * await sb process kill * globally mock storybook deps * clean lock file * Output in markdown instead of XML (#86) * add e2e tests * improve e2e scripting * add tests for mcp index * add preset tests * add telemetry tests * simplify tool test mocks * simplify mcp-handler tests, improve disableTelemetry handling * add tests for manifest availability * exclude evals from coverage * cleanup * changeset * fix preset registering handlers instead of middlewares * update tests to match changes in base branch * cleanup * await sb process kill * refactor formatter, splitting into markdown and xml, configurable, defaulting to markdown * globally mock storybook deps * clean lock file * fix context arg * fix tests * fix types * "Examples" -> "Stories", simplify tests * simplify tests and types * simplify * use ts-like prop type docs format * add script to clean experiments * add changeset * exit pre mode (#88) * Update reshaped flight booking eval (#87) * Update reshaped flight booking eval * format --------- Co-authored-by: Jeppe Reinhold <[email protected]> Co-authored-by: Jeppe Reinhold <[email protected]> * Version Packages (#80) Co-authored-by: storybook-app-bot[bot] <175111413+storybook-app-bot[bot]@users.noreply.github.com> --------- Co-authored-by: storybook-app-bot[bot] <175111413+storybook-app-bot[bot]@users.noreply.github.com> Co-authored-by: Copilot <[email protected]> Co-authored-by: Tom Coleman <[email protected]> Co-authored-by: Michael Shilman <[email protected]> Co-authored-by: JReinhold <[email protected]> Co-authored-by: Kasper Peulen <[email protected]>
This PR introduces E2E tests via the internal storybook, using the existing Vitest setup. The E2E tests also asserts that we're running with the latest prerelease of Storybook, to ensure we're not testing against outdated code. This might be a bit annoying, forcing you to keep Storybook up-to-date whenever we do a new prerelease. Other ideas welcome - we have had issues with not updating Storybook in this repo, and not catching incompatibility regressions.
A downside of adding e2e to the existing vitest setup, is that now tests depends on building packages, as you can see in the turbo config changes. e2e tests takes about 3 seconds vs 0.5 for unit tests. Maybe this is okay, at least splitting this up into completely separate testing flows seemed rather complex.
This PR also goes through all existing unit tests to improve coverage and simplify some of them.