From 35ff2f91a96292ed6b30705900eaf800b86a646a Mon Sep 17 00:00:00 2001 From: puxiaokang <1726541+joaner@users.noreply.github.com> Date: Thu, 9 Jul 2026 16:22:36 +0800 Subject: [PATCH] ci: stop installing the mcap CLI in CI (local-only debugging tool) The `mcap` CLI is meant for local fixture regeneration/debugging, not as a CI dependency. gen-test-mcap-filtered.mjs already self-skips (exit 0, no output file) when the binary isn't on PATH, and the one Playwright spec that depends on test_multi_filtered.mcap already skips itself when the fixture is absent, so removing the install step just means that one derived fixture/test no longer runs in CI; every other fixture and test is unaffected. --- .github/workflows/ci.yml | 18 +++++------------- docs/DEVELOPMENT.md | 2 +- scripts/gen-e2e-fixtures.mjs | 11 +++++++---- scripts/gen-test-mcap-filtered.mjs | 10 +++++----- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13e16b6..e02b5fb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,8 +22,6 @@ jobs: e2e: runs-on: ubuntu-latest - env: - MCAP_CLI_VERSION: 'v0.2.0' steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 @@ -32,18 +30,12 @@ jobs: cache: 'npm' - run: npm ci - run: npx playwright install chromium --with-deps - - name: Install mcap CLI - # Used by scripts/gen-test-mcap-filtered.mjs to derive a fixture via - # the real `mcap filter` command (not @mcap/core). That script - # self-skips if this step is ever removed, so it's not load-bearing - # for the rest of CI, but keeping it here exercises the multi-source - # merge feature's "mcap filter"-derived fixture on every run. - run: | - curl -sSL -o /usr/local/bin/mcap \ - "https://github.com/foxglove/mcap/releases/download/releases/mcap-cli/${MCAP_CLI_VERSION}/mcap-linux-amd64" - chmod +x /usr/local/bin/mcap - mcap --version - name: Generate E2E fixtures + # The `mcap` CLI is a local-only debugging tool (not installed in + # CI): scripts/gen-test-mcap-filtered.mjs detects its absence and + # self-skips (no test_multi_filtered.mcap), and the Playwright spec + # that depends on that fixture skips itself in turn. Everything else + # still runs. run: npm run gen:e2e:fixtures - run: npm run test:e2e env: diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 8b45e70..c2df2d6 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -40,7 +40,7 @@ Vitest layout round-trip tests import JSON directly from `test-fixtures/layouts/ Two fixtures depend on a tool that isn't an npm package. Their generator scripts detect absence and skip (exit 0, no output file) rather than failing the whole `gen:e2e:fixtures` run; Playwright specs that need one of these files check for it and `test.skip()` themselves when it's missing. -- **`mcap` CLI** (Rust rewrite, `mcap --version` ≥ 0.1) — needed only to (re)generate `test_multi_filtered.mcap` via `mcap filter`. Install with `brew install mcap` or download a binary from . CI installs it explicitly (see `.github/workflows/ci.yml`), so this fixture and its tests always run there. +- **`mcap` CLI** (Rust rewrite, `mcap --version` ≥ 0.1) — a local-only debugging tool, needed only to (re)generate `test_multi_filtered.mcap` via `mcap filter`. Install with `brew install mcap` or download a binary from . **CI does not install it**: `test_multi_filtered.mcap` and the one Playwright spec that depends on it simply skip there; everything else runs normally. - **`rosbags`** Python package — needed only to *regenerate* the committed `test-fixtures/media/minimal-multi.bag` source (e.g. after changing its schema in `scripts/gen-test-bag.py`); `npm run gen:e2e:fixtures` just copies that committed file into `public/examples/test_multi.bag`, so CI and most contributors never need to install it. If you do: `pip install rosbags`. For sample deep links (`?url=sample://…`), set `VITE_SAMPLE_DATASETS_MANIFEST_URL` in `.env` to a reachable JSON manifest (see `src/services/sampleDatasets.ts`). diff --git a/scripts/gen-e2e-fixtures.mjs b/scripts/gen-e2e-fixtures.mjs index 0a61347..909382e 100644 --- a/scripts/gen-e2e-fixtures.mjs +++ b/scripts/gen-e2e-fixtures.mjs @@ -33,13 +33,16 @@ runNode('gen-test-mcap-compressed-depth.mjs'); runPython('gen-test-hdf5.py'); runNode('gen-test-bvh.mjs'); -// Multi-source merge fixtures. gen-test-mcap-filtered.mjs and gen-test-bag.py -// each self-skip (exit 0) when their optional external tool (`mcap` CLI / -// `rosbags` package) isn't installed, so this pipeline still succeeds for -// contributors without them; CI installs both (see .github/workflows/ci.yml). +// Multi-source merge fixtures. runNode('gen-test-mcap-multi-base.mjs'); runNode('gen-test-mcap-multi-incremental.mjs'); +// Uses the local-only `mcap` CLI; self-skips (exit 0, no output file) when +// it isn't on PATH, which is always the case in CI by design (see +// .github/workflows/ci.yml) — the Playwright spec that depends on this +// fixture skips itself in turn. runNode('gen-test-mcap-filtered.mjs'); +// Copies the committed test-fixtures/media/minimal-multi.bag; only needs +// the `rosbags` Python package to *regenerate* that committed source. runPython('gen-test-bag.py'); console.log('[gen-e2e-fixtures] all fixtures ready'); diff --git a/scripts/gen-test-mcap-filtered.mjs b/scripts/gen-test-mcap-filtered.mjs index 5d424b8..ad4988c 100644 --- a/scripts/gen-test-mcap-filtered.mjs +++ b/scripts/gen-test-mcap-filtered.mjs @@ -5,11 +5,11 @@ * multi-source-merge test plan's requirement to exercise `mcap filter` for * fixture generation. * - * Requires gen-test-mcap-multi-base.mjs to have run first. Skips gracefully - * (exit 0, no output file) when the `mcap` CLI isn't on PATH, so contributors - * without it installed still get every other fixture; tests that depend on - * this file self-skip when it's absent. CI installs the CLI so it always - * runs there (see .github/workflows/ci.yml). + * The `mcap` CLI is a local-only debugging tool and is intentionally *not* + * installed in CI. Requires gen-test-mcap-multi-base.mjs to have run first. + * Skips gracefully (exit 0, no output file) when the `mcap` CLI isn't on + * PATH, so contributors (and CI) without it installed still get every other + * fixture; tests that depend on this file self-skip when it's absent. */ import { spawnSync } from 'node:child_process'; import fs from 'node:fs';