diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c4dd126..13abaed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: e2e: runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 30 steps: - name: Checkout repository @@ -58,9 +58,19 @@ jobs: - name: Start gateway run: docker compose -f e2e/docker-compose.yml up -d + # A second stack, on its own port: the rosbag specs need a fault + # manager to own the recordings, which the scripts stack does not run. + - name: Start rosbag gateway + run: docker compose -f e2e/docker-compose.rosbag.yml up -d + - name: Install Playwright browsers run: npx playwright install --with-deps chromium + # The rosbag specs skip themselves when the fixture is not there, so + # without this the job stays green whether or not they ever ran. + - name: Wait for the rosbag fixture + run: ./e2e/wait-for-rosbag-fixture.sh + - name: Run E2E tests run: npm run test:e2e @@ -77,10 +87,21 @@ jobs: if: failure() run: docker compose -f e2e/docker-compose.yml logs + - name: Dump rosbag gateway logs on failure + if: failure() + run: docker compose -f e2e/docker-compose.rosbag.yml logs + - name: Stop gateway if: always() run: docker compose -f e2e/docker-compose.yml down -v + # -v matters here: the volume holds faults.db as well as the bags, so + # a reused one starts with the fault already confirmed and seeds a + # different number of recordings than a clean run. + - name: Stop rosbag gateway + if: always() + run: docker compose -f e2e/docker-compose.rosbag.yml down -v + docker-build: runs-on: ubuntu-latest timeout-minutes: 15 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8f44930..f97302a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,10 +93,20 @@ Before opening or updating a Pull Request, you **must**: The Playwright suite in `e2e/` runs the real UI against a containerised gateway instead of mocks, so it needs Docker. -1. Start the gateway: +1. Start both gateways. The scripts stack serves most of the suite; the rosbag + specs need the second one, which also runs a fault manager so a fault can own + black-box recordings. Skip it and those three specs skip themselves: ```bash docker compose -f e2e/docker-compose.yml up -d + docker compose -f e2e/docker-compose.rosbag.yml up -d + ``` + + The rosbag stack seeds its fixture in the background. Wait for it before + running the suite, the way CI does: + + ```bash + ./e2e/wait-for-rosbag-fixture.sh ``` 2. Run the suite: @@ -107,10 +117,14 @@ The Playwright suite in `e2e/` runs the real UI against a containerised gateway 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: +3. Stop both gateways once you are done, dropping their volumes along with them. + The rosbag volume holds `faults.db` as well as the bags, so a reused one + starts with the fault already confirmed and seeds a different number of + recordings than a clean run: ```bash docker compose -f e2e/docker-compose.yml down -v + docker compose -f e2e/docker-compose.rosbag.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. @@ -118,11 +132,22 @@ The Playwright suite in `e2e/` runs the real UI against a containerised gateway 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=8090 docker compose -f e2e/docker-compose.yml up -d +E2E_GATEWAY_PORT=8090 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. +Do not reach for 8081 here: that is the rosbag stack's own default, so the two +gateways would fight over it, and with the rosbag stack down the rosbag specs +would point at the scripts gateway, find no seeded fault and skip for a reason +that has nothing to do with the code. + +`E2E_GATEWAY_PORT` is the only variable the scripts stack needs: `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. + +The rosbag stack has its own set, all optional: `E2E_ROSBAG_GATEWAY_PORT` +(default 8081), `E2E_ROSBAG_GATEWAY_URL`, `E2E_ROSBAG_FAULT_CODE`, and +`E2E_ROSBAG_FIXTURE_TIMEOUT` for the wait script. `E2E_ROSBAG_GATEWAY_IMAGE` +points the stack at a locally built gateway instead of the pinned release, which +is how you run these specs against a gateway change before it ships. ### Pull Request Checklist diff --git a/README.md b/README.md index ef7259a..a132aa1 100644 --- a/README.md +++ b/README.md @@ -87,8 +87,11 @@ npm run test:ui # Run tests with coverage npm run test:coverage -# Run the end-to-end suite against a containerised gateway +# Run the end-to-end suite against containerised gateways +# (the second stack carries the rosbag specs; see CONTRIBUTING.md) docker compose -f e2e/docker-compose.yml up -d +docker compose -f e2e/docker-compose.rosbag.yml up -d +./e2e/wait-for-rosbag-fixture.sh npm run test:e2e # Run the end-to-end suite with the Playwright UI diff --git a/e2e/docker-compose.rosbag.yml b/e2e/docker-compose.rosbag.yml index b3961ca..58ee51c 100644 --- a/e2e/docker-compose.rosbag.yml +++ b/e2e/docker-compose.rosbag.yml @@ -11,11 +11,11 @@ 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} + # The same gateway 0.7.0 digest docker-compose.yml pins, for the same + # reason: registry tags are mutable, so a tag would let an unrelated + # gateway change decide what these specs run against. Overridable so a + # locally built gateway can be dropped in ahead of a release. + image: ${E2E_ROSBAG_GATEWAY_IMAGE:-ghcr.io/selfpatch/ros2_medkit-jazzy@sha256:c706a4c16a4f4f2963690f776e93a70ac4afebbc52a0774a4edb984369a0dbd8} ports: # Loopback only, and on its own port so this stack can run alongside # the scripts one without either stealing the other's. @@ -47,21 +47,25 @@ services: # 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. + # + # The `$$` are not a typo: Compose interpolates `$VAR` in this string + # before the container ever sees it, so a single `$` would hand bash an + # empty word and leave `kill` with no operands. 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=$!; + FM=$$!; python3 /e2e/seed_recordings.py & - SEED=$!; + 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 $? + 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 @@ -69,7 +73,7 @@ services: # 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} + image: ${E2E_ROSBAG_GATEWAY_IMAGE:-ghcr.io/selfpatch/ros2_medkit-jazzy@sha256:c706a4c16a4f4f2963690f776e93a70ac4afebbc52a0774a4edb984369a0dbd8} user: root volumes: - e2e-bags:/e2e-bags @@ -78,6 +82,8 @@ services: 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. + # fault is already CONFIRMED and the first confirm captures nothing, so the + # recording count drifts from what a fresh seed produces. The specs assert + # against the count they read back, so this is hygiene rather than a + # correctness gate, but it keeps a run comparable with CI. e2e-bags: diff --git a/e2e/docker-compose.yml b/e2e/docker-compose.yml index 650b2f7..13e6992 100644 --- a/e2e/docker-compose.yml +++ b/e2e/docker-compose.yml @@ -5,8 +5,8 @@ services: # 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 + # ghcr.io/selfpatch/ros2_medkit-jazzy, gateway 0.7.0 + image: ghcr.io/selfpatch/ros2_medkit-jazzy@sha256:c706a4c16a4f4f2963690f776e93a70ac4afebbc52a0774a4edb984369a0dbd8 user: root volumes: - e2e-uploads:/e2e-uploads @@ -14,10 +14,14 @@ services: 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 + # Pinned by digest rather than by a tag: tags on this registry are + # mutable and a re-run of the publishing workflow on the same commit + # would move one. This digest is gateway 0.7.0, the release the app is + # built against: package.json takes the generated client at ^0.7.0, so + # the suite runs against the schema that client was generated from. It + # is the manifest-list digest, not a per-arch one, so an arm64 host gets + # a native image instead of running the gateway under emulation. + image: ghcr.io/selfpatch/ros2_medkit-jazzy@sha256:c706a4c16a4f4f2963690f776e93a70ac4afebbc52a0774a4edb984369a0dbd8 ports: # Bound to loopback only, on purpose: this gateway has uploads enabled # and executes uploaded shell scripts without authentication. diff --git a/e2e/wait-for-rosbag-fixture.sh b/e2e/wait-for-rosbag-fixture.sh new file mode 100755 index 0000000..6b72c3e --- /dev/null +++ b/e2e/wait-for-rosbag-fixture.sh @@ -0,0 +1,138 @@ +#!/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. +# +# Blocks until the rosbag stack holds the fixture rosbag-recordings.spec.ts +# needs, and exits non-zero if it never does. +# +# The specs skip themselves when the fixture is missing, which is right for a +# developer who did not bring the second stack up, but in CI a skip is +# indistinguishable from a pass. This script is what makes the difference +# visible. +# +# It judges the SAME app the specs will judge: appHoldingTheFault() takes the +# first app whose fault list carries the code and never looks further, so this +# stops there too. Scanning on to a later app that happens to hold enough bags +# would let this pass while the specs skip on the first one. +# +# Seeding is not instant. The fault has to confirm, be acknowledged and confirm +# again before a second bag exists, so the wait is a real one, not a formality. + +set -euo pipefail + +GATEWAY_PORT="${E2E_ROSBAG_GATEWAY_PORT:-8081}" +GATEWAY_URL="${E2E_ROSBAG_GATEWAY_URL:-http://localhost:${GATEWAY_PORT}/api/v1}" +FAULT_CODE="${E2E_ROSBAG_FAULT_CODE:-E2E_FLAPPING_SENSOR}" +TIMEOUT_SEC="${E2E_ROSBAG_FIXTURE_TIMEOUT:-240}" + +# Two, because the whole point of the specs is a fault that kept more than the +# newest recording. One bag means the gateway is up but the fixture is half +# seeded, which would skip the specs just as surely as no bag at all. +REQUIRED_RECORDINGS=2 + +deadline=$((SECONDS + TIMEOUT_SEC)) +last_state="nothing answered on ${GATEWAY_URL}" + +# Anything that is not a plain count means the gateway is not serving what the +# specs will parse. They call .json() on these same bodies and throw on a bad +# one, so a malformed answer is a fixture that is not ready yet - never an app +# to step over on the way to a healthier one. +is_count() { + [[ $1 =~ ^[0-9]+$ ]] +} + +expired() { + ((SECONDS >= deadline)) +} + +while ! expired; do + if ! apps=$(curl -fsS --max-time 5 "${GATEWAY_URL}/apps" 2>/dev/null); then + sleep 2 + continue + fi + + if ! app_ids=$(jq -r '.items[]?.id' <<<"${apps}" 2>/dev/null); then + last_state="${GATEWAY_URL} answered /apps with something jq could not read" + sleep 2 + continue + fi + + # The likeliest failure is a gateway that answers perfectly while the seeder + # never confirms the fault, so the no-fault case gets its own message rather + # than falling back to the "nothing answered" one. + saw_fault=0 + unusable=0 + + # Read line by line: an id is one line, and word splitting would break an id + # containing whitespace and glob-expand one containing * or ?. + while IFS= read -r app; do + [[ -n ${app} ]] || continue + if expired; then + unusable=1 + last_state="${GATEWAY_URL} was still being scanned when the deadline passed" + break + fi + + if ! faults=$(curl -fsS --max-time 5 "${GATEWAY_URL}/apps/${app}/faults" 2>/dev/null); then + unusable=1 + last_state="${GATEWAY_URL} did not serve faults for ${app}" + break + fi + + hits=$(jq --arg fc "${FAULT_CODE}" '[.items[]? | select(.fault_code == $fc)] | length' <<<"${faults}" 2>/dev/null) || hits='' + if ! is_count "${hits}"; then + unusable=1 + last_state="${GATEWAY_URL} answered faults for ${app} with something the specs cannot parse" + break + fi + ((hits > 0)) || continue + saw_fault=1 + + if ! bags=$(curl -fsS --max-time 5 "${GATEWAY_URL}/apps/${app}/bulk-data/rosbags" 2>/dev/null); then + unusable=1 + last_state="${GATEWAY_URL} did not serve rosbags for ${app}" + break + fi + + count=$(jq --arg fc "${FAULT_CODE}" \ + '[.items[]? | select((."x-medkit".fault_codes // []) | index($fc))] | length' <<<"${bags}" 2>/dev/null) || count='' + if ! is_count "${count}"; then + unusable=1 + last_state="${GATEWAY_URL} answered rosbags for ${app} with something the specs cannot parse" + break + fi + + # Checked again here, not only on entry: the two requests above can each + # take up to five seconds, so a scan that began in time can finish out of + # it, and a wait that reports success past its own deadline is not one. + if ((count >= REQUIRED_RECORDINGS)) && ! expired; then + echo "${app} holds ${count} recordings for ${FAULT_CODE}" + exit 0 + fi + + last_state="${app} holds ${count} recording(s) for ${FAULT_CODE}, need ${REQUIRED_RECORDINGS}" + # The specs stop at the first app whose faults carry the code, so this + # stops there too. Scanning on to a healthier app would let this pass + # while they skip on this one. + break + done <<<"${app_ids}" + + ((saw_fault || unusable)) || last_state="${GATEWAY_URL} answered, but no app reports ${FAULT_CODE} yet" + + sleep 2 +done + +echo "Timed out after ${TIMEOUT_SEC}s waiting for the rosbag fixture: ${last_state}" >&2 +exit 1