From c32b9ac9c1a198cdae4a5a8b2b978126305333af Mon Sep 17 00:00:00 2001 From: John McLear Date: Tue, 28 Apr 2026 06:09:01 +0100 Subject: [PATCH 1/2] ci(playwright): discover plugin frontend specs from node_modules + plugin_packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new globs to the Playwright testMatch so any installed plugin shipping specs at the conventional location is picked up automatically: - ../node_modules/ep_*/static/tests/frontend-new/specs/**/*.spec.ts (covers `pnpm add -w ep_*` workspace installs, e.g. CI's with-plugins matrix and dev-time pnpm installs) - plugin_packages/ep_*/static/tests/frontend-new/specs/**/*.spec.ts (covers admin-UI / live-plugin-manager installs into src/plugin_packages) Mirrors the equivalent backend pattern (`mocha ... ../node_modules/ep_*/static/tests/backend/specs/**`) which already auto-discovers plugin backend specs. This re-enables coverage that was lost in commit cc80db2d3 (2023-07) when the legacy in-page jQuery test runner was removed without a Playwright replacement. Until now plugin frontend tests have been silently dead: every plugin's CI runs `pnpm run test-ui` but core's testDir scoped only to `tests/frontend-new/`, so plugin specs at `static/tests/frontend/specs/test.js` were never executed and their green badges were misleading. See #7622. doc/PLUGIN_FRONTEND_TESTS.md documents the new convention, the import path for shared helpers (ep_etherpad-lite/tests/...), and a mocha+helper → Playwright translation table for plugin maintainers who want to migrate. Existing core test discovery is unchanged (143 tests in 38 files listed before and after). Closes #7622. **Change type:** patch (test infra; no production behavior change). Co-Authored-By: Claude Opus 4.7 (1M context) --- doc/PLUGIN_FRONTEND_TESTS.md | 103 +++++++++++++++++++++++++++++++++++ src/playwright.config.ts | 25 ++++++++- 2 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 doc/PLUGIN_FRONTEND_TESTS.md diff --git a/doc/PLUGIN_FRONTEND_TESTS.md b/doc/PLUGIN_FRONTEND_TESTS.md new file mode 100644 index 00000000000..ffacd04d776 --- /dev/null +++ b/doc/PLUGIN_FRONTEND_TESTS.md @@ -0,0 +1,103 @@ +# Plugin frontend tests + +Etherpad core's Playwright runner discovers plugin frontend specs from +the conventional path: + +``` +node_modules/ep_/static/tests/frontend-new/specs/**/*.spec.ts +``` + +When the plugin is installed alongside core (e.g. via `pnpm add -w +ep_` or in a `with-plugins` CI variant), the plugin's specs +run as part of `pnpm run test-ui`. Same pattern backend tests already +use (`mocha ... ../node_modules/ep_*/static/tests/backend/specs/**`). + +This re-enables coverage that was lost in commit `cc80db2d3` (2023-07) +when the legacy jQuery test runner (`static/tests/frontend/specs/test.js` ++ in-page mocha+helper) was removed without a Playwright replacement. +See [#7622](https://github.com/ether/etherpad/issues/7622). + +## Layout in your plugin + +``` +ep_yourplugin/ +├── ep.json +├── package.json +├── static/ +│ └── tests/ +│ └── frontend-new/ +│ └── specs/ +│ └── yourplugin.spec.ts +└── ... +``` + +A spec is a normal Playwright test file. Import shared helpers from the +core package — `ep_etherpad-lite` is symlinked into `node_modules` by +the workspace, so this resolves anywhere the plugin is installed +alongside core: + +```ts +import {expect, test} from '@playwright/test'; +import {clearPadContent, getPadBody, goToNewPad, writeToPad} + from 'ep_etherpad-lite/tests/frontend-new/helper/padHelper'; + +test.beforeEach(async ({page}) => { + await goToNewPad(page); +}); + +test.describe('ep_yourplugin', () => { + test('does the thing', async ({page}) => { + const padBody = await getPadBody(page); + await padBody.click(); + await clearPadContent(page); + await writeToPad(page, 'hello'); + // …assertions… + await expect(padBody.locator('div').first()).toHaveText('hello'); + }); +}); +``` + +## Migrating from the legacy `static/tests/frontend/specs/test.js` + +The old format used mocha + a jQuery `helper` global: + +```js +// Legacy — does not run anywhere any more. +describe('ep_yourplugin', function () { + beforeEach(function (cb) { helper.newPad(cb); }); + it('does the thing', async function () { + const chrome$ = helper.padChrome$; + const inner$ = helper.padInner$; + expect(chrome$('#yourbutton').length).to.be.greaterThan(0); + }); +}); +``` + +Translation table: + +| Legacy (mocha + helper) | Playwright | +|---|---| +| `describe(...)` / `it(...)` | `test.describe(...)` / `test(...)` | +| `helper.newPad(cb)` | `await goToNewPad(page)` | +| `helper.padChrome$('#x')` | `page.locator('#x')` | +| `helper.padInner$('div')` | `(await getPadBody(page)).locator('div')` | +| `expect(x).to.equal(y)` | `expect(x).toBe(y)` (Playwright's expect) | +| `expect($el.length).to.be.greaterThan(0)` | `await expect(page.locator('#x')).toBeVisible()` | +| `$el.sendkeys('text')` | `await page.keyboard.type('text')` | +| `$el.simulate('click')` | `await page.locator(...).click()` | + +Most legacy specs translate ~mechanically. After migrating, **delete +the legacy file** so the plugin can't accidentally ship stale tests +that nothing executes. + +## Running them + +```sh +# Inside core, with the plugin installed: +pnpm run test-ui --project=chromium +# Or via core's with-plugins CI job (see frontend-tests.yml). +``` + +`pnpm run test-ui` automatically picks up plugin specs from any +installed `ep_*` package. To gate per-plugin: use playwright's +`--grep` against your plugin's describe name. diff --git a/src/playwright.config.ts b/src/playwright.config.ts index bd306840534..3347967d927 100644 --- a/src/playwright.config.ts +++ b/src/playwright.config.ts @@ -7,8 +7,31 @@ export const defaultTestTimeout = 90 * 1000 /** * See https://playwright.dev/docs/test-configuration. */ +// Mirror of how tests/backend/specs picks up plugin specs from +// `../node_modules/ep_*/static/tests/backend/specs/**`. Plugins that +// ship Playwright frontend tests at the conventional location below are +// discovered automatically when the plugin is installed alongside core. +// +// Path is relative to `testDir` (which is './' so the same root as +// playwright resolves from). Quirk: testDir defaults to one path; we +// expand to '.' so the testMatch globs can reach both core's tests and +// node_modules paths above src/. +// +// See docs/PLUGIN_FRONTEND_TESTS.md for the per-plugin spec layout +// convention. +const testDirRoot = '.'; +const testMatchGlobs = [ + 'tests/frontend-new/specs/**/*.spec.ts', + // Plugins installed via `pnpm add -w ep_*` (CI / dev workspace). + '../node_modules/ep_*/static/tests/frontend-new/specs/**/*.spec.ts', + // Plugins installed via the admin UI / live-plugin-manager land + // here instead of node_modules. + 'plugin_packages/ep_*/static/tests/frontend-new/specs/**/*.spec.ts', +]; + export default defineConfig({ - testDir: './tests/frontend-new/', + testDir: testDirRoot, + testMatch: testMatchGlobs, /* Run tests in files in parallel */ fullyParallel: true, /* Fail the build on CI if you accidentally left test.only in the source code. */ From 89c6186aafcd9875713e0c6622e0bfea3992b6ce Mon Sep 17 00:00:00 2001 From: John McLear Date: Tue, 28 Apr 2026 06:27:06 +0100 Subject: [PATCH 2/2] fix(playwright): split into per-project testMatch; address Qodo on #7623 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real Qodo findings on the previous commit, all fixed: 1) test-ui's positional arg `tests/frontend-new/specs` filtered out plugin spec paths added to testMatch — the very thing the PR was trying to enable. Drop the positional. Discovery is now driven by per-project testMatch. 2) The single project-wide testMatch I added excluded tests/frontend-new/admin-spec, breaking pnpm run test-admin and the frontend-admin-tests workflow. Split into three projects: - chromium : core specs + plugin specs - firefox : core specs + plugin specs - chromium-admin : admin specs only test-admin now runs --project=chromium-admin (no positional). Net coverage unchanged for both workflows. 3) New code re-indented to 2 spaces per .editorconfig. Discovery verified locally: --project=chromium → 143 tests in 38 files (core) --project=firefox → 143 tests in 38 files (core) --project=chromium-admin → 11 tests in 4 files (admin) With a plugin spec installed at the conventional path: --project=chromium → +1 file, +N tests as expected. **Change type:** patch (test infra; no production behavior change). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/package.json | 8 +- src/playwright.config.ts | 162 ++++++++++++++++++++------------------- 2 files changed, 88 insertions(+), 82 deletions(-) diff --git a/src/package.json b/src/package.json index a52682173d3..6a93a780d1e 100644 --- a/src/package.json +++ b/src/package.json @@ -150,10 +150,10 @@ "prod": "cross-env NODE_ENV=production node --require tsx/cjs node/server.ts", "ts-check": "tsc --noEmit", "ts-check:watch": "tsc --noEmit --watch", - "test-ui": "cross-env NODE_ENV=production npx playwright test tests/frontend-new/specs", - "test-ui:ui": "cross-env NODE_ENV=production npx playwright test tests/frontend-new/specs --ui", - "test-admin": "cross-env NODE_ENV=production npx playwright test tests/frontend-new/admin-spec --workers 1 --project=chromium", - "test-admin:ui": "cross-env NODE_ENV=production npx playwright test tests/frontend-new/admin-spec --ui --workers 1", + "test-ui": "cross-env NODE_ENV=production npx playwright test", + "test-ui:ui": "cross-env NODE_ENV=production npx playwright test --ui", + "test-admin": "cross-env NODE_ENV=production npx playwright test --workers 1 --project=chromium-admin", + "test-admin:ui": "cross-env NODE_ENV=production npx playwright test --ui --workers 1 --project=chromium-admin", "debug:socketio": "cross-env DEBUG=socket.io* node --require tsx/cjs node/server.ts", "test:vitest": "vitest" }, diff --git a/src/playwright.config.ts b/src/playwright.config.ts index 3347967d927..be93b1cd56f 100644 --- a/src/playwright.config.ts +++ b/src/playwright.config.ts @@ -4,92 +4,98 @@ import {defineConfig, devices, test} from '@playwright/test'; export const defaultExpectTimeout = process.env.CI ? 20 * 1000 : 5000 export const defaultTestTimeout = 90 * 1000 -/** - * See https://playwright.dev/docs/test-configuration. - */ // Mirror of how tests/backend/specs picks up plugin specs from // `../node_modules/ep_*/static/tests/backend/specs/**`. Plugins that -// ship Playwright frontend tests at the conventional location below are -// discovered automatically when the plugin is installed alongside core. -// -// Path is relative to `testDir` (which is './' so the same root as -// playwright resolves from). Quirk: testDir defaults to one path; we -// expand to '.' so the testMatch globs can reach both core's tests and -// node_modules paths above src/. -// -// See docs/PLUGIN_FRONTEND_TESTS.md for the per-plugin spec layout -// convention. -const testDirRoot = '.'; -const testMatchGlobs = [ - 'tests/frontend-new/specs/**/*.spec.ts', - // Plugins installed via `pnpm add -w ep_*` (CI / dev workspace). - '../node_modules/ep_*/static/tests/frontend-new/specs/**/*.spec.ts', - // Plugins installed via the admin UI / live-plugin-manager land - // here instead of node_modules. - 'plugin_packages/ep_*/static/tests/frontend-new/specs/**/*.spec.ts', +// ship Playwright frontend tests at the conventional location below +// are discovered automatically when the plugin is installed alongside +// core. See doc/PLUGIN_FRONTEND_TESTS.md. +const CORE_SPECS = 'tests/frontend-new/specs/**/*.spec.ts'; +const ADMIN_SPECS = 'tests/frontend-new/admin-spec/**/*.spec.ts'; +const PLUGIN_SPECS = [ + // Plugins installed via `pnpm add -w ep_*` (CI / dev workspace). + '../node_modules/ep_*/static/tests/frontend-new/specs/**/*.spec.ts', + // Plugins installed via the admin UI / live-plugin-manager land + // here instead of node_modules. + 'plugin_packages/ep_*/static/tests/frontend-new/specs/**/*.spec.ts', ]; +const FRONTEND_MATCH = [CORE_SPECS, ...PLUGIN_SPECS]; +/** + * See https://playwright.dev/docs/test-configuration. + */ export default defineConfig({ - testDir: testDirRoot, - testMatch: testMatchGlobs, - /* Run tests in files in parallel */ - fullyParallel: true, - /* Fail the build on CI if you accidentally left test.only in the source code. */ - /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: process.env.CI ? [['github'], ['list']] : 'html', - expect: { timeout: defaultExpectTimeout }, - timeout: defaultTestTimeout, - retries: process.env.CI ? 2 : 0, - workers: 2, - /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ - use: { - /* Base URL to use in actions like `await page.goto('/')`. */ - // baseURL: 'http://127.0.0.1:3000', - baseURL: "localhost:9001", - /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ - trace: 'on-first-retry', - video: 'on-first-retry', - }, - - /* Configure projects for major browsers */ - projects: [ - { - name: 'chromium', - use: { ...devices['Desktop Chrome'] }, - }, + // testDir is project-root for src/ so the testMatch globs reach both + // tests under src/tests/... and node_modules/ep_*/... above src/. + testDir: '.', + /* Run tests in files in parallel */ + fullyParallel: true, + /* Fail the build on CI if you accidentally left test.only in the source code. */ + /* Reporter to use. See https://playwright.dev/docs/test-reporters */ + reporter: process.env.CI ? [['github'], ['list']] : 'html', + expect: { timeout: defaultExpectTimeout }, + timeout: defaultTestTimeout, + retries: process.env.CI ? 2 : 0, + workers: 2, + /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ + use: { + /* Base URL to use in actions like `await page.goto('/')`. */ + // baseURL: 'http://127.0.0.1:3000', + baseURL: "localhost:9001", + /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ + trace: 'on-first-retry', + video: 'on-first-retry', + }, - { - name: 'firefox', - use: { ...devices['Desktop Firefox'] }, - }, - // Webkit dropped from CI — see https://github.com/ether/etherpad-lite/issues/XXXX - // Kept chromium and firefox as the supported browsers. + /* Configure projects for major browsers */ + projects: [ + // Frontend / pad-editor specs (core + plugins). + { + name: 'chromium', + testMatch: FRONTEND_MATCH, + use: { ...devices['Desktop Chrome'] }, + }, + { + name: 'firefox', + testMatch: FRONTEND_MATCH, + use: { ...devices['Desktop Firefox'] }, + }, - /* Test against mobile viewports. */ - // { - // name: 'Mobile Chrome', - // use: { ...devices['Pixel 5'] }, - // }, - // { - // name: 'Mobile Safari', - // use: { ...devices['iPhone 12'] }, - // }, + // Admin-UI specs are isolated from the regular frontend run so the + // existing test-admin script + frontend-admin-tests workflow keep + // their own scope (different fixtures, different server state). + { + name: 'chromium-admin', + testMatch: ADMIN_SPECS, + use: { ...devices['Desktop Chrome'] }, + }, + // Webkit dropped from CI — see https://github.com/ether/etherpad-lite/issues/XXXX + // Kept chromium and firefox as the supported browsers. - /* Test against branded browsers. */ - // { - // name: 'Microsoft Edge', - // use: { ...devices['Desktop Edge'], channel: 'msedge' }, - // }, - // { - // name: 'Google Chrome', - // use: { ...devices['Desktop Chrome'], channel: 'chrome' }, - // }, - ], + /* Test against mobile viewports. */ + // { + // name: 'Mobile Chrome', + // use: { ...devices['Pixel 5'] }, + // }, + // { + // name: 'Mobile Safari', + // use: { ...devices['iPhone 12'] }, + // }, - /* Run your local dev server before starting the tests */ - // webServer: { - // command: 'npm run start', - // url: 'http://127.0.0.1:3000', - // reuseExistingServer: !process.env.CI, + /* Test against branded browsers. */ + // { + // name: 'Microsoft Edge', + // use: { ...devices['Desktop Edge'], channel: 'msedge' }, + // }, + // { + // name: 'Google Chrome', + // use: { ...devices['Desktop Chrome'], channel: 'chrome' }, // }, + ], + + /* Run your local dev server before starting the tests */ + // webServer: { + // command: 'npm run start', + // url: 'http://127.0.0.1:3000', + // reuseExistingServer: !process.env.CI, + // }, });