-
Notifications
You must be signed in to change notification settings - Fork 492
chore: merge vitest config into vite.config.mts #8132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughConsolidates Vitest configuration into Changes
Possibly related PRs
Suggested reviewers
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 |
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/17/2026, 08:55:51 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 18.1 kB (baseline 18.1 kB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 878 kB (baseline 878 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 66.6 kB (baseline 66.6 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 385 kB (baseline 385 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 4.22 kB (baseline 4.22 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 2.85 kB (baseline 2.85 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 26.3 kB (baseline 26.3 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.74 MB (baseline 2.74 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 16.6 kB (baseline 16.6 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 9.09 MB (baseline 9.09 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 5.88 MB (baseline 5.88 MB) • ⚪ 0 BBundles that do not match a named category
|
d7e80ab to
ae16759
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@vite.config.mts`:
- Around line 14-15: The Vite-only typing prevents Vitest-specific `test`
options from being type-checked; update the config typing to use Vitest-aware
types by replacing or augmenting the current `UserConfig` usage with Vitest's
config type and ensuring `defineConfig` from 'vitest/config' is parametrized
accordingly (referencing `defineConfig` and the `test` block), e.g., import the
appropriate type from 'vitest' and apply it to the config or remove the
Vite-only `satisfies` so the `test` block (and symbols around `defineConfig` and
`UserConfig`) are validated by Vitest's types.
- Around line 516-535: The Vitest configuration in the test block (the test
object in vite.config.mts) currently only includes 'src/**' and 'packages/**'
patterns; add an include pattern for the tests-ui folder so files like
tests-ui/stores/templateRankingStore.test.ts and
tests-ui/tests/composables/graph/useGraphNodeManager.test.ts are picked up.
Update the test.include array to also contain a pattern for
tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx} so Vitest will run
those test files.
| test: { | ||
| globals: true, | ||
| environment: 'happy-dom', | ||
| setupFiles: ['./vitest.setup.ts'], | ||
| retry: process.env.CI ? 2 : 0, | ||
| include: [ | ||
| 'src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}', | ||
| 'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}' | ||
| ], | ||
| coverage: { | ||
| reporter: ['text', 'json', 'html'] | ||
| }, | ||
| exclude: [ | ||
| '**/node_modules/**', | ||
| '**/dist/**', | ||
| '**/cypress/**', | ||
| '**/.{idea,git,cache,output,temp}/**', | ||
| '**/{karma,rollup,webpack,vite,vitest,jest,ava,babel,nyc,cypress,tsup,build,eslint,prettier}.config.*' | ||
| ], | ||
| silent: 'passed-only' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if tests-ui directory exists
if [ -d tests-ui ]; then
echo "=== tests-ui directory exists ==="
# Find test files in tests-ui
fd -a -e ts -e tsx -e js -e jsx -e mts -e cts -e mjs -e cjs '(\.test|\.spec)\.' tests-ui | head -20
else
echo "tests-ui directory does not exist"
fi
# Check current vite.config.mts include patterns
echo -e "\n=== Vite config include patterns ==="
sed -n '516,535p' vite.config.mtsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 905
Add tests-ui/** to Vitest include patterns.
Tests in tests-ui/ won't run with the current configuration. Real test files exist at tests-ui/stores/templateRankingStore.test.ts and tests-ui/tests/composables/graph/useGraphNodeManager.test.ts, but Vitest's include patterns only cover src/** and packages/**. This reduces coverage and misses regressions for unit/component tests in the tests-ui directory.
Add tests-ui to include
include: [
'src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}',
- 'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'
+ 'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}',
+ 'tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'
],🤖 Prompt for AI Agents
In `@vite.config.mts` around lines 516 - 535, The Vitest configuration in the test
block (the test object in vite.config.mts) currently only includes 'src/**' and
'packages/**' patterns; add an include pattern for the tests-ui folder so files
like tests-ui/stores/templateRankingStore.test.ts and
tests-ui/tests/composables/graph/useGraphNodeManager.test.ts are picked up.
Update the test.include array to also contain a pattern for
tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx} so Vitest will run
those test files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does tests-ui exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DrJKL Yes, the tests-ui directory exists. The script output in my previous analysis shows:
=== tests-ui directory exists ===
tests-ui/stores/templateRankingStore.test.ts
tests-ui/tests/composables/graph/useGraphNodeManager.test.ts
The directory contains at least two test files that won't be picked up by the current Vitest configuration since the include patterns only cover src/** and packages/**. Adding tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx} to the include array would ensure these tests run.
🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : For mocking in tests, leverage Vitest's utilities; keep module mocks contained without global mutable state
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : For component testing, use Vue Test Utils and make components easy to test
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Be parsimonious in testing; do not write redundant tests
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Do not write tests dependent on non-behavioral features like utility classes or styles
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Do not mock what you don't own; be selective with external dependencies
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Aim for behavioral coverage of critical and new features
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Unit and component test files must use `.test.ts` extension and be located in `tests-ui/` or `src/**/*.test.ts`
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Use `vi.hoisted()` if necessary to allow per-test Arrange phase manipulation of mock state
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Write tests for all changes, especially bug fixes to catch regressions
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to src/**/*.{ts,tsx,vue} : ESLint rules: Vue + TS rules, no floating promises, unused imports disallowed, i18n raw text restrictions in templates
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Do not write tests that just test the mocks; ensure tests fail when code behaves unexpectedly
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: tests-ui/tests/platform/cloud/subscription/components/SubscriptionPanel.test.ts:189-194
Timestamp: 2025-12-22T21:36:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:22.621Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and @AustinMroz !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a lint-staged check to start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lint-staged.config.mjs`:
- Around line 3-5: The current lint-staged rule for 'tests-ui/**' blocks any
staged change; replace the static rule with a function that only fails when
newly added files under that pattern are staged by running a git cached diff
filtered for added files (e.g. using "git diff --cached --name-status
--diff-filter=A") and checking whether any of those added paths match
'tests-ui/**'; if matches exist, print the deprecation message and exit 1,
otherwise return a no-op so renames/deletions/edits don't trigger the check.
Ensure you update the rule for 'tests-ui/**' in the lint-staged config to use
this function-based approach.
♻️ Duplicate comments (1)
vite.config.mts (1)
516-535: Ensure tests under tests-ui are either migrated or included.The
includepatterns only coversrc/**andpackages/**. If any tests still live intests-ui/, they won’t run. With the new lint-staged rule blocking additions, please confirm those tests are migrated or addtests-ui/**temporarily.✅ Option A: include tests-ui until migration completes
include: [ 'src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}', - 'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}' + 'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}', + 'tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}' ],#!/bin/bash # Check if any tests still exist under tests-ui/ if [ -d tests-ui ]; then fd -a -e ts -e tsx -e js -e jsx -e mts -e cts -e mjs -e cjs '(\.test|\.spec)\.' tests-ui fi
| export default { | ||
| 'tests-ui/**': () => 'echo "Files in tests-ui/ are deprecated. Colocate tests with source files." && exit 1', | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n lint-staged.config.mjsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 196
🏁 Script executed:
# Check if there are other lint-staged configs or documentation in the repo
fd -t f "lint-staged" --or -name "*lint*" -type f | grep -E "\.(mjs|js|json|md)$" | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 196
🏁 Script executed:
# Search for git diff usage in the codebase to understand patterns
rg "git diff.*--cached" -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 196
🌐 Web query:
lint-staged glob pattern function return value behavior
💡 Result:
Short answer: lint-staged lets you use functions in JS configs; those functions receive the (staged) filenames and must return either
- a string (one shell command),
- an array of strings (multiple commands), or
- a Promise resolving to one of the above.
Important details
- If you export a function as the whole config, it receives ALL staged filenames. If you use a function as a value for a specific glob key, it receives only the staged files that match that glob. [1][2]
- Returned command strings are treated as complete shell commands. They do NOT have filenames automatically appended — include the filenames in the returned string(s) yourself (or return commands that already incorporate the filenames). [2]
- Functions may be async (return a Promise); use awaiting/filtering inside the function when needed (examples shown in the docs). [2][3]
Examples (conceptual)
- Export-as-function:
(filenames) =>eslint --fix ${filenames.join(' ')}// receives all staged files - Per-glob function:
'*.js': (files) => files.length ? [eslint --fix ${files.join(' ')}] : [] // receives only matched files
Sources
- lint-staged docs (JS config / function signature & examples). [1]
- lint-staged README on GitHub (filtering + behavior for glob keys). [2]
Scope the tests-ui lint-staged block to new files only.
The rule currently blocks any staged change under tests-ui/ (including deletions and renames), preventing migrations without --no-verify. Restrict the block to new file additions only.
💡 Proposed fix (block only added files)
-import path from 'node:path'
+import path from 'node:path'
+import { execSync } from 'node:child_process'
export default {
- 'tests-ui/**': () => 'echo "Files in tests-ui/ are deprecated. Colocate tests with source files." && exit 1',
+ 'tests-ui/**': () => {
+ const added = execSync(
+ 'git diff --cached --name-status --diff-filter=A -- tests-ui',
+ { encoding: 'utf8' }
+ )
+ return added.trim()
+ ? 'echo "Files in tests-ui/ are deprecated. Colocate tests with source files." && exit 1'
+ : []
+ },🤖 Prompt for AI Agents
In `@lint-staged.config.mjs` around lines 3 - 5, The current lint-staged rule for
'tests-ui/**' blocks any staged change; replace the static rule with a function
that only fails when newly added files under that pattern are staged by running
a git cached diff filtered for added files (e.g. using "git diff --cached
--name-status --diff-filter=A") and checking whether any of those added paths
match 'tests-ui/**'; if matches exist, print the deprecation message and exit 1,
otherwise return a no-op so renames/deletions/edits don't trigger the check.
Ensure you update the rule for 'tests-ui/**' in the lint-staged config to use
this function-based approach.
Moves vitest configuration from `vitest.config.ts` into the `test` section of `vite.config.mts` and deletes the separate vitest config file. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8132-chore-merge-vitest-config-into-vite-config-mts-2eb6d73d365081ab81b5dca11fadf13a) by [Unito](https://www.unito.io)
Moves vitest configuration from
vitest.config.tsinto thetestsection ofvite.config.mtsand deletes the separate vitest config file.┆Issue is synchronized with this Notion page by Unito