diff --git a/.github/workflows/explorer-e2e.yml b/.github/workflows/explorer-e2e.yml new file mode 100644 index 00000000..d9f596c6 --- /dev/null +++ b/.github/workflows/explorer-e2e.yml @@ -0,0 +1,170 @@ +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 + + # Fast pure-logic gate (#249 PR3): unit-tests the ES modules extracted + # from explorer.qmd (assets/js/sql-builders.js + explorer-utils.js). + # Node built-ins only, sub-second — runs before the slow Quarto/ + # Playwright steps so a module regression fails fast. + - name: Unit tests (extracted modules) + run: node --test tests/unit/*.test.mjs + + - 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 00000000..3b3bef67 --- /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/_quarto.yml b/_quarto.yml index 685a07f9..c6d00354 100644 --- a/_quarto.yml +++ b/_quarto.yml @@ -3,6 +3,8 @@ project: output-dir: docs resources: - assets/js/source-palette.js + - assets/js/sql-builders.js + - assets/js/explorer-utils.js website: title: "iSamples" diff --git a/assets/js/explorer-utils.js b/assets/js/explorer-utils.js new file mode 100644 index 00000000..4e9440b4 --- /dev/null +++ b/assets/js/explorer-utils.js @@ -0,0 +1,66 @@ +// Pure helpers extracted from explorer.qmd (issue #249, PR3). +// No closure over `viewer`/`db`/DOM — safe to unit-test under Node and to +// import into the Interactive Explorer's OJS runtime (see explorer.qmd). + +// HTML-escape a value for safe interpolation into innerHTML. +export function escapeHtml(value) { + return String(value ?? '') + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + +// Split a free-text query into whitespace-delimited terms (no empties). +export function searchTerms(value) { + return String(value || '').trim().split(/\s+/).filter(Boolean); +} + +// Parse a numeric URL param with a default and optional clamping. +export function parseNum(val, def, min, max) { + if (val == null) return def; + const n = parseFloat(val); + if (!Number.isFinite(n)) return def; + if (min != null && n < min) return min; + if (max != null && n > max) return max; + return n; +} + +// Read a comma-separated URLSearchParams value into an array of trimmed, +// non-empty strings. Returns null when the key is absent, [] when present +// but empty. +export function csvParamValues(params, key) { + if (!params.has(key)) return null; + const raw = params.get(key) || ''; + if (raw.trim() === '') return []; + return raw.split(',').map(s => s.trim()).filter(Boolean); +} + +// Resolve a pid to its canonical resolver URL. All iSamples sources resolve +// via n2t.net: ARK pids (OpenContext, GEOME, Smithsonian) and IGSN pids +// (SESAR) alike. +export function sourceUrl(pid) { + if (!pid) return null; + return `https://n2t.net/${pid}`; +} + +// Decode the explorer globe state from a URL hash fragment. +// `hashStr` defaults to `location.hash` for in-browser callers (every current +// call site is zero-arg); tests pass an explicit string so `location` is never +// referenced. Numeric fields are clamped to valid geographic / altitude ranges. +export function readHash(hashStr = location.hash) { + const params = new URLSearchParams(hashStr.slice(1)); + return { + v: parseInt(params.get('v')) || 0, + lat: parseNum(params.get('lat'), null, -90, 90), + lng: parseNum(params.get('lng'), null, -180, 180), + alt: parseNum(params.get('alt'), null, 100, 40000000), + heading: parseNum(params.get('heading'), 0, 0, 360), + pitch: parseNum(params.get('pitch'), -90, -90, 0), + mode: params.get('mode') || null, + pid: params.get('pid') || null, + h3: params.get('h3') || null, + heatmap: params.get('heatmap') === '1', + }; +} diff --git a/assets/js/sql-builders.js b/assets/js/sql-builders.js new file mode 100644 index 00000000..d4c1b50b --- /dev/null +++ b/assets/js/sql-builders.js @@ -0,0 +1,41 @@ +// Pure SQL-string builders extracted from explorer.qmd (issue #249, PR3). +// No closure over `viewer`/`db`/DOM — safe to unit-test under Node and to +// import into the Interactive Explorer's OJS runtime (see explorer.qmd). +// +// Internal dependency chain: textSearch* -> escapeIlikePattern -> escSql. +// Each function references the module-local sibling directly, so importing +// these into OJS does NOT create reactive edges between the bound cells. + +// Double single-quotes for safe interpolation into a SQL string literal. +export function escSql(value) { + return String(value).replace(/'/g, "''"); +} + +// Escape a value for use inside an ILIKE '%...%' pattern with ESCAPE '\'. +// First SQL-escapes single quotes (escSql), then backslash-escapes the LIKE +// metacharacters \ % _ so they match literally. +export function escapeIlikePattern(value) { + return escSql(value).replace(/[\\%_]/g, "\\$&"); +} + +// Build a WHERE fragment: every term must match at least one column +// (terms AND-ed, columns OR-ed within a term). +export function textSearchWhere(terms, columns) { + return terms.map(raw => { + const term = escapeIlikePattern(raw); + const checks = columns.map(col => `${col} ILIKE '%${term}%' ESCAPE '\\'`); + return `(${checks.join(' OR ')})`; + }).join(' AND '); +} + +// Build a relevance-score expression: sum of per-term, per-weighted-column +// CASE contributions. Returns '0' when there are no terms. +export function textSearchScore(terms, weightedColumns) { + if (!terms.length) return '0'; + return terms.map(raw => { + const term = escapeIlikePattern(raw); + return weightedColumns.map(({ col, weight }) => + `CASE WHEN ${col} ILIKE '%${term}%' ESCAPE '\\' THEN ${weight} ELSE 0 END` + ).join(' + '); + }).map(score => `(${score})`).join(' + '); +} diff --git a/explorer.qmd b/explorer.qmd index 08b7feaa..d6719724 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -774,14 +774,25 @@ _palette = await import(new URL('assets/js/source-palette.js', document.baseURI) SOURCE_COLORS = _palette.SOURCE_COLORS SOURCE_NAMES = _palette.SOURCE_NAMES -// === Source URL: resolve pid to original repository === -function sourceUrl(pid) { - if (!pid) return null; - // All sources resolve via n2t.net: - // ARK pids (OpenContext, GEOME, Smithsonian) → n2t.net/ark:/... - // IGSN pids (SESAR) → n2t.net/IGSN:... - return `https://n2t.net/${pid}`; -} +// === Pure logic extracted to ES modules (issue #249, PR3) === +// Same path-relative dynamic-import pattern as the palette above, so these +// resolve under both the custom domain (isamples.org) and fork project-pages +// previews. The functions are closure-free (no viewer/db/DOM) and unit-tested +// under Node (tests/unit/). Each `name = _module.fn` line is a single OJS cell +// that REPLACES the former inline `function name(){...}` cell. +_sqlBuilders = await import(new URL('assets/js/sql-builders.js', document.baseURI).href) +escSql = _sqlBuilders.escSql +escapeIlikePattern = _sqlBuilders.escapeIlikePattern +textSearchWhere = _sqlBuilders.textSearchWhere +textSearchScore = _sqlBuilders.textSearchScore + +_explorerUtils = await import(new URL('assets/js/explorer-utils.js', document.baseURI).href) +escapeHtml = _explorerUtils.escapeHtml +searchTerms = _explorerUtils.searchTerms +parseNum = _explorerUtils.parseNum +csvParamValues = _explorerUtils.csvParamValues +sourceUrl = _explorerUtils.sourceUrl +readHash = _explorerUtils.readHash // === Source Filter: get active sources and build SQL clause === function getActiveSources() { @@ -821,13 +832,6 @@ DEFAULT_POINT_BUDGET = 5000 // long range while bypassing terrain occlusion at every interactive zoom. POINT_DEPTH_TEST_DISTANCE = 2.0e6 -function csvParamValues(params, key) { - if (!params.has(key)) return null; - const raw = params.get(key) || ''; - if (raw.trim() === '') return []; - return raw.split(',').map(s => s.trim()).filter(Boolean); -} - function updateSourceLegendState() { document.querySelectorAll('#sourceFilter .legend-item').forEach(li => { const cb = li.querySelector('input'); @@ -940,32 +944,6 @@ function writeQueryState(opts = {}) { } } -function searchTerms(value) { - return String(value || '').trim().split(/\s+/).filter(Boolean); -} - -function escapeIlikePattern(value) { - return escSql(value).replace(/[\\%_]/g, "\\$&"); -} - -function textSearchWhere(terms, columns) { - return terms.map(raw => { - const term = escapeIlikePattern(raw); - const checks = columns.map(col => `${col} ILIKE '%${term}%' ESCAPE '\\'`); - return `(${checks.join(' OR ')})`; - }).join(' AND '); -} - -function textSearchScore(terms, weightedColumns) { - if (!terms.length) return '0'; - return terms.map(raw => { - const term = escapeIlikePattern(raw); - return weightedColumns.map(({ col, weight }) => - `CASE WHEN ${col} ILIKE '%${term}%' ESCAPE '\\' THEN ${weight} ELSE 0 END` - ).join(' + '); - }).map(score => `(${score})`).join(' + '); -} - // === Material / Sampled Feature / Specimen Type Filters === // Checkbox semantics: start UNCHECKED (no filter; show everything). User // checks items to *include only those*. Empty = no filter. Matches the @@ -1009,10 +987,6 @@ function syncFacetNote() { el.style.display = (active && inCluster && !heatOn) ? 'block' : 'none'; } -function escSql(value) { - return String(value).replace(/'/g, "''"); -} - // Returns a portable predicate fragment (no outer-table alias dependency) // that callers append to a WHERE: ` AND ${facetFilterSQL()}`. Uses a // `pid IN (SELECT pid FROM facets WHERE ...)` subquery so it works @@ -1186,31 +1160,7 @@ function markFacetCountsRecomputing() { } // === URL State: encode/decode globe state in hash fragment === -function parseNum(val, def, min, max) { - if (val == null) return def; - const n = parseFloat(val); - if (!Number.isFinite(n)) return def; - if (min != null && n < min) return min; - if (max != null && n > max) return max; - return n; -} - -function readHash() { - const params = new URLSearchParams(location.hash.slice(1)); - return { - v: parseInt(params.get('v')) || 0, - lat: parseNum(params.get('lat'), null, -90, 90), - lng: parseNum(params.get('lng'), null, -180, 180), - alt: parseNum(params.get('alt'), null, 100, 40000000), - heading: parseNum(params.get('heading'), 0, 0, 360), - pitch: parseNum(params.get('pitch'), -90, -90, 0), - mode: params.get('mode') || null, - pid: params.get('pid') || null, - h3: params.get('h3') || null, - heatmap: params.get('heatmap') === '1', - }; -} - +// (decode side — parseNum + readHash — extracted to assets/js/explorer-utils.js, PR3) function buildHash(v) { const cam = v.camera; const carto = cam.positionCartographic; @@ -1518,15 +1468,6 @@ function updateSamples(samples) { // ORDER BY pid makes "Page N is the same N rows" actually true. TABLE_PAGE_SIZE = 100 -function escapeHtml(value) { - return String(value ?? '') - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, '''); -} - ``` ```{ojs} diff --git a/package.json b/package.json index b92e58be..8ec9f2ec 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "private": true, "scripts": { "test": "playwright test", + "test:unit": "node --test tests/unit/*.test.mjs", "test:headed": "playwright test --headed", "test:ui": "playwright test --ui", "test:debug": "playwright test --debug", diff --git a/tests/README.md b/tests/README.md index c3b8303b..b8ff4303 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,6 +1,67 @@ # 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 +``` + +## Running characterization & deeper specs via workflow_dispatch + +The characterization and deeper specs depend on live remote parquet loads and are **not** +in the CI smoke gate. They can be triggered via the GitHub Actions +*Run workflow* button on the `explorer-e2e` workflow. Set `spec_filter` +to one of the values below; empty = all specs. + +| spec_filter value | What runs | +|---|---| +| `explorer-characterization` | PR2 characterization suite (7 [data] tests) | +| `facet-viewport` | B1 viewport-aware facet counts (#234 step 3) | +| `heatmap-overlay` | Heatmap mutual-exclusion and round-trip (#233) | +| `search-real-count` | Search cap-hit real-count display (#232) | +| `url-roundtrip` | URL state round-trip regressions (#209) | +| `explorer-map-overlay` | Map overlay / table pagination (#200) | +| *(empty)* | All Playwright specs (explorer + tutorial) | + +Run any of these locally with `npx playwright test `, e.g.: + + npx playwright test explorer-characterization + +Allow up to 3 minutes per test for remote parquet loads. + ## Setup diff --git a/tests/playwright/explorer-characterization.spec.js b/tests/playwright/explorer-characterization.spec.js new file mode 100644 index 00000000..47813dfc --- /dev/null +++ b/tests/playwright/explorer-characterization.spec.js @@ -0,0 +1,278 @@ +/** + * Characterization tests for the iSamples Explorer (PR2, issue #249). + * + * These tests (all tagged [data]) pin the behaviors that Codex review of the + * PR1 smoke gate named as missing characterization coverage. They depend on + * remote parquet loads from data.isamples.org (202608 dataset) and are + * intentionally NOT in the CI smoke gate (explorer-e2e.yml stays unchanged). + * Run manually or via workflow_dispatch with spec_filter=explorer-characterization. + * + * Tightened per Codex PR2 review: every assertion pins an OBSERVABLE CONTRACT + * (the table actually filters; the deep-link propagates to the table; the + * detail card shows a real material), not just internal state that could + * false-pass during a refactor. + * + * Flakiness mitigations: test.setTimeout(180000); expect.poll (60-90s) for + * every data-dependent assertion; NEVER fixed waitForTimeout. + * + * Behavior map: + * (a+) search (known single result) -> __searchFilter active AND table total == 1 + * (a-) clear search -> __searchFilter.active === false AND filter removed (total > 1) + * (b) material facet -> table total strictly decreases (parsed ints), aria-busy settles false + * (c) heatmap -> see heatmap-overlay.spec.js (comment only, no test) + * (d1) ?search= URL -> __searchFilter restored AND #tableMeta shows the match summary + * (d2) &pid= URL -> selectedPid restored AND #clusterSection shows the sample card + * (NB: deep-link does NOT open #inMapCard — row-click-only; gap filed) + * (e) facet hydration -> >=3 source counts, material URIs, no stuck .recomputing + * (f) detail card -> known sample -> #inMapCard visible AND exact material value + */ +const { test, expect } = require('@playwright/test'); +const { explorerUrl } = require('./helpers/url'); +const { + waitForBootReady, + waitForFacetUI, + waitForFacetCountsStable, + readFacetCounts, + waitForSearchReady, + runSearch, + getSearchFilter, + getSelectedPid, +} = require('./helpers/explorer'); + +const WORLD = '#v=1&lat=20&lng=0&alt=10000000'; + +// Parse the integer total out of #tablePageInfo: "Page 1 of N (1-100 of TOTAL)". +async function tableTotal(page) { + const t = await page.locator('#tablePageInfo').textContent(); + const m = t && t.match(/of ([\d,]+)\)\s*$/); + return m ? parseInt(m[1].replace(/,/g, ''), 10) : null; +} +// Poll until #tablePageInfo first shows a numeric total, return it. +async function waitForTableTotal(page, timeout = 90000) { + await expect.poll(async () => await tableTotal(page), { timeout, intervals: [500, 1000, 2000] }) + .toBeGreaterThan(0); + return await tableTotal(page); +} + +test.describe('Explorer characterization tests [data]', () => { + test.setTimeout(180000); + + // ========================================================================= + // (a+) search-as-filter: a search must scope the TABLE to its matching set. + // We use a deterministic known sample ("Object 5404-8" -> exactly the + // one OpenContext record ark:/28722/k2p55x96j) so the proof is exact + // and stable. (Table-total SCOPE shifts between viewport/global/filtered + // states, so cross-state total comparisons are unreliable -- a known + // single-result search is the robust way to prove real filtering.) + // ========================================================================= + test('(a+) [data] search scopes the table to the matching record', async ({ page }) => { + await page.goto(explorerUrl(WORLD), { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForBootReady(page); + await waitForSearchReady(page, 90000); + await runSearch(page, 'Object 5404-8'); + + // Internal state activates... + await expect.poll( + async () => { const sf = await getSearchFilter(page); return sf && sf.active && sf.kind === 'text'; }, + { timeout: 90000, intervals: [500, 1000, 2000] } + ).toBe(true); + + // ...AND the table is scoped to exactly the one matching record... + await expect.poll( + async () => await tableTotal(page), + { timeout: 90000, intervals: [500, 1000, 2000] } + ).toBe(1); + + // ...which is the expected OpenContext sample. + await expect(page.locator('.samples-table tbody tr[data-pid="ark:/28722/k2p55x96j"]')) + .toBeVisible({ timeout: 30000 }); + }); + + // ========================================================================= + // (a-) negative: clearing a search removes the filter (active=false AND the + // table grows beyond the single matched record). + // ========================================================================= + test('(a-) [data] clear search resets active=false and removes the filter', async ({ page }) => { + await page.goto(explorerUrl(WORLD), { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForBootReady(page); + await waitForSearchReady(page, 90000); + + await runSearch(page, 'Object 5404-8'); + await expect.poll( + async () => { const sf = await getSearchFilter(page); return sf && sf.active; }, + { timeout: 90000, intervals: [500, 1000, 2000] } + ).toBe(true); + await expect.poll(async () => await tableTotal(page), { timeout: 90000, intervals: [500, 1000, 2000] }) + .toBe(1); + + const input = page.locator('#sampleSearch').first(); + await input.click(); + await input.press('ControlOrMeta+a'); + await input.press('Delete'); + await page.locator('#searchSubmitBtn').first().click(); + + // Filter gone: active=false AND the table is no longer scoped to the 1 match. + await expect.poll( + async () => { const sf = await getSearchFilter(page); return sf ? sf.active : false; }, + { timeout: 60000, intervals: [500, 1000, 2000] } + ).toBe(false); + await expect.poll(async () => await tableTotal(page), { timeout: 60000, intervals: [500, 1000, 2000] }) + .toBeGreaterThan(1); + }); + + // ========================================================================= + // (b) facet -> table coherence: material checkbox STRICTLY decreases the + // table total (parsed integers, not "text changed"). + // ========================================================================= + test('(b) [data] material facet toggle strictly decreases table total', async ({ page }) => { + await page.goto(explorerUrl(WORLD), { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForBootReady(page); + + const before = await waitForTableTotal(page, 90000); + + await page.waitForFunction(() => document.querySelectorAll('#materialFilterBody input[type="checkbox"]').length > 0, null, { timeout: 60000 }); + await page.evaluate(() => { + const cb = document.querySelector('#materialFilterBody input[type="checkbox"]'); + if (cb) { cb.checked = true; document.getElementById('materialFilterBody').dispatchEvent(new Event('change', { bubbles: true })); } + }); + + // Settle on the refetch (final aria-busy=false; the transient =true is not + // asserted -- a fast cached refresh can complete between polls, per Codex). + await expect.poll( + async () => await page.locator('#tableContainer').getAttribute('aria-busy'), + { timeout: 90000, intervals: [250, 500, 1000] } + ).toBe('false'); + + // Selecting ONE material facet must reduce the total (coherence), not just change text. + await expect.poll(async () => await tableTotal(page), { timeout: 60000, intervals: [500, 1000, 2000] }) + .toBeLessThan(before); + expect(await tableTotal(page)).toBeGreaterThan(0); + }); + + // ========================================================================= + // (c) heatmap -- fully covered by heatmap-overlay.spec.js (toggle, mutual + // exclusion with markers, URL round-trip, no-cap). Not duplicated here. + // ========================================================================= + + // ========================================================================= + // (d1) deep-link ?search=pottery -> restores state AND propagates to the table + // (#tableMeta shows the match summary, not just the internal flag). + // ========================================================================= + test('(d1) [data] ?search=pottery deep-link restores state AND filters the table', async ({ page }) => { + await page.goto(explorerUrl('?search=pottery' + WORLD), { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page.waitForSelector('#cesiumContainer', { timeout: 30000 }); + + await expect.poll( + async () => { const sf = await getSearchFilter(page); return sf && sf.active && sf.term === 'pottery'; }, + { timeout: 90000, intervals: [500, 1000, 2000] } + ).toBe(true); + + // Propagation: the table surface reflects the deep-linked search. + await expect.poll( + async () => await page.locator('#tableMeta').textContent(), + { timeout: 90000, intervals: [500, 1000, 2000] } + ).toMatch(/\d[\d,]* of [\d,]+ "pottery" match/); + }); + + // ========================================================================= + // (d2) deep-link &pid= -> restores selectedPid AND renders the sample card + // into #clusterSection (what the boot/hashchange pid path actually does: + // updateSampleCard, NOT showInMapCard). The fact that a pid deep-link + // does NOT open #inMapCard (whereas a row-click does) is a real UX + // inconsistency tracked as a follow-up (#239-family), NOT asserted here. + // ========================================================================= + test('(d2) [data] &pid= deep-link restores selectedPid and renders #clusterSection card', async ({ browser }) => { + // Phase 1: click a row, capture pid + label + the resulting pid URL. + const ctx1 = await browser.newContext(); + let capturedUrl = null, capturedPid = null, capturedLabel = null; + try { + const page1 = await ctx1.newPage(); + await page1.goto(explorerUrl(WORLD), { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page1.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForBootReady(page1); + const firstRow = page1.locator('.samples-table tbody tr[data-pid]').first(); + await expect(firstRow).toBeVisible({ timeout: 90000 }); + capturedPid = await firstRow.getAttribute('data-pid'); + capturedLabel = (await firstRow.locator('td').nth(1).textContent() || '').trim(); + expect(capturedPid).toBeTruthy(); + await firstRow.locator('td').first().click(); + await expect.poll(async () => await page1.evaluate(() => location.href), { timeout: 30000, intervals: [250, 500, 1000] }).toContain('pid='); + capturedUrl = await page1.evaluate(() => location.href); + } finally { await ctx1.close(); } + + // Phase 2: load the pid URL fresh; assert the boot pid path restored state + card. + const ctx2 = await browser.newContext(); + try { + const page2 = await ctx2.newPage(); + await page2.goto(capturedUrl, { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page2.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForBootReady(page2); + // selectedPid restored. + await expect.poll(async () => await getSelectedPid(page2), { timeout: 90000, intervals: [500, 1000, 2000] }).toBe(capturedPid); + // updateSampleCard rendered the sample into #clusterSection (the real visible effect). + await expect.poll( + async () => await page2.evaluate(() => { const el = document.getElementById('clusterSection'); return el ? el.querySelector('.cluster-card') !== null : false; }), + { timeout: 120000, intervals: [500, 1000, 2000] } + ).toBe(true); + // and the card carries the deep-linked sample's label. + if (capturedLabel) { + await expect.poll( + async () => await page2.locator('#clusterSection').textContent(), + { timeout: 30000, intervals: [500, 1000] } + ).toContain(capturedLabel); + } + } finally { await ctx2.close(); } + }); + + // ========================================================================= + // (e) facet hydration: source counts, material URIs, no stuck .recomputing + // ========================================================================= + test('(e) [data] facet hydration: source counts, material URIs, no stuck .recomputing', async ({ page }) => { + await page.goto(explorerUrl('#v=1&lat=0&lng=0&alt=15000000'), { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForFacetUI(page, 90000); + await waitForFacetCountsStable(page, 90000); + + const sourceCounts = await readFacetCounts(page, 'source'); + expect(Object.values(sourceCounts).filter(n => n > 0).length).toBeGreaterThanOrEqual(3); + + const materialValues = await page.evaluate(() => [...document.querySelectorAll('#materialFilterBody input[type="checkbox"]')].map(cb => cb.value)); + expect(materialValues.length).toBeGreaterThan(0); + for (const val of materialValues) { expect(val).toMatch(/^https?:\/\//); } + + const stuck = await page.evaluate(() => document.querySelectorAll('.facet-count.recomputing').length); + expect(stuck).toBe(0); + }); + + // ========================================================================= + // (f) detail card: a KNOWN sample (#260, ark:/28722/k2p55x96j) must show the + // in-map card with its EXACT material -- not merely "non-empty", which + // would false-pass on "Not Provided" from a failed detail query (Codex). + // ========================================================================= + test('(f) [data] known sample row-click shows #inMapCard with exact material', async ({ page }) => { + await page.goto(explorerUrl('?search=Object+5404-8' + WORLD), { waitUntil: 'domcontentloaded', timeout: 60000 }); + await page.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForSearchReady(page, 90000); + + // The deep-link search resolves to exactly this OpenContext sample. + const row = page.locator('.samples-table tbody tr[data-pid="ark:/28722/k2p55x96j"]'); + await expect(row).toBeVisible({ timeout: 120000 }); + + expect(await page.locator('#inMapCard').getAttribute('hidden')).not.toBeNull(); // starts hidden + await row.locator('td').first().click(); + + await expect.poll( + async () => await page.locator('#inMapCard').getAttribute('hidden'), + { timeout: 60000, intervals: [250, 500, 1000] } + ).toBeNull(); + + // Exact, known material (the #260 fix value) -- pins a real successful detail query. + await expect.poll( + async () => (await page.locator('#imcMaterial').textContent() || '').trim(), + { timeout: 90000, intervals: [500, 1000, 2000] } + ).toBe('Other anthropogenic material'); + }); +}); diff --git a/tests/playwright/explorer-smoke.spec.js b/tests/playwright/explorer-smoke.spec.js new file mode 100644 index 00000000..21a6426e --- /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); + }); +}); diff --git a/tests/playwright/helpers/explorer.js b/tests/playwright/helpers/explorer.js new file mode 100644 index 00000000..1f613839 --- /dev/null +++ b/tests/playwright/helpers/explorer.js @@ -0,0 +1,331 @@ +// Explorer-specific Playwright helpers. +// +// Consolidates helpers that were copy-pasted across multiple spec files so +// `explorer-characterization.spec.js` (and future specs) can import them +// without duplicating the implementation. Every function below is copied +// verbatim from the source spec noted in the block comment; the source spec +// keeps its own inline copy (unchanged) so existing tests remain independent. +// +// Module style follows `tests/playwright/helpers/url.js`: +// - CommonJS (require/module.exports) +// - No external dependencies beyond Playwright's `page` argument +// - Each helper is exported by name +// +// Source specs: +// explorer-map-overlay.spec.js -> waitForBootReady +// facet-viewport.spec.js -> waitForFacetUI, waitForFacetCountsStable, +// readFacetCounts +// heatmap-overlay.spec.js -> heatmapState, markerVisibility +// url-roundtrip.spec.js -> waitForMode, snapshot +// search-real-count.spec.js -> waitForSearchReady, runSearch +// +// NEW helpers (not in any existing spec): +// getSearchFilter -> reads window.__searchFilter +// getSelectedPid -> reads viewer._globeState.selectedPid via OJS mainModule + +// --------------------------------------------------------------------------- +// From explorer-map-overlay.spec.js +// --------------------------------------------------------------------------- + +/** Wait for the OJS runtime to attach, then wait for zoomWatcher to resolve + * (it returns "active" once boot hydration + listener registration are + * complete - same pattern used in explorer-layout-stability.spec.js). */ +async function waitForBootReady(page) { + // Wait for the OJS runtime to attach, then wait for zoomWatcher to resolve + // (it returns "active" once boot hydration + listener registration are + // complete - same pattern used in explorer-layout-stability.spec.js). + await page.waitForFunction(() => !!window._ojs && !!window._ojs.ojsConnector, null, { timeout: 60000 }); + await page.evaluate(async () => { + return await window._ojs.ojsConnector.mainModule.value('zoomWatcher'); + }); +} +// --------------------------------------------------------------------------- +// From facet-viewport.spec.js +// --------------------------------------------------------------------------- + +/** Wait until facet UI hydrates: at least one `.facet-count[data-facet="source"]` + * span is in the DOM. The facetFilters cell renders these after + * facet_summaries.parquet loads, which is the same precondition existing + * tests (facetnote-url-load) wait for. */ +async function waitForFacetUI(page, ms = 60000) { + await page.waitForFunction( + () => document.querySelectorAll('.facet-count[data-facet="source"]').length > 0, + null, { timeout: ms } + ); + // Also wait for materialFilterBody to populate - the existing + // facetnote-url-load spec uses this as the "facet UI is fully wired" + // signal, and we want to match its boot-readiness criterion. + await page.waitForFunction( + () => document.querySelectorAll('#materialFilterBody input[type="checkbox"]').length > 0, + null, { timeout: ms } + ); +} + +async function waitForFacetCountsStable(page, ms = 60000) { + await page.waitForFunction( + () => document.querySelectorAll('.facet-count.recomputing').length === 0, + null, { timeout: ms } + ); +} + +/** Read the per-value counts for a facet off the DOM. Returns `{[uri]: integer}`. + * Tolerates the `Number.toLocaleString()` thousands-grouping the renderer + * applies (en-US locale produces `1,234`; we strip commas). */ +async function readFacetCounts(page, facet) { + return page.evaluate((facet) => { + const out = {}; + document.querySelectorAll(`.facet-count[data-facet="${facet}"]`).forEach(el => { + const val = el.getAttribute('data-value'); + const m = (el.textContent || '').match(/\(([\d,]+)\)/); + if (m && val) out[val] = parseInt(m[1].replace(/,/g, ''), 10); + }); + return out; + }, facet); +} + +/** Source is the most stable facet for the unfiltered viewport-aware + * assertion: only a handful of values, and `_baselineCounts.source` + * populates at boot. */ +async function readSourceCounts(page) { return readFacetCounts(page, 'source'); } + +/** flyTo a destination, then wait for the post-moveEnd debounce + bbox query + * to settle. Uses a zero-duration flight to make the test deterministic. */ +async function flyToAndSettle(page, lat, lng, alt) { + await page.evaluate(async ({ lat, lng, alt }) => { + const v = await window._ojs?.ojsConnector?.mainModule?.value('viewer'); + if (!v) throw new Error('viewer not in OJS module'); + v.camera.flyTo({ + destination: window.Cesium.Cartesian3.fromDegrees(lng, lat, alt), + duration: 0, + }); + }, { lat, lng, alt }); + // Give the debounce + query a chance to land. waitForFacetCountsStable + // is the real synchronization point; this just kicks the event loop. + await page.waitForTimeout(50); + await waitForFacetCountsStable(page); +} + + +// --------------------------------------------------------------------------- +// From heatmap-overlay.spec.js +// --------------------------------------------------------------------------- + +async function heatmapState(page) { + await page.waitForFunction(() => !!window._ojs?.ojsConnector?.mainModule, null, { timeout: 60000 }); + return await page.evaluate(() => { + return window._ojs.ojsConnector.mainModule.value('viewer').then((v) => { + const state = v?._heatmapOverlay || {}; + return { + enabled: !!state.enabled, + hasLayer: !!state.layer, + layerShown: state.layer ? state.layer.show !== false : false, + lastRefreshAt: state.lastRefreshAt || 0, + lastPointCount: state.lastPointCount || 0, + lastImageHash: state.lastImageHash || null, + status: document.getElementById('heatmapStatus')?.textContent || '', + }; + }); + }); + } + +// Reads the live `.show` flags off the two marker collections so a test + // can assert mutual exclusion with the heatmap overlay (#233 phase 3). +async function markerVisibility(page) { + return await page.evaluate(() => { + return window._ojs.ojsConnector.mainModule.value('viewer').then((v) => ({ + clusterShown: v?.h3Points?.show === true, + pointShown: v?.samplePoints?.show === true, + mode: v?._globeState?.mode || null, + })); + }); + } + + +// --------------------------------------------------------------------------- +// From url-roundtrip.spec.js +// --------------------------------------------------------------------------- + +/** Wait until `viewer._globeState.mode` equals `expected`. + * Cold-cache point-mode boot can take 60–90s per #190; allow 3 minutes. */ +async function waitForMode(page, expected, timeoutMs = 180000) { + await page.waitForFunction( + async (expectedMode) => { + try { + const v = await window._ojs?.ojsConnector?.mainModule?.value('viewer'); + return v?._globeState?.mode === expectedMode; + } catch { return false; } + }, + expected, + { timeout: timeoutMs } + ); +} + +/** Wait until `loadViewportSamples` has settled into the point-mode done message. + * Matches on the trailing "Click one for details" phrase — it's the common + * denominator across both done-state phaseMsg branches at explorer.qmd:1610-1612 + * (normal: `" individual samples. Click one for details."` and cap-reached: + * `" samples in view (showing m — zoom in for more). Click one for details."`). + * Codex review (PR #214 round 1) suggested switching to the count phrase + * `\d[\d,]*\s+individual\s+samples`, but that pattern misses the cap-reached + * branch which has no "individual" word, so we stay with the trailing phrase. */ +async function waitForPointModeSettled(page, timeoutMs = 120000) { + await page.waitForFunction( + () => { + const msg = document.getElementById('phaseMsg')?.textContent || ''; + return msg.includes('Click one for details'); + }, + null, + { timeout: timeoutMs } + ); +} + +/** Wait for boot to complete enough that the `hashchange` listener (registered + * late in the explorer cell at explorer.qmd:2210) is installed. `_globeState` + * is initialized very early (line 871), so `waitForMode` alone is not enough + * to guarantee the listener exists. `_suppressHashWrite` flips to false either + * via zoomWatcher init or at the end of the boot deep-link section (line 2734); + * by the time it's false, the hashchange listener is definitely registered. */ +async function waitForBootSettled(page, timeoutMs = 180000) { + await page.waitForFunction( + async () => { + try { + const v = await window._ojs?.ojsConnector?.mainModule?.value('viewer'); + return v?._suppressHashWrite === false; + } catch { return false; } + }, + null, + { timeout: timeoutMs } + ); +} + +/** Wait until the URL hash's lat/lng (and optionally alt) match the expected + * values within `eps` / `altEps`. Replaces fixed sleeps with a precise settle + * condition for moveEnd-driven hash writes (regression for #205). */ +async function waitForHashLatLng(page, expectedLat, expectedLng, opts = {}) { + const eps = opts.eps ?? 0.001; + const expectedAlt = opts.alt ?? null; + const altEps = opts.altEps ?? 100; + const timeoutMs = opts.timeoutMs ?? 10000; + await page.waitForFunction( + ({ lat, lng, eps, alt, altEps }) => { + const params = new URLSearchParams(location.hash.slice(1)); + const ul = parseFloat(params.get('lat')); + const un = parseFloat(params.get('lng')); + if (!Number.isFinite(ul) || !Number.isFinite(un)) return false; + if (Math.abs(ul - lat) >= eps || Math.abs(un - lng) >= eps) return false; + if (alt != null) { + const ua = parseFloat(params.get('alt')); + if (!Number.isFinite(ua) || Math.abs(ua - alt) >= altEps) return false; + } + return true; + }, + { lat: expectedLat, lng: expectedLng, eps, alt: expectedAlt, altEps }, + { timeout: timeoutMs } + ); +} + +/** Snapshot camera + mode + selection + URL hash from the live page. + * `selectedPid` and `selectedH3` are intentionally read from `viewer._globeState` + * — that's the canonical URL-selection backing state today. If a future refactor + * renames or relocates these fields, this spec is the place to update. */ +async function snapshot(page) { + return await page.evaluate(async () => { + const v = await window._ojs?.ojsConnector?.mainModule?.value('viewer'); + const carto = v?.camera?.positionCartographic; + if (!carto) return null; + return { + url: location.href, + hash: location.hash, + lat: Cesium.Math.toDegrees(carto.latitude), + lng: Cesium.Math.toDegrees(carto.longitude), + alt: carto.height, + mode: v._globeState.mode, + selectedPid: v._globeState.selectedPid || null, + selectedH3: v._globeState.selectedH3 || null, + }; + }); +} + + +// --------------------------------------------------------------------------- +// From search-real-count.spec.js +// --------------------------------------------------------------------------- + +/** Wait until the explorer has rendered the search input. Boot sequence: + * phase1 (viewer + cluster cache) → facetFilters → search input wiring. + * The input is in the DOM from page load but only functional after wiring. */ +async function waitForSearchReady(page, ms = 60000) { + await page.locator('#sampleSearch').first().waitFor({ timeout: ms }); + await page.locator('#searchSubmitBtn').first().waitFor({ timeout: ms }); + // The search results line is created at boot too — wait for the + // facetFilters cell to settle so a search fires against a populated + // facet UI (matches what a real user would experience). + await page.waitForFunction( + () => document.querySelectorAll('#materialFilterBody input[type="checkbox"]').length > 0, + null, { timeout: ms } + ); +} + +/** Collect `isamples.search` JSON payloads from the console (matches the + * pattern used in tests/test_search_perf.py:204). */ +function attachSearchLogCollector(page) { + const captured = []; + page.on('console', (msg) => { + if (msg.type() !== 'log') return; + const text = msg.text(); + if (!text.includes('isamples.search')) return; + try { + const payload = JSON.parse(text); + if (payload?.event === 'isamples.search') captured.push(payload); + } catch { /* not a structured search log */ } + }); + return captured; +} + +async function runSearch(page, term) { + // Quarto's see-also rendering produces a duplicate #sampleSearch in the + // DOM; `document.getElementById` (which the live JS uses) resolves to + // the FIRST instance, so the test mirrors that with `.first()`. + const input = page.locator('#sampleSearch').first(); + await input.click(); + await input.press('ControlOrMeta+a'); + await input.press('Delete'); + await input.fill(term); + await page.locator('#searchSubmitBtn').first().click(); +} + +// --------------------------------------------------------------------------- +// NEW helpers (not in any existing spec) +// --------------------------------------------------------------------------- + +/** Read the current value of 'window.__searchFilter'. + * Returns the filter object '{ active, term, token, total, kind }' or null. */ +async function getSearchFilter(page) { + return page.evaluate(() => window.__searchFilter); +} + +/** Read 'viewer._globeState.selectedPid' via the OJS mainModule 'viewer' value. + * Returns the PID string, or null if none is selected. */ +async function getSelectedPid(page) { + return page.evaluate(async () => { + try { + const v = await window._ojs?.ojsConnector?.mainModule?.value('viewer'); + return v?._globeState?.selectedPid || null; + } catch { return null; } + }); +} + +module.exports = { + waitForBootReady, + waitForFacetUI, + waitForFacetCountsStable, + readFacetCounts, + heatmapState, + markerVisibility, + waitForMode, + snapshot, + waitForSearchReady, + runSearch, + getSearchFilter, + getSelectedPid, +}; diff --git a/tests/unit/explorer-utils.test.mjs b/tests/unit/explorer-utils.test.mjs new file mode 100644 index 00000000..2f4882f2 --- /dev/null +++ b/tests/unit/explorer-utils.test.mjs @@ -0,0 +1,70 @@ +// Unit tests for assets/js/explorer-utils.js (issue #249, PR3). +// Run: node --test tests/unit/ (Node built-ins only, no install) +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + escapeHtml, searchTerms, parseNum, csvParamValues, sourceUrl, readHash, +} from '../../assets/js/explorer-utils.js'; + +test('escapeHtml escapes the five HTML-significant chars; nullish -> ""', () => { + assert.equal( + escapeHtml(`O'B & C`), + '<a href="x">O'B & C</a>' + ); + assert.equal(escapeHtml(null), ''); + assert.equal(escapeHtml(undefined), ''); + assert.equal(escapeHtml(0), '0'); +}); + +test('searchTerms splits on whitespace, drops empties', () => { + assert.deepEqual(searchTerms(' hello world '), ['hello', 'world']); + assert.deepEqual(searchTerms(''), []); + assert.deepEqual(searchTerms(null), []); + assert.deepEqual(searchTerms('one'), ['one']); +}); + +test('parseNum: default for nullish/non-finite, clamps to [min,max]', () => { + assert.equal(parseNum(null, 5), 5); + assert.equal(parseNum(undefined, 7), 7); + assert.equal(parseNum('abc', 5), 5); + assert.equal(parseNum('45', 0, 0, 90), 45); + assert.equal(parseNum('100', 0, 0, 90), 90); // clamp max + assert.equal(parseNum('-5', 0, 0, 90), 0); // clamp min + assert.equal(parseNum('3.5', 0), 3.5); +}); + +test('csvParamValues: null when absent, [] when empty, trimmed non-empty list', () => { + assert.equal(csvParamValues(new URLSearchParams(''), 'x'), null); + assert.deepEqual(csvParamValues(new URLSearchParams('x='), 'x'), []); + assert.deepEqual(csvParamValues(new URLSearchParams('x=a,b, c ,'), 'x'), ['a', 'b', 'c']); +}); + +test('sourceUrl: n2t.net resolver; null for falsy', () => { + assert.equal(sourceUrl('ark:/28722/k2x'), 'https://n2t.net/ark:/28722/k2x'); + assert.equal(sourceUrl('IGSN:ABC123'), 'https://n2t.net/IGSN:ABC123'); + assert.equal(sourceUrl(''), null); + assert.equal(sourceUrl(null), null); +}); + +test('readHash: full round-trip parse', () => { + assert.deepEqual( + readHash('#v=1&lat=10&lng=20&alt=500&heading=45&pitch=-30&mode=point&pid=abc&h3=8a&heatmap=1'), + { v: 1, lat: 10, lng: 20, alt: 500, heading: 45, pitch: -30, mode: 'point', pid: 'abc', h3: '8a', heatmap: true } + ); +}); + +test('readHash: empty hash -> defaults', () => { + assert.deepEqual( + readHash(''), + { v: 0, lat: null, lng: null, alt: null, heading: 0, pitch: -90, mode: null, pid: null, h3: null, heatmap: false } + ); +}); + +test('readHash: clamps lat/lng/alt and treats heatmap!=1 as false', () => { + const h = readHash('#lat=999&lng=-999&alt=50&heatmap=0'); + assert.equal(h.lat, 90); // clamp to +90 + assert.equal(h.lng, -180); // clamp to -180 + assert.equal(h.alt, 100); // clamp to min 100 + assert.equal(h.heading, 0); // default + assert.equal(h.heatmap, false); +}); diff --git a/tests/unit/sql-builders.test.mjs b/tests/unit/sql-builders.test.mjs new file mode 100644 index 00000000..003e4770 --- /dev/null +++ b/tests/unit/sql-builders.test.mjs @@ -0,0 +1,50 @@ +// Unit tests for assets/js/sql-builders.js (issue #249, PR3). +// Run: node --test tests/unit/ (Node built-ins only, no install) +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + escSql, escapeIlikePattern, textSearchWhere, textSearchScore, +} from '../../assets/js/sql-builders.js'; + +test('escSql doubles single quotes and stringifies', () => { + assert.equal(escSql("O'Brien"), "O''Brien"); + assert.equal(escSql("plain"), "plain"); + assert.equal(escSql("''"), "''''"); + assert.equal(escSql(123), "123"); +}); + +test('escapeIlikePattern backslash-escapes LIKE metachars after SQL-escaping', () => { + assert.equal(escapeIlikePattern("100%"), "100\\%"); + assert.equal(escapeIlikePattern("a_b"), "a\\_b"); + // a single backslash becomes an escaped backslash + assert.equal(escapeIlikePattern("a\\b"), "a\\\\b"); + // single-quote doubling (escSql) happens BEFORE metachar escaping + assert.equal(escapeIlikePattern("O'Brien%"), "O''Brien\\%"); + assert.equal(escapeIlikePattern("plain"), "plain"); +}); + +test('textSearchWhere: terms AND-ed, columns OR-ed, ESCAPE clause present', () => { + assert.equal( + textSearchWhere(['cat'], ['label', 'descr']), + "(label ILIKE '%cat%' ESCAPE '\\' OR descr ILIKE '%cat%' ESCAPE '\\')" + ); + assert.equal( + textSearchWhere(['cat', 'dog'], ['label']), + "(label ILIKE '%cat%' ESCAPE '\\') AND (label ILIKE '%dog%' ESCAPE '\\')" + ); +}); + +test('textSearchScore: empty terms -> "0"; weighted CASE sum otherwise', () => { + assert.equal(textSearchScore([], [{ col: 'label', weight: 3 }]), '0'); + assert.equal( + textSearchScore(['cat'], [{ col: 'label', weight: 3 }, { col: 'descr', weight: 1 }]), + "(CASE WHEN label ILIKE '%cat%' ESCAPE '\\' THEN 3 ELSE 0 END + CASE WHEN descr ILIKE '%cat%' ESCAPE '\\' THEN 1 ELSE 0 END)" + ); +}); + +test('textSearchScore escapes terms (injection-safe)', () => { + assert.equal( + textSearchScore(["O'Brien"], [{ col: 'label', weight: 1 }]), + "(CASE WHEN label ILIKE '%O''Brien%' ESCAPE '\\' THEN 1 ELSE 0 END)" + ); +});