DIAGNOSTIC: capture frontend trace (do not merge)

[JohnMcLear]

Temporary diagnostic to capture why ace_outer never loads in CI. Will close after grabbing traces.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

PR Summary by Qodo

DIAGNOSTIC: capture Playwright traces for define.spec in CI
⚙️ Configuration changes 🕐 Less than 10 minutes

Description

• Narrow frontend CI to run only define.spec with limited retries
• Always upload Playwright traces/reports as short-lived CI artifacts
• Keep CI green while collecting diagnostics (ignore test exit code)

Diagram

graph TD
  A["GitHub Actions: frontend-tests"] --> B["Run Playwright (define.spec)"] --> C[("test-results / report")]
  C --> D["upload-artifact: pw-traces"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Gate diagnostic behavior behind a workflow input/env flag

• ➕ Avoids changing default CI behavior for all branches/PRs
• ➕ Makes it easy to toggle diagnostics on/off without reverting commits
• ➖ Slightly more workflow complexity
• ➖ Requires contributors to know/enable the flag when needed

2. Enable Playwright trace-on-failure via config, keep full test suite

• ➕ Preserves normal CI coverage while still collecting traces
• ➕ Less risk of missing regressions outside define.spec
• ➖ May generate too many/larger artifacts, increasing CI time/storage
• ➖ Harder to isolate the specific failure if CI is already unstable

Recommendation: For the stated goal (short-lived diagnostics), the PR’s focused spec run + artifact upload is appropriate. To reduce risk of accidental merge, consider gating the diagnostic mode behind an explicit workflow input/env toggle (or add branch/path filters) and revert once traces are captured.

Files changed (1) +13 / -1

Other (1) +13 / -1

frontend-tests.ymlRun only define.spec and upload Playwright traces in CI +13/-1

Run only define.spec and upload Playwright traces in CI

• Modifies the frontend test job to execute only the Playwright define.spec with a single retry and bounded failures, while allowing the job to continue for diagnostics. Adds an always-run artifact upload step to persist Playwright test-results and the HTML report with a short retention window.

.github/workflows/frontend-tests.yml

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Test failures ignored ✓ Resolved 🐞 Bug ☼ Reliability

Description

The Playwright step appends || true, forcing the step to succeed even when pnpm run test-ui
fails, so CI can report green while UI regressions exist. This undermines the repository’s primary
signal that the plugin still works in Etherpad’s frontend harness.

Code

.github/workflows/frontend-tests.yml[R72-74]

+          # DIAGNOSTIC: only ep_define spec, single retry, capture artifacts
+          pnpm run test-ui --project=chromium --retries=1 --max-failures=4 \
+            define.spec || true

Evidence

The workflow explicitly forces the Playwright test command to return success via || true,
preventing the job from failing when Playwright exits non-zero.

.github/workflows/frontend-tests.yml[55-84]

Agent prompt

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

## Issue description
The workflow’s Playwright invocation ends with `|| true`, which converts any failing test run into a successful step and can allow regressions to merge unnoticed.
## Issue Context
The workflow already uploads artifacts with `if: always()`, so the job can still capture traces/reports even if the test step fails—there’s no need to force the test command to succeed.
## Fix Focus Areas
- .github/workflows/frontend-tests.yml[55-84]

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

---

2. CI runs only one spec 🐞 Bug ≡ Correctness

Description

The workflow now passes a positional define.spec filter to Playwright, reducing CI coverage from
the full UI suite to the single define spec. This can allow unrelated frontend regressions to pass
CI because they are no longer exercised.

Code

.github/workflows/frontend-tests.yml[R73-74]

+          pnpm run test-ui --project=chromium --retries=1 --max-failures=4 \
+            define.spec || true

Evidence

Repository documentation indicates test-ui is the standard way to run the Playwright suite, but
the workflow now adds a define.spec filter. The repo contains a single matching Playwright spec
file (define.spec.ts), demonstrating the coverage reduction.

CONTRIBUTING.md[34-49]
static/tests/frontend-new/specs/define.spec.ts[1-23]
.github/workflows/frontend-tests.yml[55-84]

Agent prompt

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

## Issue description
The workflow narrows Playwright execution to `define.spec`, which reduces the CI test surface area compared to running `test-ui` without a file filter.
## Issue Context
Project docs describe running the Playwright suite via `pnpm --filter ep_etherpad-lite run test-ui` (no file filter). The only `define` Playwright spec in this repo is `static/tests/frontend-new/specs/define.spec.ts`, so this change effectively makes CI run only that single spec.
## Fix Focus Areas
- .github/workflows/frontend-tests.yml[55-84]

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

---

[JohnMcLear]

Diagnostic complete — root cause found (top-level require('wordnet') hangs the client bundle). Real fix in #92.