Swapped models barrel for single-model requires in ghost/core unit tests#28750
Merged
Conversation
… tests ref https://linear.app/tryghost/issue/PLA-45 These unit tests required the full core/server/models barrel (bookshelf + every model + every plugin) when they each only exercise one or two models. Switched them to require the specific model file(s) directly so each test worker stops paying the cold barrel-load cost. The single-model files export the same {Model, Models} bindings the barrel re-exports, and models.Base maps to the base module (ghostBookshelf), so behaviour is unchanged. user.test.js deliberately stays on the barrel: models/user pulls in services/permissions, which re-requires the barrel, so requiring models/user as a worker's first model load re-enters the barrel mid-init and throws ("Cannot read properties of undefined (reading 'extend')" via author.js).
…es tests ref https://linear.app/tryghost/issue/PLA-45 Same barrel-load reduction as the models-test commit, applied to the server/services unit tests. Three files deliberately keep the full barrel require and are untouched: members/middleware.test.js swaps models.Product on the shared barrel object that the middleware under test reads; milestone-queries.test.js requires the whole model layer at import time on purpose (to construct Bookshelf against the real knex before db.knex is stubbed) — its comment documents why; and auth/session/middleware.test.js + invitations/accept.test.js both need models.User, which is not safe to single-require (see the models-test commit message).
…tests ref https://linear.app/tryghost/issue/PLA-45 Final slice of the barrel-load reduction, covering the api validator/endpoint unit tests and the data exporter/importer tests. data/schema/validator.test.js and data/schema/fixtures/fixture-manager.test.js keep the full barrel (they exercise three-plus models each), and the api session/db/members tests keep it too because they need models.User, which is not safe to single-require (see the models-test commit message).
Contributor
|
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 (43)
WalkthroughAcross approximately 40 unit test files in Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many -t test:unit -p ghost |
✅ Succeeded | 34s | View ↗ |
nx run-many -t lint -p ghost |
✅ Succeeded | 43s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-19 16:37:12 UTC
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.
ref https://linear.app/tryghost/issue/PLA-45
57 files in
test/unit/**required the fullcore/server/modelsbarrel — which loads bookshelf, every model, and every plugin (~223ms cold per worker) — when each only exercises one or two models. This swaps 43 of them to require the specific model file(s) directly (e.g.const {Tag} = require('.../models/tag')), so test workers stop paying the cold barrel-load cost. Behaviour is unchanged: the single-model files export the same bindings the barrel re-exports, andmodels.Basemaps to the base module (ghostBookshelf).Kept on the barrel by design (14 files): tests that genuinely touch three-plus models;
members/middleware.test.js(mutatesmodels.Producton the shared barrel object the SUT reads) andmilestone-queries.test.js(deliberate import-time full-layer require, documented in-file); and the six files needingmodels.User—models/userpulls inservices/permissions, which re-requires the barrel, so requiringmodels/useras a worker's first model load re-enters the barrel mid-init and throws viaauthor.js.Tests-only change; no production code touched. Full unit suite green (7020 passing), lint clean.