Public: anyone can read this; the crew can change it.

A forge preview's check reports the held page's owner looks, desktop and phone, with how many screens tall each is #9510

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

This pull adds a test that a forge preview's check lists the held page (`/preview-admin/sample/held`) at desktop and phone, each with its screens-tall figure. It also adds one line to the docs.

shipped production is serving it deploy 3d07d6f

opened this

This pull adds a test that a forge preview's check lists the held page (/preview-admin/sample/held) at desktop and phone, each with its screens-tall figure. It also adds one line to the docs.

  • Test: test/check-looks-held.test.ts builds a looks file from every entry in PREVIEW_OWNER_LOOKS, named the way src/looks/service.ts names them. It then reads GET /api/v1/repos/alice/r/commits/:oid/check and asserts that desktop /preview-admin/sample/held and phone /preview-admin/sample/held both appear with screens: "2.5".
  • Fails without the page: with PREVIEW_HELD_PAGE commented out of PREVIEW_OWNER_LOOKS, the test fails.
  • Docs: docs/features/vitals-screens.md now says in one line that the held page is measured as an owner look, not a visitor look, because the visitor cap (LOOKS_MAX, 24) is spent.
  • Unchanged: forge.json, LOOKS_MAX, and everything under src/, including src/http/. The API already returned owner looks, so the commit message needs no Try: line.
  • Checks: the new test, bun x tsc --noEmit and bun scripts/gate.ts are green. I did not run bun scripts/fast.ts or the rest of the suite; the forge runs the full check on push.
  • Left out: the test writes the looks file itself, so it does not exercise the capture in src/looks/service.ts that produces it. It only proves that the API reports the owner looks the preview list names.

Closes #9508

commented

This pull adds a test that a forge preview's check lists the held page (/preview-admin/sample/held) at desktop and phone, each with its screens-tall figure. It also adds one line to the docs.

  • Test: test/check-looks-held.test.ts builds a looks file from every entry in PREVIEW_OWNER_LOOKS, named the way src/looks/service.ts names them. It then reads GET /api/v1/repos/alice/r/commits/:oid/check and asserts that desktop /preview-admin/sample/held and phone /preview-admin/sample/held both appear with screens: "2.5".
  • Fails without the page: with PREVIEW_HELD_PAGE commented out of PREVIEW_OWNER_LOOKS, the test fails.
  • Docs: docs/features/vitals-screens.md now says in one line that the held page is measured as an owner look, not a visitor look, because the visitor cap (LOOKS_MAX, 24) is spent.
  • Unchanged: forge.json, LOOKS_MAX, and everything under src/, including src/http/. The API already returned owner looks, so the commit message needs no Try: line.
  • Checks: the new test, bun x tsc --noEmit and bun scripts/gate.ts are green. I did not run bun scripts/fast.ts or the rest of the suite; the forge runs the full check on push.
  • Left out: the test writes the looks file itself, so it does not exercise the capture in src/looks/service.ts that produces it. It only proves that the API reports the owner looks the preview list names.
Approved

Test-and-docs only; src, forge.json and LOOKS_MAX untouched. The test asserts both held-page kinds are in PREVIEW_OWNER_LOOKS, so it fails if the page is removed. It then checks that the commit check API returns desktop and phone with screens 2.5. The docs line is present. Next: (1) the test seeds the looks file itself, so a later test could drive the real capture in src/looks/service.ts; (2) rename the tmpdir prefix forge-check-looks-api- to match this file.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs