Skip to content

✨ Add Vitest browser mode plugin (drop-in replacement for vitest visual testing)#67

Merged
Robdel12 merged 13 commits intomainfrom
claude/explore-vitest-visual-testing-011CUQiJWHuvSPy54azZcnMH
Oct 25, 2025
Merged

✨ Add Vitest browser mode plugin (drop-in replacement for vitest visual testing)#67
Robdel12 merged 13 commits intomainfrom
claude/explore-vitest-visual-testing-011CUQiJWHuvSPy54azZcnMH

Conversation

@Robdel12
Copy link
Contributor

@Robdel12 Robdel12 commented Oct 24, 2025

Vitest Browser Mode Integration

This PR adds a Vitest v4 browser mode plugin that completely replaces Vitest's native visual testing with Vizzly.

Implementation

The plugin works by:

  1. Extending expect API with a custom toMatchScreenshot matcher via a browser-side setup file
  2. Disabling Vitest's native screenshot system with screenshotFailures: false
  3. Making direct HTTP POST requests from browser to Vizzly server
  4. Returning comparison results in TDD mode, always passing in cloud mode

API

Users just add the plugin - no test changes required:

// vitest.config.js
import { vizzlyPlugin } from '@vizzly-testing/vitest'

export default defineConfig({
  plugins: [vizzlyPlugin()],
  test: {
    browser: {
      enabled: true,
      name: 'chromium',
      provider: 'playwright'
    }
  }
})
// Tests use standard Vitest API
await expect(page).toMatchScreenshot('screenshot.png', {
  properties: { theme: 'dark' },
  threshold: 5
})

Files Changed

  • clients/vitest/src/setup.js (NEW) - Custom matcher implementation in browser context
  • clients/vitest/src/index.js - Plugin that injects setup file and passes env vars
  • clients/vitest/vitest.e2e.config.js (NEW) - E2E test configuration
  • clients/vitest/tests/ - Updated tests for new implementation
  • clients/vitest/README.md - Updated docs
  • .github/workflows/ci.yml - Removed unnecessary --update flag
  • src/services/test-runner.js - Removed wrong --update flag logic

Testing

  • ✅ Unit tests: 9/9 passed
  • ✅ E2E tests: 2/2 passed in both TDD and cloud modes
  • ✅ CI workflow updated and working
  • ✅ No Vitest test artifacts created (native system fully disabled)

Adds native integration with Vitest v4's visual regression testing using the
screenshot comparator API. This allows developers to use Vitest's native
toMatchScreenshot matcher with Vizzly's visual testing platform.

Features:
- Native Vitest API - use toMatchScreenshot with no custom matchers
- Screenshot comparator integration using Vitest's comparator API
- Per-screenshot properties for multi-variant testing via comparatorOptions
- TDD mode - local comparison with honeydiff and interactive dashboard
- Cloud mode - team collaboration with visual reviews in web dashboard
- Automatic mode detection and routing (TDD vs Cloud)
- TypeScript support with module augmentation
- Full ESM package (no build step required)

The comparator receives pixel data from Vitest, converts to PNG using pngjs,
and routes to either the TDD service (local) or screenshot server (cloud).
Baselines are stored in .vizzly/baselines/ via custom resolveScreenshotPath.

Package includes:
- @vizzly-testing/vitest - Standalone SDK in clients/vitest/
- Unit tests validating plugin configuration and behavior
- E2E test project demonstrating real-world usage
- GitHub workflow for automated npm releases
- Complete documentation and TypeScript declarations
@claude

This comment was marked as resolved.

- Remove examples/vitest-browser/ (duplicate of tests/integration/vitest-plugin/e2e/)
- Remove docs/vitest-integration.md (all docs in clients/vitest/README.md)
- Revert root README.md changes (no need to document in root)
- Move tests from tests/integration/vitest-plugin/ to clients/vitest/tests/
- Update CI workflow to run tests from client directory
- Add test scripts to package.json (test:unit, test:e2e, test)
- Update README with correct test paths
- Fix import paths in unit tests
- Follows same pattern as other client packages
- Remove duplicate package.json/package-lock.json from tests/e2e/
- Move vitest.config.js to root of vitest client package
- Add Vitest dependencies to main package.json devDependencies
- Update test scripts to run from package root
- Update CI workflow paths
- Simpler structure: one package.json, one lock file
@Robdel12
Copy link
Contributor Author

@claude can you rereview this PR?

@claude

This comment was marked as resolved.

Completely replaces Vitest's native screenshot testing by extending expect API
with custom toMatchScreenshot matcher. This bypasses Vitest's baseline checking
entirely and communicates directly with Vizzly server via HTTP POST.

Changes:
- Add setup.js that extends expect with custom matcher in browser context
- Update plugin to inject setup file and pass env vars via define
- Split unit and E2E tests into separate configs
- Clean up properties API - first-class top-level options
- Disable Vitest's native system with screenshotFailures: false
- Remove --update flag hack from test-runner and CI workflow
- Update README with implementation details

The custom matcher captures screenshots in browser and POSTs directly to
Vizzly server. In cloud mode tests always pass. In TDD mode comparison
results are returned from local server.
@Robdel12 Robdel12 changed the title Add Vitest v4 browser mode integration Add Vitest browser mode plugin (drop-in replacement for native visual testing) Oct 24, 2025
Removed dead code and updated all documentation for initial v0.1.0 release:
- Remove unused vizzlyComparator function and pixelDataToPNG helper
- Remove pngjs import (no longer needed)
- Update CHANGELOG with accurate features for custom matcher approach
- Update package.json description to "Drop-in replacement"
- Update TypeScript declarations to match actual API
- Update file header comments to reflect implementation
- Simplify setup.js options extraction (no backward compatibility needed)

All tests passing. Package ready for npm publish.
@Robdel12 Robdel12 force-pushed the claude/explore-vitest-visual-testing-011CUQiJWHuvSPy54azZcnMH branch from dc1bf1f to 8a0b640 Compare October 24, 2025 21:08
@Robdel12
Copy link
Contributor Author

@claude rereview this PR -- the approach is completely different from the first two reviews now.

@claude

This comment was marked as resolved.

- **Vitest SDK**: Send file paths directly from browser to server
- **Client SDK**: Send file paths directly instead of converting to base64
- **TDD/API handlers**: Support both file paths and base64 (backward compatible)
- **New utility**: image-input-detector for identifying input type
- **Tests**: Updated to reflect new file path behavior

This eliminates unnecessary file reading and base64 conversion in the client,
making all test runners (Vitest, Playwright, Puppeteer) consistent.
- Remove unused toMatchScreenshot export from index.js (actual implementation is in setup.js)
- Remove getTDDService helper and TDD service code (not needed in Node context)
- Enhance error handling in setup.js with specific error types
- Add baseline acceptance workflow note to README
- Update TypeScript declarations to match actual exports
- Fix unit tests to verify setup.js-based implementation
@Robdel12 Robdel12 force-pushed the claude/explore-vitest-visual-testing-011CUQiJWHuvSPy54azZcnMH branch from e0748d5 to a485ec9 Compare October 25, 2025 13:50
@Robdel12
Copy link
Contributor Author

@claude rereview this PR

@claude
Copy link

claude bot commented Oct 25, 2025

Claude finished @Robdel12's task —— View job


Re-reviewing PR: Vitest Browser Mode Plugin

  • Read and understand the new implementation approach
  • Review plugin architecture and integration
  • Review browser-side setup and matcher implementation
  • Review test coverage and E2E tests
  • Review documentation and configuration
  • Provide comprehensive feedback

Review Summary

This PR successfully implements a drop-in replacement for Vitest's native visual testing. The approach has evolved significantly from earlier reviews, and the current implementation is much cleaner and more aligned with Vitest's philosophy. Overall, this is excellent work with a well-thought-out architecture.

Architecture & Design ✅

Strengths:

  1. Clean separation of concerns: Plugin configuration (index.js) vs. browser-side matcher (setup.js)
  2. Proper Vite plugin lifecycle: Uses config() hook to inject setup files and env vars via define
  3. Smart auto-discovery: Searches parent directories for .vizzly/server.json - great DX
  4. File path support: The new image-input-detector.js utility elegantly handles both base64 and file paths
  5. Dual-mode behavior: TDD mode returns comparison results (pass/fail), cloud mode always passes - this is correct

Architecture observation:
The plugin uses Vite's define feature to inject __VIZZLY_SERVER_URL__ and __VIZZLY_BUILD_ID__ as compile-time constants. This is a standard Vite pattern and works well for browser contexts.

Implementation Quality 🎯

Plugin (clients/vitest/src/index.js) - Very Good

// Line 93: Uses import.meta.dirname with __dirname fallback
resolve(import.meta.dirname || __dirname, 'setup.js')

✅ Good ESM compatibility handling

Minor suggestion: The auto-discovery logic (lines 69-87) duplicates code from src/client/index.js:50-77. Consider extracting to a shared utility if you plan to maintain consistency.

Custom Matcher (clients/vitest/src/setup.js) - Excellent

The matcher implementation is well-designed with several highlights:

Strengths:

  1. Proper error handling (lines 124-141): Differentiates between network errors, server errors, and screenshot errors with helpful messages
  2. Threshold handling (lines 103-109): Correctly passes screenshots within threshold
  3. TDD vs Cloud mode (lines 74-80): Clean conditional logic
  4. Good UX: Returns dashboard links in error messages

Observation on line 10:

let serverUrl = typeof __VIZZLY_SERVER_URL__ !== 'undefined' ? __VIZZLY_SERVER_URL__ : '';

This is correct for define usage - the constants are replaced at compile time, so they may literally be undefined in the code.

Question (not a blocker): Lines 26-33 handle both element.locator (Playwright locator) and page object. Should we document what types are expected? The TypeScript definitions show only Assertion.toMatchScreenshot but don't specify what this context is passed.

Server-Side Changes - Well Done

src/utils/image-input-detector.js (clients/vitest/src/index.js:71):

  • Comprehensive test coverage (265 lines of tests!)
  • Clear documentation with examples
  • Handles edge cases (data URIs, Windows paths, file:// URIs)
  • Strategy is sound: check file paths FIRST (more specific), then base64 (more permissive)

src/server/handlers/api-handler.js (lines 69-99):

  • Properly resolves file paths and reads them
  • Good error messages for missing files
  • Security: Uses resolve() to prevent path traversal (though input is from localhost tests, still good practice)

src/server/handlers/tdd-handler.js (lines 1-100+):

  • Same file path detection logic applied consistently
  • PNG dimension detection remains robust (bytes 16-23 per PNG spec)

TypeScript Definitions ✅

clients/vitest/src/index.d.ts:

declare module 'vitest' {
  interface Assertion {
    toMatchScreenshot(
      name?: string,
      options?: VizzlyScreenshotOptions
    ): Promise<void>;
  }
}

✅ Proper module augmentation - this extends Vitest's types correctly

Completeness: Types match the implementation well. The VizzlyScreenshotOptions interface documents all supported fields.

Test Coverage ✅

Unit Tests (clients/vitest/tests/vitest-plugin.spec.js):

  • 9 test cases covering plugin registration, config injection, TypeScript declarations, package structure
  • Tests verify setup file injection and env var passing via define

E2E Tests (clients/vitest/tests/e2e/example.test.js):

  • 2 real browser tests using the plugin
  • Tests both basic usage and properties/threshold options
  • Actually renders HTML and takes screenshots - excellent dogfooding

New Utility Tests (tests/unit/image-input-detector.spec.js):

  • 265 lines of comprehensive tests
  • Covers base64 detection, file path detection (Unix, Windows, relative, absolute), edge cases

Coverage verdict: Excellent coverage across unit, integration, and E2E levels.

CI/CD Integration ✅

.github/workflows/ci.yml (lines 340-401):

  • New test-vitest-client job runs on blacksmith runners (matching other integration tests)
  • Tests across Node 20, 22, 24 - good version coverage
  • Runs unit tests (npm run test:unit) and E2E tests via vizzly run in cloud mode
  • Properly passes secrets and git metadata
  • Integrated into ci-check job for pass/fail gating

Path filter (lines 243-256):

  • Only runs when clients/vitest/** or workflow files change - efficient CI usage

Documentation 📚

clients/vitest/README.md:

  • Comprehensive (280 lines)
  • Clear installation, quick start, API reference
  • Explains TDD vs Cloud modes
  • Multi-variant testing examples
  • TypeScript support documented
  • Troubleshooting section
  • ✅ Excellent user-facing documentation

Code comments: Both index.js and setup.js have detailed JSDoc comments explaining the approach.

Potential Issues & Questions

1. Minor: Dependency on pngjs

clients/vitest/package.json:57 lists pngjs as a dependency, but I don't see it imported in src/index.js or src/setup.js. Is this actually needed for the Vitest client?

Impact: Low - extra dependency in node_modules, but minimal size
Recommendation: Remove if unused, or document why it's needed

2. Question: Screenshot path resolution

In setup.js:26-33, Vitest browser mode returns a file path from screenshot(). The path is sent directly to the server.

Question: Are these paths always absolute? Or could they be relative to the Vitest working directory? If relative, does the server resolve them correctly?

Looking at api-handler.js:75:

const filePath = resolve(image.replace('file://', ''));

If image is a relative path like ./screenshots/test.png, resolve() will resolve it relative to the server's CWD, which should be the repo root. This should work, but I'd verify that Vitest browser mode always returns paths that resolve correctly from the server's perspective.

Impact: Medium - could cause "file not found" errors if paths don't resolve
Recommendation: Add a comment documenting expected path format, or add defensive logic

3. Minor: Error message formatting

setup.js:129:

error.message.includes('HTTP 500') || error.message.includes('HTTP 502')

This checks if the error message includes "HTTP 500", but earlier (line 69) you throw:

throw new Error(`HTTP ${response.status}: ${response.statusText}`);

So the format is "HTTP 500: Internal Server Error", which would match. But if error messages ever change format, this could break. Consider checking error.status or error.code if available.

Impact: Low - error handling is best-effort anyway
Recommendation: Consider storing status code on error object for more robust checking

4. Documentation: First-run experience

The README mentions (line 96):

New screenshots will fail the first time they run - this is expected!

This is good to document, but users might find it surprising. Consider adding a note in the custom matcher message for 'new' status to make this even clearer (you already do this on line 91, which is great).

✅ Already handled well

Security Review 🔒

Path traversal: api-handler.js:75 uses resolve() which prevents ../../../etc/passwd attacks
Input validation: Image input detector properly validates base64 and paths
Injection: No user input is interpolated into shell commands or eval

✅ No security concerns

Consistency with Vizzly Architecture ✅

Follows patterns:

  • Uses service container approach (implicit via server handlers)
  • Matches Ruby/Storybook client pattern (thin client, server does heavy lifting)
  • TDD/Cloud mode behavior is consistent with other integrations
  • Screenshot signature matching aligns with cloud product

CLAUDE.md compliance:

  • ✅ Service-oriented design
  • ✅ Client SDK philosophy (thin client, server-side processing)
  • ✅ Dogfooding (E2E tests use Vizzly itself)
  • ✅ Plugin architecture (this could even be a Vizzly CLI plugin, but as a Vite plugin it's appropriate)

Final Recommendations

Must Address (before merge):

None - the implementation is production-ready

Should Address (nice to have):

  1. Clarify pngjs dependency - remove if unused
  2. Document screenshot path format - add comment about expected path format from Vitest
  3. Consider extracting auto-discovery logic - shared utility between plugin and client

Could Address (future work):

  1. Support for custom comparators/diffing algorithms (probably out of scope)
  2. Progress indicators for long screenshot batches (low priority)

Changelog Review ✅

clients/vitest/CHANGELOG.md:

  • Documents initial release (0.1.0)
  • Lists all features appropriately
  • Mentions browser mode requirement

Overall Assessment

Code Quality: ⭐⭐⭐⭐⭐ (5/5)

  • Clean, well-documented, properly tested

Architecture: ⭐⭐⭐⭐⭐ (5/5)

  • Excellent separation of concerns, follows Vite/Vitest patterns

Documentation: ⭐⭐⭐⭐⭐ (5/5)

  • Comprehensive README, good code comments, clear examples

Testing: ⭐⭐⭐⭐⭐ (5/5)

  • Unit, integration, and E2E tests, great coverage

Integration: ⭐⭐⭐⭐⭐ (5/5)

  • CI properly configured, follows monorepo patterns

Verdict: ✅ APPROVE

This PR is ready to merge. The implementation is clean, well-tested, thoroughly documented, and follows best practices. The approach of completely replacing Vitest's native visual testing via expect.extend() is the right call - it provides a seamless DX with zero API changes for users.

The file path support is a great addition that makes this work elegantly with Vitest browser mode's screenshot API. The dual-mode behavior (TDD pass/fail vs Cloud always-pass) is architecturally correct.

Great work! 🎉


Base64-encoded images can contain '/' as a valid base64 character. The previous detection logic checked for file paths before validating base64, causing base64 strings with '/' to be misidentified as file paths.

Changed detection priority to check base64 validation FIRST, then file path patterns. This ensures base64 strings (even with '/') are correctly identified, while still supporting all file path variations.

Fixes CI failure where PNG screenshots were treated as file paths.
@Robdel12 Robdel12 changed the title Add Vitest browser mode plugin (drop-in replacement for native visual testing) ✨ Add Vitest browser mode plugin (drop-in replacement for native visual testing) Oct 25, 2025
@Robdel12 Robdel12 changed the title ✨ Add Vitest browser mode plugin (drop-in replacement for native visual testing) ✨ Add Vitest browser mode plugin (drop-in replacement for vitest visual testing) Oct 25, 2025
@Robdel12 Robdel12 enabled auto-merge (squash) October 25, 2025 19:43
@Robdel12 Robdel12 merged commit 1aeb528 into main Oct 25, 2025
25 checks passed
@Robdel12 Robdel12 deleted the claude/explore-vitest-visual-testing-011CUQiJWHuvSPy54azZcnMH branch October 25, 2025 19:45
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.

1 participant