From ee475e6982c864b119b6dc2e77f5ad248f9e34af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Tue, 7 Jul 2026 17:05:44 +0200 Subject: [PATCH 1/3] refactor: make silverstripe/htmleditor-tinymce an optional dependency Only the generate-tinymce-combined CI task needs TinyMCE; fixtures-only consumers should not have to drag it in. Move it from require to require-dev (so this module's own tests still run) plus a suggest entry, and guard the task with class_exists() so it skips cleanly when the package is absent instead of fataling on the TinyMCE classes. --- composer.json | 7 +++++-- src/Tasks/GenerateTinyMCECombinedTask.php | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 2fea7bf..c1ca9f0 100644 --- a/composer.json +++ b/composer.json @@ -15,8 +15,7 @@ }], "require": { "php": "^8.3", - "silverstripe/framework": "^6", - "silverstripe/htmleditor-tinymce": "^1.1" + "silverstripe/framework": "^6" }, "require-dev": { "cambis/silverstan": "^2.1", @@ -25,9 +24,13 @@ "phpstan/phpstan-deprecation-rules": "^2.0", "phpunit/phpunit": "^11.3", "rregeer/phpunit-coverage-check": "^0.3", + "silverstripe/htmleditor-tinymce": "^1.1", "tomasvotruba/type-coverage": "^2.0", "wernerkrauss/silverstripe-rector": "^1.0" }, + "suggest": { + "silverstripe/htmleditor-tinymce": "Required only for the generate-tinymce-combined CI task; fixtures-only consumers do not need it (^1.1)." + }, "autoload": { "psr-4": { "WeDevelop\\E2e\\": "src/" diff --git a/src/Tasks/GenerateTinyMCECombinedTask.php b/src/Tasks/GenerateTinyMCECombinedTask.php index 06ea1cb..d90a290 100644 --- a/src/Tasks/GenerateTinyMCECombinedTask.php +++ b/src/Tasks/GenerateTinyMCECombinedTask.php @@ -25,6 +25,21 @@ class GenerateTinyMCECombinedTask extends BuildTask protected function execute(InputInterface $input, PolyOutput $output): int { + // silverstripe/htmleditor-tinymce is a suggested (optional) dependency: + // only this CI helper needs it, while fixtures-only consumers do not. + // Skip cleanly when it is absent rather than fataling on the TinyMCE + // classes used below. + if (!class_exists(TinyMCECombinedGenerator::class)) { + // @codeCoverageIgnoreStart + // Unreachable in this module's own test run, which require-dev's TinyMCE. + $output->writeln( + 'silverstripe/htmleditor-tinymce is not installed; skipping TinyMCE asset generation', + ); + + return Command::SUCCESS; + // @codeCoverageIgnoreEnd + } + TinyMCECombinedGenerator::flush(); $editorConfigs = HTMLEditorConfig::get_available_configs_map(); From f07f43cf2fa4b2659fa11b54f2cd7ed596827805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Tue, 7 Jul 2026 17:05:51 +0200 Subject: [PATCH 2/3] feat: improve Playwright fixture client ergonomics - loadAndNavigate defaults request to page.request (which also shares the page's auth cookies); the signature becomes (page, fixture, request?). This is a breaking change from (page, request, fixture). - Add editorReadyTimeout option for the editor-ready wait (was a fixed Playwright default). - reset() now checks the JSON success envelope, consistent with load()/loadAll(). - Document that the Playwright host needs the module's Composer deps installed (the client ships as a vendor deep-import, not on npm), and how to compose load() + page.goto() for non-pages navigation. --- client/playwright/README.md | 25 +++++++++++++++++++++++-- client/playwright/index.ts | 18 ++++++++++++++++-- tests/E2E/specs/fixture.spec.ts | 4 ++-- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/client/playwright/README.md b/client/playwright/README.md index c724566..26ac9b3 100644 --- a/client/playwright/README.md +++ b/client/playwright/README.md @@ -2,6 +2,16 @@ Reusable Playwright helpers for the `wedevelopnl/silverstripe-e2e` fixture endpoint. +## Requirements + +The client ships only as a source file inside the Composer package +(`vendor/wedevelopnl/silverstripe-e2e/client/playwright/index.ts`) — it is not +published to npm. The machine that runs Playwright must therefore have the +module's Composer dependencies installed (i.e. `vendor/` present), even when the +application's PHP otherwise runs entirely inside Docker. In CI, run `composer +install` on the Playwright runner (or mount the container's `vendor/`) before +`playwright test`. + ## Usage in a consuming project Add a path alias in the project's `tsconfig.json` (or `tests/E2E/tsconfig.json`): @@ -34,12 +44,23 @@ import { createFixtureClient } from '@wedevelop/e2e'; const fixtures = createFixtureClient(); // defaults to /dev/e2e-fixtures -test('...', async ({ page, request }) => { - const { pageId } = await fixtures.loadAndNavigate(page, request, 'my-fixture'); +test('...', async ({ page }) => { + // `request` defaults to `page.request` (shares the page's auth cookies); pass + // an explicit APIRequestContext as the third argument to override. + const { pageId } = await fixtures.loadAndNavigate(page, 'my-fixture'); // ... }); ``` +`loadAndNavigate` is a convenience wrapper for the common case of editing the +loaded record on the CMS pages screen (`/admin/pages/edit/show/{pageId}`). To +navigate elsewhere, compose the pieces directly: + +```ts +const { pageId } = await fixtures.load(request, 'my-fixture'); +await page.goto(`/admin/some-other-section/${pageId}`); +``` + The `/dev/e2e-fixtures` endpoint and the `strict_user_agent_check` relaxation are provided automatically by the module's dev-only config when installed. Register your fixtures and `fixture_page_classes` in your project's own dev config: diff --git a/client/playwright/index.ts b/client/playwright/index.ts index 90555d4..e21a819 100644 --- a/client/playwright/index.ts +++ b/client/playwright/index.ts @@ -32,11 +32,17 @@ export interface FixtureClientOptions { * `loadAndNavigate` waits for it to become hidden after navigation. */ editorReadySelector?: string; + /** + * Timeout (ms) for the `editorReadySelector` wait in `loadAndNavigate`. When + * omitted, Playwright's default action timeout applies. + */ + editorReadyTimeout?: number; } export function createFixtureClient(options: FixtureClientOptions = {}) { const endpoint = options.endpoint ?? '/dev/e2e-fixtures'; const editorReadySelector = options.editorReadySelector ?? null; + const editorReadyTimeout = options.editorReadyTimeout; async function load(request: APIRequestContext, fixture: string): Promise { const response = await request.post(`${endpoint}/load`, { form: { fixture } }); @@ -71,17 +77,25 @@ export function createFixtureClient(options: FixtureClientOptions = {}) { if (!response.ok()) { throw new Error(`Fixture reset failed (${response.status()}): ${await response.text()}`); } + + const body = (await response.json()) as { success: boolean; error?: string }; + if (!body.success) { + throw new Error(`Fixture reset failed: ${body.error ?? 'unknown error'}`); + } } async function loadAndNavigate( page: Page, - request: APIRequestContext, fixture: string, + request: APIRequestContext = page.request, ): Promise { const result = await load(request, fixture); await page.goto(`/admin/pages/edit/show/${result.pageId}`); if (editorReadySelector !== null) { - await page.locator(editorReadySelector).waitFor({ state: 'hidden' }); + await page.locator(editorReadySelector).waitFor({ + state: 'hidden', + timeout: editorReadyTimeout, + }); } return result; diff --git a/tests/E2E/specs/fixture.spec.ts b/tests/E2E/specs/fixture.spec.ts index a064dfc..242b147 100644 --- a/tests/E2E/specs/fixture.spec.ts +++ b/tests/E2E/specs/fixture.spec.ts @@ -15,8 +15,8 @@ test('loads a fixture over HTTP and returns the created page id', async ({ reque expect(result.fixtureMap.Page).toBeDefined(); }); -test('navigates to the loaded page in the CMS', async ({ page, request }) => { - const result = await fixtures.loadAndNavigate(page, request, 'demo-home'); +test('navigates to the loaded page in the CMS', async ({ page }) => { + const result = await fixtures.loadAndNavigate(page, 'demo-home'); await expect(page).toHaveURL(new RegExp(`/admin/pages/edit/show/${result.pageId}`)); await expect(page.getByText('E2E Home').first()).toBeVisible(); From 38dad072d44ecfc09d52ff646077020b8aa9e31f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Tue, 7 Jul 2026 17:18:53 +0200 Subject: [PATCH 3/3] test: pull htmleditor-tinymce into the container test harness Now that the module declares silverstripe/htmleditor-tinymce as an optional (require-dev + suggest) dependency, the containerised test suite no longer receives it: the harness installs the module as a path-repo dependency, and Composer never installs a dependency's require-dev. recipe-cms does not pull it either (verified), so GenerateTinyMCECombinedTask would skip and its test would fail. Mirror the pin into .docker/app/composer.json's require-dev, matching how the other module dev tools (phpstan, phpunit, rector, ...) are already duplicated there, so the container can exercise the task. --- .docker/app/composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.docker/app/composer.json b/.docker/app/composer.json index 22bf89a..d9b20a1 100644 --- a/.docker/app/composer.json +++ b/.docker/app/composer.json @@ -21,6 +21,7 @@ "phpstan/phpstan-deprecation-rules": "2.0.4", "phpunit/phpunit": "11.5.55", "rregeer/phpunit-coverage-check": "0.3.1", + "silverstripe/htmleditor-tinymce": "1.1.0", "tomasvotruba/type-coverage": "2.2.1", "wernerkrauss/silverstripe-rector": "1.3.0" },