Add a Playwright end-to-end harness with a containerised gateway - #91
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds first end-to-end (Playwright) coverage for the ROS2 Medkit Web UI by running the SPA in a real browser against a containerised gateway, and wiring it into CI. This complements the existing Vitest/jsdom suite by validating real network + DOM behaviour, especially for the Scripts feature introduced in #90.
Changes:
- Add a Playwright config, global setup, and a new
e2e/suite (live gateway scenarios + mocked error-state scenarios + smoke test). - Add a pinned Docker Compose gateway stack (manifest-only discovery, scripts enabled, uploads isolated in a named volume).
- Add CI job + docs updates for running E2E locally; ignore Playwright artifacts in git.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds project reference for the new e2e/ TypeScript project. |
| README.md | Documents Scripts tab behaviour and how to run the E2E suite locally. |
| playwright.config.ts | Defines Playwright projects, serialisation strategy (1 worker), web server config, and storage state. |
| e2e/tsconfig.json | Adds TS config for Playwright specs/global setup. |
| e2e/smoke.spec.ts | Smoke test ensuring the stack boots and the entity tree renders. |
| e2e/scripts.spec.ts | Live-gateway Scripts scenarios (list/run/fail/stop/upload/write/delete/remove execution). |
| e2e/scripts-errors.spec.ts | Mocked gateway responses for error/edge UI states that are hard to induce live. |
| e2e/global-setup.ts | Waits for gateway health and seeds persisted server URL into storage state. |
| e2e/gateway/scripts/sleep.sh | Gateway fixture script for stop/terminate scenario. |
| e2e/gateway/scripts/hello.sh | Gateway fixture script for stdin-parameter echo scenario. |
| e2e/gateway/scripts/fail.sh | Gateway fixture script for non-zero exit / stderr behaviour. |
| e2e/gateway/params.yaml | Gateway params enabling bind-all + CORS + manifest-only discovery + scripts uploads. |
| e2e/gateway/manifest.yaml | Minimal manifest defining entities and managed scripts for E2E. |
| e2e/fixtures/uploaded-script.sh | Fixture uploaded script used by E2E to validate upload/run/delete path. |
| e2e/docker-compose.yml | Compose stack for pinned gateway image + init service + named volume for uploads. |
| CONTRIBUTING.md | Adds instructions and warnings for running the E2E suite locally. |
| .gitignore | Ignores Playwright output directories and E2E auth storage state. |
| .github/workflows/ci.yml | Adds an e2e job running Compose + Playwright and uploading artifacts on failure. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
e2e/scripts.spec.ts:171
- This scenario is described as writing a bash script, but the uploaded file name ends in
.sh. The gateway’s interpreter selection (documented in the UI) uses.bashfor bash and treats other extensions (including.sh) as/bin/sh, so this test won’t actually exercise the bash interpreter path.
await page.getByRole('button', { name: 'Upload' }).click();
const dialog = page.getByRole('dialog');
await dialog.getByRole('button', { name: 'Write script' }).click();
await dialog.getByLabel('File name').fill(`${scriptName}.sh`);
// Reachable by role and accessible name against the real CodeMirror
README.md:91
- Local E2E instructions omit installing Playwright browsers. On a fresh machine/checkout,
npm run test:e2ewill fail unless the Chromium browser bundle has been installed (CI does this vianpx playwright install ...).
# Run the end-to-end suite against a containerised gateway
docker compose -f e2e/docker-compose.yml up -d
npm run test:e2e
CONTRIBUTING.md:106
- The local E2E runbook doesn’t mention installing Playwright browsers. On a fresh environment,
npm run test:e2ewill fail unless Chromium has been installed at least once (CI handles this explicitly).
2. Run the suite:
```bash
npm run test:e2e
```
mfaferek93
reviewed
Jul 28, 2026
bburda
force-pushed
the
feature/scripts-tab
branch
from
August 22, 2026 17:01
4711b3d to
412e662
Compare
bburda
force-pushed
the
feature/scripts-e2e
branch
from
August 22, 2026 17:01
440aafe to
ae6cf5d
Compare
Runs the suite against a real gateway in a container: manifest-defined scripts, uploads enabled, and a named volume for uploaded files so the checkout stays clean. Seven scenarios drive the live gateway, six more use route mocking for states a healthy gateway will not produce on demand, and one smoke test proves the stack itself. The suite runs on a single worker because the mocked project still reaches the gateway for entity discovery.
Adds the end-to-end job alongside the existing checks, dumping the gateway's container logs when a run fails so a container that dies during startup can be diagnosed from the run alone. Documents the Scripts tab and how to run the suite locally.
…by digest - Give each gateway health-check attempt in global setup its own timeout via AbortSignal.timeout, since a stalled connect (as opposed to a refused one) would otherwise hang past the overall deadline with no informative error. - Read E2E_APP_URL in playwright.config.ts and derive the dev server's port from it, matching global setup, so the two cannot end up pointed at different addresses. - Scope the "no longer tracks this execution" assertion in the polling-404 scenario to the card that owns the status badge, matching every other assertion in that test. - Make the scripts spec's afterEach cleanup best-effort: a failure opening the panel or removing one leftover script no longer masks the test's own failure or stops the rest of the cleanup from being attempted. - Pin the e2e gateway image by immutable digest instead of a mutable tag, so a later re-publish of the same tag cannot change what CI pulls, keeping the human-readable tag in a comment for reference.
The e2e gateway container ran with uploads enabled and executes uploaded shell scripts without authentication, yet published its port on every interface - on a shared or untrusted network that is remote code execution for as long as the container is left running. Bind it to 127.0.0.1 instead, keeping the port override working. Several other rough edges in the harness made it unreliable or misleading: - The toolbar "Upload" button lookup matched by substring, so it also resolved to leftover rows named uploaded_<worker>_<repeat> (which contains "upload"). Match it by exact name instead. - afterEach cleanup checked isVisible() once per leftover name; once two runs left duplicates behind, that check itself threw and nothing got deleted, compounding the problem. Loop on the locator's count and delete .first() until none remain. - The gateway's CORS config only allowed the default dev server origin, so overriding E2E_APP_URL to dodge a busy port failed every request with no CORS error to explain why. Allow any origin - this is a throwaway local/CI fixture with allow_credentials left at its default false, so a wildcard carries none of the risk it would in production. - global-setup only read E2E_GATEWAY_URL, while docker-compose reads E2E_GATEWAY_PORT, so overriding just the port (the natural move) left setup polling the wrong address for its full deadline. Derive the URL from the port when the URL itself is not set. - playwright.config.ts had no forbidOnly, so a committed test.only would pass CI quietly instead of failing it. - The sleeper script and its manifest timeout ran for 300s against a global concurrency cap of 5, so a handful of interrupted runs could exhaust every execution slot with no reset short of destroying the stack. Both are now 30s, still ample for the scenario that stops it mid-run. - The "writes a bash script" scenario uploaded a .sh file, but the gateway only runs .bash under bash - .sh runs under sh like everything else - so it never covered the branch its name claims to. Give it a .bash extension. Documented the E2E_GATEWAY_PORT/E2E_APP_URL override in CONTRIBUTING.
pull_request previously triggered only for PRs targeting main, so a PR targeting any other branch got no CI signal at all. Drop the branch filter on pull_request (push stays scoped to main) so every pull request runs the check, e2e and docker-build jobs.
ScriptRow's handleDelete now calls window.confirm before deleting a script. Playwright auto-dismisses a native dialog with no handler, which returns false and turns every delete into a silent no-op, so every spec that clicks Delete needs to accept the dialog explicitly. Add clickDeleteAndConfirm, a shared helper that registers the dialog listener before the click (accepting after the click would deadlock, since window.confirm blocks the page's JS until the dialog is resolved) and asserts the dialog actually appeared with the expected message, so a removed confirmation guard would fail the test rather than pass silently. Use it everywhere a spec deletes a script: the upload/run/delete scenario, both write-script scenarios, the managed-script-rejection scenario, and the shared afterEach cleanup that removes leftovers between runs.
A fault can now hold several black-box recordings. The detail refetches on expand so one written while the page is open is reachable, the dashboard keys its cache by entity as well as code so two entities reporting the same code stop showing each other's evidence, each download button names its recording, and the saved file keeps the extension the gateway put on it. Adds a Playwright stack with a fault manager and specs covering all of it.
Distinct filenames alone would pass on a build that resolved both ids to one recording and labelled the responses differently.
A fetch in beforeAll threw before the skip guard could run, so CI went red on a missing fixture rather than reporting it as skipped.
Per-entity faultKey shared by the dashboard and the entity panel - expand, loading, clearing and the detail cache no longer tie colliding codes together, clear takes the Fault, and expansion opens before the refetch without blanking cached evidence on a 404. RFC 8187 charset+language filename parsing. The e2e stack gets its own compose project, sources ROS in the parent shell, runs the seeder as a watched job, and the seeder survives for discovery and checks both service responses.
Brought the stack up for real: '&' still bound the whole 'source && source && fault_manager' chain, so the gateway leg ran unsourced and exited 127. Semicolons scope each '&' to one command. Verified live: all three jobs up, two recordings seeded, specs' data visible in the browser.
bburda
force-pushed
the
feature/scripts-e2e
branch
from
August 22, 2026 17:09
ae6cf5d to
d39dc06
Compare
mfaferek93
approved these changes
Aug 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Adds the repository's first end-to-end coverage: Playwright driving a real browser against a gateway running in a container, plus the CI job that runs it.
Builds on #90 and is based on its branch, so the diff here is only the harness and the specs. It stays red until #90 merges, because the scenarios exercise the Scripts tab that pull request introduces.
What the harness sets up:
loadrather than for the network to fall idle, because the app holds a fault stream open and the network never falls idle.What the specs cover: seven scenarios against the live gateway, six against mocked responses for states a healthy gateway will not produce on demand, and the smoke test. The live ones assert gateway behaviour rather than only rendering: that a script receives its parameters on stdin, that a failing one surfaces its exit code and stderr with stdout discarded, that a stopped one is reported as stopped rather than failed, and that a script written in the browser in bash and in python runs and returns its output.
Two things worth knowing:
latestis overwritten on every push to the gateway's main branch, which would let unrelated changes turn this repository's CI red. Bumping it is meant to be a deliberate act.Issue
Type
Testing
16 Playwright tests pass against the containerised gateway. The live specs were also run twice in a row against the same container without restarting it, to prove that uploads are cleaned up and the suite is idempotent.
The CI job dumps the gateway's container logs when a run fails, before tearing the stack down, so a container that dies during startup can be diagnosed from the run alone. That failure mode is otherwise invisible on an ephemeral runner.
Checklist
npm run lint)npm run build)