diff --git a/.github/workflows/explorer-e2e.yml b/.github/workflows/explorer-e2e.yml new file mode 100644 index 0000000..c2288fa --- /dev/null +++ b/.github/workflows/explorer-e2e.yml @@ -0,0 +1,163 @@ +name: Explorer e2e smoke gate + +# Browser-level regression gate for explorer.html (#249 PR 1 — the safety +# net that must exist BEFORE any refactor of explorer.qmd lands). +# +# What it does: renders explorer.qmd with Quarto, serves the rendered +# docs/ with the repo's range-capable static server, and runs the +# Playwright smoke set (tests/playwright/explorer-smoke.spec.js) against +# it in headless Chromium. +# +# What it does NOT do: assert on data correctness. The explorer streams +# large parquet files from data.isamples.org; the smoke set only asserts +# structural liveness (page boots, Cesium canvas, facet sidebar, search +# box) so a slow or flaky data load cannot fail the gate. The deeper +# data-dependent specs in tests/playwright/ can be run manually via +# workflow_dispatch with a spec filter. + +on: + pull_request: + paths: + - "explorer.qmd" + - "_quarto.yml" + - "styles.css" + - "assets/**" + - "workers/**" + - "tests/playwright/**" + - "playwright.config.js" + - "package.json" + - "dev_server.py" + - ".github/workflows/explorer-e2e.yml" + push: + branches: [main] + paths: + - "explorer.qmd" + - "_quarto.yml" + - "styles.css" + - "assets/**" + - "workers/**" + - "tests/playwright/**" + - "playwright.config.js" + - "package.json" + - "dev_server.py" + - ".github/workflows/explorer-e2e.yml" + workflow_dispatch: + inputs: + spec_filter: + description: >- + Playwright test file filter. Default runs only the smoke set; + pass e.g. "explorer-map-overlay" or "" (empty for all specs) + to run the data-dependent suite manually. + required: false + default: "explorer-smoke" + +concurrency: + group: explorer-e2e-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +env: + # Pinned: the version this gate was developed and verified against. + # The deploy workflow installs latest; pinning here keeps the gate's + # failures attributable to the PR, not a Quarto release. + QUARTO_VERSION: "1.6.42" + +jobs: + smoke: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + + - name: Cache Quarto installer + uses: actions/cache@v4 + with: + path: ~/quarto.deb + key: quarto-deb-${{ env.QUARTO_VERSION }} + + - name: Install Quarto + run: | + if [ ! -f ~/quarto.deb ]; then + curl -sSL -o ~/quarto.deb \ + "https://github.com/quarto-dev/quarto-cli/releases/download/v${QUARTO_VERSION}/quarto-${QUARTO_VERSION}-linux-amd64.deb" + fi + sudo dpkg -i ~/quarto.deb + quarto --version + + - uses: actions/setup-node@v4 + with: + node-version: 22 + + # package-lock.json is gitignored in this repo, so `npm ci` and + # setup-node's built-in npm cache are unavailable; cache ~/.npm + # keyed on package.json instead. + - name: Cache npm + uses: actions/cache@v4 + with: + path: ~/.npm + key: npm-${{ runner.os }}-${{ hashFiles('package.json') }} + + - name: Install node deps + run: npm install + + - name: Resolve Playwright version + id: pw + run: echo "version=$(node -e 'console.log(require("@playwright/test/package.json").version)')" >> "$GITHUB_OUTPUT" + + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ steps.pw.outputs.version }} + + - name: Install Chromium + run: npx playwright install --with-deps chromium + + # Single-page render: explorer.qmd is OJS/HTML only, so this needs + # neither the Python deps nor generate_vocab_docs.sh that the full + # site render (quarto-pages.yml) requires. ~10s, produces + # docs/explorer.html + site_libs + assets. + - name: Render explorer page + run: quarto render explorer.qmd + + # dev_server.py is the repo's range-capable (206) static server — + # same serving semantics the explorer verify loop uses locally. + - name: Serve rendered site + run: | + python3 dev_server.py --dir docs --port 5860 > devserver.log 2>&1 & + for i in $(seq 1 30); do + curl -sf http://localhost:5860/explorer.html > /dev/null && break + sleep 1 + done + curl -sf -o /dev/null http://localhost:5860/explorer.html + + # PR/push runs are pinned to the smoke set. Only workflow_dispatch + # honors spec_filter, and it is passed as ONE quoted argument so the + # input can never smuggle extra Playwright flags (e.g. + # --pass-with-no-tests, which would green-light a zero-test run). + # Empty spec_filter on dispatch = run ALL specs. + - name: Run Playwright specs + env: + SPEC_FILTER: ${{ github.event.inputs.spec_filter }} + run: | + if [ "${{ github.event_name }}" != "workflow_dispatch" ]; then + npx playwright test explorer-smoke --reporter=list + elif [ -n "$SPEC_FILTER" ]; then + npx playwright test "$SPEC_FILTER" --reporter=list + else + npx playwright test --reporter=list + fi + + - name: Upload Playwright artifacts + if: failure() + uses: actions/upload-artifact@v4 + with: + name: playwright-artifacts + path: | + tests/playwright-report/ + test-results/ + devserver.log + retention-days: 7 + if-no-files-found: ignore diff --git a/REFACTOR_PLAN_249.md b/REFACTOR_PLAN_249.md new file mode 100644 index 0000000..3b3bef6 --- /dev/null +++ b/REFACTOR_PLAN_249.md @@ -0,0 +1,104 @@ +# Draft comment for isamplesorg/isamplesorg.github.io#249 + +> Paste-ready draft for the refactor-window plan. Edit freely — written +> 2026-06-12 alongside the PR that wires the e2e gate (PR 1 below). + +--- + +## Refactor window: concrete PR sequence + +Following up on the 2026-06-05 update above: here is the staged plan for +the refactor window, structured so every stage lands behind a green +browser-level gate and no stage requires a big-bang diff. The method is +still Option C (strangler / extract-along-seams); what's new is the +explicit sequencing and the regression gate that goes in *first*. + +### PR 1 — CI safety net (this is the only infrastructure PR) + +Wire the existing Playwright browser specs into CI before anything in +`explorer.qmd` moves: + +- New workflow `explorer-e2e.yml`: renders `explorer.qmd` with Quarto, + serves `docs/` with the repo's range-capable `dev_server.py`, runs a + headless-Chromium **smoke set** on every PR that touches the explorer + (page boots with no uncaught/OJS-cell errors, Cesium canvas draws at + non-zero size, facet sidebar renders, search box present). +- The smoke set deliberately does **not** wait on parquet loads from + data.isamples.org, so slow data can't flake the gate. The deeper + data-dependent specs (facet viewport, URL round-trip, heatmap, search + counts) stay runnable on demand via `workflow_dispatch` with a spec + filter, and locally per `tests/README.md`. +- This complements (doesn't replace) the pre-deploy DuckDB-liveness gate + already in `quarto-pages.yml` / `tests/test_smoke.py`: that one guards + *deploys to production*; this one guards *PRs*, which is where refactor + regressions need to be caught. + +**Gate rule for every PR below: the smoke set must stay green, and any +stage that touches a seam covered by a deeper spec runs that spec +manually before merge.** + +### PR 2 — characterization tests around the seams we're about to cut + +Cheap, compounding (step 1 of the 2026-06-05 plan): turn the recent +symptom reports into specs *before* moving code — #260 (detail-card +material/category), #265 (facet label provenance), #267 (facet selection +drives the map; partially latched already), back/forward + deep-link +round-trip (#239's divergence case). #266 is already covered by the +updated `explorer-map-overlay.spec.js` assertion. These are +characterization tests: they pin current *intended* behavior, not new +features. + +### PR 3 — extract pure logic into `assets/js/` ES modules (lowest risk) + +The step the original analysis sized at ~500 lines, near-zero risk: SQL +builders (`facetFilterSQL`, `sourceFilterSQL`, `textSearchWhere`/`Score`), +hash codec (`readHash`/`buildHash`), bbox math (`paddedViewportBounds`/ +`viewerBboxSQL`), card renderers, escapers. The explorer already imports +`assets/js/source-palette.js` at runtime, so the mechanism is proven. +Pure functions become unit-testable outside the browser; this PR also +directly answers #268's "where does the SQL live?" — the answer becomes a +file path instead of a line range in a 5.4k-line qmd. Includes the +parquet-URL registry / plain-English query doc from the 2026-06-05 plan. + +### PR 4 — URL/state codecs + single-writer boundary (#208) + +Extract the state codecs (search params, hash state, selection, heatmap, +source/facet selections — the `EXPLORER_STATE.md` inventory), then land +#208's two fixes on top of the now-isolated codec: collapse the dual +`mode` state to `viewer._globeState.mode` as single source of truth, and +funnel the ~8 URL writers through `writeGlobeHash` / `setExplorerMode` / +`reconcileCameraState`. This is the stage that de-risks every future +"interactive state diverges from cold-reload state" bug (#239, #262). + +### PR 5 — split controllers out of `zoomWatcher`, one seam at a time + +The god-closure (~2,200 lines) gets carved along its already-namespaced +seams, heatmap first (`viewer._heatmap*` state is already hoisted — +the seam the original analysis rated highest-risk, so it goes last among +extractions but first among controllers since its state is cleanest), +then facet panel, samples table, search panel, map mode/rendering. +Each seam = one PR, each behind the gate plus the relevant deep spec. +#189's selection controller (`selectSample`/`selectCluster`/ +`clearSelection`) joins this stage **only if** its YAGNI trigger has +fired by then — per that issue, it stays filed until a selection feature +or recurrence forces it. + +### Explicitly deferred + +- The "should this become a Vite/TS app embedded in Quarto?" question + (open question 1 in the issue body) — decide *after* PR 5, when the + module boundaries make the cost visible. Not a prerequisite for any + stage above. +- #234's filter-semantics direction (A1+B1+C3/C2) is *feature* work, not + refactor work — but PR 3/PR 4 are sequenced so that when #234 + implementation starts, search/facet predicates and URL state are + already modules it can build on instead of more closure growth. +- No UI redesign, no data-substrate changes, no Quarto replacement + (non-goals from the 2026-06-05 comment stand). + +### Coordination + +While PRs 3–5 are open, feature work in `explorer.qmd` freezes only on +the seam being moved (answer to open question 2: per-seam freeze, not a +page-wide freeze). `EXPLORER_STATE.md` gains a module-boundary section as +each extraction lands (open question 3: yes, same doc). diff --git a/tests/README.md b/tests/README.md index c3b8303..60a37be 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,6 +1,43 @@ # iSamples Testing Infrastructure -Automated tests for the iSamples Cesium tutorial UI using Playwright. +Automated tests for the iSamples website: Playwright specs for the +Interactive Explorer and Cesium tutorial UI (`tests/playwright/`), plus +pytest suites for rendered-site checks (`tests/test_*.py`). + +## Explorer e2e suite & CI smoke gate (#249) + +`tests/playwright/explorer-smoke.spec.js` is the minimal "is the explorer +alive?" set — page boots without uncaught/OJS errors, Cesium canvas draws, +facet sidebar renders, search box present. It deliberately does **not** +wait on parquet data loads, so it stays green even when +data.isamples.org is slow. CI runs it on every PR that touches the +explorer via `.github/workflows/explorer-e2e.yml`. + +Run it locally against a fresh render: + +```bash +npm install +npx playwright install --with-deps chromium +quarto render explorer.qmd # ~10s, single page +python3 dev_server.py --dir docs --port 5860 & # range-capable server +npx playwright test explorer-smoke +``` + +The other `tests/playwright/*.spec.js` files are deeper, data-dependent +explorer specs (facets, URL round-trip, heatmap, search counts, …) plus +the Cesium tutorial specs (`cesium-queries.spec.js`). Run a specific +file the same way (`npx playwright test facet-viewport`); dropping the +filter entirely runs **all** Playwright specs, explorer and tutorial +alike. Expect the deeper specs to exercise remote parquet loads from +data.isamples.org (slower, network-sensitive). In CI they can be run +manually via the `explorer-e2e` workflow's *Run workflow* button with a +different spec filter (empty filter = all specs). + +To test against a deployed site instead of a local render: + +```bash +TEST_URL=https://rdhyee.github.io/isamplesorg.github.io npx playwright test explorer-smoke +``` ## Setup diff --git a/tests/playwright/explorer-map-overlay.spec.js b/tests/playwright/explorer-map-overlay.spec.js index b6039ed..ac17285 100644 --- a/tests/playwright/explorer-map-overlay.spec.js +++ b/tests/playwright/explorer-map-overlay.spec.js @@ -291,21 +291,19 @@ test.describe('Map search overlay — Cesium toolbar coexistence (#200 / M-1A)', expect(hash).toContain(`pid=${encodeURIComponent(pid)}`); }); - test('sidebar search input mirrors in-map search input', async ({ page }) => { + test('sidebar search input is gone; in-map search is the only search box (#266)', async ({ page }) => { await page.setViewportSize({ width: 1280, height: 800 }); await page.goto(explorerUrl(), { waitUntil: 'domcontentloaded', timeout: 60000, }); await page.waitForSelector('#sampleSearch', { timeout: 30000 }); - await page.waitForSelector('#sampleSearchSidebar', { timeout: 10000 }); await waitForBootReady(page); - await page.locator('#sampleSearchSidebar').fill('pottery'); - await expect(page.locator('#sampleSearch')).toHaveValue('pottery'); - - await page.locator('#sampleSearch').fill('basalt'); - await expect(page.locator('#sampleSearchSidebar')).toHaveValue('basalt'); + // #266: the duplicate sidebar search box was removed — exactly one + // search input should exist, and it's the in-map one. + await expect(page.locator('#sampleSearchSidebar')).toHaveCount(0); + await expect(page.locator('#sampleSearch')).toHaveCount(1); }); }); diff --git a/tests/playwright/explorer-smoke.spec.js b/tests/playwright/explorer-smoke.spec.js new file mode 100644 index 0000000..21a6426 --- /dev/null +++ b/tests/playwright/explorer-smoke.spec.js @@ -0,0 +1,116 @@ +// Explorer smoke gate (#249 PR 1) — the minimal "is the explorer alive?" +// set that CI runs on every PR touching the explorer, BEFORE any refactor +// of explorer.qmd lands. +// +// Scope is deliberately tiny (one assertion per concern): +// 1. the page loads without an uncaught JS exception, +// 2. the Cesium map canvas appears with non-zero size, +// 3. the facet sidebar renders (static source-legend rows), +// 4. the search box is present. +// +// What this does NOT cover: data correctness. The explorer streams +// multi-hundred-MB parquet from data.isamples.org via DuckDB-WASM range +// requests; this gate must stay green even when that load is slow or the +// network is constrained, so nothing here waits on facet counts, sample +// dots, or search results. Deeper data-dependent specs live in the other +// files in this directory and run on demand, and the pre-deploy gate in +// quarto-pages.yml (tests/test_smoke.py) separately asserts DuckDB-WASM +// liveness before anything reaches production. +// +// Per the hard-won lesson documented in tests/test_smoke.py: ONE fresh +// context, ONE navigation, poll-for-readiness. Rapid reloads exhaust the +// DuckDB-WASM worker and produce false failures — so the four tests share +// a single page load (serial mode) instead of navigating four times. + +const { test, expect } = require('@playwright/test'); +const { explorerUrl } = require('./helpers/url'); + +test.describe('Explorer smoke gate (#249)', () => { + test.describe.configure({ mode: 'serial' }); + + /** @type {import('@playwright/test').Page} */ + let page; + /** @type {string[]} */ + let pageErrors; + + test.beforeAll(async ({ browser }) => { + const context = await browser.newContext({ + viewport: { width: 1280, height: 900 }, + // The gate asserts on our page, not on TLS. Sandboxed/proxied dev + // environments MITM outbound HTTPS (Cesium CDN, data.isamples.org) + // with a CA Chromium doesn't trust; without this the page can never + // boot there. No-op on a clean network like GitHub Actions. + ignoreHTTPSErrors: true, + }); + page = await context.newPage(); + pageErrors = []; + page.on('pageerror', (err) => pageErrors.push(String(err))); + // OJS swallows cell exceptions: a crashed cell (undefined symbol, + // syntax slip in explorer.qmd) logs "Error evaluating OJS cell" to the + // console instead of firing pageerror. That signature IS the "explorer + // is dead" signal this gate exists for, so collect it too. + page.on('console', (msg) => { + if (msg.type() === 'error' && /Error evaluating OJS cell/i.test(msg.text())) { + pageErrors.push(msg.text().slice(0, 500)); + } + }); + + await page.goto(explorerUrl(), { + waitUntil: 'domcontentloaded', + timeout: 60000, + }); + }); + + test.afterAll(async () => { + await page?.context().close(); + }); + + // The page is shared across the serial tests, so an async error that + // fires AFTER the first test (late OJS cell crash, runtime regression + // surfacing during the canvas/sidebar/search checks) must still fail + // the gate — assert the accumulator after every test, draining it so + // each error is reported exactly once. + test.afterEach(() => { + const errs = pageErrors.splice(0); + expect(errs, `uncaught page errors:\n${errs.join('\n')}`).toEqual([]); + }); + + test('explorer loads without uncaught JS errors', async () => { + await expect(page).toHaveTitle(/Interactive Explorer/i, { timeout: 30000 }); + // The Cesium container is static HTML in explorer.qmd — if it is + // missing, the render itself is broken (not just slow data). + await expect(page.locator('#cesiumContainer')).toBeAttached({ timeout: 30000 }); + // Give the boot path a moment to surface an early uncaught exception + // (OJS cell crash, undefined symbol) without waiting on data — + // afterEach below does the actual assertion for this and every test. + await page.waitForTimeout(3000); + }); + + test('map canvas appears with non-zero size', async () => { + const canvas = page.locator('.cesium-viewer .cesium-widget canvas'); + await expect(canvas).toBeAttached({ timeout: 60000 }); + // A 0x0 canvas means the widget mounted but the globe never sized — + // the "container but no globe" failure mode test_smoke.py documents. + await expect + .poll(async () => { + const box = await canvas.boundingBox(); + return box ? Math.min(box.width, box.height) : 0; + }, { timeout: 30000 }) + .toBeGreaterThan(0); + }); + + test('facet sidebar renders source facet rows', async () => { + // The four source rows are static HTML in explorer.qmd, so this + // asserts the sidebar structure rendered — it does NOT wait for the + // DuckDB-WASM facet-count fill (data-dependent, excluded from smoke). + const sourceRows = page.locator('.facet-row[data-facet="source"]'); + await expect(sourceRows.first()).toBeVisible({ timeout: 30000 }); + expect(await sourceRows.count()).toBeGreaterThanOrEqual(4); + }); + + test('search box is present', async () => { + // Post-#266 there is exactly one search input: the in-map overlay. + await expect(page.locator('#sampleSearch')).toBeVisible({ timeout: 30000 }); + await expect(page.locator('#sampleSearch')).toHaveCount(1); + }); +});