tests: extract URL helper for sub-path-safe page.goto across the suite#238
Merged
rdhyee merged 1 commit intoMay 27, 2026
Merged
Conversation
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>
7cc8aac to
5ac408f
Compare
rdhyee
added a commit
to rdhyee/isamplesorg.github.io
that referenced
this pull request
May 27, 2026
4 tasks
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
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.
Summary
Adds
tests/playwright/helpers/url.jsexposingsiteUrl(path, suffix?)andexplorerUrl(suffix?). Migrates all 8 specs intests/playwright/to use them, replacing the hand-rolledBASE_URL/EXPLORER_PATHconstants 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.jssilently 404'd. Root cause:EXPLORER_PATH = '/explorer.html'(leading slash) called viapage.goto(\${EXPLORER_PATH}${HASH}`). Playwright resolves an absolute path against the **origin** ofbaseURL, 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:
page.goto(\${BASE_URL}${EXPLORER_PATH}...`)` (4 specs)//explorer.htmlpage.goto('/explorer.html...')relying on PlaywrightbaseURL(4 specs)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
The trailing-slash strip means the helper produces the same URL whether
TEST_URLends 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_PATHconstants withconst { explorerUrl } = require('./helpers/url')(orsiteUrlforcesium-queries.spec.js, which navigates/tutorials/parquet_cesium.html).Diff is mostly mechanical:
Test plan
facet-viewport.spec.jspasses 4/4 on localhost in isolation — same as pre-PR baseline (verified before and after migration)explorer-helper.spec.jsagainst the pre-migration version — same 2/4 failures appear. Stash restored. These pre-existing failures are tracked separately from this PR.localhost, which the helper handles identically to the old code (string concat, no trailing slash to strip)Out of scope (filed separately or noted)
explorer-helper.spec.js,explorer-layout-stability.spec.js,explorer-map-overlay.spec.js, and the 14cesium-queries.spec.jstests (the latter all 404 becausedocs/tutorials/parquet_cesium.htmlisn't rendered) are not addressed by this PR.facet-viewportfailures 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.