Added unified pnpm test:watch across all Vitest packages#28038
Merged
Conversation
The last 3 mocha holdouts in test/unit now run on vitest, so the mocha unit-test path is retired: - scheduling-default.test.js: converted off mocha done() callbacks. The pingUrl tests do real HTTP (nock), which doesn't mix with the suite's sinon fake clock — they now restore real timers; the scheduling-logic tests keep the fake clock. The 503-retry test is driven by spying _pingUrl and awaiting each attempt instead of polling wall-clock time. - notify.test.js / overrides.test.js needed no changes — vitest.config's include now covers all of test/unit (test/unit/**), and the scheduling-default exclude is gone. - package.json: test:unit now runs vitest; the dead test:unit:base, test:unit:ci and test:unit:slow scripts are removed. - ci.yml: the separate "Run vitest unit tests" step is gone (test:unit is vitest now), along with GHOST_UNIT_TEST_VARIANT and the unused unit-coverage artifact (it was uploaded but never consumed). Integration, e2e-api and legacy suites still run on mocha by design.
- `pnpm test:watch` now runs a single Vitest watcher across ghost/core and
every app instead of each package only watching itself, giving one
consistent dev loop and a stable reference point for agents
- a root vitest.config.mjs wires each package up as a Vitest project; each
keeps its own environment, setup and pool
- ghost/core's test helpers assumed process.cwd() was ghost/core (content
paths, knex-migrator's MigratorConfig.js, minifier glob output); these are
now resolved explicitly so the suite passes when run from the repo root as
a project, with no change to production code
- tsx is registered via require('tsx/cjs') in ghost/core's Vitest setup
rather than a global NODE_OPTIONS='--import tsx'; the global flag broke
module resolution in the app projects and worker threads cannot scope it
- the apps' shared vitest-config resolves @src/@test from an explicit root
instead of process.cwd(), keeping the aliases package-local
- signup-form is excluded — its test:unit is a build, with no Vitest tests
Contributor
WalkthroughThis PR consolidates Vitest as the unified unit test runner across the Ghost monorepo. It establishes root-level configuration ( Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/stats/vitest.config.ts (1)
5-13: 💤 Low valueRedundant
@srcalias now thatrootis passed.With
root: __dirname,createVitestConfigalready sets@src→path.resolve(__dirname, './src'), so the explicit'@src': resolve(__dirname, './src')inaliasesduplicates the default (the spread later just overwrites it with the same path). Safe to drop for clarity; the stats-specific aliases (@assets,@components,@hooks,@utils,@views) are the only ones that genuinely need to be here.♻️ Proposed cleanup
export default createVitestConfig({ root: __dirname, aliases: { - '`@src`': resolve(__dirname, './src'), '`@assets`': resolve(__dirname, './src/assets'), '`@components`': resolve(__dirname, './src/components'), '`@hooks`': resolve(__dirname, './src/hooks'), '`@utils`': resolve(__dirname, './src/utils'), '`@views`': resolve(__dirname, './src/views') } });🤖 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 `@apps/stats/vitest.config.ts` around lines 5 - 13, Remove the redundant '`@src`' entry from the aliases object in the Vitest config: since root: __dirname is passed to createVitestConfig which already maps '`@src`' → path.resolve(__dirname, './src'), delete the explicit "'`@src`': resolve(__dirname, './src')" line and keep only the stats-specific aliases ('`@assets`', '`@components`', '`@hooks`', '`@utils`', '`@views`') so the aliases object is concise and non-duplicative.ghost/core/test/unit/server/adapters/scheduling/scheduling-default.test.js (1)
321-360: 💤 Low valueFire-and-forget call is fine here, but worth a small safety net.
scope.adapter._pingUrl(...)on line 345 isn't held in a variable or awaited; the same promise is reached later viapingSpy.returnValues[0]insidesettle(0), so it does get awaited. However, if that first promise ever rejects (e.g., the implementation starts throwing instead of logging on 5xx), node would emit anunhandledRejectionbetween the call and theawait settle(0)resolution — and Ghost's CI runs with strict unhandled-rejection handling in places. Capturing the return value and pre-attaching a.catch(() => {})(the assertion still comes frompingSpy.returnValues) keeps the test robust to future implementation changes:Proposed tweak
- scope.adapter._pingUrl({ - url: 'http://localhost:1111/ping', - time: moment().valueOf(), - extra: { - httpMethod: 'PUT' - } - }); + // Fire-and-forget: settle(...) below awaits each attempt via pingSpy.returnValues. + // Swallow on the floating promise so a future reject() doesn't trip unhandledRejection + // before settle(0) gets a chance to surface it. + scope.adapter._pingUrl({ + url: 'http://localhost:1111/ping', + time: moment().valueOf(), + extra: { + httpMethod: 'PUT' + } + }).catch(() => {});🤖 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/test/unit/server/adapters/scheduling/scheduling-default.test.js` around lines 321 - 360, The test currently calls scope.adapter._pingUrl(...) without storing its promise which can produce an unhandled rejection if the implementation starts throwing; capture the returned promise from scope.adapter._pingUrl(...) into a variable (e.g., const firstPing = scope.adapter._pingUrl(...)) and immediately attach a no-op rejection handler (e.g., firstPing.catch(() => {})) so the test still awaits the original promise via pingSpy.returnValues[0] in settle() but will not trigger unhandledRejection; leave the rest of the test (settle, pingSpy, logging assertions) unchanged.
🤖 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 `@apps/stats/vitest.config.ts`:
- Around line 5-13: Remove the redundant '`@src`' entry from the aliases object in
the Vitest config: since root: __dirname is passed to createVitestConfig which
already maps '`@src`' → path.resolve(__dirname, './src'), delete the explicit
"'`@src`': resolve(__dirname, './src')" line and keep only the stats-specific
aliases ('`@assets`', '`@components`', '`@hooks`', '`@utils`', '`@views`') so the aliases
object is concise and non-duplicative.
In `@ghost/core/test/unit/server/adapters/scheduling/scheduling-default.test.js`:
- Around line 321-360: The test currently calls scope.adapter._pingUrl(...)
without storing its promise which can produce an unhandled rejection if the
implementation starts throwing; capture the returned promise from
scope.adapter._pingUrl(...) into a variable (e.g., const firstPing =
scope.adapter._pingUrl(...)) and immediately attach a no-op rejection handler
(e.g., firstPing.catch(() => {})) so the test still awaits the original promise
via pingSpy.returnValues[0] in settle() but will not trigger unhandledRejection;
leave the rest of the test (settle, pingSpy, logging assertions) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1e90af69-f6e2-446b-9105-a03401d348ce
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.github/workflows/ci.ymlAGENTS.mdapps/admin-x-framework/src/test/vitest-config.tsapps/admin-x-settings/vitest.config.tsapps/posts/vitest.config.tsapps/stats/vitest.config.tsghost/core/package.jsonghost/core/test/unit/frontend/services/assets-minification/minifier.test.jsghost/core/test/unit/server/adapters/scheduling/scheduling-default.test.jsghost/core/test/unit/server/adapters/storage/local-images-storage.test.jsghost/core/test/unit/server/data/importer/index.test.jsghost/core/test/unit/server/web/admin/controller.test.jsghost/core/test/utils/db-utils.jsghost/core/test/utils/fixture-utils.jsghost/core/test/utils/vitest-setup.tsghost/core/vitest.config.tspackage.jsonvitest.config.mjs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #28038 +/- ##
==========================================
+ Coverage 73.81% 73.82% +0.01%
==========================================
Files 1528 1528
Lines 129409 129417 +8
Branches 15506 15509 +3
==========================================
+ Hits 95519 95538 +19
- Misses 32910 32923 +13
+ Partials 980 956 -24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch finishes the ghost/core unit-test migration to Vitest and builds on it to deliver a unified
pnpm test:watchacross the monorepo.Commits
test/unit;test:unitnow runs Vitest and the separate CI step is gone.pnpm test:watch— the focus of this PR (below).Unified
test:watchpnpm test:watchnow runs a single Vitest watcher across all 14 Vitest packages (ghost/core + 13 apps) via a rootvitest.config.mjsthat registers each package as a Vitest project — each keeps its own environment, setup and pool. Previously onlyapps/portalandapps/statshad a watch script, each watching just itself.pnpm test:watch— watch everythingpnpm test:watch apps/posts— path filter to scopecd ghost/core && pnpm test:watch— a single packageMaking ghost/core composable as a project
ghost/core could not run as a Vitest project alongside the apps until two things were fixed — both without touching production code:
process.cwd()wasghost/core(content paths inlocal-images-storage,knex-migrator'sMigratorConfig.js,minifierglob output,importerfixtures). These now resolve paths explicitly, so the suite passes whether run standalone or from the repo root.require().tsserver source. A globalNODE_OPTIONS='--import tsx'broke module resolution in the app projects, and worker threads cannot scope it viaexecArgv. tsx is now registered withrequire('tsx/cjs')inside ghost/core's Vitest setup (the patternMigratorConfig.jsalready uses), scoping it to ghost/core's workers.The apps' shared
vitest-configalso takes an explicitrootinstead of readingprocess.cwd(), keeping@src/@testaliases package-local under the unified run.signup-formis excluded — itstest:unitis a build, with no Vitest tests.Testing