test(compat): add test coverage for darkModePlugin uncovered branches#20122
test(compat): add test coverage for darkModePlugin uncovered branches#20122me-saurabhkohli wants to merge 1 commit into
Conversation
Adds a dedicated test file for the dark-mode compat plugin (src/compat/dark-mode.ts), covering branches that had no tests: - darkMode: 'class' — legacy v3 :is() selector with default .dark - darkMode: ['class', custom] — legacy :is() with custom class selector - darkMode: ['variant', '.dark'] — warns, selector must not be bare .dark - darkMode: ['variant', '.no-ampersand'] — warns, selector must contain & - darkMode: ['variant', [invalid, invalid]] — warns for each bad selector - darkMode: ['variant', [...valid]] — array of valid selectors Branch coverage for src/compat/dark-mode.ts improves from ~67% to 100%.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new Vitest suite to validate Tailwind’s dark mode compatibility behavior, including legacy class mode output and warnings for invalid variant selectors.
Changes:
- Adds smoke tests for existing
mediaandselectordark mode outputs - Adds coverage for legacy
darkMode: "class"selector generation (default + custom class) - Adds coverage for
darkMode: ["variant", ...]warning cases and valid selector output
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) | ||
|
|
||
| await run( | ||
| ['dark:underline'], | ||
| css` | ||
| @tailwind utilities; | ||
| @config "./config.js"; | ||
| `, | ||
| { | ||
| loadModule: async () => ({ | ||
| module: { darkMode: ['variant', '.dark'] }, | ||
| base: '/root', | ||
| path: '', | ||
| }), | ||
| }, | ||
| ) | ||
|
|
||
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, you must provide a selector.\nExample: `darkMode: ["variant", ".your-selector &"]`', | ||
| ) | ||
|
|
||
| warn.mockRestore() | ||
| }) | ||
|
|
||
| test('darkMode: ["variant", ".no-ampersand"] — warns when selector does not contain &', async () => { | ||
| const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) | ||
|
|
||
| await run( | ||
| ['dark:underline'], | ||
| css` | ||
| @tailwind utilities; | ||
| @config "./config.js"; | ||
| `, | ||
| { | ||
| loadModule: async () => ({ | ||
| module: { darkMode: ['variant', '.no-ampersand'] }, | ||
| base: '/root', | ||
| path: '', | ||
| }), | ||
| }, | ||
| ) | ||
|
|
||
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, your selector must contain `&`.\nExample `darkMode: ["variant", ".your-selector &"]`', | ||
| ) | ||
|
|
||
| warn.mockRestore() | ||
| }) | ||
|
|
||
| test('darkMode: ["variant", [".dark", ".no-ampersand"]] — warns for each invalid selector in the array', async () => { | ||
| const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) | ||
|
|
||
| await run( | ||
| ['dark:underline'], | ||
| css` | ||
| @tailwind utilities; | ||
| @config "./config.js"; | ||
| `, | ||
| { | ||
| loadModule: async () => ({ | ||
| module: { darkMode: ['variant', ['.dark', '.no-ampersand']] }, | ||
| base: '/root', | ||
| path: '', | ||
| }), | ||
| }, | ||
| ) | ||
|
|
||
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, you must provide a selector.\nExample: `darkMode: ["variant", ".your-selector &"]`', | ||
| ) | ||
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, your selector must contain `&`.\nExample `darkMode: ["variant", ".your-selector &"]`', | ||
| ) | ||
|
|
||
| warn.mockRestore() | ||
| }) |
| test('darkMode: ["variant", [".dark", ".no-ampersand"]] — warns for each invalid selector in the array', async () => { | ||
| const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) | ||
|
|
||
| await run( | ||
| ['dark:underline'], | ||
| css` | ||
| @tailwind utilities; | ||
| @config "./config.js"; | ||
| `, | ||
| { | ||
| loadModule: async () => ({ | ||
| module: { darkMode: ['variant', ['.dark', '.no-ampersand']] }, | ||
| base: '/root', | ||
| path: '', | ||
| }), | ||
| }, | ||
| ) | ||
|
|
||
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, you must provide a selector.\nExample: `darkMode: ["variant", ".your-selector &"]`', | ||
| ) | ||
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, your selector must contain `&`.\nExample `darkMode: ["variant", ".your-selector &"]`', | ||
| ) |
Confidence Score: 4/5Tests-only change; no production code is modified and the new tests accurately reflect the source implementation. The snapshot values are correct, and the coverage claims in the PR description match what the tests actually exercise. The only concern is that warn.mockRestore() is not inside a try/finally block in the warning tests, meaning a failing assertion would leave console.warn mocked for the remainder of the suite run. The three warning tests in dark-mode.test.ts would benefit from try/finally cleanup guards. Reviews (1): Last reviewed commit: "test(compat): add coverage for darkModeP..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, you must provide a selector.\nExample: `darkMode: ["variant", ".your-selector &"]`', | ||
| ) | ||
| expect(warn).toHaveBeenCalledWith( | ||
| 'When using `variant` for `darkMode`, your selector must contain `&`.\nExample `darkMode: ["variant", ".your-selector &"]`', | ||
| ) | ||
|
|
||
| warn.mockRestore() | ||
| }) |
There was a problem hiding this comment.
warn.mockRestore() is called unconditionally at the end of each warning test, so if a toHaveBeenCalledWith assertion throws, the spy is never restored. Subsequent tests in this file would then run with console.warn still mocked, silently swallowing warnings they depend on. Wrapping the assertions in a try/finally block guarantees cleanup regardless of assertion outcome. The same pattern appears in the ["variant", ".dark"] and ["variant", ".no-ampersand"] tests as well.
| expect(warn).toHaveBeenCalledWith( | |
| 'When using `variant` for `darkMode`, you must provide a selector.\nExample: `darkMode: ["variant", ".your-selector &"]`', | |
| ) | |
| expect(warn).toHaveBeenCalledWith( | |
| 'When using `variant` for `darkMode`, your selector must contain `&`.\nExample `darkMode: ["variant", ".your-selector &"]`', | |
| ) | |
| warn.mockRestore() | |
| }) | |
| try { | |
| expect(warn).toHaveBeenCalledWith( | |
| 'When using `variant` for `darkMode`, you must provide a selector.\nExample: `darkMode: ["variant", ".your-selector &"]`', | |
| ) | |
| expect(warn).toHaveBeenCalledWith( | |
| 'When using `variant` for `darkMode`, your selector must contain `&`.\nExample `darkMode: ["variant", ".your-selector &"]`', | |
| ) | |
| } finally { | |
| warn.mockRestore() | |
| } | |
| }) |
WalkthroughThis PR adds comprehensive Vitest coverage for the 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/tailwindcss/src/compat/dark-mode.test.ts (1)
109-132: ⚡ Quick winConsider also asserting the CSS output to verify variant is not generated.
The test correctly verifies that the warning is issued, but doesn't check that the
dark:variant is not generated when the selector is invalid. Adding a snapshot or empty-output assertion would more completely verify the behavior.🧪 Suggested enhancement
test('darkMode: ["variant", ".dark"] — warns when selector is exactly .dark (must provide a real selector)', async () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - await run( + const result = await run( ['dark:underline'], css` `@tailwind` utilities; `@config` "./config.js"; `, { loadModule: async () => ({ module: { darkMode: ['variant', '.dark'] }, base: '/root', path: '', }), }, ) expect(warn).toHaveBeenCalledWith( 'When using `variant` for `darkMode`, you must provide a selector.\nExample: `darkMode: ["variant", ".your-selector &"]`', ) + // Verify no dark variant is generated when selector is invalid + expect(result.trim()).toBe('') warn.mockRestore() })Apply similar enhancements to lines 134-157 and 159-185.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tailwindcss/src/compat/dark-mode.test.ts` around lines 109 - 132, Extend the test "darkMode: [\"variant\", \".dark\"] — warns when selector is exactly .dark (must provide a real selector)" to also assert the generated CSS does not include the dark variant: capture the output returned by run (the call that compiles ['dark:underline'] with `@tailwind` utilities and the loadModule returning darkMode: ['variant', '.dark']) and add an assertion such as expect(output.css).not.toContain('.dark') or expect(output.css).not.toContain('dark:underline') or a snapshot asserting no generated variant rules; apply the same pattern to the similar tests around lines 134-157 and 159-185 so each verifies both the warning (vi.spyOn console.warn) and that no dark variant CSS was emitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/tailwindcss/src/compat/dark-mode.test.ts`:
- Around line 109-132: Extend the test "darkMode: [\"variant\", \".dark\"] —
warns when selector is exactly .dark (must provide a real selector)" to also
assert the generated CSS does not include the dark variant: capture the output
returned by run (the call that compiles ['dark:underline'] with `@tailwind`
utilities and the loadModule returning darkMode: ['variant', '.dark']) and add
an assertion such as expect(output.css).not.toContain('.dark') or
expect(output.css).not.toContain('dark:underline') or a snapshot asserting no
generated variant rules; apply the same pattern to the similar tests around
lines 134-157 and 159-185 so each verifies both the warning (vi.spyOn
console.warn) and that no dark variant CSS was emitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3147b739-001d-4f94-885f-386860b4aab8
📒 Files selected for processing (1)
packages/tailwindcss/src/compat/dark-mode.test.ts
|
Hey! This is unnecessary. If you run into bugs, please open an issue first. |
Summary
Adds a dedicated test file
src/compat/dark-mode.test.tsfor thedarkModePluginfunction in the v3 compat layer. Coverage analysis showedsrc/compat/dark-mode.tshad ~67% branch coverage with several real code paths completely untested.What's covered by the new tests
darkMode: 'class'— legacy:is()selector with default.darkdarkMode: ['class', '.custom']— legacy:is()with custom classdarkMode: ['variant', '.dark']— warns: must provide a real selectordarkMode: ['variant', '.no-amp']— warns: selector must contain&darkMode: ['variant', [...invalid]]— warns for each bad selector in arraydarkMode: ['variant', [...valid]]— array of valid selectorsdarkMode: 'media'anddarkMode: 'selector'— smoke testsNo behaviour changes
Tests only — no source files modified.