Skip to content

Conversation

@akornmeier
Copy link
Contributor

This pull request introduces a new Storybook reporter package for TDD Guard, enabling integration of Storybook interaction test results into TDD Guard's validation workflow. The implementation includes the reporter class, TypeScript types, test data factories, comprehensive tests, documentation, and configuration files. The reporter captures results from stories with play functions, groups them by story file, and outputs them in a format compatible with TDD Guard. It also handles edge cases such as render failures and interrupted test runs.

Key changes include:

New Storybook Reporter Implementation:

  • Added StorybookReporter class that collects Storybook test results, groups them by story file, and writes them in TDD Guard's expected format. It supports both in-memory and file-based storage, and can be configured for monorepos or custom roots.
  • Defined TypeScript types for test results, errors, modules, and reporter options to ensure type safety and clarity throughout the reporter codebase.
  • Provided helper functions for generating mock test contexts for unit testing, covering passed, failed, skipped, and render error scenarios.

Testing and Validation:

  • Added comprehensive unit tests for the reporter, verifying correct output format, test state mapping, error handling, and overall status determination.

Documentation and Configuration:

  • Added a detailed design document outlining the reporter's architecture, design decisions, data flow, error handling, and testing strategy.
  • Created a README with installation, configuration, and usage instructions for end users.
  • Added package.json with dependencies, scripts, and publishing configuration for the new package.
  • Included TypeScript project configuration for build and type checking.
  • Exported public API and types from the package entry point.

Closes: #81

@nizos
Copy link
Owner

nizos commented Nov 10, 2025

This is great work @akornmeier!

I really appreciate that you captured your "mental model" such as design decisions, rationale, and so on. This is really valuable context. Could we create an ADR of the main design decision and include the other details as context? That would allow us to maintain this information in a way that is consistent with what we already do. We can then drop the plan document from the PR.

I noticed that the integration tests use mocking/simulation. I'm wondering if we could explore running real storybook tests for better confidence. I know that may be more setup, would it be difficult to setup? We can update the devcontainer if there is anything we need. I'm happy to help out if needed. Should we handle it in this PR or a follow up?

I noticed Storybook is not included in all the integration test cases. Can you confirm that we are including it where relevant?

Thanks again for this contribution! Looking forward to merging it. :)

@akornmeier
Copy link
Contributor Author

This is great work @akornmeier!

I really appreciate that you captured your "mental model" such as design decisions, rationale, and so on. This is really valuable context. Could we create an ADR of the main design decision and include the other details as context? That would allow us to maintain this information in a way that is consistent with what we already do. We can then drop the plan document from the PR.

I noticed that the integration tests use mocking/simulation. I'm wondering if we could explore running real storybook tests for better confidence. I know that may be more setup, would it be difficult to setup? We can update the devcontainer if there is anything we need. I'm happy to help out if needed. Should we handle it in this PR or a follow up?

I noticed Storybook is not included in all the integration test cases. Can you confirm that we are including it where relevant?

Thanks again for this contribution! Looking forward to merging it. :)

Thanks @nizos - I originally held off adding Storybook to the integration tests due to the complexity, but I will take another look at it. I will update the PR with possible options forward. I moved the document from docs/plan to the ADR to match your format.

Designed approach for replacing mock-based Storybook reporter tests
with real test-runner execution following existing reporter patterns
…book test-runner

Replace mock-based Storybook reporter tests with actual test-runner execution:

Test Infrastructure:
- Add story test artifacts (Calculator.js, passing/failing/import-error scenarios)
- Update factory to spawn real @storybook/test-runner commands
- Configure Storybook with .storybook/main.js and test-runner-jest.config.js
- Add Storybook to all integration test matrices

Dependencies:
- Add @storybook/react-vite, @storybook/test, storybook, react, vite
- Update package.json with required Storybook 8.x dependencies

DevContainer Support:
- Install Playwright browser dependencies (libnss3, libgbm1, etc.)
- Update README to document Storybook reporter and Playwright deps

This enables real end-to-end testing of the Storybook reporter with actual
browser-based test execution, matching the pattern of other language reporters.
Change from bin/test-storybook to dist/test-storybook to match
actual location in @storybook/test-runner package
Use join(__dirname, '../../storybook/node_modules/...') instead of
require.resolve() since @storybook/test-runner is only installed in
the storybook reporter package, not at the project root
After running npm install, @storybook/test-runner is hoisted to root
node_modules (similar to jest-cli), so we can use require.resolve()
to find the executable path
Storybook test-runner needs extra time to start dev server and launch
Playwright browser. Increased from 30s to 120s to accommodate this.
Add console.error to capture stdout/stderr when test-runner fails
to help diagnose why reporter returns null results
Start Storybook dev server in background before running test-runner:
- Make run function async to handle server startup
- Wait for server to be ready by watching stdout for 'Local:' message
- Kill server after tests complete (in finally block)
- Update ReporterConfig type to allow async run functions
- Use fixed npx path for security (sonarjs compliance)

This fixes the issue where test-runner failed because no Storybook
instance was running at http://127.0.0.1:6006
Update npx path from /usr/bin/npx to /usr/local/bin/npx and
add /usr/local/bin to PATH to fix dev server startup
Run storybook dev server using the binary from reporters/storybook/node_modules
instead of trying to use npx in temp directory. This avoids needing to run
npm install in the temp directory. Also add stderr logging for debugging.
Storybook is hoisted to root node_modules by npm workspaces.
Update path to use ../../../node_modules/.bin/storybook instead of
trying to use it from reporter's own node_modules
Add await to reporter.run() call to ensure async reporters (Storybook)
complete before temp directory cleanup. This fixes 'ENOENT: uv_cwd' error
where temp directory was deleted while Storybook was still starting.
- Update integration test expectations to match Storybook's actual data format:
  - Module paths: Use .stories file paths instead of story IDs
  - Test names: Use 'play-test' (Jest test name) instead of play function names
  - Full test names: Match 'Calculator Primary play-test' format
  - Error messages: Match Storybook's wrapped error format for import errors

- Fix port conflicts by changing range from 6000-6999 to 8000-8999
  - Avoids Chrome's unsafe port list (e.g., 6697 for IRC)
  - Prevents ERR_UNSAFE_PORT errors during test execution

- Make Rust/PHPUnit reporters optional with Promise.allSettled
  - Tests can run without these reporters installed
  - extractValues now looks up reporters by name instead of array index

- Add pretest:reporters script to ensure Playwright is installed

All 17 Storybook-specific integration tests now passing.
…ments

The 120s timeout was insufficient in devcontainer environments where all
reporters (Jest, Vitest, PHPUnit, Pytest, Go, Rust, Storybook) run
sequentially with slower I/O. Increased to 240s to accommodate.
@akornmeier
Copy link
Contributor Author

akornmeier commented Nov 13, 2025

Disregard the message above, I found the issue with the phpUnit tests, it was something I changed. All tests are now passing!!

@nizos
Copy link
Owner

nizos commented Nov 15, 2025

Excellent work Tony!! Taking a look now :)

@nizos
Copy link
Owner

nizos commented Nov 16, 2025

Are you fine with me making some simplifications before merging? Just fixing the security audit and port management if that's fine. :)

@nizos
Copy link
Owner

nizos commented Nov 16, 2025

I was looking into resolving the js-yaml vulnerability in @storybook/test-runner and found that the test-runner is being superseded by @storybook/addon-vitest. From the documentation:

The test runner has been superseded by the Vitest addon, which offers the same functionality, powered by the faster and more modern Vitest browser mode. It also enables the full Storybook Test experience, allowing you to run interaction, accessibility, and visual tests from your Storybook app. If you are using a Vite-powered Storybook framework, we recommend using the Vitest addon instead of the test runner.

I tested this approach and confirmed that tdd-guard-vitest successfully captures Storybook test results when using the Vitest addon. However, this requires Storybook 10+ and Vitest. The question becomes, should we maintain a dedicated Storybook reporter for users of older Storybook version and non-Vitest projects? Or should we pivot to documenting the vitest addon approach? What are your thoughts on this? :)

Add npm overrides to force js-yaml to version 4.1.1, fixing the prototype
pollution vulnerability (CVE) in versions < 4.1.1. This vulnerability was
in the dependency chain: @storybook/test-runner → nyc → @istanbuljs/load-nyc-config → [email protected]

The override in the root package.json ensures all nested dependencies use
the safe version, allowing StorybookReporter to work without security issues.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@akornmeier
Copy link
Contributor Author

I was looking into resolving the js-yaml vulnerability in @storybook/test-runner and found that the test-runner is being superseded by @storybook/addon-vitest. From the documentation:

The test runner has been superseded by the Vitest addon, which offers the same functionality, powered by the faster and more modern Vitest browser mode. It also enables the full Storybook Test experience, allowing you to run interaction, accessibility, and visual tests from your Storybook app. If you are using a Vite-powered Storybook framework, we recommend using the Vitest addon instead of the test runner.

I tested this approach and confirmed that tdd-guard-vitest successfully captures Storybook test results when using the Vitest addon. However, this requires Storybook 10+ and Vitest. The question becomes, should we maintain a dedicated Storybook reporter for users of older Storybook version and non-Vitest projects? Or should we pivot to documenting the vitest addon approach? What are your thoughts on this? :)

That’s great! I’m really enjoying the Vitest Browser Mode + Storybook integration and where they’re heading with it. I was using v9 on the project where I integrated TDD-Guard, so this is fantastic news for me. I’m in charge of that code, so I’d be able to upgrade the tools. Regarding your question about supporting a Storybook-reporter for backward compatibility, I’d let you make that call. As a tooling developer, I know adding features to a project comes with responsibilities and the need for ongoing maintenance. I’m really a fan of this project and how it guides the AI model to write code using the red/green approach. We could add the Storybook-reporter to give more coverage to those who might not be able to upgrade to v10 yet, document the best way to set up v10, and decide on a date to retire the old reporter.

I’m on board with whatever you decide, and I’d be happy to help keep this part running if you decide to keep it.

@nizos
Copy link
Owner

nizos commented Nov 17, 2025

Thanks Tony! I really appreciate your effort and I am always open for contributions especially from developers who can craft software like you do. I think we can merge this and maintain it while there is a need for it. Could we follow this up with 2 PRs?

  • Communicate the option to use the vitest plugin in the main README and link to an internal guide
  • Look into a way to use get-port instead of randomly generating ports

@nizos nizos merged commit ccd53b6 into nizos:main Nov 17, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storybook Test Runner Integration

2 participants