chore: migrates tests from Enzyme/Mocha to Vitest and React Testing Library#94
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates the project’s test stack from Enzyme/Mocha/Chai/Sinon/NYC to Vitest + React Testing Library (with happy-dom), modernizing tooling and updating related scripts/config.
Changes:
- Added Vitest configuration (globals, happy-dom, setup file, coverage) and updated npm scripts for run/watch/ui/coverage.
- Replaced Enzyme-based tests with RTL + Vitest tests (including updated setup).
- Removed legacy Webpack SVG config and deprecated test dependencies; added a repo
.npmrcto ease installs.
Reviewed changes
Copilot reviewed 12 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Removed orphaned Webpack SVG loader config no longer used. |
| vitest.config.js | Added Vitest config (happy-dom env, setup file, coverage settings). |
| test/unit/carousel.tests.jsx | Migrated Carousel unit tests from Enzyme/Mocha to RTL/Vitest. |
| test/unit/carousel.tests.js | Removed old Enzyme/Mocha test suite. |
| test/setup.js | Simplified test setup for RTL/Vitest (jest-dom + setImmediate polyfill). |
| src/stories/CustomArrows.jsx | Minor syntax/format fix (adds missing semicolon). |
| src/index.jsx | Normalized class-field method terminators to }; for arrow properties. |
| package.json | Updated scripts/deps for Vitest + RTL; bumped version to 3.0.1. |
| CHANGELOG.md | Added 3.0.1 entry documenting testing migration. |
| .npmrc | Added legacy-peer-deps=true to streamline installation. |
| .npmignore | Removed NYC output ignore entry since NYC is removed. |
| .eslintrc | Declared Vitest globals for linting tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/unit/carousel.tests.jsx
Outdated
There was a problem hiding this comment.
waitFor may re-run the callback multiple times; triggering goToSlide(1) inside it can cause repeated state updates and flaky behavior. Move the state-changing action (slidingCarousel.goToSlide(1)) outside waitFor (wrapped in act), and keep waitFor for assertions only.
test/unit/carousel.tests.jsx
Outdated
There was a problem hiding this comment.
Similar to the prior test: calling noneCarousel.goToSlide(1) inside waitFor can be executed multiple times and make the test flaky. Call noneCarousel.goToSlide(1) once before waitFor, then use waitFor only to assert track.style.transition.
| await waitFor(() => { | |
| noneCarousel.goToSlide(1); | |
| act(() => { | |
| noneCarousel.goToSlide(1); | |
| }); | |
| await waitFor(() => { |
| let imagesFetched; | ||
|
|
||
| global.Image = class MyImage { | ||
| set src(val) { | ||
| imagesFetched.push(val); | ||
| this.onload && this.onload(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This overrides global.Image for the entire test runtime and isn’t restored, which can leak into other test files and cause surprising failures. Store the original global.Image and restore it in afterAll (or afterEach if needed).
test/unit/carousel.tests.jsx
Outdated
There was a problem hiding this comment.
Using real timers (setTimeout) makes the test slower and can be flaky on slower/loaded CI. Prefer vi.useFakeTimers() + vi.advanceTimersByTime(20) (and vi.useRealTimers() cleanup) so the test is deterministic.
| await new Promise(resolve => setTimeout(resolve, 20)); | |
| vi.useFakeTimers(); | |
| await act(async () => { | |
| vi.advanceTimersByTime(20); | |
| }); | |
| vi.useRealTimers(); |
test/unit/carousel.tests.jsx
Outdated
There was a problem hiding this comment.
Asserting exact outerHTML is brittle across DOM implementations (happy-dom vs jsdom) and React attribute serialization differences. Prefer asserting behavior/attributes (e.g., toBeDisabled(), toHaveClass(...), toHaveAttribute('type','button')) rather than the full serialized HTML string.
| expect(topArrow.outerHTML).toBe('<button type="button" disabled="" class="carousel-arrow carousel-top-arrow carousel-arrow-default"></button>'); | |
| expect(bottomArrow).toBeTruthy(); | |
| expect(bottomArrow.outerHTML).toBe('<button type="button" class="carousel-arrow carousel-bottom-arrow carousel-arrow-default"></button>'); | |
| expect(topArrow.tagName).toBe('BUTTON'); | |
| expect(topArrow).toHaveAttribute('type', 'button'); | |
| expect(topArrow).toHaveClass('carousel-arrow', 'carousel-top-arrow', 'carousel-arrow-default'); | |
| expect(topArrow).toBeDisabled(); | |
| expect(bottomArrow).toBeTruthy(); | |
| expect(bottomArrow.tagName).toBe('BUTTON'); | |
| expect(bottomArrow).toHaveAttribute('type', 'button'); | |
| expect(bottomArrow).toHaveClass('carousel-arrow', 'carousel-bottom-arrow', 'carousel-arrow-default'); | |
| expect(bottomArrow).not.toBeDisabled(); |
test/unit/carousel.tests.jsx
Outdated
There was a problem hiding this comment.
Correct spelling in the test name from 'carousal' to 'carousel' to match the component name and avoid confusion.
test/unit/carousel.tests.jsx
Outdated
There was a problem hiding this comment.
Correct spelling in the test name from 'carousal' to 'carousel' to match the component name and avoid confusion.
.npmrc
Outdated
There was a problem hiding this comment.
Enabling legacy-peer-deps repo-wide can mask genuine peer dependency conflicts and make installs diverge from typical consumer environments. Consider removing this file and documenting an opt-in install command (or addressing specific peer dependency issues directly) so CI and local installs fail fast on real peer problems.
| legacy-peer-deps=true | |
| # Do not enable legacy-peer-deps repo-wide; use `npm install --legacy-peer-deps` explicitly if absolutely necessary. |
Summary
Migrates the test framework from Enzyme/Mocha/Chai/Sinon/NYC to Vitest and React Testing Library, replacing deprecated tooling with modern, actively maintained alternatives.
Changes
Test framework
test:watch(watch mode) andtest:ui(Vitest UI)Source files
.jsto.jsx, enabling native esbuild transformswebpack.config.js(referenced a missingreact-svg-loaderdependency; Storybook uses its own config)Dependencies
enzyme,@cfaester/enzyme-adapter-react-18,mocha,chai,sinon,sinon-chai,nyc,@babel/register,eslint-plugin-mocha,jsdom,vite-plugin-babel,cheeriooverridevitest,@vitest/ui,@vitest/coverage-v8,@testing-library/react,@testing-library/user-event,@testing-library/jest-dom,happy-domHousekeeping
.npmrcwithlegacy-peer-deps=truefor streamlined installationPerformance
33% faster end-to-end, primarily from replacing Babel transforms with native esbuild (5x faster) and using happy-dom over jsdom.
Note
The rename from
.jsto.jsxis required because Vite (which powers Vitest) uses esbuild for transforms, and esbuild will not parse JSX syntax in.jsfiles by default. Without the rename, we would need to route all files through a Babel plugin (vite-plugin-babel) just to handle JSX — negating the performance benefits of switching to Vitest in the first place. Using.jsxextensions lets esbuild handle the transforms natively, which is what brought the transform step from 264ms down to 53ms. This is also the recommended convention from Vite and aligns with broader ecosystem conventions for distinguishing files that contain JSX.This rename also lays the groundwork for a potential future migration of the build step from Babel to Vite (
vite build), and Storybook from Webpack to the Storybook Vite builder. That would allow us to drop@babel/cli,@babel/core,@babel/preset-env,@babel/preset-react,@babel/plugin-transform-runtime,babel-loader,babel-plugin-inline-react-svg,@storybook/addon-webpack5-compiler-babel,@storybook/react-webpack5,@svgr/webpack,css-loader,less-loader, andstyle-loader— unifying the entire toolchain on Vite and significantly reducing the dependency footprint.Motivation
Notes
coverage/directory withjson-summaryformat, so external tools that read from there should continue to work