diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d92807e..c4dd126 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,6 @@ name: CI on: pull_request: - branches: [main] push: branches: [main] @@ -39,6 +38,49 @@ jobs: - name: Build project run: npm run build + e2e: + runs-on: ubuntu-latest + timeout-minutes: 20 + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'npm' + + - name: Install dependencies + run: npm ci + + - name: Start gateway + run: docker compose -f e2e/docker-compose.yml up -d + + - name: Install Playwright browsers + run: npx playwright install --with-deps chromium + + - name: Run E2E tests + run: npm run test:e2e + + - name: Upload Playwright artifacts + if: failure() + uses: actions/upload-artifact@v4 + with: + name: playwright-report + path: | + playwright-report/ + test-results/ + + - name: Dump gateway logs on failure + if: failure() + run: docker compose -f e2e/docker-compose.yml logs + + - name: Stop gateway + if: always() + run: docker compose -f e2e/docker-compose.yml down -v + docker-build: runs-on: ubuntu-latest timeout-minutes: 15 diff --git a/.gitignore b/.gitignore index fd85165..d10094d 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,8 @@ dist-ssr # Serena .serena/ + +# Playwright +playwright-report/ +test-results/ +e2e/.auth/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 63b50e5..8f44930 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -87,6 +87,43 @@ Before opening or updating a Pull Request, you **must**: npm run dev ``` +> **Note:** `npm run typecheck` runs `tsc --noEmit` against the root `tsconfig.json`, which has no `files` of its own and therefore checks nothing. Type errors are actually caught by `npm run build` (`tsc -b`, which builds the referenced app, node and e2e project configs). Do not trust a green `typecheck` on its own; run `build` before opening a PR. + +### Running the End-to-End Suite Locally + +The Playwright suite in `e2e/` runs the real UI against a containerised gateway instead of mocks, so it needs Docker. + +1. Start the gateway: + + ```bash + docker compose -f e2e/docker-compose.yml up -d + ``` + +2. Run the suite: + + ```bash + npm run test:e2e + ``` + + Use `npm run test:e2e:ui` instead to step through the tests with the Playwright UI. + +3. Stop the gateway once you are done, dropping the uploads volume along with it: + + ```bash + docker compose -f e2e/docker-compose.yml down -v + ``` + +`e2e/scripts.spec.ts` uploads, runs and deletes scripts against the shared gateway container, mutating its state as it goes, so it and the other specs that touch the live gateway are pinned to a single Playwright worker (see `playwright.config.ts`). Do not attempt to parallelize these specs or run them against a gateway instance you care about keeping in a known state. + +If port 8080 or 5173 is already taken on your machine, override the gateway port and/or the dev server URL before starting the stack: + +```bash +E2E_GATEWAY_PORT=8081 docker compose -f e2e/docker-compose.yml up -d +E2E_GATEWAY_PORT=8081 npm run test:e2e +``` + +`E2E_GATEWAY_PORT` is the only variable you need for the gateway side: `e2e/global-setup.ts` derives the full gateway URL from it, and the gateway's CORS configuration allows any origin so an overridden dev server port is never rejected. Set `E2E_APP_URL` instead (e.g. `E2E_APP_URL=http://localhost:5174`) if the dev server port needs to change; `playwright.config.ts` derives the dev server's port from it. The gateway container stays bound to `127.0.0.1` regardless of the port chosen. + ### Pull Request Checklist Before submitting your PR, ensure: diff --git a/README.md b/README.md index b60bc51..ef7259a 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,9 @@ ros2_medkit_web_ui is a lightweight single-page application that connects to a S - **Entity Tree Sidebar** - Browse the hierarchical structure of SOVD entities with lazy-loading, with a readiness lamp on app and component nodes (a green disc for ready, an amber ring for not ready, a grey square for a readiness the UI has not established). The lamp is re-read while the branch is open, so it tracks an entity that stops or comes back - **Entity Detail Panel** - View raw JSON details of any selected entity - **Entity Lifecycle Status Control** - View readiness and request lifecycle transitions (start, restart, force-restart, shutdown, force-shutdown) for apps and components, degrading gracefully on an entity with no lifecycle provider, without taking the entities that have one with it. Actions are gated by the current status (a transition the current status does not allow is marked unavailable and rejected, and stays focusable so the tooltip explaining why reaches a screen reader), and every destructive transition (all but Start) asks for confirmation before dispatch. A transition is only reported as requested when the gateway accepts it; because acceptance is not completion, the readiness is dropped and re-established by the refresh rather than read back straight away +- **Scripts Tab** - List the scripts available on an entity, run one with optional parameters, watch its live status while it executes, see the output once it completes, stop or force-kill a running execution, upload a new script (from a file or written directly in the browser), and delete scripts you no longer need + +> **Note:** The Scripts tab only appears for entities whose gateway reports `capabilities.scripts` in `GET /`, and even then only for apps and components - areas and functions never show it regardless of the capability. The gateway sets this when either a script provider plugin is loaded or `scripts.scripts_dir` is configured; a plugin takes precedence over `scripts_dir`, and when one is loaded `scripts_dir` is ignored. This tool is designed for developers and integrators working with SOVD-compatible systems who need a quick way to explore and debug the entity structure. @@ -84,6 +87,13 @@ npm run test:ui # Run tests with coverage npm run test:coverage +# Run the end-to-end suite against a containerised gateway +docker compose -f e2e/docker-compose.yml up -d +npm run test:e2e + +# Run the end-to-end suite with the Playwright UI +npm run test:e2e:ui + # Format code npm run format @@ -114,7 +124,8 @@ npm run lint - **shadcn/ui** - UI components - **Zustand** - State management - **lucide-react** - Icons -- **Vitest** - Testing framework +- **Vitest** - Unit and component testing framework +- **Playwright** - End-to-end testing against a containerised gateway - **Prettier** - Code formatting - **Husky** - Git hooks diff --git a/e2e/dialog-helpers.ts b/e2e/dialog-helpers.ts new file mode 100644 index 0000000..794bd67 --- /dev/null +++ b/e2e/dialog-helpers.ts @@ -0,0 +1,44 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { expect, type Dialog, type Page } from '@playwright/test'; + +/** + * Clicks the (already-visible, uniquely-matched) Delete button for + * `scriptName` and accepts the native `confirm()` dialog that ScriptRow's + * handleDelete requires before it calls deleteScript. + * + * The listener is registered before the click and accepts inline as soon as + * the dialog opens, rather than after awaiting the click: `window.confirm` + * blocks the page's JS (and, with it, the click action itself) until the + * dialog is resolved, so anything that awaits the click before calling + * `dialog.accept()` would deadlock - the click can never settle first. + * + * Asserts the dialog actually appeared, with the expected message, instead + * of accepting whatever dialog (if any) shows up - a bare accept-everything + * handler would still pass the day someone removes the confirmation guard by + * accident. + */ +export async function clickDeleteAndConfirm(page: Page, scriptName: string): Promise { + let seenDialog: Dialog | undefined; + page.once('dialog', async (dialog) => { + seenDialog = dialog; + await dialog.accept(); + }); + + await page.getByRole('button', { name: 'Delete' }).click(); + + expect(seenDialog?.type()).toBe('confirm'); + expect(seenDialog?.message()).toBe(`Delete script "${scriptName}"? This cannot be undone.`); +} diff --git a/e2e/docker-compose.rosbag.yml b/e2e/docker-compose.rosbag.yml new file mode 100644 index 0000000..b3961ca --- /dev/null +++ b/e2e/docker-compose.rosbag.yml @@ -0,0 +1,83 @@ +# Stack for the rosbag-history specs: a gateway AND a fault manager, so a fault +# can actually own black-box recordings. docker-compose.yml next to this file +# runs a manifest-only gateway with no fault manager at all, which cannot +# produce a single bag; the two scenarios are kept apart rather than merged so +# neither has to carry the other's configuration. + +# Its own project: both compose files live in e2e/, so without this they share +# the default project name "e2e" and their `gateway` services replace each other +# instead of running side by side. +name: e2e-rosbag + +services: + gateway: + # Overridable because the recording-id contract these specs assert on + # (ros2_medkit#620) is newer than any published tag: point this at a + # locally built image to run them before that lands. Once it is + # published, pin a digest here the way docker-compose.yml does. + image: ${E2E_ROSBAG_GATEWAY_IMAGE:-ghcr.io/selfpatch/ros2_medkit-jazzy:latest} + ports: + # Loopback only, and on its own port so this stack can run alongside + # the scripts one without either stealing the other's. + - '127.0.0.1:${E2E_ROSBAG_GATEWAY_PORT:-8081}:8080' + volumes: + - ./gateway/rosbag-params.yaml:/e2e/params.yaml:ro + - ./gateway/seed_recordings.py:/e2e/seed_recordings.py:ro + - e2e-bags:/e2e-bags + # PID 1 reaps children and forwards signals; without it bash -lc keeps + # PID 1 for itself and `docker compose down` waits out the whole grace + # period before SIGKILLing a fault manager mid-write. + init: true + # Overriding the entrypoint skips /entrypoint.sh, which is what sources + # ROS and exports the RMW default - both have to be restored here. + environment: + RMW_IMPLEMENTATION: ${RMW_IMPLEMENTATION:-rmw_fastrtps_cpp} + entrypoint: ['/bin/bash', '-lc'] + # Fault manager, the seeder and the gateway in one container. Not three + # services sharing a network: the default DDS transport uses /dev/shm, + # which is per container, so the seeder's service calls would never + # complete even though discovery says the service is there. + # + # Sourced ONCE in the parent shell, then every process runs as a WATCHED + # background job: `&` binds looser than `&&`, so the earlier + # `source && source && fault_manager & gateway` form left the gateway in + # an unsourced shell ("ros2: command not found", exit 127). `wait -n` + # returns when the FIRST job dies, so a fault manager that cannot open + # its DB or a seeder that raises SystemExit takes the container down + # with its exit code instead of leaving a healthy-looking stack whose + # specs skip. The trap makes SIGTERM stop the children before the shell + # exits. + command: + - > + source /opt/ros/jazzy/setup.bash; + source /home/medkit/ws/install/setup.bash; + ros2 run ros2_medkit_fault_manager fault_manager_node + --ros-args --params-file /e2e/params.yaml & + FM=$!; + python3 /e2e/seed_recordings.py & + SEED=$!; + ros2 run ros2_medkit_gateway gateway_node + --ros-args --params-file /e2e/params.yaml & + GW=$!; + trap 'kill $FM $SEED $GW 2>/dev/null' TERM INT; + wait -n $FM $SEED $GW; + exit $? + depends_on: + init-bags: + condition: service_completed_successfully + # The gateway image runs as uid 999 and a fresh named volume is root-owned, + # so the fault manager could not write a bag into it. Same one-shot chown + # the scripts stack does for its upload volume. + init-bags: + image: ${E2E_ROSBAG_GATEWAY_IMAGE:-ghcr.io/selfpatch/ros2_medkit-jazzy:latest} + user: root + volumes: + - e2e-bags:/e2e-bags + entrypoint: ['chown', '-R', '999:999', '/e2e-bags'] + +volumes: + # Holds the bags AND faults.db, and it outlives `docker compose down`. + # Re-seed from a clean slate with `down -v` first: on a reused volume the + # fault is already CONFIRMED, the first confirm captures nothing, and the + # suite sees three recordings instead of two. + e2e-bags: diff --git a/e2e/docker-compose.yml b/e2e/docker-compose.yml new file mode 100644 index 0000000..650b2f7 --- /dev/null +++ b/e2e/docker-compose.yml @@ -0,0 +1,40 @@ +services: + # Docker creates a fresh named volume owned by root:root, but the gateway + # image runs as the unprivileged `medkit` user (uid 999) and cannot create + # script subdirectories under a root-owned mount. This one-shot service + # chowns the volume before the gateway starts; it reuses the pinned + # gateway image (which already has chown) instead of pulling another one. + init-uploads: + # ghcr.io/selfpatch/ros2_medkit-jazzy:sha-7939c94 + image: ghcr.io/selfpatch/ros2_medkit-jazzy@sha256:565db07e1e972b31684bf864fbaad7e8a70aacabf2ef0cd4510fdbd8e3281831 + user: root + volumes: + - e2e-uploads:/e2e-uploads + entrypoint: ['chown', '-R', '999:999', '/e2e-uploads'] + gateway: + # Pinned on purpose: :latest is overwritten on every push to the gateway + # main branch, which would let unrelated changes turn this repo CI red. + # Pinned by digest, not by the sha-7939c94 tag alone: tags on this + # registry are mutable and a re-run of the publishing workflow on the + # same commit would move one. ghcr.io/selfpatch/ros2_medkit-jazzy:sha-7939c94 + image: ghcr.io/selfpatch/ros2_medkit-jazzy@sha256:565db07e1e972b31684bf864fbaad7e8a70aacabf2ef0cd4510fdbd8e3281831 + ports: + # Bound to loopback only, on purpose: this gateway has uploads enabled + # and executes uploaded shell scripts without authentication. + # CONTRIBUTING has developers bring this stack up and leave it running, + # so publishing it on every interface would let anyone else on the same + # network or Wi-Fi execute arbitrary shell on this machine for as long + # as the container is up. Do not drop the `127.0.0.1:` prefix to + # "simplify" this - that reintroduces the exposure. + - '127.0.0.1:${E2E_GATEWAY_PORT:-8080}:8080' + volumes: + - ./gateway/params.yaml:/e2e/params.yaml:ro + - ./gateway/manifest.yaml:/e2e/manifest.yaml:ro + - ./gateway/scripts:/e2e-scripts:ro + - e2e-uploads:/e2e-uploads + command: ['--ros-args', '--params-file', '/e2e/params.yaml'] + depends_on: + init-uploads: + condition: service_completed_successfully +volumes: + e2e-uploads: diff --git a/e2e/fixtures/uploaded-script.sh b/e2e/fixtures/uploaded-script.sh new file mode 100755 index 0000000..3d4f653 --- /dev/null +++ b/e2e/fixtures/uploaded-script.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +set -eu +echo "uploaded script executed" diff --git a/e2e/gateway/manifest.yaml b/e2e/gateway/manifest.yaml new file mode 100644 index 0000000..4a26d4b --- /dev/null +++ b/e2e/gateway/manifest.yaml @@ -0,0 +1,36 @@ +manifest_version: '1.0' +components: + - id: 'ecu' + name: 'Test ECU' +apps: + - id: 'talker' + name: 'Talker' + is_located_on: 'ecu' +scripts: + - id: 'hello' + name: 'Hello' + description: 'Echoes the parameters it receives on stdin' + path: '/e2e-scripts/hello.sh' + format: 'bash' + timeout_sec: 30 + entity_filter: + - 'ecu' + - 'talker' + - id: 'failing' + name: 'Failing' + description: 'Exits with a non-zero code' + path: '/e2e-scripts/fail.sh' + format: 'bash' + timeout_sec: 30 + entity_filter: + - 'ecu' + - id: 'sleeper' + name: 'Sleeper' + description: 'Runs long enough to be stopped' + path: '/e2e-scripts/sleep.sh' + format: 'bash' + # Matches sleep.sh's own sleep duration - see the comment there for why + # this is kept short rather than generously long. + timeout_sec: 30 + entity_filter: + - 'ecu' diff --git a/e2e/gateway/params.yaml b/e2e/gateway/params.yaml new file mode 100644 index 0000000..9569153 --- /dev/null +++ b/e2e/gateway/params.yaml @@ -0,0 +1,23 @@ +/**: + ros__parameters: + server: + host: '0.0.0.0' + port: 8080 + cors: + # '*' rather than a hardcoded 'http://localhost:5173': playwright.config.ts + # and e2e/global-setup.ts both derive the dev server origin from + # E2E_APP_URL, so overriding that variable (e.g. to dodge a busy port) + # would otherwise leave the browser talking to an origin this gateway + # never allowed, failing every request with no + # Access-Control-Allow-Origin header and no mention of CORS anywhere + # in the symptom. This is a throwaway local/CI fixture with + # allow_credentials left at its default false, so a wildcard origin + # carries none of the risk it would in a real deployment. + allowed_origins: + - '*' + discovery: + mode: 'manifest_only' + manifest_path: '/e2e/manifest.yaml' + scripts: + scripts_dir: '/e2e-uploads' + allow_uploads: true diff --git a/e2e/gateway/rosbag-params.yaml b/e2e/gateway/rosbag-params.yaml new file mode 100644 index 0000000..2ac6c06 --- /dev/null +++ b/e2e/gateway/rosbag-params.yaml @@ -0,0 +1,36 @@ +# Gateway + fault manager for the rosbag-history scenario. +# +# Separate from params.yaml on purpose: that stack is manifest-only with script +# uploads and no fault manager at all, and this one needs the opposite - live +# ROS discovery so the seeded fault's reporting source resolves to an app, and a +# fault manager configured to keep a HISTORY of black-box recordings rather than +# overwriting on every re-confirmation. +/**: + ros__parameters: + server: + host: '0.0.0.0' + port: 8080 + cors: + # Same reasoning as params.yaml: the browser's origin is the dev + # server, not the gateway, and E2E_APP_URL is overridable. + allowed_origins: + - '*' + # Rosbag retention. 3 leaves headroom above the two occurrences the seed + # drives, so a failing spec means "a recording was lost", not "the cap + # trimmed one". + snapshots: + rosbag: + enabled: true + duration_sec: 2.0 + duration_after_sec: 0.5 + include_topics: ['/e2e/probe'] + format: 'mcap' + storage_path: '/e2e-bags' + max_bags_per_fault: 3 + # Acknowledging a fault must not delete the evidence it just + # produced - the scenario is confirm, acknowledge, confirm again. + auto_cleanup: false + lazy_start: false + confirmation_threshold: -1 + storage_type: 'sqlite' + database_path: '/e2e-bags/faults.db' diff --git a/e2e/gateway/scripts/fail.sh b/e2e/gateway/scripts/fail.sh new file mode 100644 index 0000000..22c0893 --- /dev/null +++ b/e2e/gateway/scripts/fail.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +echo "diagnostics failed: sensor unreachable" >&2 +exit 3 diff --git a/e2e/gateway/scripts/hello.sh b/e2e/gateway/scripts/hello.sh new file mode 100644 index 0000000..9e55a6a --- /dev/null +++ b/e2e/gateway/scripts/hello.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +set -eu +params="$(cat)" +echo "hello ${params}" diff --git a/e2e/gateway/scripts/sleep.sh b/e2e/gateway/scripts/sleep.sh new file mode 100644 index 0000000..19befce --- /dev/null +++ b/e2e/gateway/scripts/sleep.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# Kept short on purpose: the gateway's concurrency cap is 5 and it is global, +# not per script. A stopped-but-still-running execution (e.g. from a test run +# interrupted mid-scenario) holds its slot until this sleep exits naturally, +# and a running execution cannot be deleted - so a handful of interrupted runs +# inside a long sleep window would block every execution the suite tries to +# start afterwards, with no reset short of destroying the stack. 30 seconds is +# still ample for the "stop a running script" scenario, which stops it within +# a second or two of starting. +sleep 30 diff --git a/e2e/gateway/seed_recordings.py b/e2e/gateway/seed_recordings.py new file mode 100644 index 0000000..0ff7a2d --- /dev/null +++ b/e2e/gateway/seed_recordings.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +# Copyright 2026 mfaferek93 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Leave one fault holding two black-box recordings, for the browser to click on. + +Everything below the fault report is real: the fault manager runs its own +capture, records a topic that is genuinely being published, and writes two +separate bags to disk. Only the trigger is a service call rather than a sensor +detecting its own misconfiguration - the subject of these specs is the web UI, +and the demo nodes that detect faults on their own are not shipped in the +gateway image. + +The fault is confirmed, acknowledged, then confirmed again: that is the sequence +that used to leave a single recording behind, because the second one overwrote +the first (ros2_medkit#620). +""" + +import sys +import time + +import rclpy +from rclpy.node import Node +from rclpy.qos import HistoryPolicy, QoSProfile, ReliabilityPolicy +from ros2_medkit_msgs.msg import Fault +from ros2_medkit_msgs.srv import ClearFault, ReportFault +from std_msgs.msg import Float32 + +FAULT_CODE = 'E2E_FLAPPING_SENSOR' +# The node's own fully qualified name. The gateway attributes a fault to the app +# whose FQN matches its reporting source, so a source that belongs to no live +# node leaves the fault owned by nobody and invisible under any /apps/{id} - +# which is also why this node stays up afterwards instead of exiting. +NODE_NAME = 'e2e_rosbag_seeder' +SOURCE_ID = f'/{NODE_NAME}' +PROBE_TOPIC = '/e2e/probe' +# Must exceed the configured duration_sec so the ring buffer holds a full window +# before each confirmation; a bag flushed from an empty buffer has no content. +FILL_SECONDS = 3.0 + + +class Seeder(Node): + def __init__(self): + super().__init__(NODE_NAME) + qos = QoSProfile( + reliability=ReliabilityPolicy.BEST_EFFORT, + history=HistoryPolicy.KEEP_LAST, + depth=10, + ) + self.pub = self.create_publisher(Float32, PROBE_TOPIC, qos) + self.report = self.create_client(ReportFault, '/fault_manager/report_fault') + self.clear = self.create_client(ClearFault, '/fault_manager/clear_fault') + + def wait_for_services(self, timeout=90.0): + for client, name in ((self.report, 'report_fault'), (self.clear, 'clear_fault')): + if not client.wait_for_service(timeout_sec=timeout): + raise SystemExit(f'{name} service never appeared') + + def publish_for(self, seconds, rate_hz=20.0): + msg = Float32() + msg.data = 1.0 + deadline = time.time() + seconds + period = 1.0 / rate_hz + while time.time() < deadline: + self.pub.publish(msg) + rclpy.spin_once(self, timeout_sec=0.0) + time.sleep(period) + + def call(self, client, request, attempts=5): + # Retried rather than one-shot: wait_for_service returns as soon as the + # service is advertised, which under DDS is before the fault manager has + # finished coming up, so the very first call can time out on a server + # that is seconds away from being fine. + for _ in range(attempts): + future = client.call_async(request) + rclpy.spin_until_future_complete(self, future, timeout_sec=20.0) + result = future.result() + if result is not None: + return result + # A future that outlived its timeout must not stay in flight: the + # request is not idempotent, and a late completion next to the retry + # would hand the fault manager two EVENT_FAILED reports for one + # occurrence. + future.cancel() + time.sleep(2.0) + raise SystemExit('service call timed out after retries') + + def confirm(self): + request = ReportFault.Request() + request.fault_code = FAULT_CODE + request.event_type = ReportFault.Request.EVENT_FAILED + request.severity = Fault.SEVERITY_ERROR + request.description = 'Intermittent sensor dropout seen twice' + request.source_id = SOURCE_ID + response = self.call(self.report, request) + if not response.accepted: + # ReportFault's response carries no message field; accepted=False + # means the request itself was invalid. + raise SystemExit('ReportFault rejected the request as invalid') + return response + + def acknowledge(self): + request = ClearFault.Request() + request.fault_code = FAULT_CODE + response = self.call(self.clear, request) + # A silent "Fault not found" here would leave one bag on disk and the + # whole suite skipping, with only a DEBUG log line to say why. + if not response.success: + raise SystemExit(f'ClearFault failed: {response.message}') + return response + + +def main(): + rclpy.init() + node = Seeder() + node.wait_for_services() + # Let discovery settle before the first report; the gateway is coming up in + # the same window and a confirmation raced against it produces no bag. + time.sleep(5.0) + + # First occurrence. + node.publish_for(FILL_SECONDS) + node.confirm() + node.publish_for(FILL_SECONDS) # post-roll window, then finalize + node.acknowledge() + + # Second occurrence. Before #620 this one replaced the first recording + # outright, so the fault ended up with exactly one bag either way. + node.publish_for(FILL_SECONDS) + node.confirm() + node.publish_for(FILL_SECONDS) + + # Deliberately NOT acknowledged: a cleared fault drops out of the default + # CONFIRMED-only listing, so acknowledging this one too would leave the specs + # with two bags on disk and no fault on screen pointing at them. + print('SEEDED', flush=True) + + # Stay on the graph. The fault is attributed to this node, so letting it + # exit would take the owning app entity with it and the fault would stop + # being reachable under any /apps/{id}. + try: + rclpy.spin(node) + except KeyboardInterrupt: + pass + finally: + node.destroy_node() + rclpy.shutdown() + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/e2e/global-setup.ts b/e2e/global-setup.ts new file mode 100644 index 0000000..34a1a55 --- /dev/null +++ b/e2e/global-setup.ts @@ -0,0 +1,76 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { chromium } from '@playwright/test'; +import { mkdirSync } from 'node:fs'; + +// e2e/docker-compose.yml publishes the gateway container on E2E_GATEWAY_PORT, +// not E2E_GATEWAY_URL - deriving the URL's port from it here means overriding +// the one variable that actually changes (the port, e.g. because 8080 is +// already taken) is enough. Without this, setting only E2E_GATEWAY_PORT would +// leave global setup polling the old default port for the full two-minute +// deadline before failing. An explicit E2E_GATEWAY_URL still wins outright, +// for the rarer case where the host part needs to change too. +const GATEWAY_PORT = process.env.E2E_GATEWAY_PORT ?? '8080'; +const GATEWAY_URL = process.env.E2E_GATEWAY_URL ?? `http://localhost:${GATEWAY_PORT}/api/v1`; +const APP_URL = process.env.E2E_APP_URL ?? 'http://localhost:5173'; +const STORAGE_KEY = 'ros2_medkit_web_ui_server_url'; + +// Node's fetch has no default timeout, so a stalled TCP connect (as opposed +// to an outright refused one) would otherwise hang the await forever - the +// deadline below would never get re-checked and global setup would just sit +// there until Playwright's own timeout kills it, looking like a mysterious +// hang instead of "the gateway did not start". Each attempt gets its own +// short timeout so the loop always keeps making progress toward the deadline. +const ATTEMPT_TIMEOUT_MS = 5_000; + +async function waitForGateway(): Promise { + const deadline = Date.now() + 120_000; + let lastError = 'no attempt made'; + while (Date.now() < deadline) { + try { + const res = await fetch(`${GATEWAY_URL}/health`, { signal: AbortSignal.timeout(ATTEMPT_TIMEOUT_MS) }); + if (res.ok) return; + lastError = `HTTP ${res.status}`; + } catch (err) { + lastError = err instanceof Error ? err.message : String(err); + } + await new Promise((resolve) => setTimeout(resolve, 1000)); + } + throw new Error(`Gateway did not become healthy at ${GATEWAY_URL}: ${lastError}`); +} + +export default async function globalSetup(): Promise { + await waitForGateway(); + + const browser = await chromium.launch(); + const context = await browser.newContext(); + // The app auto-connects to the persisted URL on start, so seeding the store + // is enough and we never have to drive the connection dialog. + await context.addInitScript( + ([key, url]) => { + window.localStorage.setItem(key, JSON.stringify({ state: { serverUrl: url }, version: 0 })); + }, + [STORAGE_KEY, GATEWAY_URL] + ); + const page = await context.newPage(); + // goto() already waits for 'load'. Not 'networkidle': once connected, the + // app opens a long-lived SSE fault stream that never completes, so the + // network would never go idle and this would hang until the action timeout. + await page.goto(APP_URL); + + mkdirSync('e2e/.auth', { recursive: true }); + await context.storageState({ path: 'e2e/.auth/state.json' }); + await browser.close(); +} diff --git a/e2e/rosbag-recordings.spec.ts b/e2e/rosbag-recordings.spec.ts new file mode 100644 index 0000000..720968f --- /dev/null +++ b/e2e/rosbag-recordings.spec.ts @@ -0,0 +1,198 @@ +// Copyright 2026 mfaferek93 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * A fault that owns several black-box recordings, end to end in a browser. + * + * An intermittent fault leaves one recording per occurrence. Until + * ros2_medkit#620 the newest overwrote the previous one, so the occurrence an + * engineer actually wanted to look at was already gone by the time they opened + * the fault. These specs drive the real UI against a real gateway holding two + * real bags and assert the technician can reach both of them. + * + * Runs against e2e/docker-compose.rosbag.yml, a separate stack from the scripts + * one: it needs a fault manager, which that stack does not run. + */ + +import { readFileSync } from 'node:fs'; + +import { expect, test } from '@playwright/test'; + +const GATEWAY_PORT = process.env.E2E_ROSBAG_GATEWAY_PORT ?? '8081'; +const GATEWAY_URL = process.env.E2E_ROSBAG_GATEWAY_URL ?? `http://localhost:${GATEWAY_PORT}/api/v1`; +const STORAGE_KEY = 'ros2_medkit_web_ui_server_url'; +const FAULT_CODE = process.env.E2E_ROSBAG_FAULT_CODE ?? 'E2E_FLAPPING_SENSOR'; + +interface Descriptor { + id: string; + 'x-medkit'?: { fault_codes?: string[]; recording_id?: string }; +} + +/** fetch that answers null instead of throwing when nothing is listening. */ +async function safeFetch(url: string): Promise { + try { + return await fetch(url); + } catch { + return null; + } +} + +/** Recording ids the gateway attributes to the seeded fault, via its own API. */ +async function recordingsFromApi(appId: string): Promise { + // Network errors are swallowed here and in appHoldingTheFault so a stack that + // is not up leaves expectedRecordings empty and the specs SKIP with a named + // reason. Letting fetch throw out of beforeAll fails them instead, which says + // nothing about this repo and turns CI red on a missing fixture. + const response = await safeFetch(`${GATEWAY_URL}/apps/${appId}/bulk-data/rosbags`); + if (!response?.ok) return []; + const body = (await response.json()) as { items?: Descriptor[] }; + return (body.items ?? []) + .filter((item) => item['x-medkit']?.fault_codes?.includes(FAULT_CODE)) + .map((item) => item.id); +} + +/** The app the seeded fault is attributed to, whatever the gateway named it. */ +async function appHoldingTheFault(): Promise { + const response = await safeFetch(`${GATEWAY_URL}/apps`); + if (!response?.ok) return null; + const body = (await response.json()) as { items?: Array<{ id: string }> }; + for (const app of body.items ?? []) { + const faults = await safeFetch(`${GATEWAY_URL}/apps/${app.id}/faults`); + if (!faults?.ok) continue; + const listing = (await faults.json()) as { items?: Array<{ fault_code?: string }> }; + if ((listing.items ?? []).some((f) => f.fault_code === FAULT_CODE)) return app.id; + } + return null; +} + +let appId: string | null = null; +let expectedRecordings: string[] = []; + +test.beforeAll(async () => { + appId = await appHoldingTheFault(); + if (appId) expectedRecordings = await recordingsFromApi(appId); +}); + +// Skipped rather than failed when the stack is not up or predates the +// recording-id contract: a red suite over a missing fixture says nothing about +// this repo, and these specs are the first to need a gateway new enough to keep +// more than one bag per fault. +test.beforeEach(async ({ page }) => { + test.skip( + appId === null || expectedRecordings.length < 2, + `needs e2e/docker-compose.rosbag.yml up with ${FAULT_CODE} seeded and holding ` + + `at least two recordings (found ${expectedRecordings.length} on ${GATEWAY_URL})` + ); + // Point the app at the rosbag stack instead of the scripts one that global + // setup seeded, before any application code runs. The stored value is a + // zustand-persist envelope, not a bare URL - writing the raw string leaves + // the app unable to parse it and sitting on the connection dialog. + await page.addInitScript( + ([key, url]) => window.localStorage.setItem(key, JSON.stringify({ state: { serverUrl: url }, version: 0 })), + [STORAGE_KEY, GATEWAY_URL] as const + ); +}); + +async function openTheFault(page: import('@playwright/test').Page) { + await page.goto('/', { waitUntil: 'load' }); + await page.getByRole('button', { name: /Faults Dashboard/i }).click(); + await expect(page.getByText(FAULT_CODE).first()).toBeVisible(); + await page.getByText(FAULT_CODE).first().click(); +} + +/** The fault's rosbag download buttons, in the order the detail lists them. */ +function downloadButtons(page: import('@playwright/test').Page) { + return page.locator('button:has(svg.lucide-download)'); +} + +test('the fault detail shows every recording, not just the newest', async ({ page }) => { + await openTheFault(page); + + // One download button per recording. Before #620 the gateway could only ever + // report one, so this is the assertion the whole change exists for. + await expect(downloadButtons(page)).toHaveCount(expectedRecordings.length); + + // Each button names the recording it will fetch. Without that the icon + // buttons are indistinguishable - to a screen reader they were nameless, and + // sighted users saw N identical download icons with nothing to tell them + // apart. + const names = await downloadButtons(page).evaluateAll((els) => + els.map((el) => el.getAttribute('aria-label') ?? '') + ); + expect(new Set(names).size).toBe(expectedRecordings.length); + for (const recordingId of expectedRecordings) { + expect(names.some((name) => name.includes(recordingId))).toBe(true); + } +}); + +test('every recording downloads as its own bag', async ({ page }) => { + await openTheFault(page); + const buttons = downloadButtons(page); + await expect(buttons).toHaveCount(expectedRecordings.length); + + const filenames: string[] = []; + const payloads: Buffer[] = []; + for (let i = 0; i < expectedRecordings.length; i += 1) { + const [download] = await Promise.all([page.waitForEvent('download'), buttons.nth(i).click()]); + const name = download.suggestedFilename(); + filenames.push(name); + + // The gateway names the file and is the only party that knows the + // storage format. Saving under the descriptor's display label instead + // dropped the extension, landing a bag on disk that neither the OS nor + // `ros2 bag play` could open without a manual rename. + expect(name).toMatch(/\.(mcap|db3)$/); + + const path = await download.path(); + expect(path).toBeTruthy(); + payloads.push(readFileSync(path!)); + } + + // Distinct files, not the same bag served twice under different buttons. + expect(new Set(filenames).size).toBe(expectedRecordings.length); + + // And distinct BYTES. Names alone would still pass on a build that resolved + // both ids to one recording but labelled the responses differently; this is + // what proves each button fetched its own occurrence. + expect(new Set(payloads.map((b) => b.toString('base64'))).size).toBe(expectedRecordings.length); + for (const payload of payloads) { + expect(payload.length).toBeGreaterThan(0); + // Every bag the fixture records is mcap; the magic is the cheapest proof + // that what arrived is a bag and not an error page. + expect(payload.subarray(0, 5)).toEqual(Buffer.from([0x89, 0x4d, 0x43, 0x41, 0x50])); + } +}); + +test('re-expanding the fault asks the gateway again instead of replaying a cache', async ({ page }) => { + // The detail used to be fetched once per fault and cached forever, so a + // recording written after the first expand stayed invisible until the + // component remounted. This drives the collapse/re-expand path and pins + // that the second expand goes back to the gateway with a 2xx; seeding a + // THIRD recording mid-test would need a second seeder pass, so the + // count-grows half lives in the jsdom tests that stub the store. + await openTheFault(page); + await expect(downloadButtons(page)).toHaveCount(expectedRecordings.length); + + // Collapse. + await page.getByText(FAULT_CODE).first().click(); + + // Re-expand, armed BEFORE the click and only satisfied by a 2xx: an error + // response must not count as "refetched". + const refetch = page.waitForResponse( + (response) => response.url().includes(`/faults/${FAULT_CODE}`) && response.ok() + ); + await page.getByText(FAULT_CODE).first().click(); + await refetch; + await expect(downloadButtons(page)).toHaveCount(expectedRecordings.length); +}); diff --git a/e2e/scripts-errors.spec.ts b/e2e/scripts-errors.spec.ts new file mode 100644 index 0000000..af77375 --- /dev/null +++ b/e2e/scripts-errors.spec.ts @@ -0,0 +1,202 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { test, expect, type Page } from '@playwright/test'; +import { clickDeleteAndConfirm } from './dialog-helpers'; + +async function selectTestEcu(page: Page): Promise { + await page.goto('/'); + await page.getByText('Test ECU').click(); +} + +async function openScripts(page: Page): Promise { + await selectTestEcu(page); + await page.getByRole('button', { name: /scripts/i }).click(); +} + +test('hides the Scripts tab when the gateway does not report the capability', async ({ page }) => { + await page.route('**/api/v1/', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + api_base: '/api/v1', + endpoints: [], + name: 'ROS 2 Medkit Gateway', + version: '0.6.0', + capabilities: { + aggregation: false, + async_actions: true, + authentication: false, + bulk_data: true, + configurations: true, + cyclic_subscriptions: true, + data_access: true, + discovery: true, + faults: true, + locking: true, + logs: true, + operations: true, + scripts: false, + tls: false, + triggers: true, + updates: false, + vendor_extensions: false, + }, + }), + }); + }); + + await selectTestEcu(page); + await expect(page.getByRole('button', { name: /scripts/i })).toHaveCount(0); +}); + +test('shows the not-configured state when listing returns 501', async ({ page }) => { + await page.route('**/api/v1/components/*/scripts', async (route) => { + await route.fulfill({ + status: 501, + contentType: 'application/json', + body: JSON.stringify({ error_code: 'not-implemented', message: 'Scripts backend not configured' }), + }); + }); + + await openScripts(page); + await expect(page.getByText('Scripts are not configured on this gateway')).toBeVisible({ timeout: 30_000 }); +}); + +test('reports disabled uploads from the gateway message', async ({ page }) => { + await page.route('**/api/v1/components/*/scripts', async (route) => { + if (route.request().method() !== 'POST') { + await route.continue(); + return; + } + await route.fulfill({ + status: 400, + contentType: 'application/json', + body: JSON.stringify({ + error_code: 'invalid-request', + message: 'Script uploads are disabled on this gateway', + }), + }); + }); + + await openScripts(page); + await page.getByRole('button', { name: 'Upload' }).click(); + const dialog = page.getByRole('dialog'); + await dialog.getByLabel('File').setInputFiles({ + name: 'probe.sh', + mimeType: 'text/x-shellscript', + buffer: Buffer.from('#!/usr/bin/env bash\necho probe\n'), + }); + await dialog.getByRole('button', { name: 'Upload' }).click(); + await expect(dialog.getByRole('alert')).toHaveText('Script uploads are disabled on this gateway', { + timeout: 30_000, + }); +}); + +test('reports that a managed script cannot be deleted', async ({ page }) => { + // The live gateway would never let a managed script's Delete button + // render at all, so the only way to exercise the gateway's rejection + // message is to mock a script that claims not to be managed and have the + // delete call fail anyway - exactly what a stale client cache would see. + await page.route('**/api/v1/components/*/scripts', async (route) => { + if (route.request().method() !== 'GET') { + await route.continue(); + return; + } + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + items: [{ id: 'hello', name: 'Hello', description: 'Mocked managed script', managed: false }], + }), + }); + }); + await page.route('**/api/v1/components/*/scripts/*', async (route) => { + if (route.request().method() !== 'DELETE') { + await route.continue(); + return; + } + await route.fulfill({ + status: 409, + contentType: 'application/json', + body: JSON.stringify({ + error_code: 'x-medkit-managed-script', + message: 'Cannot delete managed script: hello', + }), + }); + }); + + await openScripts(page); + await page.getByRole('button', { name: 'Hello' }).click(); + await clickDeleteAndConfirm(page, 'Hello'); + await expect(page.getByText('Cannot delete managed script: hello')).toBeVisible({ timeout: 30_000 }); +}); + +test('reports the concurrency limit when starting an execution', async ({ page }) => { + await page.route('**/api/v1/components/*/scripts/*/executions', async (route) => { + if (route.request().method() !== 'POST') { + await route.continue(); + return; + } + await route.fulfill({ + status: 429, + contentType: 'application/json', + body: JSON.stringify({ + error_code: 'x-medkit-concurrency-limit', + message: 'Maximum concurrent executions reached (5)', + }), + }); + }); + + await openScripts(page); + await page.getByRole('button', { name: 'Hello' }).click(); + await page.getByRole('button', { name: 'Run' }).click(); + await expect(page.getByText('Maximum concurrent executions reached (5)')).toBeVisible({ timeout: 30_000 }); +}); + +test('marks an execution as no longer tracked when polling returns 404', async ({ page }) => { + await page.route('**/api/v1/components/*/scripts/*/executions', async (route) => { + if (route.request().method() !== 'POST') { + await route.continue(); + return; + } + await route.fulfill({ + status: 202, + contentType: 'application/json', + body: JSON.stringify({ id: 'exec_mocked_1', status: 'running', started_at: new Date().toISOString() }), + }); + }); + await page.route('**/api/v1/components/*/scripts/*/executions/*', async (route) => { + await route.fulfill({ + status: 404, + contentType: 'application/json', + body: JSON.stringify({ error_code: 'resource-not-found', message: 'Execution not found' }), + }); + }); + + await openScripts(page); + await page.getByRole('button', { name: 'Hello' }).click(); + await page.getByRole('button', { name: 'Run' }).click(); + const status = page.getByTestId('execution-status'); + await expect(status).toBeVisible({ timeout: 30_000 }); + // Scope to the card that owns this status badge - other Refresh/Remove + // buttons exist elsewhere on the page (the panel's list reload, other + // execution cards). Do not click Refresh: the assertion below must be + // satisfied by the store's own poll loop picking up the 404 on its next + // tick, not by the manual rescue action. + const card = page.locator('[data-slot="card"]', { has: status }).last(); + await expect(card.getByText('The gateway no longer tracks this execution')).toBeVisible({ timeout: 30_000 }); + await expect(card.getByRole('button', { name: 'Remove' })).toBeVisible({ timeout: 30_000 }); +}); diff --git a/e2e/scripts.spec.ts b/e2e/scripts.spec.ts new file mode 100644 index 0000000..5931ace --- /dev/null +++ b/e2e/scripts.spec.ts @@ -0,0 +1,279 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import path from 'node:path'; +import { test, expect, type Locator, type Page, type TestInfo } from '@playwright/test'; +import { clickDeleteAndConfirm } from './dialog-helpers'; + +async function openScripts(page: Page, entity: 'Test ECU' | 'Talker'): Promise { + await page.goto('/'); + if (entity === 'Talker') { + // Talker only appears in the tree once its parent component has been + // selected and its children have loaded, so it must be expanded first. + await page.getByText('Test ECU').click(); + await page.getByText('Talker').waitFor({ state: 'visible', timeout: 30_000 }); + } + await page.getByText(entity).click(); + await page.getByRole('button', { name: /scripts/i }).click(); +} + +/** + * A stable, unique-per-worker upload name. `testInfo` (rather than the + * module-scope `test.info()`, which throws outside a running test) is what + * lets the afterEach hook below compute the exact same name the upload test + * used, since both run in the same worker with the same repeat index. + */ +function uploadedNameFor(testInfo: TestInfo): string { + return `uploaded_${testInfo.workerIndex}_${testInfo.repeatEachIndex}`; +} + +function writtenNameFor(testInfo: TestInfo, language: 'bash' | 'python'): string { + return `written_${language}_${testInfo.workerIndex}_${testInfo.repeatEachIndex}`; +} + +test.afterEach(async ({ page }, testInfo) => { + // Uploads persist in the gateway's volume between runs, so any script left + // over from this test (including a previous, unfinished run of it) must be + // removed - otherwise the next run would find a duplicate row and a + // getByRole('button', { name: uploadedName }) lookup would no longer be unique. + // + // This is best-effort cleanup, not part of the test: if the page is + // already in a broken state because the test itself failed, a throw here + // must not replace that failure in the report, and one leftover failing + // to delete must not stop the others from being attempted. + try { + await openScripts(page, 'Test ECU'); + } catch (err) { + console.warn('afterEach cleanup: could not open the Scripts panel', err); + return; + } + + const names = [uploadedNameFor(testInfo), writtenNameFor(testInfo, 'bash'), writtenNameFor(testInfo, 'python')]; + for (const name of names) { + try { + // A substring, case-insensitive match on `name` can resolve to more + // than one row: workerIndex and repeatEachIndex are both 0 on an + // ordinary run, so uploadedNameFor/writtenNameFor produce the same + // name across separate runs, and the gateway happily accepts + // duplicate script names as separate entities. Loop on the + // locator's count instead of a single isVisible() check, deleting + // .first() each time, so every leftover is removed rather than + // only a uniquely-named one. + const row = page.getByRole('button', { name }); + let remaining = await row.count(); + while (remaining > 0) { + await row.first().click(); + await clickDeleteAndConfirm(page, name); + await expect(row).toHaveCount(remaining - 1, { timeout: 30_000 }); + remaining = await row.count(); + } + } catch (err) { + console.warn(`afterEach cleanup: failed to remove leftover script "${name}"`, err); + } + } +}); + +/** + * CodeMirror's editable region is a `contenteditable` div, not a form + * control, so it has to be driven with real key events rather than a + * Locator.fill() - select-all, delete, then type the new content, the same + * sequence a person at the keyboard would use to replace the starter + * template. Takes the resolved editable locator rather than looking it up + * itself, so callers can choose how they reach it (by test id or, in one + * scenario below, by role and accessible name). + */ +async function writeInEditor(editable: Locator, content: string): Promise { + await editable.click(); + await editable.press('ControlOrMeta+a'); + await editable.press('Delete'); + await editable.pressSequentially(content); +} + +test('lists manifest scripts and marks them managed', async ({ page }) => { + await openScripts(page, 'Test ECU'); + for (const name of ['Hello', 'Failing', 'Sleeper']) { + const row = page.getByRole('button', { name }); + await expect(row).toBeVisible({ timeout: 30_000 }); + await expect(row).toContainText('managed'); + } +}); + +test('runs a script and shows the parameters it received on stdin', async ({ page }) => { + await openScripts(page, 'Test ECU'); + await page.getByRole('button', { name: 'Hello' }).click(); + // Sending parameters is what makes the gateway open the stdin pipe at all; + // hello.sh echoes whatever JSON it reads back on stdout. + await page.getByPlaceholder('{}').fill('{"greeting":"e2e-hello"}'); + await page.getByRole('button', { name: 'Run' }).click(); + + const status = page.getByTestId('execution-status'); + await expect(status).toHaveText('completed', { timeout: 30_000 }); + await expect(status).toHaveAttribute('data-tone', 'ok'); + await expect(page.locator('pre')).toContainText('"greeting":"e2e-hello"'); +}); + +test('shows exit code and stderr for a failing script', async ({ page }) => { + await openScripts(page, 'Test ECU'); + await page.getByRole('button', { name: 'Failing' }).click(); + await page.getByRole('button', { name: 'Run' }).click(); + + const status = page.getByTestId('execution-status'); + await expect(status).toHaveText('failed', { timeout: 30_000 }); + await expect(status).toHaveAttribute('data-tone', 'error'); + // The gateway discards stdout on a non-zero exit: only the stderr message + // and the exit code are ever available to assert on. + await expect(page.getByText(/sensor unreachable/)).toBeVisible(); + await expect(page.getByText(/exit code 3/)).toBeVisible(); +}); + +test('stops a running script and reports it as stopped, not failed', async ({ page }) => { + await openScripts(page, 'Test ECU'); + await page.getByRole('button', { name: 'Sleeper' }).click(); + await page.getByRole('button', { name: 'Run' }).click(); + const status = page.getByTestId('execution-status'); + await expect(status).toHaveText('running', { timeout: 30_000 }); + await page.getByRole('button', { name: 'Stop' }).click(); + await expect(status).toHaveText('terminated', { timeout: 30_000 }); + // A successful stop must render as stopped, never as an error tone. + await expect(status).toHaveAttribute('data-tone', 'stopped'); +}); + +test('uploads, runs and deletes a script', async ({ page }, testInfo) => { + const uploadedName = uploadedNameFor(testInfo); + await openScripts(page, 'Test ECU'); + // exact: true - Playwright's role/name matching is a case-insensitive + // substring match by default, and script rows carry the script name as + // their accessible name. Leftover fixtures from this file are named + // `uploaded__`, which contains "upload", so a non-exact + // lookup for the toolbar button also resolves to every leftover row. + await page.getByRole('button', { name: 'Upload', exact: true }).click(); + const dialog = page.getByRole('dialog'); + await dialog.getByLabel('File').setInputFiles(path.join(import.meta.dirname, 'fixtures', 'uploaded-script.sh')); + await dialog.getByLabel('Name').fill(uploadedName); + await dialog.getByRole('button', { name: 'Upload' }).click(); + await expect(dialog).toBeHidden({ timeout: 30_000 }); + + const row = page.getByRole('button', { name: uploadedName }); + await expect(row).toBeVisible({ timeout: 30_000 }); + // An uploaded script is not managed, so it must expose the Delete control. + await expect(row).not.toContainText('managed'); + await row.click(); + + await page.getByRole('button', { name: 'Run' }).click(); + const status = page.getByTestId('execution-status'); + await expect(status).toHaveText('completed', { timeout: 30_000 }); + + await clickDeleteAndConfirm(page, uploadedName); + await expect(row).toBeHidden({ timeout: 30_000 }); +}); + +test('writes a bash script in the UI, runs it and shows its output', async ({ page }, testInfo) => { + const scriptName = writtenNameFor(testInfo, 'bash'); + await openScripts(page, 'Test ECU'); + await page.getByRole('button', { name: 'Upload', exact: true }).click(); + const dialog = page.getByRole('dialog'); + await dialog.getByRole('button', { name: 'Write script' }).click(); + // .bash, not .sh: the gateway only runs a script under bash when its name + // ends in .bash - .sh, like everything else, runs under sh - so this is + // the only extension that actually exercises the bash interpreter path. + await dialog.getByLabel('File name').fill(`${scriptName}.bash`); + // Reachable by role and accessible name against the real CodeMirror + // instance, not just the mocked editor the unit tests exercise - this is + // the only place that would catch ScriptEditor's aria-label regressing. + const editor = dialog.getByRole('textbox', { name: 'Script content' }); + await expect(editor).toBeVisible(); + // Reads stdin and prints a recognisable, greppable line - the same + // contract the starter template teaches, just without the placeholder text. + await writeInEditor(editor, ['read -r params', 'echo e2e-write-bash-ok $params'].join('\n')); + // Exact match: "File name" also contains the substring "Name", and + // Playwright's getByLabel matches substrings by default. + await dialog.getByLabel('Name', { exact: true }).fill(scriptName); + await dialog.getByRole('button', { name: 'Upload' }).click(); + await expect(dialog).toBeHidden({ timeout: 30_000 }); + + const row = page.getByRole('button', { name: scriptName }); + await expect(row).toBeVisible({ timeout: 30_000 }); + await row.click(); + + await page.getByPlaceholder('{}').fill('{"greeting":"e2e-write-bash"}'); + await page.getByRole('button', { name: 'Run' }).click(); + + const status = page.getByTestId('execution-status'); + await expect(status).toHaveText('completed', { timeout: 30_000 }); + await expect(page.locator('pre')).toContainText('e2e-write-bash-ok'); + await expect(page.locator('pre')).toContainText('"greeting":"e2e-write-bash"'); + + await clickDeleteAndConfirm(page, scriptName); + await expect(row).toBeHidden({ timeout: 30_000 }); +}); + +test('writes a python script in the UI, runs it and shows its output', async ({ page }, testInfo) => { + const scriptName = writtenNameFor(testInfo, 'python'); + await openScripts(page, 'Test ECU'); + await page.getByRole('button', { name: 'Upload', exact: true }).click(); + const dialog = page.getByRole('dialog'); + await dialog.getByRole('button', { name: 'Write script' }).click(); + await dialog.getByLabel('File name').fill(`${scriptName}.py`); + // The only coverage in this repository for the python3 interpreter path: + // the gateway only picks python3 when the uploaded file's name ends in .py. + await writeInEditor( + dialog.locator('[data-testid="script-editor"] .cm-content'), + [ + 'import sys', + 'import json', + '', + 'raw = sys.stdin.read()', + 'params = json.loads(raw) if raw.strip() else {}', + 'print("e2e-write-python-ok:", json.dumps(params, separators=(",", ":")))', + ].join('\n') + ); + // Exact match: "File name" also contains the substring "Name", and + // Playwright's getByLabel matches substrings by default. + await dialog.getByLabel('Name', { exact: true }).fill(scriptName); + await dialog.getByRole('button', { name: 'Upload' }).click(); + await expect(dialog).toBeHidden({ timeout: 30_000 }); + + const row = page.getByRole('button', { name: scriptName }); + await expect(row).toBeVisible({ timeout: 30_000 }); + await row.click(); + + await page.getByPlaceholder('{}').fill('{"greeting":"e2e-write-python"}'); + await page.getByRole('button', { name: 'Run' }).click(); + + const status = page.getByTestId('execution-status'); + await expect(status).toHaveText('completed', { timeout: 30_000 }); + await expect(page.locator('pre')).toContainText('e2e-write-python-ok'); + await expect(page.locator('pre')).toContainText('"greeting":"e2e-write-python"'); + + await clickDeleteAndConfirm(page, scriptName); + await expect(row).toBeHidden({ timeout: 30_000 }); +}); + +test('removes a finished execution record', async ({ page }) => { + await openScripts(page, 'Test ECU'); + await page.getByRole('button', { name: 'Hello' }).click(); + await page.getByRole('button', { name: 'Run' }).click(); + + const status = page.getByTestId('execution-status'); + await expect(status).toHaveText('completed', { timeout: 30_000 }); + await page.getByRole('button', { name: 'Remove' }).click(); + await expect(status).toHaveCount(0, { timeout: 30_000 }); +}); + +test('shows the Scripts tab and the shared script on an app as well', async ({ page }) => { + await openScripts(page, 'Talker'); + const row = page.getByRole('button', { name: 'Hello' }); + await expect(row).toBeVisible({ timeout: 30_000 }); + await expect(row).toContainText('managed'); +}); diff --git a/e2e/smoke.spec.ts b/e2e/smoke.spec.ts new file mode 100644 index 0000000..cfb0b3a --- /dev/null +++ b/e2e/smoke.spec.ts @@ -0,0 +1,20 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { test, expect } from '@playwright/test'; + +test('connects to the gateway and shows the manifest component', async ({ page }) => { + await page.goto('/'); + await expect(page.getByText('Test ECU')).toBeVisible({ timeout: 30_000 }); +}); diff --git a/e2e/tsconfig.json b/e2e/tsconfig.json new file mode 100644 index 0000000..2f2944c --- /dev/null +++ b/e2e/tsconfig.json @@ -0,0 +1,8 @@ +{ + "extends": "../tsconfig.node.json", + "compilerOptions": { + "lib": ["ES2023", "DOM"], + "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.e2e.tsbuildinfo" + }, + "include": ["../playwright.config.ts", "**/*.ts"] +} diff --git a/playwright.config.ts b/playwright.config.ts new file mode 100644 index 0000000..b2925f6 --- /dev/null +++ b/playwright.config.ts @@ -0,0 +1,58 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { defineConfig } from '@playwright/test'; + +// Read from the same E2E_APP_URL that e2e/global-setup.ts honours, and derive +// the dev server's port from it, so config and setup cannot end up visiting +// two different addresses if only one of them is overridden. +const BASE_URL = process.env.E2E_APP_URL ?? 'http://localhost:5173'; +const APP_PORT = new URL(BASE_URL).port || '5173'; + +export default defineConfig({ + testDir: './e2e', + globalSetup: './e2e/global-setup.ts', + // Fails the run in CI if a `test.only` was committed, instead of silently + // running just that one test and reporting a spuriously green suite. + forbidOnly: !!process.env.CI, + timeout: 60_000, + expect: { timeout: 30_000 }, + reporter: [['html', { open: 'never' }], ['list']], + // Both projects below hit the same single containerised gateway for + // everything they do not explicitly mock (entity discovery, health, + // faults). Left to Playwright's default per-project parallelism, the + // 'mocked' project's own worker runs concurrently with 'scripts-serial' + // and the resulting burst of simultaneous full-app connections can push + // the gateway's response time past the client's health-check timeout, + // aborting an unrelated connect() and failing an unrelated test. A single + // global worker keeps every test's gateway traffic strictly sequential. + workers: 1, + use: { baseURL: BASE_URL, storageState: 'e2e/.auth/state.json', trace: 'retain-on-failure' }, + webServer: { + command: `npm run dev -- --port ${APP_PORT} --strictPort`, + url: BASE_URL, + reuseExistingServer: !process.env.CI, + timeout: 120_000, + }, + projects: [ + // These specs mutate shared gateway state (uploads, executions, the global + // concurrency limit), so they must not run in parallel with each other. + { name: 'scripts-serial', testMatch: /(scripts|smoke)\.spec\.ts/, fullyParallel: false, workers: 1 }, + { name: 'mocked', testMatch: /.*-errors\.spec\.ts/ }, + // Its own stack (e2e/docker-compose.rosbag.yml) on its own port, because + // it needs a fault manager the scripts gateway does not run. Serial for + // the same reason as scripts-serial: one shared gateway. + { name: 'rosbag-serial', testMatch: /rosbag-.*\.spec\.ts/, fullyParallel: false, workers: 1 }, + ], +}); diff --git a/src/components/FaultsDashboard.test.tsx b/src/components/FaultsDashboard.test.tsx new file mode 100644 index 0000000..0b6ba25 --- /dev/null +++ b/src/components/FaultsDashboard.test.tsx @@ -0,0 +1,116 @@ +// Copyright 2026 mfaferek93 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * Two entities reporting the SAME fault code, which is legal - a code is only + * unique within one entity. Everything here failed while the dashboard's caches + * were keyed by code alone: expanding one row opened both, and clearing the + * second row cleared the first entity's fault. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { FaultsDashboard } from './FaultsDashboard'; +import type { Fault } from '@/lib/types'; + +const mockFetchFaults = vi.fn(); +const mockClearFault = vi.fn(); +const mockGetFaultWithEnvironmentData = vi.fn(); + +let storeState: Record = {}; + +vi.mock('@/lib/store', () => ({ + useAppStore: Object.assign( + vi.fn((selector?: (s: Record) => unknown) => (selector ? selector(storeState) : storeState)), + { getState: () => storeState } + ), +})); + +function fault(entityId: string): Fault { + return { + code: 'LIDAR_RANGE_INVALID', + message: `range invalid on ${entityId}`, + severity: 'error', + status: 'active', + timestamp: '2026-08-20T10:00:00Z', + entity_id: entityId, + entity_type: 'app', + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + mockGetFaultWithEnvironmentData.mockResolvedValue({ environment_data: { snapshots: [] } }); + storeState = { + faults: [fault('app_a'), fault('app_b')], + isLoadingFaults: false, + faultsError: null, + fetchFaults: mockFetchFaults, + clearFault: mockClearFault, + getFaultWithEnvironmentData: mockGetFaultWithEnvironmentData, + isConnected: true, + }; +}); + +describe('FaultsDashboard with colliding fault codes', () => { + it('expands only the clicked row and fetches only its entity', async () => { + render(); + // Flat list view: the grouped default splits by entity, which would + // hide the collision the caches must survive. + fireEvent.click(screen.getByRole('switch', { name: /group by entity/i })); + + const rows = screen.getAllByText('LIDAR_RANGE_INVALID'); + expect(rows).toHaveLength(2); + fireEvent.click(rows[0]!); + + await waitFor(() => expect(mockGetFaultWithEnvironmentData).toHaveBeenCalledTimes(1)); + expect(mockGetFaultWithEnvironmentData).toHaveBeenCalledWith('apps', 'app_a', 'LIDAR_RANGE_INVALID'); + // The sibling with the same code stays collapsed: exactly one row shows + // the expanded empty-environment marker. + await waitFor(() => expect(screen.getAllByText(/no environment data available/i)).toHaveLength(1)); + }); + + it("clears the clicked row's entity, not the first entity with that code", async () => { + render(); + fireEvent.click(screen.getByRole('switch', { name: /group by entity/i })); + + const clearButtons = screen.getAllByTitle('Clear fault'); + expect(clearButtons).toHaveLength(2); + fireEvent.click(clearButtons[1]!); + + await waitFor(() => expect(mockClearFault).toHaveBeenCalledTimes(1)); + expect(mockClearFault).toHaveBeenCalledWith('apps', 'app_b', 'LIDAR_RANGE_INVALID'); + }); + + it('keeps evidence on screen when a refetch answers 404 (null)', async () => { + mockGetFaultWithEnvironmentData + .mockResolvedValueOnce({ + environment_data: { snapshots: [{ type: 'freeze_frame', name: 'ff', data: { level: 82 } }] }, + }) + .mockResolvedValueOnce(null); + render(); + fireEvent.click(screen.getByRole('switch', { name: /group by entity/i })); + + const row = screen.getAllByText('LIDAR_RANGE_INVALID')[0]!; + fireEvent.click(row); + await waitFor(() => expect(screen.getByText(/snapshots \(1\)/i)).toBeInTheDocument()); + + // Collapse, re-expand: the second fetch resolves null (the store's + // documented 404 shape). The cached evidence must survive it. + fireEvent.click(row); + fireEvent.click(row); + await waitFor(() => expect(mockGetFaultWithEnvironmentData).toHaveBeenCalledTimes(2)); + expect(screen.getByText(/snapshots \(1\)/i)).toBeInTheDocument(); + }); +}); diff --git a/src/components/FaultsDashboard.tsx b/src/components/FaultsDashboard.tsx index 45b5d15..998650b 100644 --- a/src/components/FaultsDashboard.tsx +++ b/src/components/FaultsDashboard.tsx @@ -30,7 +30,7 @@ import { Skeleton } from '@/components/ui/skeleton'; import { SnapshotCard } from './SnapshotCard'; import { useAppStore } from '@/lib/store'; import type { Fault, FaultSeverity, FaultStatus, FaultResponse } from '@/lib/types'; -import { mapFaultEntityTypeToResourceType } from '@/lib/utils'; +import { faultKey, mapFaultEntityTypeToResourceType } from '@/lib/utils'; /** * Default polling interval in milliseconds @@ -129,7 +129,7 @@ function FaultRow({ isLoadingDetails, }: { fault: Fault; - onClear: (code: string) => void; + onClear: (fault: Fault) => void; isClearing: boolean; isExpanded: boolean; onToggle: () => void; @@ -189,7 +189,7 @@ function FaultRow({ size="sm" onClick={(e) => { e.stopPropagation(); - onClear(fault.code); + onClear(fault); }} disabled={isClearing} className="shrink-0" @@ -298,7 +298,7 @@ function FaultGroup({ entityId: string; entityType: string; faults: Fault[]; - onClear: (code: string) => void; + onClear: (fault: Fault) => void; clearingCodes: Set; expandedFaults: Set; onToggleFault: (fault: Fault) => void; @@ -338,14 +338,14 @@ function FaultGroup({ {faults.map((fault) => ( onToggleFault(fault)} - environmentData={faultDetails.get(fault.code)?.environment_data} - isLoadingDetails={loadingDetails.has(fault.code)} + environmentData={faultDetails.get(faultKey(fault))?.environment_data} + isLoadingDetails={loadingDetails.has(faultKey(fault))} /> ))} @@ -457,64 +457,75 @@ export function FaultsDashboard() { // Clear fault handler const handleClear = useCallback( - async (code: string) => { - setClearingCodes((prev) => new Set([...prev, code])); + // The whole Fault, not its code: two entities can report the same code, + // and resolving through `faults.find` cleared the FIRST entity's fault + // whichever row was clicked. + async (fault: Fault) => { + const key = faultKey(fault); + setClearingCodes((prev) => new Set([...prev, key])); try { - // Find the fault to get entity info - const fault = faults.find((f) => f.code === code); - if (fault) { - // Map the fault's entity_type to the correct resource type for the API - const entityGroup = mapFaultEntityTypeToResourceType(fault.entity_type); - // Use store's clearFault which has proper error handling with toasts - await clearFault(entityGroup, fault.entity_id, code); - } + // Map the fault's entity_type to the correct resource type for the API + const entityGroup = mapFaultEntityTypeToResourceType(fault.entity_type); + // Use store's clearFault which has proper error handling with toasts + await clearFault(entityGroup, fault.entity_id, fault.code); // Reload faults after clearing await fetchFaults(); } finally { setClearingCodes((prev) => { const next = new Set(prev); - next.delete(code); + next.delete(key); return next; }); } }, - [faults, fetchFaults, clearFault] + [fetchFaults, clearFault] ); // Toggle fault expansion and lazy-load environment data const handleToggleFault = useCallback( async (fault: Fault) => { - const faultCode = fault.code; - const newExpanded = new Set(expandedFaults); - - if (newExpanded.has(faultCode)) { - newExpanded.delete(faultCode); - } else { - newExpanded.add(faultCode); - - // Fetch details if not cached - if (!faultDetails.has(faultCode)) { - setLoadingDetails((prev) => new Set([...prev, faultCode])); - try { - const entityGroup = mapFaultEntityTypeToResourceType(fault.entity_type); - const details = await getFaultWithEnvironmentData(entityGroup, fault.entity_id, faultCode); - setFaultDetails((prev) => new Map(prev).set(faultCode, details as FaultResponse)); - } catch (err) { - console.error('Failed to fetch fault details:', err); - } finally { - setLoadingDetails((prev) => { - const next = new Set(prev); - next.delete(faultCode); - return next; - }); - } + const key = faultKey(fault); + // Functional update, BEFORE the await: the row opens on the click + // (the previous entry stays rendered while the refetch runs), and a + // second click during the request collapses instead of reading a + // stale closed-over set and firing another GET. + let opened = false; + setExpandedFaults((prev) => { + const next = new Set(prev); + if (next.has(key)) { + next.delete(key); + } else { + next.add(key); + opened = true; + } + return next; + }); + if (!opened) return; + + // Always refetch: a fault gains recordings while the page is open, + // and a cache filled once on first expand would keep serving the + // shorter list. + setLoadingDetails((prev) => new Set([...prev, key])); + try { + const entityGroup = mapFaultEntityTypeToResourceType(fault.entity_type); + const details = await getFaultWithEnvironmentData(entityGroup, fault.entity_id, fault.code); + // A 404 resolves to null rather than throwing; overwriting the + // cache with it would blank evidence that was already on screen. + if (details) { + setFaultDetails((prev) => new Map(prev).set(key, details as FaultResponse)); } + } catch (err) { + console.error('Failed to fetch fault details:', err); + } finally { + setLoadingDetails((prev) => { + const next = new Set(prev); + next.delete(key); + return next; + }); } - - setExpandedFaults(newExpanded); }, - [getFaultWithEnvironmentData, expandedFaults, faultDetails] + [getFaultWithEnvironmentData] ); // Filter faults @@ -819,14 +830,14 @@ export function FaultsDashboard() { {filteredFaults.map((fault) => ( handleToggleFault(fault)} - environmentData={faultDetails.get(fault.code)?.environment_data} - isLoadingDetails={loadingDetails.has(fault.code)} + environmentData={faultDetails.get(faultKey(fault))?.environment_data} + isLoadingDetails={loadingDetails.has(faultKey(fault))} /> ))} diff --git a/src/components/FaultsPanel.tsx b/src/components/FaultsPanel.tsx index 85aa1ae..96d48cf 100644 --- a/src/components/FaultsPanel.tsx +++ b/src/components/FaultsPanel.tsx @@ -19,7 +19,7 @@ import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/component import { SnapshotCard } from './SnapshotCard'; import { useAppStore, type AppState } from '@/lib/store'; import type { Fault, FaultSeverity, FaultStatus, FaultResponse, SovdResourceEntityType } from '@/lib/types'; -import { mapFaultEntityTypeToResourceType } from '@/lib/utils'; +import { faultKey, mapFaultEntityTypeToResourceType } from '@/lib/utils'; interface FaultsPanelProps { entityId: string; @@ -301,43 +301,59 @@ export function FaultsPanel({ entityId, entityType = 'components' }: FaultsPanel }, [loadFaults]); const handleToggleFault = useCallback( - async (faultCode: string) => { - const newExpanded = new Set(expandedFaults); - - if (newExpanded.has(faultCode)) { - newExpanded.delete(faultCode); - } else { - newExpanded.add(faultCode); + // The whole Fault, not its code: a component's list spans every app it + // hosts, so two apps under one component can report the same code and a + // code-keyed lookup cannot tell them apart. + async (fault: Fault) => { + const key = faultKey(fault); + // Functional update, BEFORE the await: the row opens on the click + // (the previous entry stays rendered while the refetch runs), and a + // second click during the request collapses instead of reading a + // stale closed-over set and firing another GET. + let opened = false; + setExpandedFaults((prev) => { + const next = new Set(prev); + if (next.has(key)) { + next.delete(key); + } else { + next.add(key); + opened = true; + } + return next; + }); + if (!opened) return; - // Fetch details if not cached - if (!faultDetails.has(faultCode)) { - setLoadingDetails((prev) => new Set([...prev, faultCode])); - try { - // Use the fault's own entity info (app-level) for correct bulk_data_uri. - // Components have a synthetic FQN that doesn't match fault reporting sources, - // so fetching via /components/{id}/faults/{code} produces an unusable bulk_data_uri. - const fault = faults.find((f) => f.code === faultCode); - const detailEntityType: SovdResourceEntityType = fault?.entity_type - ? mapFaultEntityTypeToResourceType(fault.entity_type) - : entityType; - const detailEntityId = fault?.entity_id || entityId; - const details = await getFaultWithEnvironmentData(detailEntityType, detailEntityId, faultCode); - setFaultDetails((prev) => new Map(prev).set(faultCode, details as FaultResponse)); - } catch (err) { - console.error('Failed to fetch fault details:', err); - } finally { - setLoadingDetails((prev) => { - const next = new Set(prev); - next.delete(faultCode); - return next; - }); - } + // Always refetch, even when a detail is already cached. A fault can + // gain recordings while the page is open - it re-confirms, the black + // box is written, the snapshot list grows - and a cache that is + // filled once on first expand would keep serving the older list with + // no way to refresh short of remounting. + setLoadingDetails((prev) => new Set([...prev, key])); + try { + // Use the fault's own entity info (app-level) for correct bulk_data_uri. + // Components have a synthetic FQN that doesn't match fault reporting sources, + // so fetching via /components/{id}/faults/{code} produces an unusable bulk_data_uri. + const detailEntityType: SovdResourceEntityType = fault.entity_type + ? mapFaultEntityTypeToResourceType(fault.entity_type) + : entityType; + const detailEntityId = fault.entity_id || entityId; + const details = await getFaultWithEnvironmentData(detailEntityType, detailEntityId, fault.code); + // A 404 resolves to null rather than throwing; overwriting the + // cache with it would blank evidence that was already on screen. + if (details) { + setFaultDetails((prev) => new Map(prev).set(key, details as FaultResponse)); } + } catch (err) { + console.error('Failed to fetch fault details:', err); + } finally { + setLoadingDetails((prev) => { + const next = new Set(prev); + next.delete(key); + return next; + }); } - - setExpandedFaults(newExpanded); }, - [getFaultWithEnvironmentData, entityType, entityId, expandedFaults, faultDetails, faults] + [getFaultWithEnvironmentData, entityType, entityId] ); const handleClear = useCallback( @@ -419,10 +435,10 @@ export function FaultsPanel({ entityId, entityType = 'components' }: FaultsPanel fault={fault} onClear={handleClear} isClearing={clearingCodes.has(fault.code)} - isExpanded={expandedFaults.has(fault.code)} - onToggle={() => handleToggleFault(fault.code)} - environmentData={faultDetails.get(fault.code)?.environment_data} - isLoadingDetails={loadingDetails.has(fault.code)} + isExpanded={expandedFaults.has(faultKey(fault))} + onToggle={() => handleToggleFault(fault)} + environmentData={faultDetails.get(faultKey(fault))?.environment_data} + isLoadingDetails={loadingDetails.has(faultKey(fault))} /> ))} diff --git a/src/components/RosbagDownloadButton.tsx b/src/components/RosbagDownloadButton.tsx index 7fded98..b36d9ac 100644 --- a/src/components/RosbagDownloadButton.tsx +++ b/src/components/RosbagDownloadButton.tsx @@ -84,6 +84,11 @@ export function RosbagDownloadButton({ snapshot, variant = 'outline', size = 'sm } const label = snapshot.size_bytes ? `Download (${formatBytes(snapshot.size_bytes)})` : 'Download rosbag'; + // A fault can hold several recordings, so the icon variant renders as N + // buttons with no text at all - identical to a screen reader and to keyboard + // navigation. Name each one after the recording it downloads so they can be + // told apart; snapshot.name carries the recording id. + const accessibleName = snapshot.name ? `${label} - ${snapshot.name}` : label; return ( @@ -93,6 +98,7 @@ export function RosbagDownloadButton({ snapshot, variant = 'outline', size = 'sm size={size} onClick={handleDownload} disabled={isDownloading} + aria-label={accessibleName} className={error ? 'border-destructive' : ''} > {isDownloading ? : } diff --git a/src/lib/store-download.test.ts b/src/lib/store-download.test.ts new file mode 100644 index 0000000..0bea934 --- /dev/null +++ b/src/lib/store-download.test.ts @@ -0,0 +1,58 @@ +// Copyright 2026 mfaferek93 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { describe, expect, it } from 'vitest'; + +import { filenameFromContentDisposition } from './store'; + +describe('filenameFromContentDisposition', () => { + it('takes the quoted filename the gateway sends for a rosbag', () => { + // The extension is the point: only the server knows the storage format, + // and a bag saved without `.mcap` is one the OS and `ros2 bag play` + // cannot open until the user renames it by hand. + expect(filenameFromContentDisposition('attachment; filename="fault_MOTOR_OVERHEAT_1738664999000.mcap"')).toBe( + 'fault_MOTOR_OVERHEAT_1738664999000.mcap' + ); + }); + + it('accepts an unquoted filename', () => { + expect(filenameFromContentDisposition('attachment; filename=bag.db3')).toBe('bag.db3'); + }); + + it('prefers the RFC 5987 form, which is the one that survives non-ASCII', () => { + expect( + filenameFromContentDisposition('attachment; filename="fallback.mcap"; filename*=UTF-8\'\'r%C3%B6ntgen.mcap') + ).toBe('röntgen.mcap'); + }); + + it('falls back to the plain form when the extended one is malformed', () => { + // A truncated percent-escape throws inside decodeURIComponent; the plain + // filename is still perfectly usable and must not be lost with it. + expect(filenameFromContentDisposition('attachment; filename="good.mcap"; filename*=UTF-8\'\'bad%ZZ')).toBe( + 'good.mcap' + ); + }); + + it('returns null rather than a filename when the header says nothing', () => { + // Null is what lets the caller fall back to the recording id. Returning + // an empty string here would save the file as "" instead. + expect(filenameFromContentDisposition(null)).toBeNull(); + expect(filenameFromContentDisposition('attachment')).toBeNull(); + expect(filenameFromContentDisposition('attachment; filename=""')).toBeNull(); + }); + + it('is case-insensitive about the parameter name', () => { + expect(filenameFromContentDisposition('attachment; FileName="x.mcap"')).toBe('x.mcap'); + }); +}); diff --git a/src/lib/store.ts b/src/lib/store.ts index c6bbc5e..9931b72 100644 --- a/src/lib/store.ts +++ b/src/lib/store.ts @@ -51,7 +51,6 @@ import { putEntityDataItem, deleteEntityConfiguration, deleteEntityConfigurations, - getEntityBulkData, getEntityLogs, getEntityLogsConfiguration, putEntityLogsConfiguration, @@ -979,6 +978,55 @@ async function fetchEntityFromApi( } } +/** + * Filename the server chose, out of a `Content-Disposition` header. + * + * Handles the plain `filename=` form (quoted or not) and RFC 8187's + * `filename*=''` - any charset and any language + * tag, not just `UTF-8''`. Returns null when the header is absent or names + * nothing, so the caller can fall back rather than saving a file called "null". + */ + +/** RFC 8187 ext-value payload to a string, or null when undecodable. UTF-8 is + * the wire norm; any single-byte charset (the RFC's other registered case is + * ISO-8859-1) decodes byte-per-byte, which maps 1:1 onto code points. */ +function decodeExtValue(charset: string, encoded: string): string | null { + try { + if (/^utf-?8$/i.test(charset)) return decodeURIComponent(encoded); + return encoded.replace(/%([0-9a-f]{2})/gi, (_, hex: string) => String.fromCharCode(parseInt(hex, 16))); + } catch { + return null; + } +} + +export function filenameFromContentDisposition(header: string | null): string | null { + if (!header) return null; + + const extended = /(?:^|;)\s*filename\*\s*=\s*([^';]+)'[^';]*'([^;\s]*)/.exec(header); + // Validated as a WHOLE before decoding: a partial match would silently + // truncate at the first bad escape ("bad%ZZ" -> "bad") instead of letting a + // well-formed plain `filename=` further down win. Decoded before trimming, + // so a value that is all `%20` is rejected as empty. + if (extended && /^(?:%[0-9a-fA-F]{2}|[^%])*$/.test(extended[2]!)) { + const name = decodeExtValue(extended[1]!, extended[2]!)?.trim(); + if (name) return name; + } + + // Quoted form next: semicolons and spaces stay inside the quotes, and a + // backslash escapes the next character. Anchored on a parameter boundary so + // `xfilename=` cannot match, and `filename*=` cannot reach here because a + // `*` sits between the name and the `=`. + const quoted = /(?:^|;)\s*filename\s*=\s*"((?:\\.|[^"\\])*)"/i.exec(header); + if (quoted) { + const name = quoted[1]!.replace(/\\(.)/g, '$1').trim(); + return name ? name : null; + } + + const plain = /(?:^|;)\s*filename\s*=\s*([^;]+)/i.exec(header); + const name = plain?.[1]?.trim(); + return name ? name : null; +} + export const useAppStore = create()( persist( (set, get) => ({ @@ -2759,13 +2807,6 @@ export const useAppStore = create()( const { client, serverUrl } = get(); if (!client || !serverUrl) return null; - // Fetch file list to get filename - const { data } = await getEntityBulkData(client, entityType, entityId, category); - if (!data) return null; - const items = (data as unknown as { items?: Array<{ id: string; name?: string }> })?.items || []; - const fileDesc = items.find((item) => item.id === fileId); - const filename = fileDesc?.name || fileId; - // Download binary via fetch (openapi-fetch doesn't support blob responses) const baseUrl = normalizeBaseUrl(serverUrl); const downloadUrl = `${baseUrl}/${entityType}/${encodeURIComponent(entityId)}/bulk-data/${encodeURIComponent(category)}/${encodeURIComponent(fileId)}`; @@ -2776,6 +2817,14 @@ export const useAppStore = create()( clearTimeout(timer); if (!response.ok) return null; const blob = await response.blob(); + // The server names the file, and it is the only party that knows + // the storage format, so only it can put the right extension on + // the end. The descriptor's `name` is a human label (" + // recording "), not a filename: saving under it lands + // a rosbag on disk with no `.mcap`/`.db3` at all, which neither + // the OS nor `ros2 bag play` can make sense of. + const filename = + filenameFromContentDisposition(response.headers.get('content-disposition')) ?? fileId; return { blob, filename }; } catch { clearTimeout(timer); diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 7bf3517..1e75558 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -50,3 +50,16 @@ export function formatDuration(seconds: number): string { const secs = Math.round(seconds % 60); return `${mins}m ${secs}s`; } + +/** + * Key for per-fault UI caches (expand state, detail cache, in-flight sets). + * + * A fault code is only unique within one entity - two apps can both report + * `LIDAR_RANGE_INVALID` - so any cache keyed by code alone ties their rows + * together: expanding one opens both, and a clear resolves to whichever + * entity's fault happens to come first. Including the entity makes the key as + * specific as the request that filled the cache. + */ +export function faultKey(fault: { code: string; entity_type?: string; entity_id?: string }): string { + return `${fault.entity_type ?? ''}/${fault.entity_id ?? ''}/${fault.code}`; +} diff --git a/tsconfig.json b/tsconfig.json index 04f1a75..070fb00 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,10 @@ { "files": [], - "references": [{ "path": "./tsconfig.app.json" }, { "path": "./tsconfig.node.json" }], + "references": [ + { "path": "./tsconfig.app.json" }, + { "path": "./tsconfig.node.json" }, + { "path": "./e2e/tsconfig.json" } + ], "compilerOptions": { "baseUrl": ".", "paths": {