Integrate local E2E test with yarn dev:forward #25552
Conversation
jonatansberg
requested review from
9larsons,
cmraible and
ibalosh
as code owners
WalkthroughThis pull request introduces a development environment mode for e2e tests that automatically detects and reuses a running Docker-based infrastructure when Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
e2e/tests/admin/analytics/web-traffic.test.ts (1)
2-5: LGTM!Consistent with the conditional skip pattern used across analytics tests.
e2e/tests/admin/analytics/location.test.ts (1)
3-5: LGTM!Consistent implementation of the analytics skip pattern.
e2e/tests/admin/analytics/post-analytics/growth.test.ts (1)
8-10: LGTM!Consistent with the analytics skip pattern.
e2e/tests/admin/analytics/newsletters.test.ts (1)
2-6: LGTM!Consistent implementation of the analytics test skip pattern.
🧹 Nitpick comments (12)
e2e/AGENTS.md (1)
25-42: Dev Environment Mode docs are clear; consider clarifying cleanup contextThe new section nicely explains how e2e integrates with
yarn dev:forwardand why this mode is recommended. To avoid confusion, you could explicitly mention thatdocker compose -p ghost-dev-e2e downshould be run from the repository root (or wherever the relevant compose file lives).e2e/README.md (1)
170-211: Adjust nested list indentation to satisfy markdownlint (MD007)The bullet lists under “Standalone Mode (Default)” and “Dev Environment Mode” use a deeper indent than markdownlint’s MD007 rule expects. To keep
markdownlintgreen, you can reduce nested bullet indentation to two spaces, for example:- - Global setup (`tests/global.setup.ts`): - - Starts shared services (MySQL, Tinybird, etc.) + - Global setup (`tests/global.setup.ts`): + - Starts shared services (MySQL, Tinybird, etc.)Apply the same pattern to other nested bullets in these sections.
e2e/helpers/environment/service-managers/mysql-manager.ts (1)
108-114: Schema-only snapshot helper is consistent with existing snapshot API
createSchemaSnapshotmirrorscreateSnapshotin structure/logging and adds the expected--no-dataflag for schema-only dumps. If you ever revisit this area, it may be worth centralising the mysqldump command construction (and escaping DB name / path once) so both snapshot methods share the same hardened path, but that’s not blocking for this PR.e2e/tests/admin/analytics/growth.test.ts (1)
3-6: Analytics skip guard is good; confirm top‑level await is supported in your test setupUsing
test.skip(await shouldSkipAnalyticsTests(), 'Tinybird not available');at the top level cleanly gates this file on Tinybird availability and honours the “never skip in CI” rule fromshouldSkipAnalyticsTests.Because this relies on top‑level
awaitin the test file, please double‑check that your Playwright + TS configuration runs tests as ESM with top‑level await enabled. If it ever causes issues, an equivalent pattern is:test.beforeAll(async () => { if (await shouldSkipAnalyticsTests()) { test.skip('Tinybird not available'); } });which avoids top‑level await while preserving behaviour.
e2e/tests/admin/analytics/post-analytics/overview.test.ts (1)
8-11: Consistent Tinybird‑based skip logic; same top‑level await caveatThis file’s use of
test.skip(await shouldSkipAnalyticsTests(), 'Tinybird not available');keeps analytics suites consistently gated on Tinybird availability, which is good.As with
growth.test.ts, please confirm your Playwright/TS setup supports top‑level await in test files; otherwise consider moving the check into atest.beforeAllblock as shown in the previous comment.e2e/helpers/playwright/fixture.ts (1)
109-133: Stronger baseURL guards and explicitUser.passwordshape are good improvementsThe explicit
if (!baseURL) throwchecks inghostAccountOwner/pageWithAuthenticatedUserand the addition ofpasswordtoUseralign the runtime shape with howloginToGetAuthenticatedSessionis used. This should make fixture failures easier to diagnose and reduce type drift.e2e/helpers/pages/public/public-page.ts (1)
1-3: Analytics request waiting with skip guard is well‑structured; consider minor robustness tweakThe combination of
enableAnalyticsRequests(),waitForPageHitRequest(), and theshouldSkipAnalyticsTests()early return gives a good balance: local runs without Tinybird won’t hang, while CI still fails if the analyticspage_hitisn’t sent.If you see unhandled‑rejection noise when navigation fails, you could wrap the logic as:
if (await shouldSkipAnalyticsTests()) { await super.goto(url, options); return; } await Promise.all([ super.goto(url, options), this.page.waitForResponse(/* ... */) ]);Not required, but slightly tighter failure behavior.
Also applies to: 88-103
e2e/helpers/environment/environment-factory.ts (1)
1-59: Environment factory design is solid; you may want slightly more defensive Docker health checksThe factory pattern with
isDevEnvironmentAvailable()+ cachedgetEnvironmentManager()is a clean way to auto‑select dev vs standalone environments while keeping a per‑worker singleton.Two small robustness tweaks you might consider:
- When checking MySQL, guard against unexpected shapes by verifying
containers[0]?.Statusbefore calling.includes('healthy'), and/or iterating all matching containers instead of assuming the first is the right one.- If you ever change container naming, keep
DEV_ENVIRONMENT.mysql.hostin sync with thecompose.dev.yamlservice/container name so the name filter continues to match.These are defensive rather than required changes; the current implementation should work given consistent compose config.
e2e/helpers/environment/dev-environment-manager.ts (1)
48-60: Consider adding error handling for partial failures during global setup.If
runMigrationsorcreateSnapshotfails afterrecreateBaseDatabasesucceeds, the environment is left in an inconsistent state. The next run'scleanupResources()should handle this, but logging the specific failure point would aid debugging.The overall flow is sound—mirrors the standalone environment pattern as documented.
e2e/helpers/environment/service-managers/dev-ghost-manager.ts (3)
60-62: Hardcoded port base may conflict with other services.The gateway port calculation
30000 + workerIndexcould conflict with services running on those ports. This is acceptable for local dev but worth documenting.Consider adding a comment or making the base port configurable:
getGatewayPort(): number { + // Base port 30000 chosen to avoid common service ports; adjust if conflicts occur return 30000 + this.config.workerIndex; }
137-161: Consider exponential backoff for readiness polling.The fixed 500ms polling interval is fine for most cases, but under load the Ghost container might take longer. Exponential backoff would reduce unnecessary requests during slow startups.
Current implementation is functional; this is a minor optimization.
274-280: Silent failure in removeContainer may hide issues.The catch block swallows all errors, only logging the container ID. In some cases (permission issues, Docker daemon problems), this could mask root causes.
private async removeContainer(container: Container): Promise<void> { try { await container.remove({force: true}); - } catch { - debug('Failed to remove container:', container.id); + } catch (err) { + debug('Failed to remove container:', container.id, err); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
compose.dev.analytics.yaml(1 hunks)compose.dev.yaml(1 hunks)docker/ghost-dev/Dockerfile(1 hunks)docker/ghost-dev/entrypoint.sh(1 hunks)e2e/AGENTS.md(1 hunks)e2e/README.md(3 hunks)e2e/helpers/environment/constants.ts(1 hunks)e2e/helpers/environment/dev-environment-manager.ts(1 hunks)e2e/helpers/environment/environment-factory.ts(1 hunks)e2e/helpers/environment/index.ts(1 hunks)e2e/helpers/environment/service-availability.ts(1 hunks)e2e/helpers/environment/service-managers/dev-ghost-manager.ts(1 hunks)e2e/helpers/environment/service-managers/index.ts(1 hunks)e2e/helpers/environment/service-managers/mysql-manager.ts(2 hunks)e2e/helpers/pages/public/public-page.ts(2 hunks)e2e/helpers/playwright/fixture.ts(5 hunks)e2e/tests/admin/analytics/growth.test.ts(1 hunks)e2e/tests/admin/analytics/location.test.ts(1 hunks)e2e/tests/admin/analytics/newsletters.test.ts(1 hunks)e2e/tests/admin/analytics/overview.test.ts(1 hunks)e2e/tests/admin/analytics/post-analytics/growth.test.ts(1 hunks)e2e/tests/admin/analytics/post-analytics/overview.test.ts(1 hunks)e2e/tests/admin/analytics/utm-tracking.test.ts(1 hunks)e2e/tests/admin/analytics/web-traffic.test.ts(1 hunks)e2e/tests/global.setup.ts(1 hunks)e2e/tests/global.teardown.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
e2e/**/*.test.ts
📄 CodeRabbit inference engine (e2e/AGENTS.md)
e2e/**/*.test.ts: Always follow ADRs in../adr/folder (ADR-0001: AAA pattern, ADR-0002: Page Objects)
Never use CSS/XPath selectors - only use semantic locators or data-testid
Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'
Test names should be lowercase and follow the format 'what is tested - expected outcome'
Each test should represent one scenario only - never mix multiple scenarios in a single test
Follow the AAA (Arrange, Act, Assert) pattern in test structure
Prefer semantic locators (getByRole,getByLabel,getByText) over test IDs and never use CSS selectors, XPath, nth-child, or class names
UsegetByTestId()only when semantic locators are unavailable, and suggest addingdata-testidto Ghost codebase when needed
Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation
Never use hard-coded waits likewaitForTimeout()
Never usenetworkidlein waits
Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Usetest.only()for debugging single tests
Manual login should not be used - authentication is automatic via fixture
Files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/admin/analytics/growth.test.tse2e/tests/admin/analytics/overview.test.tse2e/tests/admin/analytics/newsletters.test.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.ts
e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (e2e/AGENTS.md)
Prefer less comments and give things clear names
Files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/helpers/environment/environment-factory.tse2e/tests/admin/analytics/post-analytics/growth.test.tse2e/helpers/environment/index.tse2e/tests/global.teardown.tse2e/tests/admin/analytics/growth.test.tse2e/helpers/environment/service-managers/mysql-manager.tse2e/helpers/environment/service-availability.tse2e/helpers/environment/dev-environment-manager.tse2e/tests/global.setup.tse2e/helpers/pages/public/public-page.tse2e/helpers/environment/service-managers/index.tse2e/tests/admin/analytics/overview.test.tse2e/helpers/environment/constants.tse2e/tests/admin/analytics/newsletters.test.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/playwright/fixture.ts
e2e/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
E2E tests should use Playwright with Docker container isolation
Files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/helpers/environment/environment-factory.tse2e/tests/admin/analytics/post-analytics/growth.test.tse2e/helpers/environment/index.tse2e/tests/global.teardown.tse2e/tests/admin/analytics/growth.test.tse2e/helpers/environment/service-managers/mysql-manager.tse2e/helpers/environment/service-availability.tse2e/helpers/environment/dev-environment-manager.tse2e/tests/global.setup.tse2e/helpers/pages/public/public-page.tse2e/helpers/environment/service-managers/index.tse2e/tests/admin/analytics/overview.test.tse2e/helpers/environment/constants.tse2e/tests/admin/analytics/newsletters.test.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/playwright/fixture.ts
e2e/helpers/pages/**/*.ts
📄 CodeRabbit inference engine (e2e/AGENTS.md)
e2e/helpers/pages/**/*.ts: Page Objects should be located inhelpers/pages/directory
Expose locators aspublic readonlyin Page Objects when used with assertions
Page Object methods should use semantic names (e.g.,login()instead ofclickLoginButton())
UsewaitFor()for guards in Page Objects, never useexpect()in page objects
Files:
e2e/helpers/pages/public/public-page.ts
🧠 Learnings (41)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use `yarn dev:forward` for hybrid Docker + host development with backend in Docker and frontend dev servers on host
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to e2e/**/*.{ts,js} : E2E tests should use Playwright with Docker container isolation
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: After integrating ShadCN components, run `yarn lint`, `yarn test`, and verify in Storybook before merging
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Each test receives fresh Ghost instance for automatic isolation
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Ensure tests have fresh Ghost instance isolation (automatic) and do not create test dependencies where Test B needs Test A
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Analytics using Tinybird should reference scripts in `ghost/core/core/server/data/tinybird/scripts/` and datafiles in `ghost/core/core/server/data/tinybird/`
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tsdocker/ghost-dev/entrypoint.she2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/admin/analytics/growth.test.tse2e/helpers/environment/service-availability.tse2e/tests/admin/analytics/overview.test.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.tscompose.dev.analytics.yaml
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/README.mde2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/global.teardown.tse2e/tests/admin/analytics/growth.test.tse2e/helpers/environment/service-availability.tse2e/helpers/environment/dev-environment-manager.tse2e/tests/global.setup.tse2e/helpers/pages/public/public-page.tse2e/tests/admin/analytics/overview.test.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use `getByTestId()` only when semantic locators are unavailable, and suggest adding `data-testid` to Ghost codebase when needed
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/README.mde2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/global.teardown.tse2e/tests/admin/analytics/growth.test.tse2e/helpers/pages/public/public-page.tse2e/tests/admin/analytics/overview.test.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/admin/analytics/growth.test.tse2e/tests/admin/analytics/overview.test.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/README.mde2e/AGENTS.mde2e/tests/admin/analytics/growth.test.tse2e/tests/admin/analytics/overview.test.tse2e/tests/admin/analytics/utm-tracking.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/post-analytics/overview.test.tse2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/admin/analytics/growth.test.tse2e/tests/admin/analytics/overview.test.tse2e/tests/admin/analytics/utm-tracking.test.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/helpers/pages/**/*.ts : Use `waitFor()` for guards in Page Objects, never use `expect()` in page objects
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/growth.test.tse2e/helpers/pages/public/public-page.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Each test receives fresh Ghost instance for automatic isolation
Applied to files:
e2e/tests/admin/analytics/post-analytics/overview.test.tse2e/README.mde2e/AGENTS.mde2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/admin/analytics/growth.test.tse2e/helpers/environment/dev-environment-manager.tse2e/tests/admin/analytics/overview.test.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Check `e2e/CLAUDE.md` for detailed E2E testing guidance and debugging tips
Applied to files:
e2e/README.mde2e/AGENTS.md
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Always refer to `.claude/E2E_TEST_WRITING_GUIDE.md` for comprehensive testing guidelines and patterns when creating or modifying E2E tests
Applied to files:
e2e/README.mde2e/AGENTS.md
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
e2e/README.mde2e/AGENTS.mddocker/ghost-dev/Dockerfilee2e/tests/admin/analytics/growth.test.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/environment-factory.tsdocker/ghost-dev/Dockerfilee2e/helpers/environment/dev-environment-manager.tse2e/helpers/environment/service-managers/index.tse2e/helpers/environment/constants.tse2e/helpers/environment/service-managers/dev-ghost-manager.tscompose.dev.analytics.yamle2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use `yarn dev:forward` for hybrid Docker + host development with backend in Docker and frontend dev servers on host
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/constants.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:unit` for unit tests in all packages or `yarn test:single` for individual test files
Applied to files:
e2e/README.mde2e/AGENTS.md
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:dev` to start Ghost in Docker with hot reload
Applied to files:
e2e/README.mde2e/AGENTS.mddocker/ghost-dev/Dockerfilee2e/helpers/environment/dev-environment-manager.tse2e/helpers/environment/service-managers/dev-ghost-manager.tscompose.dev.analytics.yamle2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to e2e/**/*.{ts,js} : E2E tests should use Playwright with Docker container isolation
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/environment-factory.tse2e/tests/global.teardown.tse2e/helpers/environment/service-availability.tse2e/helpers/environment/dev-environment-manager.tse2e/tests/global.setup.tse2e/helpers/pages/public/public-page.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn lint` to lint all packages or `cd ghost/core && yarn lint` to lint Ghost core (server, shared, frontend, tests)
Applied to files:
e2e/README.mddocker/ghost-dev/Dockerfile
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Ensure tests have fresh Ghost instance isolation (automatic) and do not create test dependencies where Test B needs Test A
Applied to files:
e2e/README.mde2e/helpers/environment/dev-environment-manager.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-06-19T22:57:05.880Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23941
File: .github/workflows/ci.yml:911-914
Timestamp: 2025-06-19T22:57:05.880Z
Learning: In Ghost, when NODE_ENV is set to testing-mysql, Ghost uses config.testing-mysql.json configuration which sets the server port to 2369 instead of the default 2368. This also applies to other testing environments like testing and testing-browser.
Applied to files:
e2e/README.mde2e/helpers/environment/dev-environment-manager.tse2e/helpers/environment/constants.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Use the existing setup (Vitest + Testing Library + jsdom) when adding tests
Applied to files:
e2e/README.mde2e/tests/global.setup.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation
Applied to files:
e2e/README.mde2e/helpers/environment/environment-factory.tse2e/tests/admin/analytics/post-analytics/growth.test.tse2e/tests/global.teardown.tse2e/helpers/environment/service-managers/mysql-manager.tse2e/helpers/environment/dev-environment-manager.tse2e/tests/global.setup.tse2e/tests/admin/analytics/newsletters.test.tse2e/helpers/playwright/fixture.ts
📚 Learning: 2025-08-11T19:37:41.029Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/integration/services/q-email-addresses.test.js:138-142
Timestamp: 2025-08-11T19:37:41.029Z
Learning: In Ghost's test suite, prefer explicit cleanup calls (like `configUtils.restore()`) in test teardown even when they might be called by other utilities (like `urlUtils.restore()`), as this improves test readability and prevents issues if test dependencies change in the future.
Applied to files:
e2e/README.mde2e/helpers/playwright/fixture.ts
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
e2e/README.mdcompose.dev.analytics.yamle2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-24T17:28:42.084Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:28:42.084Z
Learning: AGENTS.md - Documentation file for agent specifications and guidelines
Applied to files:
e2e/AGENTS.md
📚 Learning: 2025-11-24T17:28:51.262Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/comments-ui/.cursor/rules/playwright-e2e.mdc:0-0
Timestamp: 2025-11-24T17:28:51.262Z
Learning: Applies to apps/comments-ui/{package.json,**/.github/workflows/**,**/playwright.config.{ts,js},**/*.{sh,bash}} : Set PLAYWRIGHT_REPORTER=list environment variable when running Playwright e2e tests as an AI agent for better parsing
Applied to files:
e2e/AGENTS.mde2e/helpers/playwright/fixture.ts
📚 Learning: 2025-11-25T13:09:33.918Z
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
Applied to files:
docker/ghost-dev/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Applied to files:
docker/ghost-dev/Dockerfilee2e/helpers/environment/service-availability.tscompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:build` to build Docker images and delete ephemeral volumes
Applied to files:
docker/ghost-dev/Dockerfile
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Applied to files:
docker/ghost-dev/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/helpers/pages/**/*.ts : Expose locators as `public readonly` in Page Objects when used with assertions
Applied to files:
e2e/tests/global.teardown.tse2e/helpers/pages/public/public-page.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Never use hard-coded waits like `waitForTimeout()`
Applied to files:
e2e/tests/admin/analytics/growth.test.tse2e/helpers/pages/public/public-page.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Always follow ADRs in `../adr/` folder (ADR-0001: AAA pattern, ADR-0002: Page Objects)
Applied to files:
e2e/helpers/pages/public/public-page.tse2e/tests/admin/analytics/location.test.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/helpers/pages/**/*.ts : Page Objects should be located in `helpers/pages/` directory
Applied to files:
e2e/helpers/pages/public/public-page.ts
📚 Learning: 2025-09-11T20:38:41.537Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24891
File: e2e/helpers/pages/portal/SignUpSuccessPage.ts:61-67
Timestamp: 2025-09-11T20:38:41.537Z
Learning: In Playwright, page.$() is deprecated and should be replaced with page.locator(). When checking element visibility with Locator.isVisible(), the method returns false both when the element doesn't exist and when it exists but isn't visible, eliminating the need for separate null checks that were required with ElementHandle.
Applied to files:
e2e/helpers/pages/public/public-page.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Prefer semantic locators (`getByRole`, `getByLabel`, `getByText`) over test IDs and never use CSS selectors, XPath, nth-child, or class names
Applied to files:
e2e/helpers/pages/public/public-page.ts
📚 Learning: 2025-08-01T12:44:07.467Z
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24557
File: apps/admin-x-settings/src/components/settings/general/TimeZone.tsx:7-7
Timestamp: 2025-08-01T12:44:07.467Z
Learning: In Ghost development, PRs may depend on unpublished changes from SDK packages. When this occurs, TypeScript compilation errors for missing exports are expected and documented in the PR description until the dependency packages are published and updated. This is normal workflow for cross-repository feature development.
Applied to files:
e2e/helpers/environment/service-managers/index.ts
📚 Learning: 2025-11-03T12:33:31.093Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25320
File: apps/admin/src/layout/app-sidebar/NavHeader.tsx:10-10
Timestamp: 2025-11-03T12:33:31.093Z
Learning: The Ghost admin apps (apps/admin/**) do not use SSR, so accessing browser APIs like `navigator`, `window`, or `document` at module load time is safe and does not require typeof guards.
Applied to files:
e2e/tests/admin/analytics/overview.test.ts
📚 Learning: 2025-10-07T12:19:15.174Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25031
File: ghost/core/test/utils/fixtures/config/defaults.json:68-80
Timestamp: 2025-10-07T12:19:15.174Z
Learning: In Ghost's configuration system (ghost/core/core/shared/config/), environment-specific config files (e.g., config.development.json, config.production.json) automatically fall back to values defined in defaults.json. It's only necessary to specify changed overrides on a per-env basis. Missing keys in env configs are not an issue—they're intentional and will use the default values.
Applied to files:
e2e/helpers/environment/constants.tscompose.dev.analytics.yaml
📚 Learning: 2025-05-29T10:37:26.369Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23588
File: ghost/core/test/e2e-api/admin/backup.test.js:136-148
Timestamp: 2025-05-29T10:37:26.369Z
Learning: In the Ghost test framework, when testing CSV exports via the admin API, the response `body` field is empty while the actual CSV data is provided through the `text` field. Tests should use `expectEmptyBody()` and then validate the CSV content via `.expect(({text}) => ...)` - this is not contradictory behavior.
Applied to files:
e2e/tests/admin/analytics/location.test.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Manual login should not be used - authentication is automatic via fixture
Applied to files:
e2e/helpers/playwright/fixture.ts
🧬 Code graph analysis (14)
e2e/tests/admin/analytics/web-traffic.test.ts (2)
e2e/helpers/playwright/fixture.ts (1)
test(83-145)e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/tests/admin/analytics/post-analytics/overview.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/tests/admin/analytics/post-analytics/growth.test.ts (2)
e2e/helpers/playwright/fixture.ts (1)
test(83-145)e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/tests/global.teardown.ts (2)
e2e/helpers/environment/service-managers/dev-ghost-manager.ts (1)
teardown(104-117)e2e/helpers/environment/environment-factory.ts (1)
getEnvironmentManager(53-59)
e2e/tests/admin/analytics/growth.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/helpers/environment/service-availability.ts (1)
e2e/helpers/environment/constants.ts (1)
TINYBIRD(28-33)
e2e/tests/global.setup.ts (1)
e2e/helpers/environment/environment-factory.ts (1)
getEnvironmentManager(53-59)
e2e/helpers/pages/public/public-page.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/tests/admin/analytics/overview.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/tests/admin/analytics/newsletters.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/helpers/environment/service-managers/dev-ghost-manager.ts (2)
e2e/helpers/environment/constants.ts (2)
DEV_ENVIRONMENT(47-68)TINYBIRD(28-33)e2e/helpers/environment/service-availability.ts (1)
isTinybirdAvailable(14-51)
e2e/tests/admin/analytics/utm-tracking.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/tests/admin/analytics/location.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(65-78)
e2e/helpers/playwright/fixture.ts (1)
e2e/helpers/environment/environment-factory.ts (1)
getEnvironmentManager(53-59)
🪛 LanguageTool
e2e/README.md
[style] ~172-~172: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ... ### Test Isolation Test isolation is extremely important to avoid flaky tests that are hard to d...
(EN_WEAK_ADJECTIVE)
[style] ~172-~172: To elevate your writing, try using a synonym here.
Context: ...important to avoid flaky tests that are hard to debug. For the most part, you should...
(HARD_TO)
🪛 markdownlint-cli2 (0.18.1)
e2e/README.md
196-196: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
198-198: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
199-199: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
201-201: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
202-202: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
204-204: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
206-206: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
208-208: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (23)
compose.dev.yaml (1)
41-49: Additional Mailpit port mapping looks safe and scoped for e2eExposing
8026:8025alongside the existing8025:8025mapping is a non-breaking way to give e2e a dedicated web UI port. Just ensure any e2e docs/configs reference8026consistently.e2e/README.md (1)
22-44: Dev Environment Mode section aligns well with the new workflowThe description of running
yarn dev:forwardfrom the repo root andyarn testfrome2e/plus the benefits/cleanup command match the new environment-manager behaviour and should make the dev flow much clearer for contributors.e2e/helpers/environment/index.ts (1)
1-5: Environment helper exports are cohesiveRe-exporting
dev-environment-manager,environment-factory, andservice-availabilityalongside the existing managers keeps the environment API surface consistent and discoverable for tests and fixtures.e2e/helpers/environment/service-managers/mysql-manager.ts (1)
77-98: Expanded base DB exclusions in cleanup make senseUpdating
dropAllTestDatabasesto excludeghost_testing,ghost_e2e_base, andghost_devaligns with the new environment flows and avoids accidentally dropping base/dev databases when cleaning up interrupted tests.e2e/helpers/environment/service-managers/index.ts (1)
1-1: LGTM!The re-export follows the existing pattern and maintains alphabetical ordering.
e2e/tests/admin/analytics/overview.test.ts (1)
8-10: Conditional skip pattern looks good.The top-level await with
shouldSkipAnalyticsTests()appropriately skips analytics tests when Tinybird is unavailable in development, while ensuring tests fail in CI if the service is missing.docker/ghost-dev/Dockerfile (1)
30-38: LGTM - Standard Docker entrypoint pattern.The entrypoint script is correctly installed and configured to run before the CMD. The script path, permissions, and execution flow are all properly set up.
docker/ghost-dev/entrypoint.sh (3)
1-3: LGTM - Robust error handling.The shebang and
set -euo pipefailprovide appropriate error handling for the entrypoint script.
7-18: LGTM - Graceful Tinybird configuration handling.The script correctly handles optional Tinybird configuration with appropriate checks and warnings. The variable naming follows Ghost's configuration convention, and error messages are clear and directed to stderr.
20-21: LGTM - Standard entrypoint execution pattern.The
exec "$@"correctly executes the CMD from the Dockerfile, replacing the shell process.e2e/tests/admin/analytics/utm-tracking.test.ts (1)
4-6: Top-levelawaitintest.skip– verify compatibility with your Playwright/TS setupThe conditional skip wiring via
shouldSkipAnalyticsTests()looks correct and matches the rest of the analytics suites. Please just confirm your Playwright + TypeScript configuration supports top‑levelawaitin test files so this compiles/runs in all environments (CI + local).e2e/helpers/playwright/fixture.ts (1)
88-103: Environment manager factory usage and per‑test lifecycle look soundSwitching
ghostInstanceto useawait getEnvironmentManager()withperTestSetup/perTestTeardownis consistent with the new factory and caching model. Per‑test isolation is preserved and teardown is guaranteed afteruse, so the lifecycle wiring here looks good.e2e/tests/global.teardown.ts (1)
1-7: Global teardown correctly switched togetEnvironmentManagerUsing
await getEnvironmentManager()and thenmanager.globalTeardown()keeps teardown semantics intact while aligning with the new factory/singleton pattern.e2e/helpers/environment/constants.ts (1)
43-68: DEV_ENVIRONMENT config looks coherent – verify it matches your dev compose namingThe structure of
DEV_ENVIRONMENT(network name, MySQL/Redis hosts, image names, Ghost port/workdir) aligns with howenvironment-factory.tsprobes Docker. Please double‑check these values againstcompose.dev.yaml(network and container names especially) soisDevEnvironmentAvailable()reliably detectsyarn dev:forward.e2e/tests/global.setup.ts (1)
1-7: Global setup correctly migrated to usegetEnvironmentManagerObtaining the manager via
await getEnvironmentManager()and callingmanager.globalSetup()is consistent with the new environment factory and keeps setup semantics unchanged.compose.dev.analytics.yaml (1)
6-82: Tinybird analytics wiring in dev compose looks correct; confirm paths and profilesThe added
shared-configvolume, Tinybird‑specific env vars onghost-dev, and thetb-cliseeding flow are coherent and match the expected/mnt/shared-config/.env.tinybirdpattern and linux/amd64 analytics image requirement. Please just verify:
- The ghost Tinybird data path
./ghost/core/core/server/data/tinybirdmatches the actual repo layout.ghost-devstill has all its existing volumes/depends_on fromcompose.dev.yamlafter compose‑file merging (lists should be appended, not replaced).If both are correct, this compose overlay should work well with
yarn dev:analytics/yarn dev:forward.e2e/helpers/environment/dev-environment-manager.ts (4)
1-11: LGTM!Imports are well-organized and appropriately minimal for the orchestration responsibilities of this class.
21-42: LGTM!Good use of readonly modifiers for immutable configuration. The worker index derivation from
TEST_PARALLEL_INDEXcorrectly supports Playwright's parallel test execution model.
113-127: LGTM!Clean teardown logic. The
shouldPreserveEnvironmenttoggle is useful for debugging test failures locally.
79-108: Looking at the verification results:Key findings:
ghostContainerIdis a getter (line 52-54 in dev-ghost-manager.ts) that returnsstring | null:get ghostContainerId(): string | null { return this.ghostContainer?.id ?? null; }
setup()guarantees container initialization: The method callsgetOrCreateContainer()(lines 71-72), which:
- Either returns an existing container
- Or starts a stopped container
- Or creates and starts a new container
- Crucially: All code paths explicitly
returnaContainerobject—there's no path that silently fails or returns nullError propagation is proper: If
getOrCreateContainer()fails, it throws an error (caught by the try-catch at line 95, but then attempts creation). If creation fails, the error propagates up andsetup()throws.By line 101, the container is guaranteed: Since
perTestSetup()successfully awaitsthis.ghost.setup()and checks initialization,this.ghostContainermust be set to aContainerobject, makingghostContainerIdreturn astring(not null).The non-null assertion is justified and safe. The code cannot reach line 101 with an uninitialized container.
Verify non-null assertion on
ghostContainerIdis justified. Aftersetup()completes successfully,this.ghostContaineris guaranteed to be initialized to aContainerobject (viagetOrCreateContainer()), so the getter will always return a string, never null. The non-null assertion is safe and does not require additional guards.e2e/helpers/environment/service-managers/dev-ghost-manager.ts (3)
14-21: LGTM!Clean interface definition that matches the structure from
DEV_ENVIRONMENTconstants.
163-207: LGTM!Good conditional Tinybird configuration. The async check via
isTinybirdAvailable()ensures tests gracefully degrade when Tinybird isn't running.
285-300: LGTM!Good use of label-based cleanup to ensure all e2e containers are removed regardless of naming.
jonatansberg
force-pushed
the
ber-3077-integrate-e2e-yarn-dev-forward
branch
6 times, most recently
from
606283a to
61fe875
Compare
rob-ghost
changed the base branch from
main
to
ber-3077-fixed-yarn-dev-forward-analytics
rob-ghost
left a comment
rob-ghost
left a comment
There was a problem hiding this comment.
I've run the following scenarios locally and all but one passed:
- ✅ Running with
yarn dev:forward- the running containers are used, analytics tests are skipped - ✅ Running standalone - e2e spins up the required containers (including tinybird)
I did find that running yarn dev:analytics which spins up tinybird and then running the tests they were still skipped even though the containers are available. I think this is intentional but I also think it would be possible to use them with a slight tweak to the service availability module. See this comment.
Assuming that might be out-of-scope, PR approved 👍🏻 (pending linting is fixed and the base branch is merged)
| const containers = await docker.listContainers({ | ||
| filters: { | ||
| name: [TINYBIRD.LOCAL_HOST], | ||
| status: ['running'] | ||
| } | ||
| }); |
There was a problem hiding this comment.
Issue: TINYBIRD.LOCAL_HOST is defined as tinybird-local but yarn dev:analytics spins up ghost-dev-tinybird so running yarn dev:analytics and then running the tests still skips over the tinybird tests.
There was a problem hiding this comment.
We should align those so we can reuse the Tinybird instance.
There was a problem hiding this comment.
I tried fixing this. Now it works great for the dev:analytics case, but it's for some unknown reason a bit racy for the standalone E2E tests. I'll come back to this tomorrow.
|
Oh man, that's a lot of linting things. I think my local ESLint setup is broken for the e2e project for some reason... Edit: I've fixed them now. |
jonatansberg
force-pushed
the
ber-3077-integrate-e2e-yarn-dev-forward
branch
from
61fe875 to
ec3dd15
Compare
There was a problem hiding this comment.
I’ll take another pass tomorrow with a fresher head to fully understand the newly introduced managers, so please take this initial review with a bit of caution. I wanted to get some early feedback to you quickly.
Some of the concerns I had by looking at the implementation:
Too much behaviour is hidden behind auto detection
- get environment manager silently decides dev and test (standalone) mode. If something is missconfigured, you silently fallback instead of failing loudly
- something like const useDev = process.env.E2E_DEV === 'true' maybe?
- also for analytics exclusion I would simply use tag annotations
there's a lot of coupling to docker internals, like if changing compose.dev.yaml naming, hard to find the error (I admit though our initial implementation has similar issue, but it's less complex in my opinion)
Two environment managers instead of one
Original already encapsulates the general idea of start, cleanup, stop etc, why create one more? Ideally, they should be two configurations, rather than two classes plus factory and auto detection?
We might want to add simpler setup of env to choose, simplify a bit env manager, and per config and then just:
export function getEnvironmentManager(): EnvironmentManager {
const config = loadFromEnv();
return new EnvironmentManager(config);
}
Thats some initial reactions I had by look today. I do realize that this can't be simple, but we should keep it still friendly if possible to new-commers to the e2e repo and keep it very well documented to be able to read it. At first glance, it seems more complex
I also got errors on custom-views.tests when yarn dev:forward is running in one shell and I do yarn test in other
| * Configuration for dev environment mode. | ||
| * Used when yarn dev:forward infrastructure is detected. | ||
| */ | ||
| export const DEV_ENVIRONMENT = { |
There was a problem hiding this comment.
To avoid confusion, these seem to make more sense to be a separate file, so we have something like:
- constants.test-env.ts
- constants.dev-end.ts
we will have some duplicates, but cleaner separation between environment configurations and cleaner switch between them in future. At the moment, this looks confusing, with having configurations overlap in a single file and no clear shape, which will differ depending on file from which you are loading them
| * Returns true if Tinybird is not available AND we're not in CI. | ||
| * In CI, we want tests to fail if Tinybird isn't available. | ||
| */ | ||
| export async function shouldSkipAnalyticsTests(): Promise<boolean> { |
There was a problem hiding this comment.
It feels like we are over-complicating this with detection whether tinybird is available. Tomorrow we will add something else, and the process will be even more complex. Ti feels wrong for tests to know, what to skip and prone to future errors.
I think that by default all tests should run, and you can control whether you want that or not by something simpler like tag annotations, which you can use in your dev environment to run specific tests that you would like.
With tags, you could choose which tests to include or exclude, by single or multiple groups. I would rather go for:
- start what you want 2. run tests that you chose against it
- rather than start what you want and tests detect what they should ignore (this feels like it could go wrong in future, and too much of an optimization)
There was a problem hiding this comment.
I tend to agree on principle. While this is slick and a proactive setup, I'd rather force the engineer to be more prescriptive.
On the other hand, I think if you run yarn test:e2e it should run what we expect to run on CI. If that means we need to boot extra containers, then we should do that. And we shouldn't require the dev to know that they need to disable tinybird. What Jonathan is trying to do here is conserve resources/increase speed of running the tests.
I don't think I like the default being a slimmer version of our tests, and this seems like a reasonable compromise. I'm ultimately on the fence.
There was a problem hiding this comment.
I'm happy to consider other ways of doing this but I don't buy the slippery slope argument in this regard. Would you be open to iterating on that post merge, or do you consider this a blocker?
With that said, I do actually think having the tests be conditional makes sense. Tinybird is an optional component that's not a requirement for running Ghost. On top of that Ti is exceptionally expensive to run and uses more resources than everything else combined. The way the tests have been set up mean they won't even run on most people's machines. I mean, my M4 Pro MacBook with 24 GB of RAM is struggling most of the time, especially if you're also running the dev environment alongside the dedicated E2E test containers.
If we don't make these tests conditional on the availability of Tinybird then either:
A. You can't run the E2E tests against the existing dev containers without forcing everyone to either always run Tinybird (and whatever pinned version of the analytics backend we happen to have in our Docker configuration).
B. We need a special E2E testing command rather than the "default one".
C. We always skip the tests unless you opt in.
Having the test be conditional means that if you explicitly start the analytics stuff, they will run, and if you don't, they won't. They always run in CI. To me, this feels like a very pragmatic trade-off that avoids the very clear drawbacks of all of the above alternatives.
If/when the analytics backend is properly integrated into the monorepo, going with the "always run everything option" seems more sensible. By then we should hopefully be rid of Ember as well, which would free up a lot of resources.
| async goto(url?: string, options?: pageGotoOptions): Promise<void> { | ||
| await this.enableAnalyticsRequests(); | ||
| const pageHitPromise = this.pageHitRequestPromise(); | ||
| const pageHitPromise = this.waitForPageHitRequest(); |
There was a problem hiding this comment.
thinkingOutLoud yeah, this wait should probably be just a choice for analytics tests, goto should ignore this for everything else (that way it would work even if we use tags and certain services are off like analytics
| * Check if the dev environment (yarn dev:forward) is running. | ||
| * Detects by checking for the ghost_dev network and running MySQL container. | ||
| */ | ||
| export async function isDevEnvironmentAvailable(): Promise<boolean> { |
There was a problem hiding this comment.
Do you find this way sufficient to check whether dev environment is running locally?
There was a problem hiding this comment.
It's been working well enough in my local testing. Any specific reason you're concerned about this?
| try { | ||
| const containers = (await Promise.all([ | ||
| findTinybirdContainers(docker, DOCKER_COMPOSE_CONFIG.PROJECT), | ||
| findTinybirdContainers(docker, 'ghost-dev') |
There was a problem hiding this comment.
Shouldn't this be in the env/constants?
| } | ||
|
|
||
| // Base environment matching compose.dev.yaml - only set what Ghost needs | ||
| const BASE_GHOST_ENV = [ |
There was a problem hiding this comment.
If we have /environment/dev-environment-manager this almost feels appropriate to house there, and awkward to have here.
|
I'm not seeing this actually faster. Could you clarify how it's faster? I'm actually seeing much slower rates locally, and suspect something is different on my end. Would you mind sending over a clip/vid of you running this with and without local dev? |
9larsons
left a comment
9larsons
left a comment
There was a problem hiding this comment.
In a note above, plus a couple questions.
Same for me as for @9larsons , I have not seen speed increase, or at least not sure which one. I am wondering if I am missing something when I run it |
|
@9larsons @ibalosh The biggest speed increases come from:
If I instead kill my dev environment to run the tests, then yes, the tests run faster, but the overall iteration cycle becomes worse as I now need to restart my dev environment if I want to e.g. verify a change in my browser, before killing it to run the tests again. If you have machines with more working memory, I imagine you'd get different results. Also, serving the app from the dev servers will be slower than serving the built version. So if your testing patterns mostly involve iterating on backend code, leaving the frontend unchanged, that's also a completely different story. |
|
@ibalosh I missed your main comment, sorry. Your concerns around auto-detection and having two managers instead of one are definitely reasonable. I went for building a second manager and auto-detection rather than trying to iterate on the existing one, as doing so while maintaining support for both modes of operation became a mess. The auto-detection makes the default DX nicer, but we can definitely have an Ultimately my goal would be for what's currently called the dev environment manager in this PR to supersede the current one. In that world, we would still have two different modes of running the tests, either against the local development environment or against pre-built assets, but we would be rid of most of the duplication. This would also allow running the tests locally in the same way as today, which would be slightly faster if you're only doing backend changes. |
refs https://linear.app/ghost/issue/BER-3077/improve-e2e-testing-integration Detect the presence of containers initialized through yarn dev:forward. When present, reuse existing resources and use the lightweight dev backend image and gateway setup for E2E test isolation.
refs https://linear.app/ghost/issue/BER-3077/improve-e2e-testing-integration The yarn dev:forward command won't start Tinybird by default since it's expensive to run. These changes mean that we automatically skip tests that depend on Tinybird when running locally and that service isn't available.
jonatansberg
force-pushed
the
ber-3077-integrate-e2e-yarn-dev-forward
branch
from
ec3dd15 to
528e9a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
e2e/README.md (1)
191-210: Minor markdown indentation issue flagged by linter.The nested lists under "Dev Environment Mode" use 4-space indentation, but markdownlint expects 2-space indentation. This is cosmetic and doesn't affect rendering in most viewers.
- Global setup: - - Creates a database snapshot in the existing `ghost-dev-mysql` + - Creates a database snapshot in the existing `ghost-dev-mysql` - Worker setup (once per Playwright worker): - - Creates a Ghost container for the worker - - Creates a Caddy gateway container for routing + - Creates a Ghost container for the worker + - Creates a Caddy gateway container for routingApply similar changes to the remaining nested list items.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
compose.dev.yaml(1 hunks)e2e/AGENTS.md(1 hunks)e2e/README.md(3 hunks)e2e/helpers/environment/constants.ts(1 hunks)e2e/helpers/environment/dev-environment-manager.ts(1 hunks)e2e/helpers/environment/environment-factory.ts(1 hunks)e2e/helpers/environment/index.ts(1 hunks)e2e/helpers/environment/service-availability.ts(1 hunks)e2e/helpers/environment/service-managers/dev-ghost-manager.ts(1 hunks)e2e/helpers/environment/service-managers/index.ts(1 hunks)e2e/helpers/environment/service-managers/mysql-manager.ts(2 hunks)e2e/helpers/pages/public/public-page.ts(2 hunks)e2e/helpers/playwright/fixture.ts(5 hunks)e2e/tests/admin/analytics/growth.test.ts(1 hunks)e2e/tests/admin/analytics/location.test.ts(1 hunks)e2e/tests/admin/analytics/newsletters.test.ts(1 hunks)e2e/tests/admin/analytics/overview.test.ts(1 hunks)e2e/tests/admin/analytics/post-analytics/growth.test.ts(1 hunks)e2e/tests/admin/analytics/post-analytics/overview.test.ts(1 hunks)e2e/tests/admin/analytics/utm-tracking.test.ts(1 hunks)e2e/tests/admin/analytics/web-traffic.test.ts(1 hunks)e2e/tests/global.setup.ts(1 hunks)e2e/tests/global.teardown.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- e2e/helpers/environment/constants.ts
- e2e/tests/admin/analytics/post-analytics/growth.test.ts
- e2e/tests/admin/analytics/utm-tracking.test.ts
- e2e/tests/admin/analytics/post-analytics/overview.test.ts
- e2e/helpers/pages/public/public-page.ts
- e2e/helpers/environment/environment-factory.ts
- e2e/helpers/environment/dev-environment-manager.ts
- compose.dev.yaml
- e2e/tests/global.setup.ts
- e2e/helpers/environment/service-availability.ts
- e2e/tests/global.teardown.ts
- e2e/helpers/playwright/fixture.ts
- e2e/tests/admin/analytics/newsletters.test.ts
- e2e/tests/admin/analytics/growth.test.ts
- e2e/tests/admin/analytics/overview.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (e2e/AGENTS.md)
Prefer less comments and give things clear names
Files:
e2e/helpers/environment/service-managers/mysql-manager.tse2e/helpers/environment/index.tse2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/environment/service-managers/index.ts
e2e/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
E2E tests should use Playwright with Docker container isolation
Files:
e2e/helpers/environment/service-managers/mysql-manager.tse2e/helpers/environment/index.tse2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/location.test.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/environment/service-managers/index.ts
e2e/**/*.test.ts
📄 CodeRabbit inference engine (e2e/AGENTS.md)
e2e/**/*.test.ts: Always follow ADRs in../adr/folder (ADR-0001: AAA pattern, ADR-0002: Page Objects)
Never use CSS/XPath selectors - only use semantic locators or data-testid
Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'
Test names should be lowercase and follow the format 'what is tested - expected outcome'
Each test should represent one scenario only - never mix multiple scenarios in a single test
Follow the AAA (Arrange, Act, Assert) pattern in test structure
Prefer semantic locators (getByRole,getByLabel,getByText) over test IDs and never use CSS selectors, XPath, nth-child, or class names
UsegetByTestId()only when semantic locators are unavailable, and suggest addingdata-testidto Ghost codebase when needed
Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation
Never use hard-coded waits likewaitForTimeout()
Never usenetworkidlein waits
Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Usetest.only()for debugging single tests
Manual login should not be used - authentication is automatic via fixture
Files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/location.test.ts
🧠 Learnings (37)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use `yarn dev:forward` for hybrid Docker + host development with backend in Docker and frontend dev servers on host
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25552
File: e2e/helpers/environment/service-managers/dev-ghost-manager.ts:210-247
Timestamp: 2025-12-01T08:42:35.294Z
Learning: In e2e/helpers/environment/service-managers/dev-ghost-manager.ts, the hardcoded volume name 'ghost-dev_shared-config' at line 231 is intentional. E2E tests run under the 'ghost-dev-e2e' project namespace but deliberately mount the shared-config volume from the main 'ghost-dev' project to access Tinybird credentials created by yarn dev:forward. This is cross-project volume sharing by design.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:dev` to start Ghost in Docker with hot reload
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to e2e/**/*.{ts,js} : E2E tests should use Playwright with Docker container isolation
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Check `e2e/CLAUDE.md` for detailed E2E testing guidance and debugging tips
Applied to files:
e2e/README.md
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Always refer to `.claude/E2E_TEST_WRITING_GUIDE.md` for comprehensive testing guidelines and patterns when creating or modifying E2E tests
Applied to files:
e2e/README.mde2e/AGENTS.md
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/environment/service-managers/index.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use `yarn dev:forward` for hybrid Docker + host development with backend in Docker and frontend dev servers on host
Applied to files:
e2e/README.mde2e/AGENTS.md
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:unit` for unit tests in all packages or `yarn test:single` for individual test files
Applied to files:
e2e/README.mde2e/AGENTS.md
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:dev` to start Ghost in Docker with hot reload
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to e2e/**/*.{ts,js} : E2E tests should use Playwright with Docker container isolation
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn lint` to lint all packages or `cd ghost/core && yarn lint` to lint Ghost core (server, shared, frontend, tests)
Applied to files:
e2e/README.md
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Each test receives fresh Ghost instance for automatic isolation
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-12-01T08:42:35.294Z
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25552
File: e2e/helpers/environment/service-managers/dev-ghost-manager.ts:210-247
Timestamp: 2025-12-01T08:42:35.294Z
Learning: In e2e/helpers/environment/service-managers/dev-ghost-manager.ts, the hardcoded volume name 'ghost-dev_shared-config' at line 231 is intentional. E2E tests run under the 'ghost-dev-e2e' project namespace but deliberately mount the shared-config volume from the main 'ghost-dev' project to access Tinybird credentials created by yarn dev:forward. This is cross-project volume sharing by design.
Applied to files:
e2e/README.mde2e/AGENTS.mde2e/helpers/environment/index.tse2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/environment/service-managers/index.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Ensure tests have fresh Ghost instance isolation (automatic) and do not create test dependencies where Test B needs Test A
Applied to files:
e2e/README.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-06-19T22:57:05.880Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23941
File: .github/workflows/ci.yml:911-914
Timestamp: 2025-06-19T22:57:05.880Z
Learning: In Ghost, when NODE_ENV is set to testing-mysql, Ghost uses config.testing-mysql.json configuration which sets the server port to 2369 instead of the default 2368. This also applies to other testing environments like testing and testing-browser.
Applied to files:
e2e/README.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Use the existing setup (Vitest + Testing Library + jsdom) when adding tests
Applied to files:
e2e/README.md
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation
Applied to files:
e2e/README.mde2e/helpers/environment/service-managers/mysql-manager.tse2e/helpers/environment/index.ts
📚 Learning: 2025-08-11T19:37:41.029Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/integration/services/q-email-addresses.test.js:138-142
Timestamp: 2025-08-11T19:37:41.029Z
Learning: In Ghost's test suite, prefer explicit cleanup calls (like `configUtils.restore()`) in test teardown even when they might be called by other utilities (like `urlUtils.restore()`), as this improves test readability and prevents issues if test dependencies change in the future.
Applied to files:
e2e/README.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
e2e/README.mde2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/location.test.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use `getByTestId()` only when semantic locators are unavailable, and suggest adding `data-testid` to Ghost codebase when needed
Applied to files:
e2e/README.mde2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/location.test.ts
📚 Learning: 2025-11-24T17:28:42.084Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:28:42.084Z
Learning: AGENTS.md - Documentation file for agent specifications and guidelines
Applied to files:
e2e/AGENTS.md
📚 Learning: 2025-11-24T17:28:51.262Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/comments-ui/.cursor/rules/playwright-e2e.mdc:0-0
Timestamp: 2025-11-24T17:28:51.262Z
Learning: Applies to apps/comments-ui/{package.json,**/.github/workflows/**,**/playwright.config.{ts,js},**/*.{sh,bash}} : Set PLAYWRIGHT_REPORTER=list environment variable when running Playwright e2e tests as an AI agent for better parsing
Applied to files:
e2e/AGENTS.mde2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'
Applied to files:
e2e/AGENTS.mde2e/tests/admin/analytics/web-traffic.test.tse2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Analytics using Tinybird should reference scripts in `ghost/core/core/server/data/tinybird/scripts/` and datafiles in `ghost/core/core/server/data/tinybird/`
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/location.test.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.tse2e/tests/admin/analytics/location.test.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/helpers/pages/**/*.ts : Use `waitFor()` for guards in Page Objects, never use `expect()` in page objects
Applied to files:
e2e/tests/admin/analytics/web-traffic.test.ts
📚 Learning: 2025-05-29T10:37:26.369Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23588
File: ghost/core/test/e2e-api/admin/backup.test.js:136-148
Timestamp: 2025-05-29T10:37:26.369Z
Learning: In the Ghost test framework, when testing CSV exports via the admin API, the response `body` field is empty while the actual CSV data is provided through the `text` field. Tests should use `expectEmptyBody()` and then validate the CSV content via `.expect(({text}) => ...)` - this is not contradictory behavior.
Applied to files:
e2e/tests/admin/analytics/location.test.ts
📚 Learning: 2025-08-01T12:44:07.467Z
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24557
File: apps/admin-x-settings/src/components/settings/general/TimeZone.tsx:7-7
Timestamp: 2025-08-01T12:44:07.467Z
Learning: In Ghost development, PRs may depend on unpublished changes from SDK packages. When this occurs, TypeScript compilation errors for missing exports are expected and documented in the PR description until the dependency packages are published and updated. This is normal workflow for cross-repository feature development.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.tse2e/helpers/environment/service-managers/index.ts
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-24T11:12:15.712Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24673
File: ghost/i18n/lib/i18n.js:34-35
Timestamp: 2025-11-24T11:12:15.712Z
Learning: In the Ghost i18n package (ghost/i18n/lib/i18n.js), changing existing locale codes requires backwards compatibility handling for users who have already configured those locales. Such changes should be done in a separate PR with migration logic rather than included in feature PRs.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-02-06T10:30:53.440Z
Learnt from: allouis
Repo: TryGhost/Ghost PR: 22127
File: .github/scripts/release-apps.js:72-74
Timestamp: 2025-02-06T10:30:53.440Z
Learning: In the Ghost repository, the path to defaults.json is correctly structured as 'ghost/core/core/shared/config/defaults.json' with an intentional double 'core' in the path.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-05-20T21:08:21.026Z
Learnt from: cathysarisky
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-10-30T17:13:26.190Z
Learnt from: sam-lord
Repo: TryGhost/Ghost PR: 25303
File: ghost/core/core/server/services/email-service/BatchSendingService.js:19-19
Timestamp: 2025-10-30T17:13:26.190Z
Learning: In ghost/core/core/server/services/email-service/BatchSendingService.js and similar files in the Ghost codebase, prefer using `{...options}` spread syntax without explicit guards like `...(options || {})` when spreading potentially undefined objects, as the maintainer prefers cleaner syntax over defensive patterns when the behavior is safe.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
📚 Learning: 2025-10-09T08:10:18.956Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25075
File: ghost/admin/app/services/whats-new.js:77-83
Timestamp: 2025-10-09T08:10:18.956Z
Learning: In ghost/admin/app/services/whats-new.js, reusing `this._whatsNewSettings` (initialized once in `init()`) when persisting defaults for new users without `lastSeenDate` is acceptable. The team is aware that this means sequential logins by different admins in the same tab could share the service initialization timestamp, but this tradeoff is not a concern for their use case.
Applied to files:
e2e/helpers/environment/service-managers/dev-ghost-manager.ts
🧬 Code graph analysis (3)
e2e/tests/admin/analytics/web-traffic.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(69-82)
e2e/tests/admin/analytics/location.test.ts (1)
e2e/helpers/environment/service-availability.ts (1)
shouldSkipAnalyticsTests(69-82)
e2e/helpers/environment/service-managers/dev-ghost-manager.ts (1)
e2e/helpers/environment/constants.ts (1)
DEV_ENVIRONMENT(47-68)
🪛 LanguageTool
e2e/README.md
[style] ~172-~172: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ... ### Test Isolation Test isolation is extremely important to avoid flaky tests that are hard to d...
(EN_WEAK_ADJECTIVE)
[style] ~172-~172: To elevate your writing, try using a synonym here.
Context: ...important to avoid flaky tests that are hard to debug. For the most part, you should...
(HARD_TO)
🪛 markdownlint-cli2 (0.18.1)
e2e/README.md
196-196: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
198-198: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
199-199: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
201-201: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
202-202: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
204-204: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
206-206: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
208-208: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push
🔇 Additional comments (14)
e2e/helpers/environment/index.ts (1)
3-5: LGTM!Clean barrel exports for the new dev environment modules. The export order is consistent with the existing pattern.
e2e/AGENTS.md (1)
25-42: LGTM!Clear, concise documentation of the dev environment mode workflow with practical examples and cleanup instructions.
e2e/helpers/environment/service-managers/index.ts (1)
1-5: LGTM!Barrel exports properly include the new
dev-ghost-managerandtinybird-managermodules while maintaining alphabetical ordering.e2e/helpers/environment/service-managers/mysql-manager.ts (2)
79-85: LGTM!Correctly excludes
ghost_devfrom cleanup to protect the dev environment database when running e2e tests alongsideyarn dev:forward.
108-114: LGTM!The
createSchemaSnapshotmethod follows the same pattern ascreateSnapshotwith the--no-dataflag for schema-only dumps, which is appropriate for the dev environment workflow.e2e/README.md (1)
22-43: LGTM!Clear documentation of the dev environment mode with practical terminal examples and cleanup instructions.
e2e/helpers/environment/service-managers/dev-ghost-manager.ts (6)
14-35: LGTM!The
DevGhostConfiginterface andBASE_GHOST_ENVconstant are well-structured and align with theDEV_ENVIRONMENTconstants andcompose.dev.yamlconfiguration.
41-75: LGTM!Clean class structure with proper initialization and worker-scoped container setup. The container naming convention (
ghost-e2e-worker-{index}) provides clear identification.
119-161: LGTM!The
restartWithDatabasemethod properly removes and recreates the container with the new database, enabling per-test isolation. ThewaitForReadymethod has reasonable timeout and polling intervals.
163-207: LGTM!The
buildEnvmethod correctly constructs the environment configuration, with conditional Tinybird setup based on availability. The async check forisTinybirdAvailable()ensures tests gracefully handle missing analytics infrastructure.
226-231: Intentional cross-project volume mount.The hardcoded
ghost-dev_shared-configvolume name is by design—it mounts the shared-config volume from the mainghost-devproject to access Tinybird credentials created byyarn dev:forward. The inline comment on lines 228-229 now documents this well. Based on learnings from prior review discussion.
301-358: LGTM!The
runMigrationsmethod correctly usesAutoRemove: falseto ensure logs can be captured on failure before container cleanup. This addresses the prior review feedback.e2e/tests/admin/analytics/location.test.ts (1)
3-5: Suite-level conditional skip for Tinybird-dependent tests looks goodImporting
shouldSkipAnalyticsTestsand using a top-leveltest.skip(await shouldSkipAnalyticsTests(), 'Tinybird not available')cleanly ensures this whole file is skipped when Tinybird is unavailable locally, while still running (and failing) in CI. This matches the PR’s intent and keeps the test body unchanged.Please just confirm that your Playwright/TypeScript setup supports top-level
awaitin test files; if not, the same logic could be moved into atest.beforeAll(async () => { if (await shouldSkipAnalyticsTests()) test.skip(); });inside thedescribeblock.e2e/tests/admin/analytics/web-traffic.test.ts (1)
3-5: Consistent Tinybird-aware skip for web-traffic analytics suiteAdding
shouldSkipAnalyticsTestsand the top-leveltest.skip(await shouldSkipAnalyticsTests(), 'Tinybird not available');gives this suite the same behavior as the other analytics tests—skipping locally when Tinybird isn’t running but always executing in CI. The change is minimal and keeps test logic untouched.As with the other file, please verify that top-level
awaitis supported by your Playwright/TS config; otherwise, consider moving the check into abeforeAllhook inside thedescribeblock and callingtest.skip()from there.
I've extended the default E2E test fixtures to detect the presence of containers initialized through
yarn dev:forward. When those containers are present we can reuse existing resources and use the lightweight dev backend image and gateway setup for E2E test isolation.Since the
yarn dev:forwardcommand won't start Tinybird by default I've tweaked the test so that we automatically skip tests that depend on Tinybird when running locally and that service isn't available. In CI, the tests will still fail if Tinybird isn't available.Note: This PR includes the changes from #25551, which should be merged first.