Define multiple vitest configs to separate the different test scenarios better #8630
Conversation
📝 WalkthroughWalkthroughThis change introduces dedicated Vitest configuration files for different test types and updates the npm scripts in Changes
Possibly related PRs
Suggested labels
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
89-91: Dedicated configs for e2e and screenshot tests—consider moving inline flags into config files
The newtest-e2e,test-screenshot, andtest-wkorg-screenshotscripts correctly point to their specific Vitest configs. The inline flags (--fileParallelism=false,--testTimeout,--hookTimeout) are shared across screenshot tests and could be declared directly withinvitest_screenshot.config.tsandvitest_wkorg_screenshot.config.tsfor cleaner CLI commands and single-source maintenance."test-screenshot": "vitest --config vitest_screenshot.config.ts run --testTimeout=300000 --hookTimeout=20000", "test-wkorg-screenshot": "vitest --config vitest_wkorg_screenshot.config.ts run --testTimeout=300000 --hookTimeout=20000",→
"test-screenshot": "vitest --config vitest_screenshot.config.ts run", "test-wkorg-screenshot": "vitest --config vitest_wkorg_screenshot.config.ts run",And add to each config:
// vitest_screenshot.config.ts import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { testTimeout: 300_000, hookTimeout: 20_000, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package.json(1 hunks)vitest.config.ts(2 hunks)vitest_e2e.config.ts(1 hunks)vitest_screenshot.config.ts(1 hunks)vitest_spec.config.ts(1 hunks)vitest_wkorg_screenshot.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
vitest_e2e.config.ts (1)
vitest.config.ts (1)
config(9-25)
vitest_spec.config.ts (1)
vitest.config.ts (1)
config(9-25)
vitest_wkorg_screenshot.config.ts (1)
vitest.config.ts (1)
config(9-25)
vitest_screenshot.config.ts (1)
vitest.config.ts (1)
config(9-25)
🔇 Additional comments (8)
vitest_e2e.config.ts (1)
1-10: Clean configuration separation for E2E testsGood approach creating a dedicated configuration for E2E tests. This separation helps prevent accidental running of E2E tests that could affect the database, as mentioned in the base config comments.
vitest_screenshot.config.ts (1)
1-10: Good configuration for screenshot testsThis dedicated config file for Puppeteer screenshot tests follows the same pattern as the other configs, maintaining consistency while providing a specific test scope.
vitest_wkorg_screenshot.config.ts (1)
1-10: Appropriate configuration for WKORG screenshot testsGood job further specializing the screenshot tests with a dedicated config for the WKORG-specific screenshot tests. This granular separation allows for more targeted test runs.
vitest_spec.config.ts (1)
1-10: Well-structured configuration for unit testsThis configuration for unit tests (*.spec.ts files) follows the same clean pattern as the other configs, completing the set of specialized test configurations.
vitest.config.ts (3)
4-9: Good documentation of the motivation for these changesThe comments clearly explain the reasoning behind this refactoring - preventing accidental execution of E2E tests that could affect the database. This context is valuable for other developers.
13-15: Effective safeguard against accidental test executionUsing a descriptive message in the include pattern is an elegant way to prevent accidental test runs. This ensures users must explicitly choose a test configuration.
25-27: Clean export structureGood separation of the configuration object from the default export. This allows other config files to import and extend the base config while maintaining the proper Vitest configuration structure.
package.json (1)
85-87:✅ Verification successful
Explicit spec Vitest config in npm scripts is a good improvement
Thetest,test-changed, andtest-watchscripts now correctly referencevitest_spec.config.ts, centralizing all spec-test settings. Please ensure this config file lives at the repo root and that its glob patterns cover all intended spec tests.Run this quick check to verify the config file exists:
🏁 Script executed:
#!/bin/bash if [ ! -f vitest_spec.config.ts ]; then echo "ERROR: vitest_spec.config.ts not found" exit 1 fiLength of output: 38
🏁 Script executed:
#!/bin/bash # Verify that vitest_spec.config.ts lives at the repo root and display its include patterns if [ -f vitest_spec.config.ts ]; then echo "✅ vitest_spec.config.ts found at repo root" else echo "❌ vitest_spec.config.ts not found at repo root" exit 1 fi echo -e "\n🔍 Glob patterns in vitest_spec.config.ts:" grep -R "include" -n vitest_spec.config.ts || echo "No 'include' patterns found"Length of output: 334
Vitest config verified at root with include patterns
- ✅
vitest_spec.config.tsis located at the repository root.- 🔍 Its
includepatterns currently cover./frontend/javascripts/test/**/*.spec.ts.If this pattern encompasses all of your spec files, no further action is needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build_test_deploy.yml (1)
44-44: Use npm script alias for frontend tests
You’ve updated the workflow to point at the new Vitest config directly—which is perfect for clarity—but we already defined an npm script (e.g.test:spec) inpackage.jsonencapsulating this command. To avoid duplication and ensure consistency, consider replacing this line with:run: yarn test:specThis centralizes any future tweaks to the Vitest invocation in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build_test_deploy.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
2 participants
Follow-up to #8479 etc. See comments in diff for motivation.