Migrated test/unit/frontend bucket to vitest#27992
Conversation
- continues the vitest migration (after #27898 / #27900 / #27974 / #27991) by moving all 135 files in test/unit/frontend (1804 tests) from mocha to vitest Conversions: - 39 files using mocha's suite-scope before()/after() -> beforeAll()/afterAll() - frontend-caching + handle-image-sizes: this.beforeEach/this.afterEach (mocha-API misuse) -> top-level beforeEach/afterEach - 6 files using the mocha done() callback converted off it: - static-theme.test.js: 42 uniform middleware tests -> a callStaticTheme() promise helper - handle-image-sizes + 4 routing/controller files -> a new shared test/utils/deferred.js helper that bridges `done` onto a promise (done() resolves, done(err) rejects), so existing call sites are unchanged - handle-image-sizes: removed 5 dead `fakeReq.url.match = fn` lines — assigning a property to a string primitive is a silent no-op in mocha's sloppy mode and throws under vitest's strict mode; the override never actually installed anything Snapshot support (first migrated bucket with snapshot tests — ghost-head): - vitest-setup.ts: added a beforeEach that bridges @tryghost/jest-snapshot's per-test config. The mocha hook reads `this.currentTest`; we derive the same testPath/testTitle from the vitest task (testTitle must match mocha's fullTitle() exactly or committed .snap keys won't resolve) - vitest.config.ts: set resolveSnapshotPath to a separate path so vitest's native snapshot system doesn't adopt the jest-snapshot-managed __snapshots__/*.snap files and report their entries as obsolete - vitest.config.ts include += test/unit/frontend; removed from test:unit:base
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR updates Vitest config to include frontend tests and custom snapshot paths, adds a deferred() test utility and a Vitest setup hook, removes frontend tests from the Mocha unit script, converts many frontend tests from Mocha callback-style to promise/async patterns, and replaces per-test Mocha lifecycle hooks with suite-level hooks (beforeAll/afterAll) across the frontend test suite. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ghost/core/test/unit/frontend/web/middleware/static-theme.test.js`:
- Around line 35-38: The helper callStaticTheme currently always resolves the
Promise; change its Promise executor to accept (resolve, reject) and invoke
staticTheme()(req, res, (err) => { if (err) return reject(err); resolve(); }) so
that when the middleware calls next(err) the test helper rejects with the error
instead of resolving; update references to callStaticTheme to handle returned
rejections where needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 896bac57-8212-4457-9be7-94cdbee6fa13
📒 Files selected for processing (49)
ghost/core/package.jsonghost/core/test/unit/frontend/apps/private-blogging/controller.test.jsghost/core/test/unit/frontend/helpers/asset.test.jsghost/core/test/unit/frontend/helpers/body-class.test.jsghost/core/test/unit/frontend/helpers/cancel-link.test.jsghost/core/test/unit/frontend/helpers/comment-count.test.jsghost/core/test/unit/frontend/helpers/concat.test.jsghost/core/test/unit/frontend/helpers/content-api-url.test.jsghost/core/test/unit/frontend/helpers/content.test.jsghost/core/test/unit/frontend/helpers/foreach.test.jsghost/core/test/unit/frontend/helpers/img-url.test.jsghost/core/test/unit/frontend/helpers/link-class.test.jsghost/core/test/unit/frontend/helpers/link.test.jsghost/core/test/unit/frontend/helpers/match.test.jsghost/core/test/unit/frontend/helpers/meta-description.test.jsghost/core/test/unit/frontend/helpers/meta-title.test.jsghost/core/test/unit/frontend/helpers/navigation.test.jsghost/core/test/unit/frontend/helpers/pagination.test.jsghost/core/test/unit/frontend/helpers/price.test.jsghost/core/test/unit/frontend/helpers/raw.test.jsghost/core/test/unit/frontend/helpers/recommendations.test.jsghost/core/test/unit/frontend/helpers/search.test.jsghost/core/test/unit/frontend/helpers/social-accounts.test.jsghost/core/test/unit/frontend/helpers/social-url.test.jsghost/core/test/unit/frontend/helpers/split.test.jsghost/core/test/unit/frontend/helpers/t-new.test.jsghost/core/test/unit/frontend/helpers/t.test.jsghost/core/test/unit/frontend/helpers/url.test.jsghost/core/test/unit/frontend/meta/context-object.test.jsghost/core/test/unit/frontend/meta/description.test.jsghost/core/test/unit/frontend/meta/paginated-url.test.jsghost/core/test/unit/frontend/public/private.test.jsghost/core/test/unit/frontend/services/assets-minification/assets-minification-base.test.jsghost/core/test/unit/frontend/services/assets-minification/minifier.test.jsghost/core/test/unit/frontend/services/card-assets.test.jsghost/core/test/unit/frontend/services/routing/controllers/entry.test.jsghost/core/test/unit/frontend/services/routing/controllers/rss.test.jsghost/core/test/unit/frontend/services/routing/controllers/static.test.jsghost/core/test/unit/frontend/services/rss/generate-feed.test.jsghost/core/test/unit/frontend/services/sitemap/manager.test.jsghost/core/test/unit/frontend/services/theme-engine/handlebars/helpers.test.jsghost/core/test/unit/frontend/services/theme-engine/preview.test.jsghost/core/test/unit/frontend/utils/frontend-apps.test.jsghost/core/test/unit/frontend/web/middleware/frontend-caching.test.jsghost/core/test/unit/frontend/web/middleware/handle-image-sizes.test.jsghost/core/test/unit/frontend/web/middleware/static-theme.test.jsghost/core/test/utils/deferred.jsghost/core/test/utils/vitest-setup.tsghost/core/vitest.config.ts
- the test helper passed `resolve` straight in as the express `next` callback, so a `next(err)` resolved the promise (with the error as an ignored value) instead of failing the test - this mirrored the pre-migration mocha test, which used `function next()` with no err param + a bare `done()` — same blind spot, carried over faithfully during the migration - reject on error instead, matching the shared deferred() helper. No call sites change: all are bare `await callStaticTheme()`, so a rejection now surfaces as a test failure
no ref Final big bucket of the vitest migration (after #27898 / #27900 / #27974 / #27991 / #27992 / #27996) — moves `test/unit/server/services` (256 files, ~3290 tests) from mocha to vitest. It also fixes a **vitest worker-teardown bug** that this DB-heavy bucket exposed (the previous buckets happened not to trigger it).
Summary
Continues the vitest migration (after #27898 / #27900 / #27974 / #27991) by moving the
test/unit/frontendbucket — 135 files, 1804 tests — from mocha to vitest.Conversions
before()/after()→beforeAll()/afterAll()frontend-caching+handle-image-sizes:this.beforeEach/this.afterEach(mocha-API misuse) → top-levelbeforeEach/afterEachdone()callback converted off it:static-theme.test.js— 42 uniform middleware tests → a localcallStaticTheme()promise helperhandle-image-sizes+ 4services/routing/controllersfiles → a new sharedtest/utils/deferred.jshelper that bridgesdoneonto a promise (done()resolves,done(err)rejects), so existing call sites stay unchangedhandle-image-sizes: removed 5 deadfakeReq.url.match = fnlines — assigning a property to a string primitive is a silent no-op in mocha's sloppy mode and throws under vitest's strict mode. The override never actually installed anything; the tests still assertnext()is reached.Snapshot support
This is the first migrated bucket with snapshot tests (
ghost-head.test.js, 83 snapshots). Two pieces of infrastructure:vitest-setup.ts— added abeforeEachthat bridges@tryghost/jest-snapshot's per-test config. The mocha hook readsthis.currentTest; vitest has no mochathis, so we derive the sametestPath/testTitlefrom the vitest task.testTitlemust exactly match mocha'sfullTitle()(describe names + test name joined by spaces) or committed.snapkeys won't resolve.vitest.config.ts— setresolveSnapshotPathto a separate path. Ghost's snapshot tests are managed by@tryghost/jest-snapshot(its own__snapshots__/*.snapfiles); without this, vitest's native snapshot system adopts those files, rewrites their header, and reports every entry as obsolete (failing the run under CI). Redirecting vitest's path isolates the two systems. Verified withCI=true(which forbids snapshot writes): 83/83 match,.snapuntouched.Test plan
pnpm test:vitest test/unit/frontend: 135 files / 1804 tests passCI=true pnpm test:vitest test/unit/frontend: same, and no.snapfile modifiedpnpm test:vitest(full): 254 files / 2963 tests pass — snapshotbeforeEachdoesn't disturb other bucketspnpm test:unit:base(mocha): 3582 tests passpnpm exec eslintclean for changed files