Skip to content

ci(playwright): discover plugin frontend specs (closes #7622)#7623

Merged
JohnMcLear merged 2 commits intoether:developfrom
JohnMcLear:ci/plugin-frontend-tests
Apr 28, 2026
Merged

ci(playwright): discover plugin frontend specs (closes #7622)#7623
JohnMcLear merged 2 commits intoether:developfrom
JohnMcLear:ci/plugin-frontend-tests

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

Closes #7622.

Adds two new globs to Playwright's testMatch so any installed plugin shipping specs at the conventional location is picked up automatically:

  • ../node_modules/ep_*/static/tests/frontend-new/specs/**/*.spec.tspnpm add -w workspace installs (CI's with-plugins jobs, dev workspace).
  • plugin_packages/ep_*/static/tests/frontend-new/specs/**/*.spec.ts — admin-UI / live-plugin-manager installs into src/plugin_packages.

Mirrors the equivalent backend pattern that has worked for years:

mocha ... tests/backend/specs/**.ts ../node_modules/ep_*/static/tests/backend/specs/**

Change type: patch (test infra; no production behavior change).

Why this is a real gap

Plugin frontend tests have been silently dead since commit cc80db2d3 (2023-07-02), when the legacy in-page jQuery test runner was removed without a Playwright replacement. Every plugin still ships static/tests/frontend/specs/test.js (mocha + helper.padChrome$ style) and every plugin's CI runs pnpm run test-ui — but test-ui resolves to playwright test tests/frontend-new/specs --project=chromium, scoped to core only. Plugin specs are never imported by any runner, anywhere.

PR #7609 caught one direction of regression — core changes that break core's tests when plugins are loaded. This PR closes the other direction — core (or plugin) changes that break a plugin's own behavior tests. Both directions matter.

Verified locally

$ pnpm exec playwright test --list --project=chromium | tail -1
Total: 143 tests in 38 files                       # before
$ pnpm exec playwright test --list --project=chromium | tail -1
Total: 146 tests in 39 files                       # after, with one ep_headings2 spec migrated

The migrated ep_headings2 spec (3 tests) runs and produces real signal — found a real a11y bug in published v0.2.79 where #heading-selection lacks an aria-label (the fix is already in ep_headings2 master, just unreleased). That's exactly the kind of regression the existing infrastructure couldn't surface.

The ep_headings2 migration ships in a separate PR against ether/ep_headings2. After this PR lands and ep_headings2 publishes a new version, #7609's with-plugins job will start exercising plugin specs too.

Documentation

doc/PLUGIN_FRONTEND_TESTS.md documents:

  • The conventional layout (static/tests/frontend-new/specs/*.spec.ts)
  • How to import shared helpers (ep_etherpad-lite/tests/frontend-new/helper/padHelper)
  • A mocha+helper → Playwright translation table for plugin maintainers migrating their legacy specs

Test plan

  • Existing core test discovery unchanged (143 tests in 38 files before; same after the change in any without-plugins context).
  • Plugin specs at the conventional location are discovered when the plugin is installed.
  • Migrated ep_headings2 spec runs and produces pass/fail signal — proven locally.
  • CI green on this PR (no plugins installed, so no plugin specs run; tests should match develop).
  • After ep_headings2 publishes the migrated PR, ci: run frontend tests with /ether plugin set (closes #7608) #7609's chrome-with-plugins job should start running its 3 specs.

🤖 Generated with Claude Code

…ugin_packages

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 cc80db2 (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 ether#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 ether#7622.

**Change type:** patch (test infra; no production behavior change).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Enable automatic plugin frontend test discovery in Playwright

🧪 Tests ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Auto-discover plugin frontend specs from node_modules and plugin_packages
• Enable Playwright to run plugin tests alongside core tests
• Document plugin frontend test convention and migration guide
• Mirror existing backend plugin test discovery pattern
Diagram
flowchart LR
  A["Playwright Config"] -->|testMatch globs| B["Core Tests"]
  A -->|testMatch globs| C["Plugin Tests<br/>node_modules"]
  A -->|testMatch globs| D["Plugin Tests<br/>plugin_packages"]
  B --> E["Test Execution"]
  C --> E
  D --> E
Loading

Grey Divider

File Changes

1. src/playwright.config.ts ⚙️ Configuration changes +24/-1

Configure Playwright to discover plugin frontend tests

• Changed testDir from scoped path to root directory to enable glob expansion
• Added testMatchGlobs array with three patterns: core tests, workspace-installed plugins, and
 admin-UI installed plugins
• Added detailed comments explaining the plugin discovery pattern and path resolution
• Mirrors backend mocha test discovery for consistency

src/playwright.config.ts


2. doc/PLUGIN_FRONTEND_TESTS.md 📝 Documentation +103/-0

Add plugin frontend test documentation and migration guide

• Documents conventional plugin test layout at static/tests/frontend-new/specs/**/*.spec.ts
• Provides example Playwright spec with shared helper imports from core package
• Includes migration table translating legacy mocha+helper syntax to Playwright
• Explains how to run plugin tests and references the re-enabled coverage from commit cc80db2d3

doc/PLUGIN_FRONTEND_TESTS.md


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 28, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. test-ui path filters plugins📎 Requirement gap ≡ Correctness
Description
The Playwright config adds plugin testMatch globs, but pnpm run test-ui runs `npx playwright
test tests/frontend-new/specs`, which restricts discovery to core specs and can prevent plugin-owned
frontend specs from running in CI.
Code

src/playwright.config.ts[R22-34]

+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,
Evidence
PR Compliance ID 1 requires plugin-owned frontend tests to actually execute in CI. The PR adds
plugin testMatch globs in src/playwright.config.ts, but the test-ui script still passes
tests/frontend-new/specs as a positional filter, which can exclude plugin spec paths (for example
../node_modules/ep_*/...) from execution.

Plugin-owned frontend tests must actually run in CI (not only core frontend-new specs)
src/playwright.config.ts[22-30]
src/package.json[153-154]
doc/PLUGIN_FRONTEND_TESTS.md[10-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Plugin frontend specs may still not run in CI because the `test-ui` script passes `tests/frontend-new/specs` to `playwright test`, which can filter out plugin spec paths added via `testMatch`.
## Issue Context
The PR’s goal (Compliance ID 1) is that plugin-owned Playwright specs (for example under `../node_modules/ep_*/static/tests/frontend-new/specs/**`) execute when plugins are installed. This requires the CI entrypoint (`pnpm run test-ui`) to not restrict discovery to core-only paths.
## Fix Focus Areas
- src/package.json[153-154]
- src/playwright.config.ts[22-34]
- doc/PLUGIN_FRONTEND_TESTS.md[10-13]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Admin tests excluded🐞 Bug ≡ Correctness
Description
src/playwright.config.ts now defines an explicit testMatch that does not include
tests/frontend-new/admin-spec, so the admin Playwright suite will not be discovered. This breaks
pnpm run test-admin and the frontend-admin-tests GitHub Actions workflow (likely “No tests found” /
missing coverage).
Code

src/playwright.config.ts[R22-35]

+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 */
Evidence
The new Playwright configuration limits discovery to core frontend specs and plugin spec locations
only, omitting the admin-spec directory. The repo’s test-admin script and CI workflow rely on
discovering tests under tests/frontend-new/admin-spec, and those tests exist as *.spec.ts files, so
they will not match the configured globs.

src/playwright.config.ts[22-35]
src/package.json[144-156]
.github/workflows/frontend-admin-tests.yml[71-92]
src/tests/frontend-new/admin-spec/adminsettings.spec.ts[1-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Playwright config now sets an explicit `testMatch` list that excludes `tests/frontend-new/admin-spec/**/*.spec.ts`, so admin UI tests are no longer discovered.
### Issue Context
`pnpm run test-admin` (and `.github/workflows/frontend-admin-tests.yml`) runs Playwright against `tests/frontend-new/admin-spec`, which depends on config-based test discovery.
### Fix Focus Areas
- src/playwright.config.ts[22-30]
### Suggested change
Add an additional glob for admin tests, e.g.:
- `tests/frontend-new/admin-spec/**/*.spec.ts`
Optionally, consolidate with a brace pattern for readability:
- `tests/frontend-new/{specs,admin-spec}/**/*.spec.ts`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. test-ui runs admin project🐞 Bug ☼ Reliability
Description
src/package.json now runs npx playwright test with no path or project filter, so it will execute
all configured projects including chromium-admin. The admin specs require extra setup (admin UI
enabled, admin frontend built) that is not part of the regular frontend test setup, causing
failures/flakes when pnpm run test-ui is run without --project.
Code

src/package.json[R153-154]

+    "test-ui": "cross-env NODE_ENV=production npx playwright test",
+    "test-ui:ui": "cross-env NODE_ENV=production npx playwright test --ui",
Evidence
The test-ui script no longer scopes to tests/frontend-new/specs, and the Playwright config now
defines three projects, including a dedicated chromium-admin project that matches admin specs. The
repo’s CI workflows demonstrate that admin tests require additional setup steps (enabling Admin UI
tests and building the admin frontend) that are not done for regular frontend tests, so running all
projects together from test-ui can break in environments that previously only ran pad-editor
specs.

src/package.json[144-156]
src/playwright.config.ts[49-70]
.github/workflows/frontend-admin-tests.yml[66-92]
.github/workflows/frontend-tests.yml[49-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pnpm run test-ui` now runs `npx playwright test` without limiting projects, so it can execute the `chromium-admin` project (admin specs) in contexts that do not prepare the Admin UI environment.
### Issue Context
Admin specs require setup that regular frontend runs do not do (enabling admin UI tests, building admin assets). CI avoids this by explicitly passing `--project=chromium`/`--project=firefox` for frontend and using a separate workflow/script for admin.
### Fix Focus Areas
- src/package.json[153-156]
- src/playwright.config.ts[49-70]
### Suggested change
Update the `test-ui` / `test-ui:ui` scripts to explicitly run only the frontend projects (e.g., `--project=chromium --project=firefox`), keeping `test-admin` as the only entry point that runs `chromium-admin`. Alternatively, move admin specs into a separate Playwright config and have `test-admin` pass `-c` to avoid including the admin project in the default config.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. testMatchGlobs uses 4-space indent📘 Rule violation ⚙ Maintainability
Description
New/modified lines in src/playwright.config.ts use 4-space indentation, but the repository’s
.editorconfig requires 2-space indentation.
Code

src/playwright.config.ts[R23-34]

+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,
Evidence
PR Compliance ID 8 requires 2-space indentation for changed/added code. .editorconfig sets
indent_size = 2, but the added/modified config lines are indented with 4 spaces.

.editorconfig[3-8]
src/playwright.config.ts[23-34]
Best Practice: Repository guidelines

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Changed/added code in `src/playwright.config.ts` does not follow the repository’s required 2-space indentation.
## Issue Context
`.editorconfig` specifies `indent_size = 2` for all files by default.
## Fix Focus Areas
- src/playwright.config.ts[23-52]
- .editorconfig[3-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/playwright.config.ts Outdated
Comment thread src/playwright.config.ts Outdated
JohnMcLear added a commit to ether/ep_spellcheck that referenced this pull request Apr 28, 2026
The legacy mocha+helper specs in static/tests/frontend/ have been
silently dead since core removed the in-page jQuery test runner in
2023 (cc80db2d3). Migrate to Playwright at the new conventional
location ether/etherpad#7623 introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

The legacy file is removed because it never executed and would
otherwise rot further.

Change type: patch (test-only).
JohnMcLear added a commit to ether/ep_markdown that referenced this pull request Apr 28, 2026
The legacy mocha+helper specs in static/tests/frontend/ have been
silently dead since core removed the in-page jQuery test runner in
2023 (cc80db2d3). Migrate to Playwright at the new conventional
location ether/etherpad#7623 introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

The legacy file is removed because it never executed and would
otherwise rot further.

Change type: patch (test-only).
JohnMcLear added a commit to ether/ep_subscript_and_superscript that referenced this pull request Apr 28, 2026
The legacy mocha+helper specs in static/tests/frontend/ have been
silently dead since core removed the in-page jQuery test runner in
2023 (cc80db2d3). Migrate to Playwright at the new conventional
location ether/etherpad#7623 introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

The legacy file is removed because it never executed and would
otherwise rot further.

Change type: patch (test-only).
JohnMcLear added a commit to ether/ep_align that referenced this pull request Apr 28, 2026
The legacy mocha+helper specs in static/tests/frontend/ have been
silently dead since core removed the in-page jQuery test runner in
2023 (cc80db2d3). Migrate to Playwright at the new conventional
location ether/etherpad#7623 introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

The legacy file is removed because it never executed and would
otherwise rot further.

Change type: patch (test-only).
JohnMcLear added a commit to ether/ep_set_title_on_pad that referenced this pull request Apr 28, 2026
The legacy mocha+helper specs in static/tests/frontend/ have been
silently dead since core removed the in-page jQuery test runner in
2023 (cc80db2d3). Migrate to Playwright at the new conventional
location ether/etherpad#7623 introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

The legacy file is removed because it never executed and would
otherwise rot further.

Change type: patch (test-only).
JohnMcLear added a commit to ether/ep_font_size that referenced this pull request Apr 28, 2026
The legacy mocha+helper specs in static/tests/frontend/ have been
silently dead since core removed the in-page jQuery test runner in
2023 (cc80db2d3). Migrate to Playwright at the new conventional
location ether/etherpad#7623 introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

The legacy file is removed because it never executed and would
otherwise rot further. The l10n.js spec is intentionally not
migrated yet — language-switching tests are global state and need
extra care; will follow up.

Change type: patch (test-only).
…her#7623

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) <noreply@anthropic.com>
@JohnMcLear
Copy link
Copy Markdown
Member Author

/review

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 28, 2026

Persistent review updated to latest commit 89c6186

@JohnMcLear JohnMcLear merged commit 7f76aa2 into ether:develop Apr 28, 2026
19 checks passed
JohnMcLear added a commit to ether/ep_author_hover that referenced this pull request Apr 28, 2026
This plugin previously had no frontend test coverage. Add minimal
smoke tests at the conventional location ether/etherpad#7623
introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

Coverage is intentionally minimal (plugin-loaded smoke + a
plugin-specific structural assertion) — these are scaffolding for
plugin-specific behaviour tests to grow into. Better than the prior
zero-test baseline.

Change type: patch (test-only).
JohnMcLear added a commit to ether/ep_cursortrace that referenced this pull request Apr 28, 2026
This plugin previously had no frontend test coverage. Add minimal
smoke tests at the conventional location ether/etherpad#7623
introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

Coverage is intentionally minimal (plugin-loaded smoke + a
plugin-specific structural assertion) — these are scaffolding for
plugin-specific behaviour tests to grow into. Better than the prior
zero-test baseline.

Change type: patch (test-only).
JohnMcLear added a commit to ether/ep_table_of_contents that referenced this pull request Apr 28, 2026
This plugin previously had no frontend test coverage. Add minimal
smoke tests at the conventional location ether/etherpad#7623
introduces:

  static/tests/frontend-new/specs/

Once that core PR lands and a new release of this plugin ships, the
specs will run automatically in:
  - This plugin's own frontend-tests CI (against core+plugin).
  - Core's chrome-with-plugins CI (ether/etherpad#7609).

Coverage is intentionally minimal (plugin-loaded smoke + a
plugin-specific structural assertion) — these are scaffolding for
plugin-specific behaviour tests to grow into. Better than the prior
zero-test baseline.

Change type: patch (test-only).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revive plugin-own frontend tests (the legacy mocha+helper specs)

1 participant