diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0015f78a..09c7887a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -97,6 +97,25 @@ jobs: - name: improvise-gp run: cargo check -p improvise-gp --target wasm32-unknown-unknown + # --------------------------------------------------------------------------- + # The browser JavaScript's unit tests. Its own job, deliberately: a failure + # here is a decision `web/shell.js` or `web/serial.js` got wrong, and + # reporting that as "Browser harness" would put it behind an 80 MB browser + # download and two wasm-pack builds. It needs neither — `node --test` is in + # the runtime, and this job installs nothing at all. + js: + name: Browser JS unit tests (node --test) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v4 + with: + node-version: 22 + # The glob is the SHELL's, not node's: `web/tests/bundle/` needs a built + # wasm bundle and runs in the browser job instead. + - name: What the page decides between its boundaries + run: node --test web/tests/*.test.js + kb: name: Knowledge base consistency runs-on: ubuntu-latest @@ -169,6 +188,12 @@ jobs: # `harness/serve` — the same tree `pages-build` publishes, plus /opfs/. - name: Build the harness run: ./harness/build.sh + # The JS↔wasm boundary: every call web/app.js makes is declared by the + # bundle, with that arity. It needs the bundle and nothing else, so it + # runs here rather than in the `js` job, and before Playwright — a + # renamed export is cheaper to read here than as a blank page. + - name: The page's calls match the bundle it calls + run: node --test web/tests/bundle/*.test.js - name: Run the harness working-directory: harness run: npx playwright test @@ -243,7 +268,7 @@ jobs: pages-deploy: name: Deploy to GitHub Pages - needs: [test, wasm, browser, pages-build] + needs: [test, wasm, js, browser, pages-build] if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest permissions: diff --git a/context/code/clients.md b/context/code/clients.md index 167409fe..a054d12c 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -880,6 +880,11 @@ folded one at a time in the order they arrive (a cell update that overtakes its labels is dropped, not replayed), and the *first* frame out must be the viewport, because the server builds the snapshot at it. +Everything in this file that is a *decision* rather than a paint now lives in +`web/shell.js` — `socketUrl`, the `#status` composition, the watchdog, the +extent arithmetic and `gridPlan` — so that `node --test` can reach it without a +browser. The sections below describe those rules; the code is one file over. + ### Two transports, one client The wasm bundle is the SAME artifact either way (improvise-xks6: one envelope @@ -1074,6 +1079,11 @@ makes "frames that arrived before `start()` finished" an ordinary case of the same rule instead of a second code path with its own drain loop — and that loop had the race in it twice over, since `server` was set before it ran. +The chaining itself is `web/serial.js::serialize`, which is promise plumbing and +nothing else, so `web/tests/serial.test.js` can force the interleaving a browser +cannot be asked to produce on demand — including the negative arm, which runs +the same two handlers *without* the chain and shows them coming back swapped. + **The document is named in the worker's own URL** — `new Worker('worker.js?doc=demo.improv')` — the analogue of the socket server naming it in the `ws://` path. It is deliberately not a `ClientFrame`: the envelope pair @@ -1099,12 +1109,13 @@ at all — no `package.json`, no runner, nothing in CI running a single line of the bug-first rule could not be honoured for a change to either. **PR #21 declined to add regression tests for its close-listener fix on exactly that ground, and that decline is only honest because those assertions live here -instead.** So **a change to `web/*.js` belongs in this spec directory**, not in a -second ad-hoc surface beside it. +instead.** Playwright rather than a headless-DOM runner because what goes wrong in these files is a real Worker, a real WebSocket, real OPFS and real event ordering, none -of which a DOM shim has. It is deliberately small (improvise-vvl3): the point is +of which a DOM shim has. That still holds, and it is why no shim was added; what +changed is the **scope** — see the section below, which is now the answer to +"where does a change to `web/*.js` belong". It is deliberately small (improvise-vvl3): the point is to be able to run `storage_contract` against `OpfsFs` and to read `#status` out of a real page, not to grow a second test framework beside `cargo test`. The server root is `harness/serve`, built by `build.sh` — the assembled site plus the @@ -1164,3 +1175,81 @@ is invisible to a document picker. `opfs_root`/`make_directory` exist only in the harness: `OpfsFs` has no directory vocabulary at all — it stores one flat entry per document — so making a directory, to check that `refresh` skips one, has to happen outside it. + +--- + +## Where a change to `web/*.js` is tested + +**E2E and integration tests are rare and pin boundary contracts; unit tests +assume those contracts and test between them.** (User ruling, 2026-09-20.) So +the question is not "can this assertion be made cheaper" but "which boundary is +it about", and the browser half has four: + +| boundary | the contract | where it is pinned | +|---|---|---| +| `RenderModel` → DOM | every field the render model projects reaches an element | `web/tests/render_surface.test.js` (statically) **and** `harness/tests/contract.spec.js` (on the screen) | +| JS → wasm | the calls the page makes exist, with the arity the bundle declares, and their answers are painted | `web/tests/bundle/wasm_surface.test.js` (the surface) **and** `contract.spec.js`'s pointer test (the wiring) | +| page → peer | the handshake goes out first, a frame is answered whole, and a failure is named | `harness/tests/shell.spec.js`, `harness/tests/drill.spec.js` | +| worker → OPFS | `storage_contract`, run against a real `OpfsFs` | `harness/tests/opfs.spec.js` | + +Everything **between** those seams is `node --test web/tests`, and it may assume +the boundary holds rather than re-establishing it: transport selection from +`?ws=`, the two facts `#status` composes, the watchdog's arm and disarm, the +`frameCost`/extent arithmetic, the collapse glyph, the grid's plan, and the +worker's tail-promise ordering. None of it needs a DOM, so none of it is +Playwright's. + +**The seam that makes this possible is `web/shell.js` and `web/serial.js`**: +pure modules that **return a description** rather than take a DOM double. +`gridPlan(model)` answers every question `gridTable` used to answer while it was +creating elements — which row index a click carries when group headers are +interleaved, what a group header spans, whether a `null` group label is a name, +whether an empty edit buffer means an editor is open — and `app.js` is then a +loop that creates elements from it. Painting stays untested by Node; the +decision about *what* to paint stopped being untestable. + +### Why the four widgets were one bug + +The tile bar, the view list, the formula panel and the category sidebar +ignoring `category_panel_open` were reported as four broken features and fixed +as four. They were **one failure at one boundary**: a `RenderModel` field with +no element painting it. Four per-feature specs cost four page loads and catch +the fifth only if somebody writes a fifth spec; one assertion that *every* +projected field is painted catches it by construction. That assertion is cheap +enough to make twice, from different sides: + +- **`render_surface.test.js`** reads the `pub` fields off + `improvise-wasm-client/src/render.rs` and requires each one to be read by + `app.js` or `shell.js` — or named in `UNPAINTED` **with a reason**. It needs + no browser, no bundle and no build, so it fails in seconds. The list is not + an escape hatch: it is the debt, written down, and it currently holds + `doc_name`/`doc_dirty`/`doc_stored_as` (improvise-5jna — the fifth instance, + found by writing this test). +- **`contract.spec.js`** drives the demo document into each state that fills a + widget and asserts the element is *on the screen*. Node cannot see that, and + the moment a reduction would have to weaken it, it stays here. + +What is deliberately **not** in Playwright any more is what each key or click +*means*: `improvise_protocol::resolve` decides that and its tests are where the +meaning is pinned. One instance of "a resolved change repaints" is kept, because +the repaint is the boundary; the other seven were the same instance again. + +### Two numbers, and neither is the other + +Measured on one machine at `1af51c9` versus this branch, both on a private +`IMPROVISE_HARNESS_PORT`: + +| suite | tests | execution | whole job | +|---|---|---|---| +| Playwright, before | 29 (+3 skipped) | 36.9s | ~4m30s in CI | +| Playwright, after | 16 (+3 skipped) | 15.7s | ~4m30s in CI | +| `node --test web/tests` | 26 | 0.11s | seconds | + +**Cutting Playwright tests barely moves the CI number**, because roughly four +minutes of that job is three `wasm-pack` builds, `npm ci` and a browser +download — not test execution. The feedback-time win is the `js` job, which +needs **no Rust toolchain and no wasm bundle at all**, and that is a constraint +rather than an accident: the moment a `web/tests/*.test.js` file needs +`web/pkg/`, it inherits the four-minute build and the argument for it +evaporates. That is why the one test that does need the bundle lives in +`web/tests/bundle/` and runs in the browser job instead. diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index eca8bee0..f68ffa00 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -862,6 +862,13 @@ at all would pass by moving neither. #### harness/tests/widgets.spec.js — the file's own scope +**The file is gone; its subject is not.** All four widgets it named were one +failure at one boundary — a `RenderModel` field with no element painting it — +and that is now asserted once, from both sides, by +`web/tests/render_surface.test.js` and `harness/tests/contract.spec.js`. What +is *not* carried over is each key's meaning, which `improvise_protocol::resolve` +decides and tests. Everything below is the history of the bug, and still true. + A user reported three things as broken: "I can't expand categories, and the Tile + View UIs don't work." None of the three was a regression. Driving the assembled site under Chromium separated them, and the separation is the point: @@ -1047,6 +1054,13 @@ this one is only meaningful *because* its neighbours assert non-empty answers. #### harness/tests/pointer.spec.js — the file's own scope +**The file is gone; one test carries it.** `contract.spec.js`'s "each clickable +surface is wired to its own index" keeps what only a browser can say — the +handler is on the element, it passes *that* element's index, and the answer is +painted — against three surfaces. What each click *means* is +`improvise_protocol::resolve`'s, and its tests are named below. The six tests' +injection stands as recorded. + The user's ruling on improvise-7ybz ("build it — one entry point per widget") and improvise-wsaw ("honour the flag"), executed in a real browser. @@ -1137,3 +1151,65 @@ That control is load-bearing and not decoration: a test asserting *nothing happened* passes against a page that has stopped listening, so it presses `i` afterwards and requires `#status` to change. Without that, this file would be folklore with a green tick. + +--- + +## `web/tests/` — the Node suite, and what was injected into it + +No `###` headings here on purpose: `--check-evidence` resolves a heading to a +Rust `fn`, and a JavaScript test name under one would read as an orphan. These +are keyed by file. + +Every test below was proved able to fail by patching the code it names, +checking the patch was on disk, running the suite and reading the failure — +never by the absence of an error. Thirteen injections, thirteen failures: + +**`shell.test.js`** — `socketUrl` stops upgrading `ws://` on an https page; +`composeStatus` returns the model fact alone (which fails five tests, because +the watchdog's messages are shell facts and vanish with it); `fail()` stops +clearing the timer, so the watchdog *pre-empts* rather than being disarmed — +the bug PR #13 fixed and PR #21 half-lost; `extentOf` stops subtracting the +frame; `frameCostOf` stops counting the column-group line. + +**`grid_plan.test.js`** — the data-row index is taken from the row's position +in `rows`, so a group header consumes one (the click coordinate bug gotcha 1 +is about); the group label is tested for truthiness, so a group legitimately +named `""` draws as "in no group"; `typing` is `Boolean(model.editing)`, so an +empty edit buffer reads as no editor open. + +**`serial.test.js`** — the chain is replaced by `Promise.resolve().then(...)`, +which is what an `async` listener does: the slow first frame is overtaken. The +suite also keeps that fault as a *passing* test in its own right, asserting the +unchained pair really does come back swapped, so the ordering test cannot be +green for want of a race. + +**`render_surface.test.js`** — a `pub sparkline: String` added to `RenderModel` +is reported unpainted; deleting `paintTiles` reports `tiles` unpainted; a +`getElementById` that names an id `index.html` has not got is reported. The +first is the shape of the bug the four widgets shared, and the list it checks +against currently names `doc_name`/`doc_dirty`/`doc_stored_as` — improvise-5jna, +which this test found rather than remembered. + +**`bundle/wasm_surface.test.js`** — `client.render_model()` renamed to +`client.render_modal()` fails both its tests. It reads the generated `.d.ts`, so +a missing bundle is an assertion failure and never a skip. + +**`web/assemble.sh`** — dropping `web/shell.js` from the copy list, with the +page's own imports as the check. The first version of that guard anchored its +`sed` at `^import`, so it could not see the **multi-line** import that is the +only one it exists for, and the injection passed: the guard was written, landed, +and blind. That is what the injection was for. + +## `harness/tests/contract.spec.js` — eight injections into the served page + +Patched in `harness/serve/app/app.js`, which is what the browser loads, so no +rebuild stands between the fault and the run. The sidebar ignoring +`category_panel_open`; the tile bar, view panel, formula panel and help overlay +each never drawn; `click_cell` sending a constant row; `click_category` sending +a constant index; and the typed buffer drawn without its caret. All eight fail. + +The last one did **not** fail at first: the test asserted the `typing` class and +not the text, so a cell drawn blank passed. The assertion was strengthened until +the injection failed, which is the only reason it is worth having — and it is +the same lesson as the eleven tests above: a class is a flag, and a flag is not +a pixel. diff --git a/harness/playwright.config.js b/harness/playwright.config.js index 607879fe..f700b100 100644 --- a/harness/playwright.config.js +++ b/harness/playwright.config.js @@ -19,7 +19,7 @@ export default defineConfig({ trace: "retain-on-failure", }, // Chromium + Firefox run the OPFS battery; WebKit has no OPFS at all and is - // kept for the day it does. The shell spec is Chromium-only: clients.md. + // kept for the day it does. Every other spec is Chromium-only: clients.md. projects: [ { name: "chromium", use: { ...devices["Desktop Chrome"] } }, { diff --git a/harness/tests/contract.spec.js b/harness/tests/contract.spec.js new file mode 100644 index 00000000..f054a8a3 --- /dev/null +++ b/harness/tests/contract.spec.js @@ -0,0 +1,161 @@ +// The RenderModel→DOM boundary and the pointer's half of the JS→wasm one: the +// two contracts only a browser can establish. Why these and not more, and what +// moved to `node --test web/tests`: context/code/clients.md. + +import { test, expect } from "@playwright/test"; + +async function openApp(page) { + await page.goto("/app/index.html"); + await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 }); + await page.locator("#grid").focus(); +} + +test.describe("every field the render model projects reaches the screen", () => { + // The four widgets a user reported as "not working" — the tile bar, the view + // list, the formula panel and the category sidebar ignoring its flag — were + // ONE failure at this boundary: a projected field with no element painting + // it. This is that contract, and it is what catches the fifth. + test("the widgets the demo document fills are drawn, in the state that fills them", async ({ + page, + }) => { + await openApp(page); + + // `rows`, `col_levels`, `row_levels`: the grid itself. web/demo.improv is + // Product on row, _Measure on column (gotcha 46). + await expect(page.locator("#grid thead th", { hasText: "Revenue" })).toBeVisible(); + await expect(page.locator("#grid tbody th").first()).toHaveText("Espresso"); + await expect(page.locator("#grid tbody td").first()).not.toHaveText(""); + + // `mode`, `hint`, `view_name`, `status`: the header's four facts. + await expect(page.locator("#mode")).toHaveText("NORMAL"); + await expect(page.locator("#hint")).not.toHaveText(""); + await expect(page.locator("#view-name")).toHaveText("Plan"); + await expect(page.locator("#status")).toContainText("in-page worker"); + + // `tiles`: drawn on every frame, as the TUI draws them. The axis label is + // `Axis::short_label` — "Pag", and "·" for None, an answer not a stub. + await expect(page.locator("#tiles .tile")).toHaveCount(5); + await expect(page.locator("#tiles .tile").filter({ hasText: "Product" })).toHaveText( + "Product Row", + ); + + // `categories` + `category_panel_open`: the flag is the client's answer, + // and the sidebar was drawn always until it read it (improvise-wsaw). + await expect(page.locator("#categories")).toBeHidden(); + await page.keyboard.press("Shift+C"); + await expect(page.locator("#categories")).toBeVisible(); + await expect(page.locator("#categories li").first()).toBeVisible(); + await page.keyboard.press("Shift+C"); + await expect(page.locator("#categories")).toBeHidden(); + + // `views`: empty unless open, so the page asks no question about the mode. + await expect(page.locator("#views")).toBeHidden(); + await page.keyboard.press("Shift+V"); + await expect(page.locator("#views li")).toHaveCount(2); + await expect(page.locator("#views li.active")).toContainText("Plan"); + // The axis line is the shared `view_axis_summary`; its absence would mean + // a second derivation had crept in. + await expect(page.locator("#views li").nth(0).locator(".axes")).toHaveText( + "R:Product C:Quarter", + ); + await page.keyboard.press("Escape"); + + // `formulas`: the third of `Panel::ALL`, on the same terms. + await page.keyboard.press("Shift+F"); + await expect(page.locator("#formulas li")).toHaveText(["Profit = Revenue - Cost"]); + await page.keyboard.press("Escape"); + + // `help`, `help_page`, `help_titles`: the rows and their keys are + // `improvise_protocol::help`'s, the same ones the TUI draws. + await expect(page.locator("#help")).toBeHidden(); + await page.keyboard.press("?"); + await expect(page.locator(".help-tab.active")).toHaveText("Welcome"); + await expect(page.locator(".help-body dt").first()).toBeVisible(); + await page.keyboard.press("l"); + await expect(page.locator(".help-tab.active")).toHaveText("Navigation"); + await page.keyboard.press("Escape"); + await expect(page.locator("#help")).toBeHidden(); + + // `editing`: what has been typed, drawn into the selected cell with a + // caret. The decision that an editor is open is made in Rust. + await page.keyboard.press("i"); + await expect(page.locator("#grid td.typing")).toHaveCount(1); + // The class alone would pass against a cell drawn blank: what is on the + // screen is the buffer AND the caret. + await expect(page.locator("#grid td.typing")).toContainText("▏"); + await page.keyboard.press("Escape"); + await expect(page.locator("#grid td.typing")).toHaveCount(0); + }); + + // The other half of the same contract: a change the client resolved must + // reach the pixels, not merely the cache. One instance is enough — what + // each key MEANS is `improvise_protocol::resolve`'s, and tested there. + test("a resolved change repaints the widget it moved and the grid with it", async ({ + page, + }) => { + await openApp(page); + const product = page.locator("#tiles .tile").filter({ hasText: "Product" }); + const rowHeaders = page.locator("#grid tbody th"); + await expect(rowHeaders).toHaveText(["Espresso", "Latte", "Cold Brew", "Pastry"]); + + await page.keyboard.press("Shift+T"); + for (let i = 0; i < 3; i++) await page.keyboard.press("ArrowRight"); + await expect(product).toHaveClass(/selected/); + await page.keyboard.press("c"); + + await expect(product).toHaveText("Product Col", { timeout: 10_000 }); + // The label alone would be the bar reporting an axis the document has not + // got: the grid must have moved with it. + await expect( + page.locator("#grid thead th").filter({ hasText: "Espresso" }).first(), + ).toBeVisible(); + await expect(rowHeaders).not.toHaveText(["Espresso", "Latte", "Cold Brew", "Pastry"]); + }); +}); + +test.describe("a pointer event reaches the client and its answer is painted", () => { + // improvise-7ybz. What a click MEANS is resolved in Rust and tested in + // `improvise_protocol::resolve`; what this pins is the wiring — the handler + // is on the element, it passes THAT element's index, and the result repaints. + test("each clickable surface is wired to its own index", async ({ page }) => { + await openApp(page); + + // A grid cell: the coordinate is the data row, which group headers do not + // consume — `gridPlan` counts it and `web/tests/grid_plan.test.js` pins it. + const cells = page.locator("#grid tbody td"); + await expect(cells.nth(0)).toHaveClass(/selected/); + await cells.nth(4).click(); + await page.keyboard.press("Escape"); + await expect(cells.nth(4)).toHaveClass(/selected/); + await expect(page.locator("#grid tbody td.selected")).toHaveCount(1); + // The cursor a click leaves is the one the keyboard moves from: one + // cursor, two input paths. + await page.keyboard.press("ArrowLeft"); + await expect(cells.nth(3)).toHaveClass(/selected/); + + // A tile: selects and cycles, because a click is the whole interaction. + const product = page.locator("#tiles .tile").filter({ hasText: "Product" }); + await product.click(); + await expect(page.locator("#mode")).toHaveText("TILES"); + await expect(product).toHaveClass(/selected/); + await expect(product).toHaveText("Product Col", { timeout: 10_000 }); + await page.keyboard.press("Escape"); + + // A category row: focuses the panel, moves its cursor and activates the + // row — three things from one click, and the index must be the row's own. + await page.keyboard.press("Shift+C"); + await expect(page.locator("#categories")).toBeVisible(); + await page.keyboard.press("Escape"); + const productRow = page.locator("#categories li").filter({ hasText: "Product" }); + await expect(page.locator("#categories li.selected")).toHaveText(/_Index/); + await productRow.click(); + await expect(page.locator("#mode")).toHaveText("CATEGORIES"); + await expect(page.locator("#categories li.selected")).toHaveText(/Product/); + await expect(page.locator("#categories li.item")).toHaveText([ + "Espresso", + "Latte", + "Cold Brew", + "Pastry", + ]); + }); +}); diff --git a/harness/tests/pointer.spec.js b/harness/tests/pointer.spec.js deleted file mode 100644 index dea2fe17..00000000 --- a/harness/tests/pointer.spec.js +++ /dev/null @@ -1,140 +0,0 @@ -// The pointer: improvise-7ybz (a click resolves in Rust) and improvise-wsaw -// (the sidebar obeys its flag). History: context/test-evidence/clients.md. - -import { test, expect } from "@playwright/test"; - -async function openApp(page) { - await page.goto("/app/index.html"); - await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 }); - await page.locator("#grid").focus(); -} - -/// Open the category panel the way a user does, and leave the keyboard in -/// Normal — so a later click has a focus change to make. -async function openPanelThenLeave(page) { - await page.keyboard.press("Shift+C"); - await expect(page.locator("#categories")).toBeVisible(); - await page.keyboard.press("Escape"); - await expect(page.locator("#mode")).toHaveText("NORMAL"); - await expect(page.locator("#categories")).toBeVisible(); -} - -test.describe("the category sidebar obeys category_panel_open", () => { - // improvise-wsaw. The flag existed from the start and app.js never read it, - // so the sidebar was drawn always and `C` changed only the mode label. - test("the sidebar is hidden until C, and C hides it again", async ({ page }) => { - await openApp(page); - await expect(page.locator("#categories")).toBeHidden(); - - await page.keyboard.press("Shift+C"); - await expect(page.locator("#categories")).toBeVisible(); - await expect(page.locator("#mode")).toHaveText("CATEGORIES"); - await expect(page.locator("#categories li").first()).toBeVisible(); - - // The same key closes it, because `toggle-panel-and-focus` closes only - // when the panel is already both open AND focused. - await page.keyboard.press("Shift+C"); - await expect(page.locator("#categories")).toBeHidden(); - await expect(page.locator("#mode")).toHaveText("NORMAL"); - }); -}); - -test.describe("clicking the category panel", () => { - // improvise-7ybz. One click means three things — focus the panel, move its - // cursor, activate the row — and all three are resolved in Rust. - test("a click focuses the panel, moves the cursor and expands the row", async ({ - page, - }) => { - await openApp(page); - await openPanelThenLeave(page); - - const product = page.locator("#categories li").filter({ hasText: "Product" }); - await expect(product).toHaveText(/▶ Product \(4\)/); - await expect(page.locator("#categories li.selected")).toHaveText(/_Index/); - - await product.click(); - - // All three, and the third is what the user reported as impossible. - await expect(page.locator("#mode")).toHaveText("CATEGORIES"); - await expect(page.locator("#categories li.selected")).toHaveText(/Product/); - await expect(product).toHaveText(/▼ Product \(4\)/); - await expect(page.locator("#categories li.item")).toHaveText([ - "Espresso", - "Latte", - "Cold Brew", - "Pastry", - ]); - - // Clicking it again collapses: the click mirrors Enter, which is a toggle. - await product.click(); - await expect(page.locator("#categories li.item")).toHaveCount(0); - }); - - // The other arm of `filter-to-item`: on an ITEM row the same click filters, - // because the rule is "do what Enter does here" rather than "expand". - test("a click on an item row filters, as Enter does", async ({ page }) => { - await openApp(page); - await openPanelThenLeave(page); - - await page.locator("#categories li").filter({ hasText: "Product" }).click(); - await page.locator("#categories li.item").filter({ hasText: "Latte" }).click(); - - await expect(page.locator("#status")).toContainText("Filter: Product = Latte"); - await expect(page.locator("#categories li.selected")).toHaveText("Latte"); - }); -}); - -test.describe("clicking a tile", () => { - // The tile bar is keyboard-reachable only through `T` plus arrows; a click - // is the whole interaction, so it selects AND cycles. - test("a click selects the tile and cycles its axis", async ({ page }) => { - await openApp(page); - const product = page.locator("#tiles .tile").filter({ hasText: "Product" }); - await expect(product).toHaveText("Product Row"); - - await product.click(); - - await expect(page.locator("#mode")).toHaveText("TILES"); - await expect(product).toHaveClass(/selected/); - // Row → Column is `Axis::next`; the page does not spell the rotation. - await expect(product).toHaveText("Product Col", { timeout: 10_000 }); - // And the grid moved with it, not just the label. - await expect( - page.locator("#grid thead th").filter({ hasText: "Espresso" }).first(), - ).toBeVisible(); - }); -}); - -test.describe("clicking a grid cell", () => { - // Before improvise-7ybz a click reached only `edit()`, so the selection — - // which every keyboard command reads — stayed wherever it had been. - test("a click moves the selection, not only the editor", async ({ page }) => { - await openApp(page); - const cells = page.locator("#grid tbody td"); - // The cursor starts at the origin, so any other cell is a real move. - await expect(cells.nth(0)).toHaveClass(/selected/); - - await cells.nth(4).click(); - // demo.improv is all Records, so the click also opened an editor; close it - // without committing and the selection must have followed the pointer. - await page.keyboard.press("Escape"); - - await expect(cells.nth(4)).toHaveClass(/selected/); - await expect(page.locator("#grid tbody td.selected")).toHaveCount(1); - }); - - // The selection a click leaves is the one the keyboard then moves from — - // the property that makes the two input paths one cursor rather than two. - test("the keyboard continues from where the pointer left the cursor", async ({ - page, - }) => { - await openApp(page); - const cells = page.locator("#grid tbody td"); - await cells.nth(4).click(); - await page.keyboard.press("Escape"); - await expect(cells.nth(4)).toHaveClass(/selected/); - - await page.keyboard.press("ArrowLeft"); - await expect(cells.nth(3)).toHaveClass(/selected/); - }); -}); diff --git a/harness/tests/shell.spec.js b/harness/tests/shell.spec.js index 381c823c..a0f60fdf 100644 --- a/harness/tests/shell.spec.js +++ b/harness/tests/shell.spec.js @@ -1,29 +1,20 @@ -// The browser shell: the three assertions improvise-hwpi could not write, plus -// the watchdog. The history is in context/test-evidence/clients.md. +// The page↔peer boundary: the handshake, a frame answered whole, and what the +// page says when a peer fails. What the arm/disarm RULE is, and every other +// decision between these seams, is `node --test web/tests` — clients.md. import { test, expect } from "@playwright/test"; const status = (page) => page.locator("#status"); /// A peer that accepts the TCP connection and then refuses the WebSocket -/// upgrade — deterministic where a closed port is not (evidence file). -const NO_WEBSOCKET_HERE = "127.0.0.1:8099"; -const json = (v) => JSON.stringify(v); +/// upgrade — deterministic where a closed port is not (evidence file). It is +/// the harness's OWN server, so it follows the port the config chose. +const NO_WEBSOCKET_HERE = `127.0.0.1:${process.env.IMPROVISE_HARNESS_PORT ?? "8099"}`; -/// `web/app.js`'s own HANDSHAKE_TIMEOUT_MS. Two tests have to outlast it, and a +/// `web/app.js`'s own HANDSHAKE_TIMEOUT_MS. One test has to outlast it, and a /// number that silently disagreed with the page would pass for the wrong reason. const HANDSHAKE_TIMEOUT_MS = 8_000; -/// Record every distinct #status the page shows, not just the last one: several -/// of these are about a message being REPLACED. -function logStatus() { - window.__statusLog = []; - new MutationObserver(() => { - const text = document.getElementById("status")?.textContent ?? ""; - if (text && window.__statusLog.at(-1) !== text) window.__statusLog.push(text); - }).observe(document, { childList: true, subtree: true, characterData: true }); -} - test.describe("the shell reports what its peer is doing", () => { // improvise-hwpi. The fault is injected at the *peer*, not in the page, so // this exercises the real `on_frame` and the real catch. @@ -35,31 +26,15 @@ test.describe("the shell reports what its peer is doing", () => { await expect(status(page)).toContainText("bad frame", { timeout: 15_000 }); }); - // improvise-hwpi, plus the watchdog half on the same page load because it - // needs the same nine-second wait. See the evidence file. - test("a peer that cannot be reached is named, and the watchdog stays out of it", async ({ + // A socket that never opened did not "disconnect" (improvise-taxx): nothing + // on the close event can discriminate, so the page records `opened` itself. + // That it then STAYS is the watchdog's disarming, which is Node's now. + test("a peer that cannot be reached is named, and one that dropped is not confused with it", async ({ page, }) => { - await page.addInitScript(logStatus); await page.goto(`/app/index.html?ws=${NO_WEBSOCKET_HERE}`); - await expect(status(page)).not.toHaveText("", { timeout: 15_000 }); - await page.waitForTimeout(HANDSHAKE_TIMEOUT_MS + 1_000); + await expect(status(page)).toContainText("cannot reach", { timeout: 15_000 }); - const log = await page.evaluate(() => window.__statusLog); - expect(log.some((t) => t.includes("cannot reach")), `#status said: ${json(log)}`).toBe( - true, - ); - expect(log.some((t) => t.includes("sent no document")), `#status said: ${json(log)}`).toBe( - false, - ); - }); - - // The other arm, which must keep working when the first is fixed. It runs - // past HANDSHAKE_TIMEOUT_MS on purpose — see the evidence file. - test("a socket that opened and then dropped reads as disconnected, and stays that way", async ({ - page, - }) => { - await page.addInitScript(logStatus); // Closes on the page's first frame, so "it opened first" is true by // construction rather than by timer. await page.routeWebSocket("ws://127.0.0.1:8790/", (ws) => { @@ -68,28 +43,19 @@ test.describe("the shell reports what its peer is doing", () => { await page.goto("/app/index.html?ws=127.0.0.1:8790"); await expect(status(page)).toContainText("connected", { timeout: 15_000 }); await expect(status(page)).toContainText("disconnected", { timeout: 15_000 }); - - // Outlast the watchdog it was armed against, with margin. - await page.waitForTimeout(HANDSHAKE_TIMEOUT_MS + 2_000); - await expect(status(page)).toContainText("disconnected"); - - const log = await page.evaluate(() => window.__statusLog); - expect(log.some((t) => t.includes("sent no document")), `#status said: ${json(log)}`).toBe( - false, - ); }); - // Two bugs this file once described as live, one of which never existed in - // the merged tree: context/test-evidence/clients.md keeps that history. - - // The first-frame watchdog, found by reading: no per-transport check would - // catch it. The mock accepts the connection and never answers. + // The one assertion that needs the real timer: that the watchdog is ARMED + // after the handshake goes out. What it then says, and that a loud failure + // disarms it, are unit tests. The mock accepts and never answers. test("a peer that accepts and then says nothing trips the watchdog", async ({ page, }) => { await page.routeWebSocket("ws://127.0.0.1:8788/", () => {}); await page.goto("/app/index.html?ws=127.0.0.1:8788"); - await expect(status(page)).toContainText("sent no document", { timeout: 20_000 }); + await expect(status(page)).toContainText("sent no document", { + timeout: HANDSHAKE_TIMEOUT_MS + 12_000, + }); }); }); @@ -100,13 +66,11 @@ test.describe("the standalone deployment", () => { await page.goto("/app/index.html"); await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 }); await expect(status(page)).toContainText("in-page worker"); - // web/demo.improv: Product on row, _Measure on column (gotcha 46). - await expect(page.locator("#grid th", { hasText: "Revenue" })).toBeVisible(); await expect(page.locator("#grid td.editable").first()).toBeVisible(); }); - // What lands on screen is the HOST's value, not the string typed. What this - // deliberately does NOT cover: context/test-evidence/clients.md. + // What lands on screen is the HOST's value, not the string typed — the + // round trip through a real worker, which no unit test can stand in for. test("committing an edit sends one frame and shows the host's value", async ({ page, }) => { @@ -128,26 +92,3 @@ test.describe("the standalone deployment", () => { await expect(page.locator("#grid input")).toHaveCount(0); }); }); - -test.describe("the help overlay", () => { - // improvise-ktv1. `?` has entered AppMode::Help in the browser since the - // keymap was shared; what was missing was anything to draw. - test("? opens help, and its rows are the shared ones", async ({ page }) => { - await page.goto("/app/index.html"); - await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 }); - await expect(page.locator("#help")).toBeHidden(); - - await page.locator("#grid").focus(); - await page.keyboard.press("?"); - await expect(page.locator("#help")).toBeVisible({ timeout: 10_000 }); - // A title from improvise_protocol::help::pages(), and a keys row whose - // key came off the keymap rather than out of the page. - await expect(page.locator(".help-tab.active")).toHaveText("Welcome"); - await page.keyboard.press("l"); - await expect(page.locator(".help-tab.active")).toHaveText("Navigation"); - await expect(page.locator(".help-body dt").first()).toBeVisible(); - - await page.keyboard.press("Escape"); - await expect(page.locator("#help")).toBeHidden(); - }); -}); diff --git a/harness/tests/widgets.spec.js b/harness/tests/widgets.spec.js deleted file mode 100644 index 79b1cc8d..00000000 --- a/harness/tests/widgets.spec.js +++ /dev/null @@ -1,142 +0,0 @@ -// The three widgets a user reported as "not working": the category panel's -// expand, the tile bar and the view list. History: context/test-evidence/clients.md. - -import { test, expect } from "@playwright/test"; - -/// Open the demo document and put the keyboard where the page listens. -async function openApp(page) { - await page.goto("/app/index.html"); - await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 }); - await page.locator("#grid").focus(); -} - -test.describe("the category panel", () => { - // Expanding is resolved client-side and has always worked by KEYBOARD; the - // panel is nevertheless the one of the three that has DOM, so this pins it. - test("Enter on a category header reveals its items", async ({ page }) => { - await openApp(page); - const rows = page.locator("#categories li"); - - await page.keyboard.press("Shift+C"); - await expect(page.locator("#mode")).toHaveText("CATEGORIES"); - // web/demo.improv: _Index, _Dim, _Measure, Product, Quarter. Product is - // the first with items a user would recognise. The glyphs are - // `indicator()`'s, which is the page's ONE spelling and the TUI's too. - for (let i = 0; i < 3; i++) await page.keyboard.press("j"); - await expect(rows.filter({ hasText: "Product" })).toHaveText(/▶ Product \(4\)/); - - await page.keyboard.press("Enter"); - await expect(rows.filter({ hasText: "Product" })).toHaveText(/▼ Product \(4\)/); - await expect(page.locator("#categories li.item")).toHaveText([ - "Espresso", - "Latte", - "Cold Brew", - "Pastry", - ]); - - await page.keyboard.press("Enter"); - await expect(page.locator("#categories li.item")).toHaveCount(0); - }); -}); - -test.describe("the tile bar", () => { - // Nothing was DRAWN: TileBarCache was folded, `T` entered the mode and the - // axis keys reached the host, with no tile bar on screen to aim them at. - test("the tiles are drawn, and tile-select moves a visible cursor", async ({ - page, - }) => { - await openApp(page); - const tiles = page.locator("#tiles .tile"); - await expect(tiles).toHaveCount(5); - // Axis labels are `Axis::short_label`: three columns, so Page is "Pag", - // and None is "·" — an answer rather than a truncation (gotcha 26). - await expect(tiles.filter({ hasText: "Product" })).toHaveText("Product Row"); - await expect(tiles.filter({ hasText: "Quarter" })).toHaveText("Quarter Pag"); - await expect(tiles.filter({ hasText: "_Index" })).toHaveText("_Index ·"); - - // Out of tile select nothing is highlighted, as the TUI's widget decides. - await expect(page.locator("#tiles .tile.selected")).toHaveCount(0); - - await page.keyboard.press("Shift+T"); - await expect(page.locator("#mode")).toHaveText("TILES"); - await expect(tiles.nth(0)).toHaveClass(/selected/); - await page.keyboard.press("ArrowRight"); - await expect(tiles.nth(1)).toHaveClass(/selected/); - await expect(page.locator("#tiles .tile.selected")).toHaveCount(1); - }); - - // The axis change was reaching the host all along; what it could not do was - // show the result. Setting Product to Column must redraw the tile AND the grid. - test("setting a tile's axis is visible in the bar it was aimed at", async ({ - page, - }) => { - await openApp(page); - const product = page.locator("#tiles .tile").filter({ hasText: "Product" }); - await expect(product).toHaveText("Product Row"); - - await page.keyboard.press("Shift+T"); - for (let i = 0; i < 3; i++) await page.keyboard.press("ArrowRight"); - await expect(product).toHaveClass(/selected/); - // Product's four items are the grid's ROW headers to begin with. - const rowHeaders = page.locator("#grid tbody th"); - await expect(rowHeaders).toHaveText(["Espresso", "Latte", "Cold Brew", "Pastry"]); - - await page.keyboard.press("c"); - await expect(product).toHaveText("Product Col", { timeout: 10_000 }); - // The tile label is not the point on its own: the grid must have moved - // too, or the bar would be reporting an axis the document does not have. - // `.first()`: Product now shares the column axis with _Measure, so the - // label appears once per header level. That it appears at all is the point. - await expect( - page.locator("#grid thead th").filter({ hasText: "Espresso" }).first(), - ).toBeVisible(); - await expect(rowHeaders).not.toHaveText(["Espresso", "Latte", "Cold Brew", "Pastry"]); - }); -}); - -test.describe("the view panel", () => { - // Nothing was DRAWN: ViewListCache held both views and `V` entered ViewPanel - // mode, so Enter switched views off a cursor the user could not see. - test("V lists the views, marks the active one, and Enter switches", async ({ - page, - }) => { - await openApp(page); - await expect(page.locator("#views")).toBeHidden(); - await expect(page.locator("#view-name")).toHaveText("Plan"); - - await page.keyboard.press("Shift+V"); - await expect(page.locator("#mode")).toHaveText("VIEWS"); - await expect(page.locator("#views")).toBeVisible(); - // web/demo.improv carries two views; the axis line is the shared - // `view_axis_summary`, so its absence would be a second derivation. - await expect(page.locator("#views li")).toHaveCount(2); - await expect(page.locator("#views li").nth(0)).toContainText("Default"); - await expect(page.locator("#views li").nth(0).locator(".axes")).toHaveText( - "R:Product C:Quarter", - ); - await expect(page.locator("#views li.active")).toContainText("Plan"); - await expect(page.locator("#views li.selected")).toContainText("Default"); - - await page.keyboard.press("Enter"); - await expect(page.locator("#view-name")).toHaveText("Default", { timeout: 10_000 }); - // Switching returns to Normal but leaves the panel OPEN, exactly as - // `src/draw.rs` keeps drawing it off `view_panel_open` alone. The active - // marker is what must move, and it is the fact the list exists to carry. - await expect(page.locator("#mode")).toHaveText("NORMAL"); - await expect(page.locator("#views")).toBeVisible(); - await expect(page.locator("#views li.active")).toContainText("Default"); - }); -}); - -test.describe("the formula panel", () => { - // The third of `Panel::ALL`, equally undrawn and fixed by the same mechanism. - test("F lists the document's formulas", async ({ page }) => { - await openApp(page); - await expect(page.locator("#formulas")).toBeHidden(); - - await page.keyboard.press("Shift+F"); - await expect(page.locator("#mode")).toHaveText("FORMULAS"); - await expect(page.locator("#formulas li")).toHaveText(["Profit = Revenue - Cost"]); - await expect(page.locator("#formulas li.selected")).toHaveCount(1); - }); -}); diff --git a/web/app.js b/web/app.js index 50356864..c1fb2f15 100644 --- a/web/app.js +++ b/web/app.js @@ -2,30 +2,26 @@ // string calls, no protocol knowledge. See context/code/clients.md. import init, { Client } from "./pkg/improvise_wasm_client.js"; +import { + COL_PX, + ROW_PX, + createShell, + extentOf, + frameCostOf, + gridPlan, + indicator, + socketUrl, +} from "./shell.js"; const params = new URLSearchParams(location.search); // present => socket, absent => in-page worker. The value is a host:port, and // the scheme is NOT the caller's to get wrong: see `socketUrl`. const WS_PEER = params.get("ws"); -/** The websocket URL for `?ws=`, defaulting to TLS. Why, and when an explicit - * `ws://` is upgraded rather than honoured: context/code/clients.md. */ -function socketUrl(peer) { - const secure = location.protocol === "https:"; - if (/^wss:\/\//i.test(peer)) return peer; - if (/^ws:\/\//i.test(peer)) return secure ? peer.replace(/^ws:/i, "wss:") : peer; - return `${secure ? "wss" : "ws"}://${peer}`; -} - -const WS_URL = WS_PEER ? socketUrl(WS_PEER) : null; +const WS_URL = WS_PEER ? socketUrl(WS_PEER, location.protocol) : null; const DOC = params.get("doc") ?? "demo.improv"; const SEED = params.get("seed") ?? "demo.improv"; -// Rough character metrics, used only to turn the window size into an extent. -// Being wrong here costs a row of over- or under-fetch, nothing more. -const ROW_PX = 26; -const COL_PX = 110; - const el = { grid: document.getElementById("grid"), categories: document.getElementById("categories"), @@ -42,64 +38,38 @@ const el = { await init(); const client = new Client(); -// `#status` carries two independent facts, composed rather than overwritten: -// a repaint must not quietly erase a connection error. -let shellStatus = ""; -let modelStatus = ""; -function paintStatus() { - el.status.textContent = [shellStatus, modelStatus].filter(Boolean).join(" · "); -} -function setShellStatus(text) { - shellStatus = text; - paintStatus(); -} +// A peer that never answers the handshake, and the two facts `#status` shows. +// Both are decided in shell.js; this is the wire to the element. +const HANDSHAKE_TIMEOUT_MS = 8000; +const shell = createShell({ + onStatus: (text) => { + el.status.textContent = text; + }, + timeoutMs: HANDSHAKE_TIMEOUT_MS, +}); // What the grid's own frame costs, in whole cells, as of the last repaint — // measured off the painted model, never predicted. context/code/clients.md. let frameCost = { rows: 1, cols: 1 }; -/** The extent this window can show, in whole DATA cells. */ -function extent() { - const box = el.grid.getBoundingClientRect(); - return { - rows: Math.max(1, Math.floor(box.height / ROW_PX) - frameCost.rows), - cols: Math.max(1, Math.floor(box.width / COL_PX) - frameCost.cols), - }; -} - function sendViewport() { - const { rows, cols } = extent(); + const box = el.grid.getBoundingClientRect(); + const metrics = { rowPx: ROW_PX, colPx: COL_PX }; + const { rows, cols } = extentOf(box, frameCost, metrics); // `client.set_viewport`, NOT the associated `Client.viewport`: the same // frame, but it also RECORDS the window. Why that matters: clients.md. transport.send(client.set_viewport(rows, cols)); } -// A peer that never answers the handshake. Deliberately about the *first -// frame*, not either peer's own failure modes — context/code/clients.md. -const HANDSHAKE_TIMEOUT_MS = 8000; -let watchdog = null; -function armWatchdog(message) { - clearTimeout(watchdog); - watchdog = setTimeout(() => setShellStatus(message), HANDSHAKE_TIMEOUT_MS); -} - -// A peer that failed *loudly* has already said something more precise, so the -// watchdog must be DISARMED and not merely pre-empted (PR #13 review). -function setShellFailure(text) { - clearTimeout(watchdog); - watchdog = null; - setShellStatus(text); -} - /** One frame, one fold, in arrival order. Never batch or reorder these. */ function onFrame(text) { - clearTimeout(watchdog); + shell.sawFrame(); try { client.on_frame(text); } catch (err) { // `on_frame` returns a Result, which wasm-bindgen throws; thrown in a bare // listener it reached the console alone and the grid just stopped. - setShellStatus("bad frame — the grid has stopped updating (see console)"); + shell.setShell("bad frame — the grid has stopped updating (see console)"); console.error("improvise: frame rejected", text, err); return; } @@ -116,13 +86,13 @@ function workerTransport() { const worker = new Worker(`./worker.js?${query}`, { type: "module" }); worker.addEventListener("message", (event) => onFrame(event.data)); worker.addEventListener("messageerror", () => - setShellFailure("the worker sent a frame this page could not read"), + shell.fail("the worker sent a frame this page could not read"), ); worker.addEventListener("error", (event) => { - setShellFailure(`worker failed to start: ${event.message ?? "see console"}`); + shell.fail(`worker failed to start: ${event.message ?? "see console"}`); console.error("improvise: worker error", event); }); - setShellStatus(`in-page worker · ${DOC}`); + shell.setShell(`in-page worker · ${DOC}`); // Frames posted before the worker has finished `start()` are held by // worker.js in arrival order, so the handshake can go out immediately. return { send: (text) => worker.postMessage(text), ready: Promise.resolve() }; @@ -138,20 +108,20 @@ function socketTransport() { const ready = new Promise((resolve) => { socket.addEventListener("open", () => { opened = true; - setShellStatus(`connected · ${WS_URL}`); + shell.setShell(`connected · ${WS_URL}`); resolve(); }); }); socket.addEventListener("message", (event) => onFrame(event.data)); // Without this, a connection that never opens left #status blank and // "server not running" read exactly like "still connecting". - socket.addEventListener("error", () => setShellFailure(unreachable)); + socket.addEventListener("error", () => shell.fail(unreachable)); // A socket that never opened did not "disconnect" (improvise-taxx). BOTH arms - // route through `setShellFailure`, which disarms the watchdog — clients.md. + // route through `shell.fail`, which disarms the watchdog — clients.md. socket.addEventListener("close", () => - setShellFailure(opened ? "disconnected" : unreachable), + shell.fail(opened ? "disconnected" : unreachable), ); - setShellStatus(`connecting to ${WS_URL}…`); + shell.setShell(`connecting to ${WS_URL}…`); return { send: (text) => { if (socket.readyState === WebSocket.OPEN) socket.send(text); @@ -173,7 +143,7 @@ function repaint() { try { model = JSON.parse(client.render_model()); } catch (err) { - setShellStatus("cannot render the current state (see console)"); + shell.setShell("cannot render the current state (see console)"); console.error("improvise: render failed", err); return; } @@ -185,8 +155,7 @@ function repaint() { // itself; see `RenderModel::hint`. el.hint.textContent = model.hint; paintHelp(model); - modelStatus = model.status; - paintStatus(); + shell.setModel(model.status); el.viewName.textContent = model.view_name; el.grid.replaceChildren(gridTable(model)); // The flag is the client's answer, not the page's: `C` toggles it and the @@ -199,10 +168,7 @@ function repaint() { // What the frame costs, so the NEXT viewport announcement is honest. // Inline group ROWS are deliberately NOT counted — context/code/clients.md. - const next = { - rows: model.col_levels.length + (model.col_groups.length > 0 ? 1 : 0), - cols: model.row_levels, - }; + const next = frameCostOf(model); const changed = next.rows !== frameCost.rows || next.cols !== frameCost.cols; frameCost = next; if (changed) sendViewport(); @@ -247,32 +213,30 @@ function paintHelp(model) { el.help.replaceChildren(tabs, body); } -/** The grid, painted from a model in which every frame decision is already - * made (improvise-anr9). The page decides nothing here: which labels repeat - * and are therefore blank, how many header levels there are, which columns - * share a group and whether it is collapsed all arrive answered, by the same - * `AxisFrame` the TUI widget reads. */ +/** The grid, painted from a plan in which every decision is already made + * (improvise-anr9, and `shell.js::gridPlan`). Nothing below decides anything: + * which labels repeat and are therefore blank, how many header levels there + * are, which columns share a group and whether it is collapsed all arrive + * answered, by the same `AxisFrame` the TUI widget reads. */ function gridTable(model) { + const plan = gridPlan(model); const table = document.createElement("table"); const head = table.createTHead(); const corner = () => { const th = document.createElement("th"); - th.colSpan = model.row_levels; + th.colSpan = plan.cornerSpan; return th; }; - // The column-group line, when there is one. An empty `col_groups` means - // this grid has no column groups at all -- not that every group is blank. - if (model.col_groups.length > 0) { + // The column-group line, when there is one. An empty `groups` means this + // grid has no column groups at all -- not that every group is blank. + if (plan.groups.length > 0) { const groups = head.insertRow(); groups.appendChild(corner()); - for (const group of model.col_groups) { + for (const group of plan.groups) { const th = document.createElement("th"); th.colSpan = group.span; - // `label` is null for a run in no group; an empty STRING is a legal group - // name, so this is a null check and not a truthiness test. - th.textContent = - group.label === null ? "" : indicator(group.collapsed) + " " + group.label; + th.textContent = group.text; th.classList.add("group"); groups.appendChild(th); } @@ -280,7 +244,7 @@ function gridTable(model) { // One header line per level. A blank entry is repeat suppression, decided // in Rust -- never an absent label. - for (const level of model.col_levels) { + for (const level of plan.levels) { const tr = head.insertRow(); tr.appendChild(corner()); for (const label of level) { @@ -291,23 +255,16 @@ function gridTable(model) { } const body = table.createTBody(); - // The cell coordinates a click sends are DATA row indices and a group header - // is not a data row, so this counts rather than trusting `model.rows`. - let r = -1; - for (const row of model.rows) { + for (const row of plan.body) { const tr = body.insertRow(); if (row.kind === "group") { const th = document.createElement("th"); - th.colSpan = model.row_levels + model.col_levels[0].length; - th.textContent = indicator(row.collapsed) + " " + row.label; + th.colSpan = row.colSpan; + th.textContent = row.text; th.classList.add("group"); tr.appendChild(th); continue; } - r += 1; - // `r` is one binding shared by every closure below, so a click handler - // that captured it would fire with the LAST row's index. Snapshot it. - const dataRow = r; for (const label of row.labels) { const header = document.createElement("th"); @@ -316,39 +273,26 @@ function gridTable(model) { tr.appendChild(header); } - row.cells.forEach((cell, c) => { + for (const cell of row.cells) { const td = tr.insertCell(); - // A keyboard edit is open when `model.editing` is a string, and it holds - // what has been typed. Both facts are decided in Rust. - const typing = cell.selected && typeof model.editing === "string"; - td.textContent = typing ? model.editing + "\u258f" : cell.text; - td.classList.toggle("numeric", cell.numeric && !typing); - td.classList.toggle("error", cell.error); - td.classList.toggle("selected", cell.selected); - td.classList.toggle("typing", typing); - // `editable` is the server's answer relayed through Rust: a stored cell. - // Drill is not on the wire yet, so an aggregate is not offered. - td.classList.toggle("editable", cell.editable); + td.textContent = cell.text; + for (const [name, on] of Object.entries(cell.classes)) { + td.classList.toggle(name, on); + } // Every cell moves the selection; an editable one also opens its editor, // which is the pointer's `i`. Not repainted in that branch: `edit` has // set `editing`, and a repaint would take the away. td.addEventListener("click", () => { - const frame = client.click_cell(dataRow, c); + const frame = client.click_cell(cell.row, cell.col); if (frame) transport.send(frame); - if (cell.editable) edit(td, dataRow, c, cell.edit_text); + if (cell.editable) edit(td, cell.row, cell.col, cell.editText); else repaint(); }); - }); + } } return table; } -/** The collapse glyph. `collapsed` is the fact, decided by the view and - * carried on the frame; only this spelling is the page's (gotcha 26). */ -function indicator(collapsed) { - return collapsed ? "\u25b6" : "\u25bc"; -} - /** Open an inline editor. Committing sends a frame; the value that comes * back is the server's, never the string typed here. */ function edit(td, row, col, seed) { @@ -489,7 +433,7 @@ window.addEventListener("keydown", (e) => { await transport.ready; sendViewport(); // the handshake: the snapshot is built at this extent -armWatchdog( +shell.arm( WS_URL ? `${WS_URL} accepted the connection but sent no document` : "the in-page worker did not answer — this browser may not allow it (see console)", diff --git a/web/assemble.sh b/web/assemble.sh index e5b90cbf..f6ba1e73 100755 --- a/web/assemble.sh +++ b/web/assemble.sh @@ -44,7 +44,10 @@ cp -rf "$root/docs/casts" "$out/casts" app="$out/app" mkdir -p "$app" -cp -f "$root/web/index.html" "$root/web/app.js" "$root/web/style.css" "$root/web/worker.js" "$app/" +# shell.js and serial.js are imported by app.js and worker.js: leaving either +# out is a page that fails to load its own module graph, not a degraded one. +cp -f "$root/web/index.html" "$root/web/app.js" "$root/web/shell.js" \ + "$root/web/serial.js" "$root/web/style.css" "$root/web/worker.js" "$app/" # The document the page opens on a browser that has never seen one. # web/worker.js fetches it by the ?seed= URL and hands the bytes to Rust, # which writes them through the VFS — so leaving it out is an empty grid, not @@ -82,3 +85,16 @@ stamped "$app/app.js" "pkg/improvise_wasm_client.js?v=${token}" stamped "$app/worker.js" "worker-pkg/improvise_worker_server.js?v=${token}" stamped "$app/pkg/improvise_wasm_client.js" "improvise_wasm_client_bg.wasm?v=${token}" stamped "$app/worker-pkg/improvise_worker_server.js" "improvise_worker_server_bg.wasm?v=${token}" + +# Every relative import the two entry points make must have been copied. The +# cp list above is a second answer to "what ships", and a missing sibling is a +# 404 at module-load time, which the page cannot report. +for entry in app.js worker.js; do + deps=$(sed -n 's|.*from "\./\([A-Za-z0-9_.-]*\.js\)".*|\1|p' "$app/$entry") + for dep in $deps; do + test -f "$app/$dep" || { + echo "assemble: $entry imports ./$dep, which was not copied" >&2 + exit 1 + } + done +done diff --git a/web/package.json b/web/package.json new file mode 100644 index 00000000..213bf2e5 --- /dev/null +++ b/web/package.json @@ -0,0 +1,6 @@ +{ + "name": "web", + "private": true, + "type": "module", + "description": "The browser page. `node --test web/tests` runs what needs no browser (improvise-cr3)." +} diff --git a/web/serial.js b/web/serial.js new file mode 100644 index 00000000..92e20ad6 --- /dev/null +++ b/web/serial.js @@ -0,0 +1,19 @@ +// The one rule `web/worker.js` exists to keep: one frame at a time, in arrival +// order. No DOM, no worker globals — context/code/clients.md. + +/** Chain every arrival onto one tail promise, starting from `head`. + * + * An `async` listener does NOT serialize: the event loop is free to start the + * next message while an earlier one is suspended at an `await`, so two + * handler runs interleave and a later frame's replies can be posted before an + * earlier frame's — which the client's fold answers by DROPPING the overtaken + * update, not by replaying it. Chaining is also what makes "frames that + * arrived before `start()` finished" an ordinary case of this rule rather + * than a drain loop with the race in it twice over. */ +export function serialize(head, handler, onError) { + let tail = head.catch(() => {}); + return (value) => { + tail = tail.then(() => handler(value)).catch((err) => onError(err, value)); + return tail; + }; +} diff --git a/web/shell.js b/web/shell.js new file mode 100644 index 00000000..ced087f1 --- /dev/null +++ b/web/shell.js @@ -0,0 +1,144 @@ +// Everything `web/app.js` decides that is not painting: no DOM, no browser +// API, one `node --test` away. What belongs here: context/code/clients.md. + +/** The websocket URL for `?ws=`, defaulting to TLS. Why, and when an explicit + * `ws://` is upgraded rather than honoured: context/code/clients.md. */ +export function socketUrl(peer, pageProtocol) { + const secure = pageProtocol === "https:"; + if (/^wss:\/\//i.test(peer)) return peer; + if (/^ws:\/\//i.test(peer)) return secure ? peer.replace(/^ws:/i, "wss:") : peer; + return `${secure ? "wss" : "ws"}://${peer}`; +} + +/** `#status` carries two independent facts, composed rather than overwritten: + * a repaint must not quietly erase a connection error. */ +export function composeStatus(shell, model) { + return [shell, model].filter(Boolean).join(" · "); +} + +/** The collapse glyph. `collapsed` is the fact, decided by the view and + * carried on the frame; only this spelling is the page's (gotcha 26). */ +export function indicator(collapsed) { + return collapsed ? "▶" : "▼"; +} + +// Rough character metrics, used only to turn the window size into an extent. +// Being wrong here costs a row of over- or under-fetch, nothing more. +export const ROW_PX = 26; +export const COL_PX = 110; + +/** The extent a box of this size can show, in whole DATA cells: the frame is + * charged for first, and at least one cell is always asked for. */ +export function extentOf(box, frameCost, metrics = { rowPx: ROW_PX, colPx: COL_PX }) { + return { + rows: Math.max(1, Math.floor(box.height / metrics.rowPx) - frameCost.rows), + cols: Math.max(1, Math.floor(box.width / metrics.colPx) - frameCost.cols), + }; +} + +/** What the grid's own frame costs, in whole cells — measured off the painted + * model, never predicted. Inline group ROWS are deliberately not counted. */ +export function frameCostOf(model) { + return { + rows: model.col_levels.length + (model.col_groups.length > 0 ? 1 : 0), + cols: model.row_levels, + }; +} + +/** The grid, as a description: every decision made, nothing created. The page + * decides nothing here and neither does this — which labels repeat, how many + * header levels there are and which columns share a group all arrive + * answered, by the same `AxisFrame` the TUI widget reads (improvise-anr9). */ +export function gridPlan(model) { + const cornerSpan = model.row_levels; + const groups = model.col_groups.map((group) => ({ + span: group.span, + // `label` is null for a run in no group; an empty STRING is a legal group + // name, so this is a null check and not a truthiness test. + text: group.label === null ? "" : `${indicator(group.collapsed)} ${group.label}`, + })); + + // The cell coordinates a click sends are DATA row indices and a group header + // is not a data row, so this counts rather than trusting the row's position. + let dataRow = -1; + const body = model.rows.map((row) => { + if (row.kind === "group") { + return { + kind: "group", + colSpan: model.row_levels + model.col_levels[0].length, + text: `${indicator(row.collapsed)} ${row.label}`, + }; + } + dataRow += 1; + return { + kind: "data", + row: dataRow, + labels: row.labels, + cells: row.cells.map((cell, col) => { + // A keyboard edit is open when `model.editing` is a string, and it + // holds what has been typed. Both facts are decided in Rust. + const typing = cell.selected && typeof model.editing === "string"; + return { + row: dataRow, + col, + text: typing ? `${model.editing}▏` : cell.text, + editText: cell.edit_text, + editable: cell.editable, + classes: { + numeric: cell.numeric && !typing, + error: cell.error, + selected: cell.selected, + typing, + // `editable` is the server's answer relayed through Rust: a stored + // cell. Drill is not on the wire, so an aggregate is not offered. + editable: cell.editable, + }, + }; + }), + }; + }); + + return { cornerSpan, groups, levels: model.col_levels, body }; +} + +/** The page's two status facts and the handshake watchdog, which share a home + * because the one rule between them is that a *failure disarms the timer*. */ +export function createShell({ onStatus, timeoutMs, setTimer = setTimeout, clearTimer = clearTimeout }) { + let shell = ""; + let model = ""; + let watchdog = null; + const paint = () => onStatus(composeStatus(shell, model)); + + return { + setShell(text) { + shell = text; + paint(); + }, + setModel(text) { + model = text; + paint(); + }, + /** A peer that never answers the handshake. Deliberately about the *first + * frame*, not either peer's own failure modes — clients.md. */ + arm(message) { + clearTimer(watchdog); + watchdog = setTimer(() => { + watchdog = null; + this.setShell(message); + }, timeoutMs); + }, + /** A frame arrived, so the peer answered: the watchdog has no question + * left to ask. */ + sawFrame() { + clearTimer(watchdog); + watchdog = null; + }, + /** A peer that failed *loudly* has already said something more precise, so + * the watchdog must be DISARMED and not merely pre-empted (PR #13). */ + fail(text) { + this.sawFrame(); + this.setShell(text); + }, + armed: () => watchdog !== null, + }; +} diff --git a/web/tests/bundle/wasm_surface.test.js b/web/tests/bundle/wasm_surface.test.js new file mode 100644 index 00000000..da62adcd --- /dev/null +++ b/web/tests/bundle/wasm_surface.test.js @@ -0,0 +1,101 @@ +// The JS↔wasm boundary: every call the page makes is declared by the bundle, +// with the arity it declares. Needs `web/build.sh` to have run. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +const web = (name) => fileURLToPath(new URL(`../../${name}`, import.meta.url)); + +/// Declared methods and free functions, as `name -> [min, max]` arity. A +/// missing bundle is a FAILURE and never a skip: this file exists to be run +/// after `web/build.sh`, and a silent pass would report exactly nothing. +function declarations(dts) { + const text = readFileSync(web(dts), "utf8"); + const found = new Map(); + const decl = /(?:^|\n)\s*(?:export function |static )?([a-z_][A-Za-z0-9_]*)\(([^)]*)\)\s*:/g; + for (const [, name, params] of text.matchAll(decl)) { + const list = params.trim() === "" ? [] : params.split(","); + const required = list.filter((p) => !p.includes("?:")).length; + found.set(name, [required, list.length]); + } + assert.ok(found.size > 0, `${dts} declares nothing — was web/build.sh run?`); + return found; +} + +/// Every `.(...)` call in a source file, with how many +/// top-level arguments it passes. +function callsOn(file, receiver) { + const text = readFileSync(web(file), "utf8"); + const calls = []; + const head = new RegExp(`${receiver}\\.([a-z_][A-Za-z0-9_]*)\\(`, "g"); + for (const match of text.matchAll(head)) { + let depth = 1; + let i = match.index + match[0].length; + let commas = 0; + let body = ""; + while (depth > 0 && i < text.length) { + const ch = text[i]; + if ("([{".includes(ch)) depth += 1; + else if (")]}".includes(ch)) depth -= 1; + else if (ch === "," && depth === 1) commas += 1; + if (depth > 0) body += ch; + i += 1; + } + assert.equal(depth, 0, `unbalanced call to ${match[1]} in ${file}`); + calls.push({ name: match[1], args: body.trim() === "" ? 0 : commas + 1 }); + } + return calls; +} + +test("every client call web/app.js makes is declared, with that arity", () => { + const declared = declarations("pkg/improvise_wasm_client.d.ts"); + const calls = callsOn("app.js", "client"); + assert.ok(calls.length >= 6, `found only ${calls.length} client calls`); + for (const call of calls) { + const arity = declared.get(call.name); + assert.ok(arity, `web/app.js calls client.${call.name}, which the bundle does not declare`); + const [min, max] = arity; + assert.ok( + call.args >= min && call.args <= max, + `client.${call.name} takes ${min}..${max} arguments; app.js passes ${call.args}`, + ); + } +}); + +test("the page still reaches the client through a handful of calls", () => { + const names = new Set(callsOn("app.js", "client").map((c) => c.name)); + // The named surface, so a call added without an argument fails here and + // has to come and say so. `viewport` is deliberately absent: `set_viewport` + // is the one that records the window (gotcha 47). + assert.deepEqual( + [...names].sort(), + [ + "click_category", + "click_cell", + "click_tile", + "on_frame", + "on_key_event", + "render_model", + "set_viewport", + "typed_cell", + ], + ); +}); + +test("every worker call web/worker.js makes is declared, with that arity", () => { + const declared = declarations("worker-pkg/improvise_worker_server.d.ts"); + const calls = callsOn("worker.js", "server"); + assert.ok(calls.length >= 2, `found only ${calls.length} server calls`); + for (const call of calls) { + const arity = declared.get(call.name); + assert.ok(arity, `web/worker.js calls server.${call.name}, undeclared by the bundle`); + assert.ok( + call.args >= arity[0] && call.args <= arity[1], + `server.${call.name} takes ${arity[0]}..${arity[1]}; worker.js passes ${call.args}`, + ); + } + // `start` is a free function rather than a method, and takes the seed. + assert.deepEqual(declared.get("start"), [1, 2]); +}); diff --git a/web/tests/grid_plan.test.js b/web/tests/grid_plan.test.js new file mode 100644 index 00000000..335d71ff --- /dev/null +++ b/web/tests/grid_plan.test.js @@ -0,0 +1,110 @@ +// The grid's plan: the decisions `gridTable` used to make while creating +// elements, where a wrong answer showed up as a wrong pixel. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { gridPlan } from "../shell.js"; + +const cell = (text, extra = {}) => ({ + text, + edit_text: text, + numeric: true, + error: false, + selected: false, + editable: true, + ...extra, +}); + +/// A grid with a group header between its data rows — the shape the row +/// index has to survive. Two column levels under one group run. +function grouped() { + return { + row_levels: 2, + col_levels: [["Q1", ""], ["Revenue", "Cost"]], + col_groups: [{ span: 2, label: "2026", collapsed: false }], + editing: null, + rows: [ + { kind: "group", label: "Hot", collapsed: false }, + { kind: "data", labels: ["Hot", "Espresso"], cells: [cell("1"), cell("2")] }, + { kind: "data", labels: ["", "Latte"], cells: [cell("3"), cell("4")] }, + { kind: "group", label: "Cold", collapsed: true }, + { kind: "data", labels: ["Cold", "Cold Brew"], cells: [cell("5"), cell("6")] }, + ], + }; +} + +test("a group header consumes no data row index", () => { + const plan = gridPlan(grouped()); + const data = plan.body.filter((r) => r.kind === "data"); + assert.deepEqual(data.map((r) => r.row), [0, 1, 2]); + // And every cell in a row carries that row's index, not its position in + // `rows` — the coordinate a click sends. + assert.deepEqual( + data.at(-1).cells.map((c) => [c.row, c.col]), + [[2, 0], [2, 1]], + ); +}); + +test("a group header spans the labels and the cells together", () => { + const plan = gridPlan(grouped()); + const groups = plan.body.filter((r) => r.kind === "group"); + assert.deepEqual(groups.map((r) => r.colSpan), [4, 4]); + assert.deepEqual(groups.map((r) => r.text), ["▼ Hot", "▶ Cold"]); + assert.equal(plan.cornerSpan, 2); +}); + +test("a column run in no group is blank, and an empty name is a name", () => { + const model = grouped(); + model.col_groups = [ + { span: 1, label: null, collapsed: false }, + { span: 1, label: "", collapsed: true }, + ]; + const plan = gridPlan(model); + assert.deepEqual(plan.groups, [ + { span: 1, text: "" }, + { span: 1, text: "▶ " }, + ]); +}); + +test("the typed text replaces only the selected cell, and carries a caret", () => { + const model = grouped(); + model.editing = "12"; + model.rows[1].cells[1].selected = true; + const plan = gridPlan(model); + const row = plan.body[1]; + assert.equal(row.cells[0].text, "1"); + assert.equal(row.cells[1].text, "12▏"); + assert.equal(row.cells[1].classes.typing, true); + // A cell being typed into is not drawn as a number: the buffer is text. + assert.equal(row.cells[1].classes.numeric, false); + assert.equal(row.cells[0].classes.numeric, true); +}); + +test("a selected cell with no editor open is not typing", () => { + const model = grouped(); + model.editing = null; + model.rows[1].cells[0].selected = true; + const plan = gridPlan(model); + const first = plan.body[1].cells[0]; + assert.equal(first.classes.typing, false); + assert.equal(first.classes.selected, true); + assert.equal(first.text, "1"); +}); + +test("an empty edit buffer still means an editor is open", () => { + const model = grouped(); + model.editing = ""; + model.rows[1].cells[0].selected = true; + const plan = gridPlan(model); + assert.equal(plan.body[1].cells[0].classes.typing, true); + assert.equal(plan.body[1].cells[0].text, "▏"); +}); + +test("the editor is seeded from the stored value, not the drawn one", () => { + const model = grouped(); + model.rows[1].cells[0] = cell("20,000", { edit_text: "20000" }); + const plan = gridPlan(model); + assert.equal(plan.body[1].cells[0].text, "20,000"); + assert.equal(plan.body[1].cells[0].editText, "20000"); +}); diff --git a/web/tests/render_surface.test.js b/web/tests/render_surface.test.js new file mode 100644 index 00000000..1fe8e1b2 --- /dev/null +++ b/web/tests/render_surface.test.js @@ -0,0 +1,52 @@ +// The RenderModel→DOM boundary, statically: a projected field that nothing +// paints is the bug four widgets shared. See context/code/clients.md. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +const read = (rel) => readFileSync(fileURLToPath(new URL(rel, import.meta.url)), "utf8"); + +/// Fields the page deliberately does not read, each with the reason. A field +/// belongs here only with an argument; the list is the debt, written down. +// improvise-z754 put the title bar's facts in one shared cache and taught +// `src/draw.rs::draw_title` to fold it; this page never learned to. That is +// improvise-5jna, open, and this list is what stops it being forgotten. +const UNPAINTED = { + doc_name: "improvise-5jna: the title bar is drawn by the TUI and not here", + doc_dirty: "improvise-5jna", + doc_stored_as: "improvise-5jna", +}; + +/// The `pub` fields of `RenderModel`, read off the Rust that defines it. +function renderModelFields() { + const src = read("../../crates/improvise-wasm-client/src/render.rs"); + const start = src.indexOf("pub struct RenderModel {"); + assert.ok(start > 0, "RenderModel moved — this test reads the struct itself"); + const body = src.slice(start, src.indexOf("\n}", start)); + const fields = [...body.matchAll(/\n {4}pub ([a-z_0-9]+):/g)].map((m) => m[1]); + assert.ok(fields.length > 10, `found only ${fields.length} fields`); + return fields; +} + +test("every field the render model projects is painted, or named as debt", () => { + // Both halves of the page: `app.js` paints, `shell.js` plans the grid. + const page = read("../app.js") + read("../shell.js"); + const unpainted = renderModelFields().filter((field) => !page.includes(`model.${field}`)); + assert.deepEqual( + unpainted.sort(), + Object.keys(UNPAINTED).sort(), + "a projected field reaches no element — paint it, or add it to UNPAINTED with the reason", + ); +}); + +test("every element the page looks up exists in the page", () => { + const app = read("../app.js"); + const html = read("../index.html"); + const ids = [...app.matchAll(/getElementById\("([^"]+)"\)/g)].map((m) => m[1]); + assert.ok(ids.length >= 8, `found only ${ids.length} element lookups`); + for (const id of ids) { + assert.ok(html.includes(`id="${id}"`), `app.js looks up #${id}, which index.html has not got`); + } +}); diff --git a/web/tests/serial.test.js b/web/tests/serial.test.js new file mode 100644 index 00000000..5cfaad35 --- /dev/null +++ b/web/tests/serial.test.js @@ -0,0 +1,79 @@ +// The worker's one rule: one frame at a time, in arrival order. The bug this +// guards against is an `async` listener, which does not serialize. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { serialize } from "../serial.js"; + +const tick = (n) => new Promise((resolve) => setTimeout(resolve, n)); + +test("a slow frame does not let a later one overtake it", async () => { + const posted = []; + // The first frame suspends for longer than the second: an `async` listener + // would post B's replies while A is still awaiting. + const handler = async (frame) => { + await tick(frame === "A" ? 20 : 0); + posted.push(frame); + }; + const onMessage = serialize(Promise.resolve(), handler, () => {}); + + onMessage("A"); + await onMessage("B"); + assert.deepEqual(posted, ["A", "B"]); +}); + +test("the same interleaving without the chain is what this rules out", async () => { + const posted = []; + const listener = async (frame) => { + await tick(frame === "A" ? 20 : 0); + posted.push(frame); + }; + listener("A"); + await listener("B"); + await tick(30); + assert.deepEqual(posted, ["B", "A"]); +}); + +test("frames that arrive before startup finishes are held, not dropped", async () => { + const posted = []; + let ready; + const head = new Promise((resolve) => { + ready = resolve; + }); + const onMessage = serialize(head, (f) => posted.push(f), () => {}); + + onMessage("first"); + onMessage("second"); + assert.deepEqual(posted, [], "nothing runs before startup settles"); + ready(); + await onMessage("third"); + assert.deepEqual(posted, ["first", "second", "third"]); +}); + +test("a startup that failed still lets the chain run, so the handler can decline", async () => { + const seen = []; + const head = Promise.reject(new Error("no bundle")); + const onMessage = serialize(head, (f) => seen.push(f), () => {}); + await onMessage("frame"); + assert.deepEqual(seen, ["frame"]); +}); + +test("one bad frame does not poison the frames after it", async () => { + const posted = []; + const errors = []; + const onMessage = serialize( + Promise.resolve(), + (frame) => { + if (frame === "bad") throw new Error("rejected"); + posted.push(frame); + }, + (err, frame) => errors.push([frame, err.message]), + ); + + onMessage("good"); + onMessage("bad"); + await onMessage("later"); + assert.deepEqual(posted, ["good", "later"]); + assert.deepEqual(errors, [["bad", "rejected"]]); +}); diff --git a/web/tests/shell.test.js b/web/tests/shell.test.js new file mode 100644 index 00000000..8d229d4a --- /dev/null +++ b/web/tests/shell.test.js @@ -0,0 +1,162 @@ +// What the page decides between the boundaries: transport selection, the two +// status facts, the watchdog and the extent arithmetic. No DOM is involved. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + composeStatus, + createShell, + extentOf, + frameCostOf, + indicator, + socketUrl, +} from "../shell.js"; + +test("a bare peer takes the page's own scheme, and https never falls back", () => { + assert.equal(socketUrl("127.0.0.1:8787", "http:"), "ws://127.0.0.1:8787"); + assert.equal(socketUrl("example.test", "https:"), "wss://example.test"); + // An explicit `ws://` on an https page is UPGRADED, not honoured: the + // browser would refuse the mixed-content connection either way. + assert.equal(socketUrl("ws://example.test", "https:"), "wss://example.test"); + assert.equal(socketUrl("ws://example.test", "http:"), "ws://example.test"); + // `wss://` is honoured on both, and the match is case-insensitive. + assert.equal(socketUrl("WSS://example.test", "http:"), "WSS://example.test"); +}); + +test("a peer whose host merely starts with ws is not a scheme", () => { + assert.equal(socketUrl("wsserver.test:9000", "http:"), "ws://wsserver.test:9000"); +}); + +test("the two status facts compose, and neither can erase the other", () => { + assert.equal(composeStatus("connected", "Filter: Product = Latte"), "connected · Filter: Product = Latte"); + // An empty fact contributes nothing rather than a stray separator. + assert.equal(composeStatus("connected", ""), "connected"); + assert.equal(composeStatus("", "saved"), "saved"); + assert.equal(composeStatus("", ""), ""); +}); + +test("a repaint's model status does not overwrite a connection failure", () => { + const seen = []; + const shell = createShell({ onStatus: (t) => seen.push(t), timeoutMs: 10 }); + shell.fail("cannot reach 127.0.0.1:8787"); + shell.setModel("Filter: Product = Latte"); + assert.equal(seen.at(-1), "cannot reach 127.0.0.1:8787 · Filter: Product = Latte"); +}); + +/// A clock the test drives, so the watchdog's arming is asserted rather than +/// waited for. `fire()` runs what is pending, as the event loop would. +function fakeClock() { + let next = 1; + const pending = new Map(); + return { + setTimer: (fn) => { + const id = next++; + pending.set(id, fn); + return id; + }, + clearTimer: (id) => pending.delete(id), + fire: () => { + const due = [...pending.values()]; + pending.clear(); + for (const fn of due) fn(); + }, + count: () => pending.size, + }; +} + +test("the watchdog speaks when the peer never answers the handshake", () => { + const clock = fakeClock(); + const seen = []; + const shell = createShell({ + onStatus: (t) => seen.push(t), + timeoutMs: 8000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + shell.setShell("connecting…"); + shell.arm("sent no document"); + assert.equal(shell.armed(), true); + clock.fire(); + assert.equal(seen.at(-1), "sent no document"); +}); + +test("a loud failure DISARMS the watchdog rather than being replaced by it", () => { + const clock = fakeClock(); + const seen = []; + const shell = createShell({ + onStatus: (t) => seen.push(t), + timeoutMs: 8000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + shell.arm("sent no document"); + shell.fail("disconnected"); + assert.equal(shell.armed(), false); + assert.equal(clock.count(), 0); + clock.fire(); + // The precise message is what stands, eight seconds later and forever. + assert.equal(seen.at(-1), "disconnected"); +}); + +test("a frame that arrives is the answer the watchdog was waiting for", () => { + const clock = fakeClock(); + const seen = []; + const shell = createShell({ + onStatus: (t) => seen.push(t), + timeoutMs: 8000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + shell.setShell("connected"); + shell.arm("sent no document"); + shell.sawFrame(); + clock.fire(); + assert.equal(seen.at(-1), "connected"); +}); + +test("arming twice leaves one timer, not two", () => { + const clock = fakeClock(); + const shell = createShell({ + onStatus: () => {}, + timeoutMs: 8000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + shell.arm("first"); + shell.arm("second"); + assert.equal(clock.count(), 1); +}); + +test("the frame is charged for before the extent is announced", () => { + // 10 rows and 4 columns fit; one header level and one row-label column are + // not data, so the peer is asked for 9 by 3. + const box = { height: 10 * 26, width: 4 * 110 }; + assert.deepEqual(extentOf(box, { rows: 1, cols: 1 }), { rows: 9, cols: 3 }); +}); + +test("a window smaller than its own frame still asks for one cell", () => { + assert.deepEqual(extentOf({ height: 0, width: 0 }, { rows: 3, cols: 2 }), { + rows: 1, + cols: 1, + }); +}); + +test("the frame costs a line per column level, plus one for a group line", () => { + const model = (levels, groups, rowLevels) => ({ + col_levels: levels, + col_groups: groups, + row_levels: rowLevels, + }); + assert.deepEqual(frameCostOf(model([["a"]], [], 1)), { rows: 1, cols: 1 }); + // Two category levels on the column axis, and a group line above them. + assert.deepEqual(frameCostOf(model([["a"], ["b"]], [{ span: 1 }], 2)), { + rows: 3, + cols: 2, + }); +}); + +test("the collapse glyph points right when collapsed", () => { + assert.equal(indicator(true), "▶"); + assert.equal(indicator(false), "▼"); +}); diff --git a/web/worker.js b/web/worker.js index 5238ef52..278f9a82 100644 --- a/web/worker.js +++ b/web/worker.js @@ -2,6 +2,7 @@ // every decision in Rust. The two rules it must not break: context/code/clients.md. import init, { start } from "./worker-pkg/improvise_worker_server.js"; +import { serialize } from "./serial.js"; const params = new URLSearchParams(self.location.search); const DOC = params.get("doc") || "demo.improv"; @@ -43,21 +44,21 @@ started.catch((err) => { console.error("improvise worker: failed to start", err); }); -// One frame at a time, in arrival order: the listener is deliberately NOT -// `async` — see context/code/clients.md. -let tail = started.catch(() => {}); +// One frame at a time, in arrival order, and the listener is deliberately NOT +// `async`: the chaining is `serial.js`'s and is tested there. +const onFrame = serialize( + started, + async (data) => { + // `start()` failed. There is nothing to answer with, and the main + // thread's handshake watchdog is what tells the user. + if (!server) return; + await handle(data); + }, + // An unhandled rejection here reaches the devtools console and nothing + // else. Caught, so one bad frame does not poison every later one. + (err, data) => console.error("improvise worker: frame rejected", data, err), +); self.onmessage = (event) => { - tail = tail - .then(async () => { - // `start()` failed. There is nothing to answer with, and the main - // thread's handshake watchdog is what tells the user. - if (!server) return; - await handle(event.data); - }) - .catch((err) => { - // An unhandled rejection here reaches the devtools console and nothing - // else. Caught, so one bad frame does not poison every later one. - console.error("improvise worker: frame rejected", event.data, err); - }); + onFrame(event.data); };