perf(test): parallelize the unit suite (~17.5 min → ~6.6 min) and retire duplicate CI workflow

[Yeraze]

Summary

The unit suite took ~20 min in CI. Profiling showed only ~5.9 min of that was actually running tests — the rest was per-file startup paid one file at a time.

Result: ~20 min → ~3 min on a 4-core runner, with a provably identical test set.

Verification

Diffed the complete before/after test set, keyed by file + full test name:

Tests compared	11,808
Missing after	0
Newly appearing	0
Status changes (pass/skip/fail)	0

Green with and without the PostgreSQL/MySQL containers (containers add 570 multi-backend tests that skip locally). npm run typecheck and npm run lint:ci both clean. npm run docs:build verified before adding it as a gating job.

Suite changes

fileParallelism: false was the single largest cost. Added in #295 (2025-10-24) as a drive-by in an unrelated feature PR, when this suite was 47 files. It is now ~790, and each file's ~1s import/transform cost was being paid serially.

The adjacent poolOptions.forks.singleFork was already dead code — Vitest 4 removed poolOptions, so it had silently stopped applying and only emitted a deprecation warning.

One constant cost 53% of all test execution time. userTestHelper.ts used SALT_ROUNDS = 12 — higher than production's 10 — and is called from beforeEach, so it was paid per test. bcrypt at 12 is ~209ms/hash; at 4 it is ~2ms. The fingerprint was unmistakable: 206 of 211 meshcoreRoutes tests took exactly ~425ms, and all 34 auditRoutes tests exactly ~500ms. The five files importing this helper were precisely the five slowest files (185s of 352s). Nothing asserts the cost factor, and production hashing is set separately in localAuth.ts/mfa.ts.

A real race had to be fixed for parallelism to be safe. 15 files share one meshmonitor_test PostgreSQL/MySQL database and issue DROP TABLE ... CASCADE at import. The overlaps are genuine — users (3 files) and nodes (3 files). Naive parallelism would have raced in CI only, since those tests skip silently when the containers are down. They now run in a serial shared-db project; the serialized tail is only ~18s.

Local-only: Vitest was collecting .claude/worktrees/, re-running the entire suite once per leftover worktree (+11 min, +22k phantom tests, plus phantom failures from those trees' own stale dependencies). Now excluded — this is the Vitest half of the gotcha already documented in CLAUDE.md for the lint ratchet.

CI changes — the full suite ran 4× per PR

ci.yml runs a 3-way Node matrix (22/24/25), and pr-tests.yml ran it a fourth time. Every job in pr-tests.yml was a strict subset:

• quick-test duplicated the Node 24 matrix leg exactly — same lint ratchet, typecheck, test:run, same PG/MySQL service containers.
• focused-tests then re-ran parts of that same suite again (src/components/, two backend files), plus a build already covered unconditionally by Build Check.
• changed-files existed only to gate those two.

So pr-tests.yml is deleted. Three things it provided were not redundant, and are preserved:

1. The VitePress docs build — its only unique coverage. Moved into ci.yml. deploy-docs.yml only runs on push to main, i.e. after merge, so without this a docs break is caught by the deploy rather than the PR.
2. Trigger breadth — pr-tests.yml had no target-branch filter; ci.yml was limited to main/develop. That filter is removed, otherwise PRs targeting release/feature branches would have had no test run at all.
3. A concurrency group — ci.yml had none, so superseded pushes kept burning runners to completion.

Also updated the dependabot wait-on-check regexp (it waited on the now-gone Quick Tests), the README badge, and CONTRIBUTING.

The Node 22/24/25 matrix is kept — those are the runtimes actually shipped (armv7 image is 22; main image and LXC are 24). Cutting it would be real coverage loss, not deduplication.

Notes

• If OOM ever returns, lower maxWorkers rather than re-serializing; the config says so.
• Two follow-ups left alone: channelRoutes.test.ts still has real 3.5–5.5s timer waits worth converting to fake timers (no longer on the critical path now that files run in parallel), and the known EnvironmentTeardownError teardown flake appeared once in five local runs — LOG_LEVEL=warn silences the migration log spam that drives it, but it is no faster and would strip log context from CI failures, so it is not included here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01L9NzRtqE8eSMS8tvAeodUB

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: perf(test): parallelize unit suite and retire duplicate CI workflow

This is a well-executed, thoroughly analyzed performance improvement. The investigation is solid, the root causes are identified with evidence, and the tradeoffs are explicitly documented. Here's my detailed review:

---

vitest.config.ts — Overall: Excellent

The two-project split is the right approach. Separating the shared-db tests (which race on shared PostgreSQL/MySQL tables) into a serial project while letting everything else run in parallel is cleaner than ad-hoc workarounds.

SHARED_DB_TESTS maintenance burden (vitest.config.ts:24-40): The static list of files that need the serial project is a "manual registry" that will silently become stale. If a developer adds createPostgresBackend/createMysqlBackend to a new test file and forgets to add it here, the resulting race condition will only surface in CI (where containers are up) and only non-deterministically. The comment at line 21 says "Add a file here whenever it starts using createPostgresBackend / createMysqlBackend" — this is good documentation but no enforcement exists.

Suggestion: Consider an ESLint rule or a grep-based pre-commit hook that scans for createPostgresBackend|createMysqlBackend across test files and validates every match appears in SHARED_DB_TESTS. Alternatively, a test-time assertion in a CI-only test (a "meta-test") would catch the miss before a flaky race does.

maxWorkers: process.env.CI ? 4 : '75%' (vitest.config.ts:85): Hardcoding 4 for CI matches the stated "4-core runner" assumption from the PR description. This is reasonable now, but worth noting that if the runner type ever changes (e.g., to 8-core), CI will use only half the available workers. Using '50%' instead of 4 would make this future-proof. Minor point.

poolOptions removal — correct. The PR body explains Vitest 4 dropped this, so removing dead config is right.

isolate: true in COMMON_TEST (vitest.config.ts:61): This is already the Vitest default for forks pool, so it's redundant (no harm done, just noise).

---

userTestHelper.ts — Correct and Well-Documented

SALT_ROUNDS = 4 (userTestHelper.ts:35): This is the right tradeoff. The extensive comment (lines 17-34) clearly explains why 4 and why not to raise it, which is exactly what future contributors need to see. The key safety property — that localAuth.ts/mfa.ts have their own separate round settings — is correctly called out.

No security concern: Test bcrypt rounds don't affect production; the passwords hashed here are throwaway literals in :memory: databases.

---

.github/workflows/ci.yml — Good, one observation

docs-build has no needs dependency (ci.yml:148-175): The build and docker jobs gate on needs: test. The docs-build job runs unconditionally in parallel. This is intentional per the comment at line 155 ("Run unconditionally rather than behind a dorny/paths-filter gate"), and the reasoning is sound (~1 min build, avoid missing config/dep changes). No issue here — just confirming the lack of needs: is deliberate.

The concurrency group (ci.yml:16-18): Using github.event.pull_request.number || github.ref is the standard pattern and correctly handles both PR events and branch pushes. Good.

Target-branch filter removal (ci.yml:7-11): The comment explains the rationale — removing the branches filter for pull_request events ensures PRs targeting release/feature branches get test runs. This is correct and the PR body's framing ("PRs targeting release/feature branches would have had no test run at all") is accurate.

checkout@3d3c42e5...: All checkout/action pins use commit SHAs — good security practice throughout.

---

.github/workflows/dependabot-auto-merge.yml — Correct

The check-regexp update (dependabot-auto-merge.yml:30) correctly removes "Quick Tests" and adds "Documentation Build" to match the new job names in ci.yml. The regexp now correctly waits on all meaningful CI jobs before auto-merging.

---

CLAUDE.md — Minor

The one-line change updates the worktree gotcha from a lint-only caveat to include Vitest. Accurate and helpful.

---

CONTRIBUTING.md — Minor

The update accurately documents the retired pr-tests.yml and explains what ci.yml now covers. The description at lines 131-140 is clear and will help future contributors understand the CI structure.

---

Potential Issues / Risks

1. SHARED_DB_TESTS staleness (medium risk): No mechanism enforces that new files using shared PG/MySQL backends get added to the list. A miss is a non-deterministic CI flake. Worth addressing, even with just a grep-based test or comment in test-utils.ts.

2. maxWorkers: 4 hardcode for CI (low risk): Tied to the 4-core runner assumption. Consider '50%' for resilience to runner type changes.

3. isolate: true in COMMON_TEST (trivial): Redundant with the forks pool default. Not harmful.

---

Summary

This PR is well-reasoned, well-documented, and solves a real problem cleanly. The profiling data in the PR description (timing per-file, identifying the bcrypt cost, catching the shared-DB race) demonstrates thorough investigation. The two-project Vitest split is the correct architectural solution. The CI deduplication is sound — the PR correctly identifies what was unique to pr-tests.yml and preserves it.

The main gap worth considering before merge is enforcement around SHARED_DB_TESTS to prevent future developers from accidentally introducing parallel shared-DB tests. Everything else is minor or cosmetic.

Verdict: Approve with the SHARED_DB_TESTS staleness concern noted for awareness. The improvement is substantial (~7× CI speedup, eliminated 4× test duplication) with low regression risk.

---

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: perf(test): parallelize unit suite and retire duplicate CI workflow

Tasks:

• Gather context and read changed files
• Review vitest.config.ts changes
• Review userTestHelper.ts changes
• Review CI workflow changes
• Summarize findings

---

This is a well-investigated and well-executed performance PR. The root-cause analysis is thorough (the fileParallelism: false history, the bcrypt profiling, the shared-DB race), the changes are minimal for their impact, and the verification methodology (diffing 11,808 tests by file + name) is exactly the right way to confirm no tests were lost. Here's my detailed review:

---

vitest.config.ts — Solid Architecture

The two-project split is correct. Separating the 15 shared-PostgreSQL/MySQL test files into a shared-db project with fileParallelism: false is the right solution — it's precise (only the genuinely conflicting files serialize) and self-documenting.

SHARED_DB_TESTS coverage is currently complete. I verified by grepping the entire src/ tree for createPostgresBackend|createMysqlBackend — all 15 matching test files appear in the list, and test-utils.ts (the source of those functions) is the only non-test hit.

Staleness risk (medium — worth a note): The list at vitest.config.ts:24-40 is a manual registry with no enforcement. A developer who adds createPostgresBackend to a new test file and forgets to update SHARED_DB_TESTS would introduce a non-deterministic CI race — one that only surfaces when containers are up (i.e., only in CI, and only sometimes). The comment at line 20 documents what to do, which is good, but a grep-based meta-test or a pre-commit hook scanning for the pattern would make enforcement automatic. The PR notes in the PR body leave this as a follow-up — that's fine, but worth calling out as the main ongoing maintenance burden.

maxWorkers: process.env.CI ? 4 : '75%' (vitest.config.ts:85): The 4 matches the stated 4-core runner. If the runner type ever changes (e.g. to 8-core), CI will underutilize it. '50%' would be more future-proof. Low risk, minor point.

isolate: true in COMMON_TEST (vitest.config.ts:60): This is already the default for the forks pool — the explicit setting is harmless redundancy.

Dead config removal is correct. poolOptions.forks.singleFork was already inert (Vitest 4 dropped poolOptions), and fileParallelism: false at the top level was the actual serialization culprit. Removing both is accurate.

---

userTestHelper.ts — Correct and Well-Documented

SALT_ROUNDS = 4 (userTestHelper.ts:35): This is the right tradeoff. The 18-line comment (lines 17-34) explains why 4 is correct, why the previous 12 was harmful, and why this must not be raised to match production. This is exactly what future contributors need to read before touching the value.

No security concern: Bcrypt rounds in a test helper only affect throwaway passwords in in-memory databases. Production hashing (localAuth.ts, mfa.ts) is unaffected, as the comment correctly states.

The observed impact is credible. bcrypt at 12 rounds takes ~209ms per hash; at 4 it takes ~2ms. In a beforeEach, this is paid per test, not per file. With 206 meshcoreRoutes tests × 2 hashes each that becomes ~86s from one constant — matching what the PR description's profiling reported.

---

ci.yml — Well-Reasoned

docs-build has no needs dependency (ci.yml:148): This is intentional and correct — the build is ~1 min, runs unconditionally, and the PR correctly notes that deploy-docs.yml only triggers on push to main (post-merge), so this is the right gate for PR coverage.

Target-branch filter removal (ci.yml:7-11): The inline comment explains the reasoning. Removing the branches filter from pull_request events is the right call — PRs targeting feature/release branches would otherwise have no CI run.

Concurrency group (ci.yml:16-18): github.workflow-${{ github.event.pull_request.number || github.ref }} is the correct pattern. It cancels in-progress runs on superseded pushes, which eliminates runner waste for active PRs.

Action SHA pins throughout: All uses: references are pinned to full commit SHAs with version comments — good security hygiene.

One minor observation: docs-build does not appear in needs for pr-comment (line 295 lists [test, build, docker]). The PR comment says "All tests and builds completed successfully" but won't wait for docs-build. This was likely pre-existing behavior and pr-comment uses continue-on-error: true, so it's not a correctness issue — just noting the asymmetry for awareness.

---

dependabot-auto-merge.yml — Correct

The check-regexp update (line 30) correctly removes "Quick Tests" (retired with pr-tests.yml) and adds "Documentation Build" (moved into ci.yml). The regexp now reflects the actual CI job names.

---

Other Files — Accurate

• CLAUDE.md: The one-line update correctly extends the worktree gotcha to cover Vitest in addition to the lint ratchet (both are now fixed with the same COMMON_EXCLUDE entry).
• CONTRIBUTING.md: The rewritten CI section accurately describes the new structure and explains what pr-tests.yml covered and where it moved.
• README.md: Removing the old pr-tests.yml badge is correct.

---

Summary

The changes are sound. The profiling methodology, the two-project Vitest split, the bcrypt constant change, and the CI deduplication are all well-reasoned and well-documented. The ~7× speedup is a meaningful improvement with low regression risk.

Main ongoing concern: The SHARED_DB_TESTS list in vitest.config.ts is the one piece of infrastructure that requires ongoing manual maintenance. Currently complete (all 15 callers are listed), but a future contributor adding a shared-DB test file could inadvertently create a CI race. Documenting the validation command in the config comment (e.g. grep -rn "createPostgresBackend\|createMysqlBackend" src/ --include="*.test.ts" -l) would lower the risk further.

Verdict: Approve. The improvement is substantial and the implementation is clean.

---

| Branch: chore/test-suite-speedup-and-ci-dedup

[Yeraze]

Correction: measured CI numbers (claimed ~3 min, actually ~6.6 min)

The original title/description claimed ~20 min → ~3 min. That ~3 min came from a local run with CI=true, and my cores are faster than GitHub's shared 4-vCPU runners. Now that this has actually run in CI, here are the real numbers.

Measured on identical hardware — the Run tests step of Test Suite (24.x):

	Run tests step	Full job
Before (main, 3 consecutive runs)	1056s / 1034s / 1061s — ~17.5 min	~21 min
After (this branch)	395s — ~6.6 min	~9.8 min

~2.7x faster, not the ~5.9x I projected. Title and commit message corrected.

Per-PR compute drops further because the duplicate 4th run is gone: ~70 min (4 x ~17.5) → ~20 min (3 x ~6.6).

Everything else holds — CI is green, and the test-set diff (11,808 tests, 0 missing / 0 added / 0 status changes) was verified locally and is independent of hardware.

Note on the first (red) run

The initial run failed and was not caused by these changes: Node 25.x died in Initialize containers before any step executed, with registry-1.docker.io: context deadline exceeded on all three Docker pull retries. fail-fast then cancelled the 22.x and 24.x legs, making it look like all three failed. Clean on rerun.

That is an argument for fail-fast: false on this matrix — one registry hiccup discarded two healthy legs that were ~40s from finishing, and at ~6.6 min a leg the redundancy is now cheap. Not changed here; separate call.