Skip to content

Raised the test service-probe timeout and refreshed test DX docs#28759

Open
9larsons wants to merge 1 commit into
mainfrom
local-dev-test-audit
Open

Raised the test service-probe timeout and refreshed test DX docs#28759
9larsons wants to merge 1 commit into
mainfrom
local-dev-test-audit

Conversation

@9larsons

@9larsons 9larsons commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

ref https://linear.app/tryghost/issue/PLA-147

Local-dev hardening from a test-DX audit (ghost/core is fully on Vitest now):

  • Service-probe timeout 400ms → 1s (vitest-globalsetup-services.ts). The 400ms ceiling could falsely SKIP an adapter suite whose service is actually up: socket.setTimeout is an inactivity timeout, and the globalSetup probes while the event loop is briefly blocked (before forks spawn) — a ~500ms block made a live service read as "down". Reproduced the false-skip at 400ms / correct at 1s. When the service is down, connection-refused fires instantly regardless of the ceiling, so this adds zero latency to the skip path.
  • Refreshed 3 stale code comments referencing the deleted test/utils/overrides.js, and the AGENTS.md Testing section (corrected test:e2e, noted the sqlite adapter auto-skip, documented single-file DB watch).

Verified: unit / integration / single-file / watch all green; pnpm lint (incl. tsc) clean.

Test/docs infrastructure only.

@9larsons
Hardened local DX of the Vitest test suites
d8b8345 
ref PLA-147

Post-migration audit of running ghost/core's suites locally (sqlite, no
services). Test/docs infra only — no production or boot-path changes.

- Bumped the adapter service-probe timeout from 400ms to 1s. The probe runs
  in the vitest main process as the forks spawn, when the event loop is
  busiest; socket.setTimeout is an inactivity timeout, so under load a
  running service could miss the 400ms window and be reported down, silently
  skipping an adapter suite that should run. A down service still resolves
  instantly (connection refused), so the wider ceiling costs nothing locally.
- Fixed three comments that still referenced the deleted test/utils/overrides.js
  (now test/utils/vitest-setup-db.ts / the vitest setup files).
- Testing docs: corrected the test:e2e description (server-side suites, not
  "API only"), noted suites run on sqlite with adapter auto-skip, and
  documented the single-file DB watch path (default watch is unit-only).
@coderabbitai

coderabbitai Bot commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 89a2acfe-da0f-4588-9983-5104bbcf77cf

📥 Commits

Reviewing files that changed from the base of the PR and between 74e1e3e and d8b8345.

📒 Files selected for processing (4)
  • AGENTS.md
  • ghost/core/test/unit/shared/config/loader.test.js
  • ghost/core/test/utils/e2e-framework-mock-manager.js
  • ghost/core/test/utils/vitest-globalsetup-services.ts

Walkthrough

The pull request makes four small changes to the test infrastructure. In vitest-globalsetup-services.ts, the TCP probe timeout (PROBE_TIMEOUT_MS) is increased from 400ms to 1000ms, which affects the reachability checks that export GHOST_TEST_REDIS_AVAILABLE and GHOST_TEST_MINIO_AVAILABLE environment flags. An inline comment in e2e-framework-mock-manager.js is updated to reflect that the intercept hook runs when Vitest setup files require the module rather than via overrides.js. A comment in loader.test.js is updated to note that database:connection:filename and url are sourced from process.env via vitest-setup-db.ts. AGENTS.md expands the E2E testing section with dedicated commands, sqlite/adapter skip behavior, and a new command for targeting DB-backed test files.

Possibly related PRs

  • TryGhost/Ghost#27900: Both PRs modify ghost/core/test/unit/shared/config/loader.test.js comments in relation to the Vitest migration and DB config sourcing behavior.
  • TryGhost/Ghost#28753: Both PRs modify ghost/core/test/utils/vitest-globalsetup-services.ts to control Redis/MinIO availability probing, with this PR specifically adjusting the TCP probe timeout to 1000ms to support the same skip-if gating used by adapter integration tests.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the main changes: raising the service-probe timeout and refreshing test documentation. It is concise and clearly summarizes the primary focus of the changeset.
Description check ✅ Passed The description is detailed and directly related to the changeset, explaining the rationale for the timeout increase, the three stale comment fixes, and the documentation updates. It provides clear context for all changes.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch local-dev-test-audit

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

nx-cloud Bot commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 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 d8b8345

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 5s View ↗
nx run ghost:test:ci:integration:no-coverage ✅ Succeeded 2m 11s View ↗
nx run ghost:test:ci:e2e ✅ Succeeded 7m 40s View ↗
nx run ghost:test:ci:e2e:no-coverage ✅ Succeeded 6m 33s View ↗
nx run ghost:test:ci:legacy ✅ Succeeded 2m 48s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 30s View ↗
nx run-many -t lint -p ghost-monorepo,ghost ✅ Succeeded 41s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗

☁️ Nx Cloud last updated this comment at 2026-06-20 01:32:31 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@9larsons