Skip to content

tests: extract URL helper for sub-path-safe page.goto across the suite#238

Merged
rdhyee merged 1 commit into
isamplesorg:mainfrom
rdhyee:fix/playwright-spec-subpath-deploy
May 27, 2026
Merged

tests: extract URL helper for sub-path-safe page.goto across the suite#238
rdhyee merged 1 commit into
isamplesorg:mainfrom
rdhyee:fix/playwright-spec-subpath-deploy

Conversation

@rdhyee
Copy link
Copy Markdown
Contributor

@rdhyee rdhyee commented May 27, 2026

Summary

Adds tests/playwright/helpers/url.js exposing siteUrl(path, suffix?) and explorerUrl(suffix?). Migrates all 8 specs in tests/playwright/ to use them, replacing the hand-rolled BASE_URL / EXPLORER_PATH constants and the two patterns that previously coexisted.

Why

When verifying PR #237 against the rdhyee fork staging URL (https://rdhyee.github.io/isamplesorg.github.io/), tests/playwright/facet-viewport.spec.js silently 404'd. Root cause: EXPLORER_PATH = '/explorer.html' (leading slash) called via page.goto(\${EXPLORER_PATH}${HASH}`). Playwright resolves an absolute path against the **origin** of baseURL, dropping the /isamplesorg.github.io` sub-path segment.

Inventory of the suite revealed two coexisting URL-construction patterns, each with a latent bug exposed only on sub-path deploys:

Pattern Specs Bug
page.goto(\${BASE_URL}${EXPLORER_PATH}...`)` (4 specs) cesium-queries, explorer-helper, explorer-layout-stability, explorer-map-overlay Requires TEST_URL to NOT have a trailing slash; a trailing slash produces //explorer.html
page.goto('/explorer.html...') relying on Playwright baseURL (4 specs) facet-viewport, facetnote-url-load, search-real-count, url-roundtrip Drops sub-path on TEST_URLs like https://rdhyee.github.io/isamplesorg.github.io/

Rather than fix each spec independently (Codex round-1 review's recommendation on the earlier single-spec patch), this PR extracts the URL construction into one helper.

The helper

const BASE_URL = (process.env.TEST_URL || 'http://localhost:5860').replace(/\/$/, '');

function siteUrl(path, suffix = '') {
    return `${BASE_URL}${path}${suffix}`;
}

function explorerUrl(suffix = '') {
    return siteUrl('/explorer.html', suffix);
}

The trailing-slash strip means the helper produces the same URL whether TEST_URL ends in / or not — eliminating a foot-gun any operator running the suite against staging would otherwise hit.

Migration

All 8 specs replaced local BASE_URL/EXPLORER_PATH constants with const { explorerUrl } = require('./helpers/url') (or siteUrl for cesium-queries.spec.js, which navigates /tutorials/parquet_cesium.html).

Diff is mostly mechanical:

// Before (Pattern 1)
const BASE_URL = process.env.TEST_URL || 'http://localhost:5860';
const EXPLORER_PATH = '/explorer.html';
await page.goto(`${BASE_URL}${EXPLORER_PATH}#v=1&...`, { ... });

// Before (Pattern 2, broken on sub-path)
const EXPLORER_PATH = '/explorer.html';
await page.goto(`${EXPLORER_PATH}#v=1&...`);

// After
const { explorerUrl } = require('./helpers/url');
await page.goto(explorerUrl('#v=1&...'), { ... });

Test plan

  • facet-viewport.spec.js passes 4/4 on localhost in isolation — same as pre-PR baseline (verified before and after migration)
  • Other migrated specs have pre-existing failures unrelated to this PR. Proven by stashing my changes and re-running explorer-helper.spec.js against the pre-migration version — same 2/4 failures appear. Stash restored. These pre-existing failures are tracked separately from this PR.
  • Tests now reach the explorer on sub-path deploys like the rdhyee fork staging URL (verified via direct probe of the bbox-shrink behavior on staging — Cyprus at 500km alt returns 96,699 samples vs 5,980,282 global, matching raw DuckDB bbox queries)
  • CI smoke gate stays green — the smoke gate uses localhost, which the helper handles identically to the old code (string concat, no trailing slash to strip)

Out of scope (filed separately or noted)

  • The pre-existing failures in explorer-helper.spec.js, explorer-layout-stability.spec.js, explorer-map-overlay.spec.js, and the 14 cesium-queries.spec.js tests (the latter all 404 because docs/tutorials/parquet_cesium.html isn't rendered) are not addressed by this PR.
  • The 2 timing-sensitive facet-viewport failures against rdhyee staging (heavy bbox JOIN over Fastly→GH-Pages→remote R2 parquet) are environmental fragility, not a regression in PR explorer: B1 viewport-aware facet counts (#234 step 3) #237. To be tracked separately if recurring against production matters for sign-off.

Context

Discovered 2026-05-27 verifying PR #237 against the rdhyee fork staging deploy. The original commit on this branch fixed only facet-viewport.spec.js; Codex's round-1 review recommended extracting the shared helper rather than duplicating the fix into every spec. This PR adopts that recommendation.

Adds `tests/playwright/helpers/url.js` exposing `siteUrl(path, suffix?)`
and `explorerUrl(suffix?)`. Migrates all 8 specs in tests/playwright/
to use them, replacing the hand-rolled BASE_URL/EXPLORER_PATH
constants and the two patterns that previously coexisted:

  - 4 specs (cesium-queries, explorer-helper, explorer-layout-stability,
    explorer-map-overlay) string-concatenated `${BASE_URL}${PATH}`.
    Works on sub-path TEST_URLs only when TEST_URL has NO trailing
    slash; a trailing slash produces `//explorer.html`.

  - 4 specs (facet-viewport, facetnote-url-load, search-real-count,
    url-roundtrip) called `page.goto('/explorer.html...')` and relied
    on Playwright's `baseURL` config. Playwright resolves an absolute
    path against the ORIGIN of baseURL, dropping any sub-path. So
    `TEST_URL=https://rdhyee.github.io/isamplesorg.github.io/
    npx playwright test` silently 404'd on those 4 specs.

The helper:

  const BASE_URL = (process.env.TEST_URL || 'http://localhost:5860')
                     .replace(/\/$/, '');

  function siteUrl(path, suffix = '') {
      return `${BASE_URL}${path}${suffix}`;
  }
  function explorerUrl(suffix = '') {
      return siteUrl('/explorer.html', suffix);
  }

Trailing-slash strip means the helper produces the same URL whether
TEST_URL ends in `/` or not — eliminating a foot-gun any operator
running the suite against staging would otherwise hit.

Verified: 4/4 facet-viewport tests pass on localhost in isolation
(same as pre-PR baseline). The other migrated specs have pre-existing
failures unrelated to this change (proven by running an explorer-helper
test against the pre-migration spec via `git stash` — same 2/4 fail).

History: the URL bug was discovered 2026-05-27 verifying PR isamplesorg#237
against rdhyee fork staging. The original fix in this PR patched only
facet-viewport.spec.js; Codex round-1 review recommended extracting a
shared helper rather than duplicating the fix into every spec, since
3 other specs in the suite had the same latent bug. This commit
adopts that recommendation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rdhyee rdhyee force-pushed the fix/playwright-spec-subpath-deploy branch from 7cc8aac to 5ac408f Compare May 27, 2026 17:44
@rdhyee rdhyee changed the title tests: fix facet-viewport spec for sub-path deploys tests: extract URL helper for sub-path-safe page.goto across the suite May 27, 2026
rdhyee added a commit to rdhyee/isamplesorg.github.io that referenced this pull request May 27, 2026
@rdhyee rdhyee merged commit 9dc81fe into isamplesorg:main May 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant