Skip to content

Removed unused session-sqlite plumbing from vitest setup#28115

Merged
9larsons merged 1 commit into
mainfrom
chore/strip-session-sqlite-plumbing
May 25, 2026
Merged

Removed unused session-sqlite plumbing from vitest setup#28115
9larsons merged 1 commit into
mainfrom
chore/strip-session-sqlite-plumbing

Conversation

@9larsons
Copy link
Copy Markdown
Contributor

@9larsons 9larsons commented May 25, 2026

The ghost/core unit suite's vitest-setup.ts wired a per-worker session sqlite path, a random server port, and a URL env override before booting Ghost. This existed as a safety net for unit tests that opened the default knex pool or spun up a server — but nothing in the suite actually does either of those things. Running the full 6585-test suite produces zero /tmp/ghost-test-*.db files. The plumbing has no observable effect today and is a maintenance assumption to remember.

Removed in this PR:

  • the sessionId / database__connection__* / server__port / url env block at the top of vitest-setup.ts
  • the snapshot port-normalization block — only needed because the port was randomized; Ghost's testing config (config.testing.json) already defaults to 2369, which matches the canonical port committed in snapshots, so once the random port is gone the normalization is dead code
  • the afterAll knex-pool-drain loop — guards against a pool that no unit test connects
  • vitest-global-setup.ts — only existed to clean up sqlite files that never got created. File deleted, globalSetup entry removed from vitest.config.ts

Verification: full pnpm test:vitest run is green (550 files / 6575 tests passed, 7.57s, within noise of pre-PR), and /tmp/ghost-test-*.db stays empty across the run.

- vitest-setup.ts wired a per-worker session sqlite path, a random server port, and a URL env override before booting Ghost
- nothing in the unit suite opens the default knex pool or starts a server; running the full 6585-test suite produces zero /tmp/ghost-test-*.db files
- the snapshot port-normalization block existed only because the port was randomized; Ghost's testing config already defaults to 2369 (the canonical port committed in snapshots), so removing the random port makes the normalization dead code
- the afterAll knex pool drain was guarding against a pool that no test connects, also dead
- vitest-global-setup.ts only existed to clean up sqlite files that never got created; deleted, and the globalSetup entry removed from vitest.config.ts
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Review Change Stack

Walkthrough

This PR refactors Ghost's Vitest test lifecycle by removing the global setup teardown mechanism and simplifying per-test initialization. The vitest-global-setup.ts file is deleted entirely, its globalSetup reference is removed from vitest.config.ts, and the per-test vitest-setup.ts is reorganized: Ghost runtime overrides load earlier, per-worker session randomization (DB filenames, ports, crypto) is eliminated, and the afterAll hook no longer performs Knex pool draining—only invoking snapshot cleanup hooks if present.

Possibly related PRs

  • TryGhost/Ghost#28074: Directly modifies the same Vitest global setup and teardown flow in vitest-global-setup.ts, vitest-setup.ts, and vitest.config.ts.
  • TryGhost/Ghost#28012: Modifies the afterAll teardown logic in vitest-setup.ts, affecting the same Knex pool cleanup path.
  • TryGhost/Ghost#27898: Introduced the initial Vitest runner integration including vitest-setup.ts and the core Vitest configuration that this PR now refactors.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Removed unused session-sqlite plumbing from vitest setup' directly and specifically summarizes the main change—removing dead test setup code related to session sqlite and server port configuration.
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.
Description check ✅ Passed The PR description clearly relates to the changeset, explaining the removal of unused session-sqlite plumbing and providing verification details.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/strip-session-sqlite-plumbing

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

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

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented May 25, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 75a2d3e

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m View ↗
nx run ghost:test:ci:e2e ✅ Succeeded 7m 37s View ↗
nx run ghost:test:ci:legacy ✅ Succeeded 3m 1s View ↗
nx run-many -t lint -p ghost ✅ Succeeded 38s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 29s View ↗

☁️ Nx Cloud last updated this comment at 2026-05-25 19:59:50 UTC

9larsons added a commit that referenced this pull request May 25, 2026
ref #28115

Shared Vitest config changes can affect more tests than Nx marks as changed, so CI now widens unit-test project selection for root config changes while keeping Ghost core config changes scoped to Ghost core.
@9larsons 9larsons merged commit b046b8f into main May 25, 2026
29 checks passed
@9larsons 9larsons deleted the chore/strip-session-sqlite-plumbing branch May 25, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant