Skip to content

Stop the E2E suite asserting against the towers hostnames - #387

Merged
Babissimo merged 1 commit into
mainfrom
fix/retire-towers-host-from-e2e
Sep 15, 2026
Merged

Babissimo merged 1 commit into
mainfrom
fix/retire-towers-host-from-e2e

Conversation

@Babissimo

@Babissimo Babissimo commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Why

Since 2026-09-14 a Cloudflare Origin Rule routes towers.retina.fm and staging-towers.retina.fm to tower-finder-service's own edge on 8443, so nothing this repo builds answers on those names. frontend/playwright.config.ts still pointed hosts.frontend at them in both environments, and frontend/e2e/tower-finder.spec.ts asserted retina-server's own bundle there.

The first staging E2E since the flip (run 34956933515, after #384 unblocked the smoke gate) came back 104 passed, 2 failed, both in tower-finder.spec.ts: the Tower Search tab and the results-table markup, neither of which the service's UI has. A red staging E2E skips deploy-production, so production has not deployed since 2026-09-14. Worse, production's hosts.frontend was towers.retina.fm, also rule-captured, so the same two failures would have tripped the e2e-prod rollback and undone the first deploy to get through.

What

  • Delete tower-finder.spec.ts rather than adjust its two selectors. Adjusting would have this repo's E2E permanently assert another repo's markup, and the way that breaks is a rolled-back production. tower-finder-service's own Playwright suite (frontend/e2e/tower-finder.spec.ts there, run in its CI) already covers eleven of the twelve tests against the UI it ships: header, form fields, no JS errors, HTML5 validation, source=auto, results table, no-results, error banner, Leaflet container, and a measured-frequencies test stronger than our toggle test. The Tower Search tab test asserts a tab bar that UI does not have. The one behaviour it did not cover, the altitude prefill from /api/elevation, is rehomed in Assert the altitude prefill from /api/elevation in the E2E suite tower-finder-service#34.
  • Drop hosts.frontend from the config and make hosts.map the suite's baseURL. The key named "the tower finder frontend", which is now a host this repo does not serve; map is the frontend/dist vhost every environment still has. No spec uses a relative page.goto, so baseURL only ever resolves a relative URL.
  • Move the /api/config check in api.spec.ts to the map host. That vhost includes snippets/towers-proxy.conf, so the request exercises our own proxy; on the towers host it would have been answered by the service's edge and proved nothing about this repo.

Verified

  • npm run typecheck:e2e clean; pre-commit run --all-files clean.
  • curl to /api/config on staging-map.retina.fm and map.retina.fm: 200, JSON, and POST /api/geocode 404 on staging-map (the discriminator: retina-server still owns that vhost).
  • Full suite run locally against staging and against production (the production suite is read-only by design: node registration, the synthetic map and the admin surface all skip there). Results below.
Suite (local, no RADAR_API_KEY, no Access token) passed failed skipped
E2E_ENV=staging 41 2 54
E2E_ENV=prod 23 1 73

The skips are nodes.spec.ts, which registers nothing without the key. The failures are all in dashboard.spec.ts, which reads none of the changed keys:

  • no JavaScript errors on login page load (both environments): waitForLoadState("networkidle") timing out under four parallel workers. It passes alone in ~2 s on both environments, and CI runs it with two retries.
  • the admin vhost renders the admin console (staging): the Cloudflare Access login page, because the service token is a CI secret. The config's accessHeaders comment describes exactly this.

Nothing on the towers hostnames is asserted any more, so the two rule-captured failures from run 34956933515 cannot recur, on staging or on production.

Not in this PR

The Tower Search surface in frontend/src/App.tsx is now unreachable on every deployed environment: frontend/dist is served on the towers, map and testmap vhosts only, and the map ones hide the tab via isMapDomain. Retiring it, and HOST_MAIN with it, is the cleanup the claude-shared ADR 2026-09-07-tower-search-ui-location.md describes in its §6; tracked on the ticket. The production smoke in ci.yml still probes towers.retina.fm for GET / and the seam; those pass today because the service answers, but they are the same another-repo coupling #384 removed from staging.

ClickUp Playwright's frontend host points at the rule-captured towers hostnames.

🤖 Generated with Claude Code

Since 2026-09-14 a Cloudflare Origin Rule routes towers.retina.fm and
staging-towers.retina.fm to tower-finder-service's own edge, so nothing
this repo builds answers there. tower-finder.spec.ts was written against
retina-server's bundle, and two of its assertions (the Tower Search tab,
the results-table markup) fail against the service's UI. On staging that
is a red E2E, which skips deploy-production; on production the same two
failures would trip the E2E rollback and undo the deploy.

The spec goes rather than being adjusted. Adjusting it would have this
repo's E2E permanently assert another repo's markup, and the way that
breaks is a rolled-back production. tower-finder-service's own Playwright
suite already covers everything the file tested, against its own UI; the
one test with no counterpart there asserts a tab bar that UI does not
have.

hosts.frontend goes with it. It named "the tower finder frontend", which
after the flip is a host this repo does not serve, and it was also the
suite's baseURL. hosts.map replaces it: the frontend/dist vhost every
environment still has. The only other reader, the /api/config check in
api.spec.ts, moves to the map host, where the route is served through
our own towers-proxy.conf include; verified 200 on both staging-map and
map.retina.fm.

ClickUp 123zgec2wb0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown

Claude finished @Babissimo's task in 1m 52s —— View job


Reviewing PR #387

  • Read CLAUDE.md and gather PR context
  • Inspect the diff (api.spec.ts, deleted tower-finder.spec.ts, playwright.config.ts)
  • Verify claims against the rest of the repo (nginx vhost config, CI workflow, smoke test)
  • Post review

Review

Verified against the repo, not just the PR description:

  • hosts.map does carry snippets/towers-proxy.conf. deploy/nginx/nginx.conf.template:154,173 shows HOST_MAP's server block includes that snippet, so moving the /api/config assertion in frontend/e2e/api.spec.ts off hosts.frontend onto hosts.map does exercise this repo's own proxy, as the updated comment claims.
  • No dangling references. Grepped the whole repo for hosts.frontend, tower-finder.spec, staging-towers/towers.retina.fm inside frontend/, and BASE inside frontend/src — nothing left pointing at the removed key or the deleted spec. hosts.map is a non-nullable string in all three HOSTS entries (staging/prod/local), so baseURL: hosts.map can't resolve to null the way hosts.testmap/hosts.admin could have.
  • Proxy coverage for /api/towers isn't lost by deleting the spec. deploy/staging-smoke-test.sh:307-310 already probes ${BASE_URL}/api/towers, ${MAP_URL}/api/towers, etc. against the tower-finder-service seam independently of the deleted Playwright file, matching the PR body's claim that this routing was never covered by tower-finder.spec.ts in the first place (every test there stubbed /api/towers).
  • The "Not in this PR" caveat about ci.yml is accurate. .github/workflows/ci.yml:1066,1074,1090 still curls towers.retina.fm directly in the production smoke step — a live coupling to the other repo that this PR correctly scopes out rather than trying to fix here.

No correctness issues found in the diff itself. I could not re-run npm run typecheck:e2e or pre-commit run --all-files in this sandbox (command execution required approval that isn't available in this headless context), so those results are taken on the PR author's word rather than independently reproduced here — everything I could verify statically (vhost config, grep for dangling references, smoke-test coverage) checks out.

One thing worth a second pair of eyes rather than a blocker: deleting tower-finder.spec.ts outright (vs. porting any of it) means retina-server's CI no longer runs any assertion against the Tower Search UI path, relying entirely on tower-finder-service's own suite in its own CI. The PR body addresses this directly and the reasoning (this repo shouldn't assert another service's markup) is sound — just flagging it as the one design choice here that isn't purely mechanical.

Babissimo added a commit to offworldlabs/tower-finder-service that referenced this pull request Sep 15, 2026
The form looks elevation up as soon as coordinates are typed and writes
the answer into the altitude field (SearchForm.tsx, the auto-lookup
effect). Nothing asserted that: the unit tests and this suite both stub
/api/elevation, the stub's own comment says it exists only so the
request cannot reject noisily, and the deploy smoke only proves the
route is up. The one test that did cover the behaviour lived in
retina-server's suite and was deleted there in
offworldlabs/retina-server#387, since that repo no longer serves this
UI. This is that test, rehomed and made deterministic: the file-level
mock answers 43, so the field must read 43.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Babissimo
Babissimo merged commit 95d1732 into main Sep 15, 2026
27 checks passed
Babissimo added a commit to offworldlabs/tower-finder-service that referenced this pull request Sep 15, 2026
The form looks elevation up as soon as coordinates are typed and writes
the answer into the altitude field (SearchForm.tsx, the auto-lookup
effect). Nothing asserted that: the unit tests and this suite both stub
/api/elevation, the stub's own comment says it exists only so the
request cannot reject noisily, and the deploy smoke only proves the
route is up. The one test that did cover the behaviour lived in
retina-server's suite and was deleted there in
offworldlabs/retina-server#387, since that repo no longer serves this
UI. This is that test, rehomed and made deterministic: the file-level
mock answers 43, so the field must read 43.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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