Skip to content

Conversation

@TabishB
Copy link
Contributor

@TabishB TabishB commented Jan 14, 2026

Summary

  • Cap Vitest worker parallelism to prevent runaway CPU/memory usage when running tests
  • Explicitly set pool: 'forks' since tests rely on process isolation (process.cwd() assumptions)
  • Add resolveMaxWorkers() function that caps workers at min(4, availableCPUs)
  • Allow VITEST_MAX_WORKERS env override for CI/automation tuning

Test plan

  • Run npm test locally to verify tests still pass with the new config
  • Verify resource usage is more controlled during test runs
  • Test with VITEST_MAX_WORKERS=2 to confirm env override works

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Optimized test execution configuration with OS-aware worker allocation. Tests now intelligently scale worker count based on available system resources, with environment variable override support for custom configurations.

✏️ Tip: You can customize this high-level summary in your review settings.

Vitest v3 defaults to `pool: "forks"` and scales worker processes with
CPU count. This repo's tests spawn many Node processes (CLI invocations,
temp FS operations), which can cause runaway CPU/memory usage when
combined with high parallelism.

Changes:
- Explicitly set pool to 'forks' (tests rely on process isolation)
- Add resolveMaxWorkers() to cap workers at min(4, availableCPUs)
- Allow VITEST_MAX_WORKERS env override for CI/automation tuning
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Vitest configuration updated to dynamically calculate max worker count based on OS CPU availability with environment variable override support, capping workers between 1 and 4. Changes include a new resolveMaxWorkers helper function and explicit pool: 'forks' setting.

Changes

Cohort / File(s) Summary
Vitest worker configuration
vitest.config.ts
Added resolveMaxWorkers helper to read VITEST_MAX_WORKERS env var with fallback to os.availableParallelism() or os.cpus().length; caps workers to max 4, min 1; wired to explicit pool: 'forks' setting; replaced pool comment with note on process isolation requirements.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A worker's tale, so fine and fleet,
OS counts the cores we'll meet,
Four workers dance, or less, no more,
Capped with care, this config's core! 🔄✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main change: capping worker parallelism to prevent process storms, which aligns with the core objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
vitest.config.ts (1)

4-21: Consider truncating the env var to an integer.

The validation accepts fractional values (e.g., VITEST_MAX_WORKERS=2.5), but worker counts should be integers. Vitest may behave unexpectedly with non-integer values.

Additionally, the return type number | undefined is never undefined in practice since the CPU-based fallback always returns a number.

Suggested fix
 function resolveMaxWorkers(): number | undefined {
-  // Allow callers (CI/agents) to override without editing config.
   const raw = process.env.VITEST_MAX_WORKERS;
   if (raw) {
-    const parsed = Number(raw);
+    const parsed = Math.floor(Number(raw));
     if (Number.isFinite(parsed) && parsed > 0) {
       return parsed;
     }
   }
 
-  // Vitest v3 defaults to `pool: "forks"` and scales worker processes with CPU.
-  // This repo's tests can spawn many Node processes (CLI invocations, temp FS),
-  // so cap parallelism to avoid runaway CPU/memory usage in automation.
   const cpuCount = typeof os.availableParallelism === 'function'
     ? os.availableParallelism()
     : os.cpus().length;
   return Math.min(4, Math.max(1, cpuCount));
 }

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08c3493 and f1a48d8.

📒 Files selected for processing (1)
  • vitest.config.ts
🔇 Additional comments (2)
vitest.config.ts (2)

1-2: LGTM!

Clean import using the node: protocol prefix.


28-30: LGTM!

Good use of explicit pool: 'forks' with a clear comment explaining the rationale. Wiring maxWorkers to the resolver function keeps the configuration clean.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@TabishB TabishB merged commit 322bfd4 into main Jan 14, 2026
7 checks passed
@TabishB TabishB deleted the TabishB/vitest-proc-storm branch January 14, 2026 05:41
@vibe-kanban-cloud
Copy link

Review Complete

Your review story is ready!

View Story

Comment !reviewfast on this PR to re-generate the story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants