Explorer test infrastructure + first refactor step (#249)#287
Merged
Conversation
* tests: port isamplesorg#266 sidebar-search spec fix from upstream (stale on fork main) Upstream commit bde53e2 (PR isamplesorg#269 follow-up) updated this spec when the duplicate sidebar search box was removed; fork main got the explorer.qmd change but not the spec alignment, leaving the suite red on a selector that no longer exists. Verbatim port of the upstream hunk. https://claude.ai/code/session_01R4oXhmYe3qJiGBbHwGG7zn * ci: explorer Playwright e2e smoke gate (isamplesorg#249 PR 1) Browser-level regression gate that must exist before any refactor of explorer.qmd: renders explorer.qmd (single-page, ~10s), serves docs/ with the repo's range-capable dev_server.py, and runs the new explorer-smoke.spec.js in headless Chromium on every PR touching the explorer. The smoke set asserts structural liveness only (boot without uncaught/OJS-cell errors, Cesium canvas at non-zero size, facet sidebar rows, single search box) and deliberately never waits on parquet loads from data.isamples.org, so slow data cannot flake the gate. Deeper data-dependent specs stay runnable via workflow_dispatch spec_filter and locally per tests/README.md. NOTE: the workflow ships in .github/workflows-pending/ because the automation token lacks the Workflows permission to write into .github/workflows/. Activate with: git mv .github/workflows-pending/explorer-e2e.yml .github/workflows/ Caching: Quarto .deb (pinned 1.6.42), ~/.npm (package-lock.json is gitignored, so npm ci/setup-node cache are unavailable), and Playwright browsers keyed on the resolved @playwright/test version. REFACTOR_PLAN_249.md is the paste-ready draft comment for issue isamplesorg#249 laying out the staged refactor-window sequence (PR 1 = this gate, then characterization tests, pure-logic extraction, URL/state codecs per isamplesorg#208, zoomWatcher controller splits per isamplesorg#189-when-triggered), each stage gated on this suite staying green. https://claude.ai/code/session_01R4oXhmYe3qJiGBbHwGG7zn * ci: activate explorer e2e gate — move workflow into .github/workflows/ Pushed from laptop with workflow-scoped credentials; the cloud sandbox PAT couldn't write under .github/workflows/ (no Workflows permission), so PR 1 shipped the file in .github/workflows-pending/ with this exact move documented as the activation step. Activation-note header removed. https://claude.ai/code/session_01R4oXhmYe3qJiGBbHwGG7zn Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci+tests: close Codex round-1 findings on the e2e gate 1 (MAJOR): spec_filter was expanded unquoted, letting a dispatch input smuggle Playwright flags (--pass-with-no-tests = fail-open green run), and the documented empty-filter-runs-all never worked because the || fallback swallowed it. PR/push runs are now hardcoded to the smoke set; dispatch passes the filter as one quoted argument; empty filter runs all specs. 2 (MAJOR): pageErrors was asserted only in the first serial test, so an async OJS/runtime error during the later shared-page tests passed silently. afterEach now drains and asserts the accumulator after every test. 3 (NIT): README claimed dropping the filter runs the other explorer specs — it runs ALL Playwright specs including the Cesium tutorial suite; wording fixed. Verified locally post-fix: 4 passed (6.5s) against a fresh render. https://claude.ai/code/session_01R4oXhmYe3qJiGBbHwGG7zn Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
* test(isamplesorg#249 PR2): characterization specs for explorer behaviors Consolidated helper module (tests/playwright/helpers/explorer.js) + 7 characterization tests (explorer-characterization.spec.js) pinning the behaviors Codex flagged as uncovered by the smoke gate: search-as-filter, facet-to-table coherence, deep-link (search + pid), facet hydration, detail card. Heatmap exclusivity already covered by heatmap-overlay.spec.js. Additive only; no explorer.qmd change. Runs via workflow_dispatch (data- dependent), smoke stays the PR gate. 1 test (pid deep-link) marked fixme: surfaces a latent boot race (selectedPid restored but inMapCard stays hidden). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(isamplesorg#249 PR2): tighten characterization assertions per Codex review - (a+) prove real filtering via a deterministic single-result search (Object 5404-8 -> ark:/28722/k2p55x96j), not cross-state total compares (table-total scope shifts viewport/global/filtered -> unreliable). - (a-) clear -> active=false AND filter removed (total grows past the 1 match). - (b) strict integer decrease of table total, drop flaky transient aria-busy=true. - (d1) also assert #tableMeta match summary (propagation), not just internal state. - (d2) corrected: deep-link sets selectedPid + renders #clusterSection (NOT #inMapCard, which is row-click-only); un-fixme'd, pins real behavior. - (f) known sample asserts EXACT material (Other anthropogenic material), dodging the 'Not Provided' false-pass on a failed detail query. All 7 pass against live 202608. Surfaced 2 real behavior gaps (pid deep-link doesn't open in-map card; clearing search loses viewport scoping) -> follow-ups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(isamplesorg#249 PR2): fix stale behavior-map comments (Codex nit) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…es (#12) Extract 10 closure-free functions out of the explorer.qmd OJS god-block into two unit-testable ES modules, following the source-palette.js dynamic-import precedent. No behavior change. - assets/js/sql-builders.js: escSql, escapeIlikePattern, textSearchWhere, textSearchScore - assets/js/explorer-utils.js: escapeHtml, searchTerms, parseNum, csvParamValues, sourceUrl, readHash. readHash gains a hashStr=location.hash default param so it is Node-testable; all 3 call sites are zero-arg and unchanged. Wiring: each former `function name(){...}` OJS cell is replaced in place by a single `name = _module.name` binding cell. OJS forbids two cells of the same name, so the import block and the inline definitions cannot coexist -- the swap is atomic rather than "import above, inline below". - tests/unit/{sql-builders,explorer-utils}.test.mjs: 13 node:test cases pinning exact outputs (escapeIlikePattern metachar escaping + single-quote doubling; readHash clamping and defaults). - .github/workflows/explorer-e2e.yml: add a `node --test` step as a fast PR gate ahead of the slow Quarto/Playwright steps. - _quarto.yml: add the two modules to the resources allowlist so they are copied into docs/ on render and deploy (same treatment as source-palette.js). - package.json: `npm run test:unit` convenience script. Verified behavior-neutral locally: unit (13) + smoke (4) + characterization (7) all green; explorer renders and boots with no uncaught JS errors. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explorer test infrastructure + first refactor step (#249)
Promotes three already-reviewed changes from the rdhyee fork (
main) to upstream, as a single PR with the three commits preserved. This establishes the browser-test safety net before theexplorer.qmdrefactor proceeds, and takes the first behavior-neutral extraction step.Commits
ci:explorer Playwright e2e smoke gate (Architecture: should we refactor explorer.qmd before the next big feature? (analysis + options) #249 PR 1) —.github/workflows/explorer-e2e.ymlrendersexplorer.qmd, servesdocs/with the range-capabledev_server.py, and runs a liveness smoke set (page boots, Cesium canvas, facet sidebar, search box). Data-correctness specs stay off the PR gate (run viaworkflow_dispatch). Addstests/playwright/explorer-smoke.spec.js+REFACTOR_PLAN_249.md.tests/playwright/explorer-characterization.spec.js(7[data]tests: search-as-filter, clear, facet→table coherence,?search=/&pid=deep-links, facet hydration, detail card) + a consolidatedtests/playwright/helpers/explorer.js. Pins current behavior so the refactor can't silently change it.explorer.qmdOJS block intoassets/js/sql-builders.js(escSql,escapeIlikePattern,textSearchWhere,textSearchScore) andassets/js/explorer-utils.js(escapeHtml,searchTerms,parseNum,csvParamValues,sourceUrl,readHash), following thesource-palette.jsdynamic-import precedent. Adds 13node:testunit tests + a fastnode --testCI step. Behavior-neutral.Why behavior-neutral is trustworthy here
function name(){}OJS cell is replaced in place by a singlename = _module.namebinding cell (OJS forbids duplicate cell names, so import+inline can't coexist). The only intentional code change isreadHash()→readHash(hashStr = location.hash)— all call sites are zero-arg; the default is lazy.explorer.qmdedit cherry-picked onto this upstream base with zero conflicts and the identical −79/+20 diff — i.e. the inline helpers here were byte-identical to the fork's.Verification on this branch (upstream base)
quarto render explorer.qmd✓Note
_quarto.ymlgains the two modules in itsresources:allowlist so they deploy (same assource-palette.js).🤖 Generated with Claude Code