test(v3): vitest browser mode #605
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
""" WalkthroughThe changes overhaul the project's testing infrastructure, migrating from Testing Library and Vitest's jsdom environment to a browser-based setup using Playwright and Vitest's browser mode. Test scripts, dependencies, and configurations are updated accordingly. Test files are refactored to use Playwright-style querying and assertions, with improved handling for browser-specific behaviors and asynchronous operations. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Vitest (Browser)
participant Playwright (Chromium)
participant Test Utils
participant App Under Test
Developer->>Vitest (Browser): Run tests (via script or CI)
Vitest (Browser)->>Playwright (Chromium): Launch browser context
Vitest (Browser)->>Test Utils: Execute setup files (base, browser)
Test Utils->>App Under Test: Render components for testing
Vitest (Browser)->>App Under Test: Run test cases (queries/interactions via Playwright)
App Under Test-->>Vitest (Browser): Return results/events
Vitest (Browser)-->>Developer: Report test outcomes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ntatoud
force-pushed
the
v3/vitest-browser-mode
branch
from
6011e34 to
5b86e85
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
app/components/form/field-otp/field-otp.spec.tsx (1)
1-147: Fix missingVITE_BASE_URLfor Vitest unit testsThe unit‐test suite imports modules that validate
VITE_BASE_URL, but it isn’t set in the default test environment—causing the “VITE_BASE_URLis required but undefined” import error. You can resolve this in one of two ways:• In vitest.config.ts under the
unitproject’stestblock, add anenvsection:--- vitest.config.ts @@ export default defineConfig({ test: { projects: [ { /* browser config */ }, { test: { name: 'unit', environment: 'node', + env: { + VITE_BASE_URL: process.env.VITE_BASE_URL || 'http://localhost:3000', + SKIP_ENV_VALIDATION: 'true', + }, include: ['app/**/*.unit.{test,spec}.?(c|m)[jt]s?(x)'], setupFiles: [resolve('app/tests/setup.base.ts')], }, resolve: { /* … */ }, }, ], },• Or, at the top of app/tests/setup.base.ts before any imports:
// app/tests/setup.base.ts process.env.VITE_BASE_URL ||= 'http://localhost:3000'; process.env.SKIP_ENV_VALIDATION = 'true'; import '@/lib/dayjs/config'; // …Either change will ensure
VITE_BASE_URLis defined and skip the validation error in your CI.
♻️ Duplicate comments (1)
app/components/form/field-radio-group/field-radio-group.spec.tsx (1)
4-9: Same environment configuration issue as other test files.This file has the same import structure and will be affected by the missing
VITE_BASE_URLenvironment variable causing pipeline failures.
🧹 Nitpick comments (1)
app/components/form/field-radio-group/field-radio-group.spec.tsx (1)
251-255: Minor inconsistency: Missingtrial: trueoption in error handling.The error handling pattern is correct but differs slightly from the checkbox test which includes
trial: truein the click options. For consistency, consider adding the trial option:try { - await user.click(disabledRadio, { timeout: FAILED_CLICK_TIMEOUT_MS }); + await user.click(disabledRadio, { + trial: true, + timeout: FAILED_CLICK_TIMEOUT_MS + }); } catch { // Expected to fail since input is disabled }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.github/workflows/code-quality.yml(1 hunks)app/components/form/field-checkbox/field-checkbox.spec.tsx(5 hunks)app/components/form/field-otp/field-otp.spec.tsx(5 hunks)app/components/form/field-radio-group/field-radio-group.spec.tsx(7 hunks)app/components/form/field-text/field-text.spec.tsx(5 hunks)app/components/ui/calendar.spec.tsx(4 hunks)app/tests/setup.base.ts(1 hunks)app/tests/setup.browser.ts(1 hunks)app/tests/setup.ts(0 hunks)app/tests/utils.tsx(2 hunks)app/tests/vitest.d.ts(1 hunks)package.json(4 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/tests/setup.ts
🧰 Additional context used
🧠 Learnings (1)
app/tests/utils.tsx (2)
Learnt from: DecampsRenan
PR: #537
File: src/features/devtools/EnvHintDevPopover.tsx:0-0
Timestamp: 2024-10-11T14:57:53.600Z
Learning: When using React.cloneElement, remember that it automatically merges the child's existing props with the new props provided, so manually spreading children.props is unnecessary.
Learnt from: ivan-dalmet
PR: #532
File: src/features/auth/PageOAuthCallback.tsx:43-45
Timestamp: 2024-09-30T11:07:14.833Z
Learning: When suggesting changes to useEffect dependencies in React components, ensure that removing dependencies doesn't cause React Hook errors about missing dependencies.
🧬 Code Graph Analysis (3)
app/components/form/field-checkbox/field-checkbox.spec.tsx (1)
app/tests/utils.tsx (1)
FAILED_CLICK_TIMEOUT_MS(26-26)
app/components/form/field-radio-group/field-radio-group.spec.tsx (1)
app/tests/utils.tsx (1)
FAILED_CLICK_TIMEOUT_MS(26-26)
app/components/form/field-otp/field-otp.spec.tsx (1)
app/tests/utils.tsx (1)
FAILED_CLICK_TIMEOUT_MS(26-26)
🪛 GitHub Actions: 🔎 Code Quality
app/components/form/field-text/field-text.spec.tsx
[error] 1-1: Failed to import test file due to invalid environment variables: 'VITE_BASE_URL' is required but undefined.
app/components/form/field-checkbox/field-checkbox.spec.tsx
[error] 1-1: Failed to import test file due to invalid environment variables: 'VITE_BASE_URL' is required but undefined.
app/components/form/field-radio-group/field-radio-group.spec.tsx
[error] 1-1: Failed to import test file due to invalid environment variables: 'VITE_BASE_URL' is required but undefined.
app/components/ui/calendar.spec.tsx
[error] 1-1: Failed to import test file due to invalid environment variables: 'VITE_BASE_URL' is required but undefined.
app/components/form/field-otp/field-otp.spec.tsx
[error] 1-1: Failed to import test file due to invalid environment variables: 'VITE_BASE_URL' is required but undefined.
🔇 Additional comments (28)
app/tests/setup.base.ts (1)
1-1: LGTM! Clean base setup implementation.The Day.js configuration import ensures consistent date/time handling across all test environments.
app/tests/setup.browser.ts (1)
1-4: LGTM! Proper browser test cleanup setup.The
afterEach(cleanup)registration ensures DOM cleanup between tests, which is essential for test isolation in browser environments..github/workflows/code-quality.yml (1)
84-85: LGTM! Essential step for Playwright browser testing.The Playwright browser installation with
--with-depsis correctly positioned and necessary for the new browser-based test environment.app/components/form/field-otp/field-otp.spec.tsx (3)
4-9: LGTM! Proper migration to browser testing utilities.The import changes correctly replace testing library utilities with browser-based equivalents, including the
pageobject andFAILED_CLICK_TIMEOUT_MSconstant.
38-44: LGTM! Correct implementation of clipboard simulation.The explicit clipboard write operation before paste ensures proper simulation of clipboard behavior in the browser environment.
135-139: LGTM! Proper error handling for disabled input interaction.The try-catch block with timeout correctly handles the expected failure when clicking a disabled input, following the established pattern from other form field tests.
app/tests/vitest.d.ts (1)
1-1: LGTM! Correct type definitions for browser testing.The triple-slash directive properly references Playwright provider types, replacing the previous vitest-axe matchers that are no longer needed after the migration.
app/components/form/field-checkbox/field-checkbox.spec.tsx (3)
45-45: LGTM: DOM queries correctly migrated to browser testing utilities.The consistent replacement of
screenwithpagefor DOM queries aligns perfectly with the migration to Vitest browser mode. All role-based queries are appropriate for the checkbox elements being tested.Also applies to: 52-52, 80-80, 89-89, 116-116, 147-147, 159-159
47-47: LGTM: Assertions correctly converted to asynchronous pattern.The migration from synchronous
expect(...)toawait expect.element(...)is appropriate for browser-based testing and provides better reliability when elements may not be immediately available.Also applies to: 50-50, 83-83, 87-87, 117-117, 148-149, 157-157
151-158: LGTM: Proper error handling for disabled element interactions.The try-catch pattern with
trial: trueandFAILED_CLICK_TIMEOUT_MScorrectly handles the expected failure when attempting to click a disabled checkbox. This approach gracefully manages the error while still verifying that the checkbox remains unchecked, which is the intended behavior.app/components/form/field-radio-group/field-radio-group.spec.tsx (2)
58-58: LGTM: Comprehensive and consistent DOM query migration.All DOM queries have been correctly migrated from
screentopagewith appropriate role-based selectors for radio buttons, labels, and form elements. The migration is thorough and maintains the same testing logic.Also applies to: 64-64, 92-93, 101-101, 129-131, 148-148, 179-179, 182-182, 214-214, 217-217, 248-248, 258-258
59-59: LGTM: Consistent async assertion pattern implemented.All assertions have been properly converted to the asynchronous
await expect.element(...)pattern, providing better reliability for browser-based testing of radio group interactions.Also applies to: 62-62, 95-95, 99-99, 134-134, 137-137, 139-139, 142-142, 145-146, 180-180, 215-215, 249-249, 256-256
app/components/form/field-text/field-text.spec.tsx (4)
4-4: LGTM: Import changes appropriate for text input testing.The imports correctly include the necessary testing utilities. The absence of
FAILED_CLICK_TIMEOUT_MSis appropriate since this file uses a different error handling approach for disabled/readonly inputs.
28-28: LGTM: Proper element access patterns for different use cases.The code correctly uses
.element()when direct DOM property access is needed (e.g.,input.value) and uses the locator directly for user interactions. This demonstrates proper understanding of the browser testing API.Also applies to: 57-57, 86-86, 119-119
87-91: LGTM: Appropriate error handling for text input interactions.The try-catch blocks around
user.type()operations correctly handle expected failures when typing into disabled or readonly inputs. The approach is appropriate for text input testing and doesn't require the timeout configuration used for click operations.Also applies to: 120-124
32-32: LGTM: Button queries correctly migrated.All submit button queries have been consistently updated to use
page.getByRole()following the migration pattern.Also applies to: 59-59, 92-92, 125-125
package.json (2)
29-29: Script changes enable browser-based testing with appropriate defaults.The test script changes correctly implement browser-based testing:
testscript now runs in headless browser mode by default (good for CI/automation)test:uiscript runs in non-headless mode for visual debuggingThis provides a good balance between performance and debuggability.
Also applies to: 31-31
1-175: Dependency cleanup aligns with testing migration.The removal of Testing Library dependencies (
@testing-library/jest-dom,@testing-library/react,@testing-library/user-event) and replacement of@vitest/uiwith@vitest/browsercorrectly reflects the migration to browser-based testing infrastructure.app/components/ui/calendar.spec.tsx (4)
2-2: LGTM: Mocking approach correctly updated for browser testing.The module mocking using
vi.mockwithspy: trueandmockImplementationis appropriate for Vitest browser mode. The comment about browser limitations provides helpful context for the mocking strategy.Also applies to: 10-14
18-18: LGTM: Test functions correctly converted to async.Converting test functions to async is necessary to support the
awaitexpressions used for assertions and user interactions in browser-based testing.Also applies to: 46-46, 73-73
21-23: LGTM: Async assertions consistently implemented.The migration to
await expect.element()pattern is correctly applied for all element assertions, providing better reliability for browser-based testing of the Calendar component.Also applies to: 25-27, 66-66, 92-92
68-68: LGTM: User interactions correctly updated for browser testing.The
await button.click()pattern properly handles asynchronous user interactions in the browser testing environment.Also applies to: 94-94
app/tests/utils.tsx (3)
1-3: LGTM! Clean migration to Vitest browser utilities.The import changes correctly replace React Testing Library with Vitest's browser-based equivalents. The
userEventfrom@vitest/browser/contextandrenderfromvitest-browser-reactprovide the same functionality in the browser environment.
20-26: Good additions for browser testing setup.The export changes and new utilities are well-designed:
- Re-exporting from the new libraries maintains API compatibility
setupUser()provides a clean abstraction for user event setupFAILED_CLICK_TIMEOUT_MSconstant suggests proper error handling for flaky browser interactions
13-13: All tests confirmed compatible with new ComponentRenderOptionsVerified that the only invocation of our custom
renderhelper is withinapp/tests/utils.tsxitself—and no test files pass anoptionsobject directly. Sincewrapperis correctly omitted and no external usage ofoptionsexists, there are no compatibility concerns with the switch toComponentRenderOptionsfromvitest-browser-react.vitest.config.ts (3)
5-5: LGTM! Good helper function for path resolution.The
resolvehelper function improves readability and ensures consistent path resolution throughout the configuration.
40-52: Unit test project configuration looks good.The unit project properly:
- Uses Node environment for unit tests
- Includes only
.unit.{test,spec}files- Uses only the base setup file (no browser-specific setup needed)
- Maintains consistent alias configuration
10-39: Browser testing setup verified and approved.
- Confirmed
app/tests/setup.base.tsandapp/tests/setup.browser.tsexist.- Both files contain the expected configurations for Day.js and browser cleanup.
- No further changes needed—everything looks solid.
ntatoud
force-pushed
the
v3/vitest-browser-mode
branch
from
5b86e85 to
208bf94
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/code-quality.yml (2)
77-82: Consider moving the “Get pnpm store directory” step aftersetup-node
pnpm store pathis evaluated with whatever Node version is on the runner at that moment (the pre-installed one, not the matrix version).
Running it afteractions/setup-nodeguarantees the store path matches the Node version used for installation and avoids duplicated caches when the default runner Node differs from the matrix version.Re-order:
Setup Node.jsGet pnpm store directoryCache node modulesNo functional break, just cleaner cache-key hygiene.
Also applies to: 88-99
103-105: Heavy Playwright download on every run – add a browser cache
playwright install chromium --with-depsfetches ~120 MB each run.
Add a cache keyed on~/.cache/ms-playwright(or usemicrosoft/playwright-github-action) to cut CI time by ~30–40 s.Example snippet:
+ - name: Cache Playwright binaries + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: ${{ runner.os }}-playwright-${{ hashFiles('pnpm-lock.yaml') }} + - name: Install Playwright Chromium run: pnpm exec playwright install chromium --with-deps
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/code-quality.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/code-quality.yml
70-70: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: 🔬 Tests (20)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (1)
.github/workflows/code-quality.yml (1)
64-66: 👍 ExplicitVITE_BASE_URLlooks correctNo issues spotted with the new env variable – mirrors the default dev URL and shouldn’t interfere with the Vitest browser run.
ntatoud
force-pushed
the
v3/vitest-browser-mode
branch
from
208bf94 to
a158879
Compare
ntatoud
force-pushed
the
v3/vitest-browser-mode
branch
from
46b598b to
ec2d610
Compare
|
Summary by CodeRabbit
New Features
Refactor
Chores