Skip to content

Assert the altitude prefill from /api/elevation in the E2E suite - #34

Merged
Babissimo merged 1 commit into
mainfrom
test/e2e-elevation-prefill
Sep 15, 2026
Merged

Babissimo merged 1 commit into
mainfrom
test/e2e-elevation-prefill

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

Why

SearchForm.tsx looks elevation up as soon as coordinates are typed and writes the answer into the altitude field. Nothing in this repo asserted that behaviour: SearchForm.test.tsx and this suite both stub /api/elevation, the stub's own comment says it is there so the request cannot reject noisily, and deploy/smoke-test.sh only proves the route answers. The one test that covered it lived in retina-server's Playwright suite (auto-fetches elevation when lat/lon are entered) and was deleted there in offworldlabs/retina-server#387, because since the Origin Rule flip that repo no longer serves this UI.

What

One test in frontend/e2e/tower-finder.spec.ts, in the search-form group: fill latitude and longitude, expect the altitude field to read 43, the value the file-level beforeEach mock returns. Stronger than the original, which ran unmocked and accepted either a populated field or a request having fired.

Verified

  • npm run test:e2e against the built dist: 15 passed (the 14 existing tests plus this one).
  • Mutation check: with the mock changed to 99 the test fails with Expected: "43", Received: "99", so the assertion is live.
  • npm run typecheck clean.

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

🤖 Generated with Claude Code

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>
@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown

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


Review: PR #34 — Assert altitude prefill from /api/elevation

  • Gather context (PR body, diff)
  • Read changed file and surrounding test suite
  • Verify claims (mock value, test structure)
  • Post review feedback

Summary: Small, well-scoped addition — a single new test in the "search form" describe block (frontend/e2e/tower-finder.spec.ts:168-175).

Verified:

  • The file-level beforeEach (tower-finder.spec.ts:53-61) stubs /api/elevation to return elevation_m: 43, matching the new test's expectation of "43" on the altitude field.
  • Cross-checked SearchForm.tsx:61-84: elevation lookup is debounced (400ms) via useEffect on [lat, lon], independent of form submission — consistent with the PR's claim that the field populates without clicking submit.
  • Confirmed via grep that SearchForm.test.tsx stubs /api/elevation only to prevent noisy rejections and never asserts on the resulting altitude value (as the PR description states), so this is genuinely new coverage, not a duplicate.
  • The assertion uses expect(...).toHaveValue(...), which polls/retries — appropriate for the debounced update, no manual wait/sleep needed and no flakiness risk from timing.
  • Comment style, coordinate values, and structure match the existing tests in the same describe block (e.g. tower-finder.spec.ts:98-120).

Not independently re-run: npm run test:e2e and npm run typecheck require sandboxed shell commands not permitted in this environment, so I relied on static review plus the PR author's reported verification (15/15 passing, mutation check on the mock value, clean typecheck).

Verdict: No issues found. The change is minimal, targeted, and does exactly what the PR description claims — no requested changes.

@Babissimo
Babissimo merged commit c885aa7 into main Sep 15, 2026
7 checks 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