Migrated 80 ghost/core unit tests to vitest (bin/shared/adapters/data/lib/web)

[9larsons]

Summary

Second in the planned sequence migrating ghost/core's tests from mocha to vitest (after #27898). Expands vitest to cover test/unit/{bin,shared,server/{adapters,api,data,lib,web}} — 81 files / 824 tests.

Update (post-review): the original approach ran each migrated file under both runners as a safety net. That broke down on lexical.test.js — see findings below. Switched to vitest-only for migrated buckets: each bucket migration removes paths from mocha's run list and adds them to vitest's include glob. No more dual-compat constraint on individual files; future migrations are mechanical.

What's changed

• vitest.config.ts — include expanded to the new buckets; six done()-callback files stay excluded (deferred to a focused follow-up; ~50 callbacks to convert to promises)
• package.json — test:unit:base now lists the remaining mocha paths explicitly (unmigrated dirs + the six deferred done() files), instead of scanning all of ./test/unit
• vitest-setup.ts — kept the snapshot/port normalization and the bridged mochaHooks; no before/after shim
• test/.eslintrc.js — added beforeAll/afterAll to globals so the vitest-only files lint cleanly
• .c8rc.json — disabled the coverage gate (reporters still run). See "Coverage gate" below.

Findings from this bucket

1. describe(name, {options}, fn) is not valid Mocha syntax. Vitest accepts it, but Mocha treats the options object as the fn arg — the describe registers 0 tests and leaves the suite stack open, so subsequent files (e.g. mrr.test.js) end up nested under it. That's the source of the 14 failures on the previous run. Reverting lexical.test.js to its original this.timeout(5000) would have worked in mocha but failed in vitest (this is undefined), confirming there was no clean dual-compat option. Fix: lexical keeps the vitest-native {timeout: 5000} syntax and runs under vitest only.
2. before / after aren't vitest globals. Original PR aliased them in vitest-setup.ts. Removed the shim and converted the 5 affected files in migrated buckets to beforeAll/afterAll (storage/index, scheduling/utils, web/shared/middleware/brute, shared/config/loader, shared/config/adapter-config).
3. Three files use mocha-this patterns inside describes — lexical.test.js (this.timeout(5000)), sentry.test.js and importer/index.test.js (this.beforeEach(fn)). The this.beforeEach calls were already mocha-API misuses (they happen to work because mocha binds this to the Suite object); replaced with top-level beforeEach(fn).
4. 6 files use the mocha done() callback — vitest 4.x removed support. Listed under exclude in vitest.config.ts and explicitly included in test:unit:base so they continue to run under mocha. Deferred to a focused follow-up PR:
   • test/unit/server/adapters/scheduling/scheduling-default.test.js
   • test/unit/server/adapters/storage/local-images-storage.test.js
   • test/unit/server/lib/image/blog-icon.test.js
   • test/unit/server/lib/image/gravatar.test.js
   • test/unit/server/lib/package-json/parse.test.js
   • test/unit/server/web/api/middleware/cors.test.js

Coverage gate

c8 only sees mocha's run. Moving tests to vitest shrinks c8's covered-files count while the denominator (all source files in core/) stays constant, so coverage % drops on every migration PR even though true coverage is unchanged. If the gate stays on, each PR has to lower the threshold — pure noise.

Proper fix is to merge vitest's coverage into c8's report, but two blockers:

• @vitest/coverage-v8 instrumentation conflicts with the tsx loader: require('./bulk-filters') fails to resolve .ts files when coverage is enabled
• Vitest's forks pool workers don't flush native V8 coverage on exit (NODE_V8_COVERAGE propagates, but the worker doesn't write its coverage-<pid>.json before being torn down)

For now: check-coverage: false, reporters left on so numbers stay visible (~63% statements, ~62% functions, ~85% branches at this point). Re-enable a merged gate once the migration completes or a vitest+tsx coverage combo becomes viable.

Test plan

• pnpm test:vitest (from ghost/core): 81 files / 824 tests pass (+ 3 skipped)
• pnpm test:unit:base (mocha): 5689 tests pass — no more cross-file pollution from the broken describe options syntax
• pnpm exec eslint clean for changed files
• CI green

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 413eb04f-e125-41b2-bdcd-57fcdcce1240

📥 Commits

Reviewing files that changed from the base of the PR and between acaf5f9 and 141fe7d.

📒 Files selected for processing (1)

• ghost/core/.c8rc.json

✅ Files skipped from review due to trivial changes (1)

• ghost/core/.c8rc.json

---

Walkthrough

Updates Vitest configuration and test infra, disables strict c8 coverage thresholds, and adjusts the unit test script. Multiple unit test files are converted to Vitest-compatible patterns: suite timeouts moved to describe options, Mocha hook registrations standardized, before/after hooks replaced with beforeAll/afterAll where appropriate, and arrow callbacks changed to function() when using Mocha-style this bindings.

Possibly related PRs

• TryGhost/Ghost#27898: Introduced initial Vitest config and setup that this change extends.
• TryGhost/Ghost#27901: CI/automation changes that run Vitest and align with the updated vitest.config.ts and test script selection.

Suggested reviewers

• EvanHahn

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: migrating 80 ghost/core unit tests to vitest across the specified directories (bin, shared, adapters, data, lib, web).
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, explaining the migration strategy, findings, configuration changes, and test results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch vitest-ghost-core-unit-safe-buckets

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

ghost/core/.c8rc.json (1)

9-12: ⚡ Quick win

Add an explicit expiration/follow-up marker for lowered thresholds.

Lowering gates is understandable for this migration, but this can easily become permanent drift. Please add a nearby comment (or linked issue ID in commit/PR metadata) with a clear restore target so the 60% thresholds are temporary by contract, not convention.

🤖 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 `@ghost/core/.c8rc.json` around lines 9 - 12, The coverage thresholds for
"statements", "branches", "functions", and "lines" are temporarily lowered to
60%; add an explicit expiration/follow-up marker so this change isn’t permanent
drift by adding a clear adjacent comment or a linked issue/PR ID noting the
restore target and date (e.g., restore thresholds to previous values or X% by
YYYY-MM-DD) and reference the exact keys ("statements", "branches", "functions",
"lines") in that comment so reviewers and automation can find and enforce the
rollback.

🤖 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 `@ghost/core/.c8rc.json`:
- Around line 9-12: The coverage thresholds for "statements", "branches",
"functions", and "lines" are temporarily lowered to 60%; add an explicit
expiration/follow-up marker so this change isn’t permanent drift by adding a
clear adjacent comment or a linked issue/PR ID noting the restore target and
date (e.g., restore thresholds to previous values or X% by YYYY-MM-DD) and
reference the exact keys ("statements", "branches", "functions", "lines") in
that comment so reviewers and automation can find and enforce the rollback.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ff81a8f1-325f-4f39-90ea-61860b6ff9d0

📥 Commits

Reviewing files that changed from the base of the PR and between 87a1256 and acaf5f9.

📒 Files selected for processing (1)

• ghost/core/.c8rc.json