Add a browser test suite, and fix the two faults it found

.github/workflows/ci.yml

@@ -39,3 +39,45 @@ jobs:
 
       - name: Check the build produced an entry point
         run: test -f dist/index.html
+
+  # The same app, driven in a real browser.
+  #
+  # Once rather than per Node version: what this checks is what the browser
+  # does with the built site, and that does not vary with the Node that built
+  # it. The build above already covers both.
+  browser:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: '22'
+          cache: npm
+
+      - name: Install
+        run: npm ci
+
+      # Only Chromium, and only what it needs to start. The suite has one
+      # browser project, so fetching the other two is a minute of download
+      # for nothing.
+      - name: Get the browser
+        run: npx playwright install --with-deps chromium
+
+      # This builds the site and serves it before running; see
+      # playwright.config.ts, which tests what ships rather than the dev
+      # server.
+      - name: Browser tests
+        run: npm run test:browser
+
+      # Traces and screenshots for whatever failed, which is the only way to
+      # see what a headless run on somebody else's machine actually did.
+      - uses: actions/upload-artifact@v4
+        if: failure()
+        with:
+          name: playwright-report
+          path: |
+            playwright-report/
+            test-results/
+          retention-days: 7

.gitignore

@@ -11,3 +11,10 @@ tools/.wk/
 # The assembled design canvas, which bundles the whole editor into one 2.5 MB
 # file. Generated from the .dc.html sources beside it, so it is not kept.
 design/**/beat-studio-*.html
+
+# What a browser test run leaves behind: the traces and screenshots kept for
+# a failure, and the report built from them. Both are about one run on one
+# machine and neither is worth keeping.
+test-results/
+playwright-report/
+blob-report/

README.md

@@ -28,19 +28,33 @@ npm run build      # type check, then build into dist/
 npm run preview    # serve the built files
 npm test           # run the tests once
 npm run test:watch # run them again whenever a file changes
+npm run test:browser # drive the built site in a real browser
 ```
 
+The browser tests need a browser to drive. `npx playwright install chromium`
+fetches the one they are pinned to. On a machine that already has a Chromium
+and cannot fetch another, point `CHROMIUM_PATH` at it instead.
+
 After a build, the `dist/` folder contains plain static files. You can host that
 folder on any web server. Paths in the build are relative, so it also works from
 a subfolder.
 
 Every push and pull request runs the type check, the tests and the build on
-both versions of Node. The workflow is at `.github/workflows/ci.yml`.
+both versions of Node, and the browser tests once. The workflow is at
+`.github/workflows/ci.yml`.
 
 ## Tests
 
-The tests run under Vitest and live beside what they test, as `*.test.ts`. Run
-them with `npm test`.
+There are two suites, and the split is not about speed. The Vitest one answers
+questions that have an answer on paper: given this curve, what kind of moment
+is it. The browser one answers questions that only a browser can be asked:
+given this stylesheet and this layout, what does the pointer look like over a
+sound.
+
+### What is a plain function of its inputs
+
+These run under Vitest and live beside what they test, as `*.test.ts`. Run them
+with `npm test`.
 
 What is tested is the part of the app that is a plain function of its inputs:
 what kind of moment a curve in the picture describes, what sound belongs on it,
@@ -70,8 +84,37 @@ when they always go up, since they are rebuilt by adding deltas and a delta is
 never written negative however wrong it is. One invented a curve to stand in
 for real measurements and tuned it until it passed.
 
-What is not tested yet: anything that draws, and the parts that reach the audio
-graph.
+### What only a browser can answer
+
+These run under Playwright, live in `test/browser/` as `*.spec.ts`, and are run
+with `npm run test:browser`. They drive the built site rather than the
+development server, because one of the faults below was found by comparing what
+shipped to the design and would not have shown up any other way.
+
+They exist because a particular kind of fault kept getting through. A tool
+cursor set on the timeline viewport, which every surface inside it overrides,
+so picking up the blade changed nothing you could see. A cursor drawn from a
+data URI that the browser accepted, reported back intact, and silently failed
+to decode. The blade itself looking for `.tl__cue`, a class that has never
+existed. A panel dropped into the empty column landing nowhere, because tearing
+down the drag collapsed that column before the drop was worked out. Six panel
+tabs overflowing their column into a scrollbar that had been switched off, so
+two panels could not be opened at all. None of those is reachable from Node,
+none is a type error, and every one of them shipped.
+
+Each test names the fault it is for. `test/browser/app.ts` holds what they all
+need: opening the app past the walkthrough, and a clip. The clip is a few
+seconds of canvas recorded through `MediaRecorder` in the page — about ninety
+kilobytes, made in under a second, and no binary in the repository that
+somebody has to take on trust.
+
+Every test here was checked the same way as the ones above, by putting the
+fault back and watching it fail, and writing them found two more: the transport
+was hiding its last three controls behind a scrollbar it had disabled, and the
+frame rate measurement that runs just after a clip loads was undoing a play
+started while it ran.
+
+What is not tested yet: the parts that reach the audio graph.
 
 ## Deploying
 
@@ -936,6 +979,7 @@ src/
       work-panel.ts the three tabs the right panel shows one at a time
   styles/          design tokens and stylesheets
 test/fixtures/     measurements taken off real clips, for the tests to read
+test/browser/      the suite that drives the built site in a real browser
 public/            files copied to the site root, which is where the icons live
 ```
 

package.json

@@ -2,7 +2,7 @@
   "name": "beat-studio",
   "version": "1.0.0",
   "private": true,
-  "description": "Toolcraft ST-88 Beat Studio \u2014 sound design for video in the browser: it reads a clip, says what belongs on each moment, and exports WAV, MP3, MIDI and a marker list that line up.",
+  "description": "Toolcraft ST-88 Beat Studio — sound design for video in the browser: it reads a clip, says what belongs on each moment, and exports WAV, MP3, MIDI and a marker list that line up.",
   "type": "module",
   "engines": {
     "node": "^20.19.0 || >=22.12.0"
@@ -12,6 +12,8 @@
     "build": "tsc --noEmit && vite build",
     "test": "vitest run",
     "test:watch": "vitest",
+    "test:browser": "playwright test",
+    "test:browser:ui": "playwright test --ui",
     "preview": "vite preview",
     "typecheck": "tsc --noEmit"
   },
@@ -21,6 +23,7 @@
     "smplr": "^1.0.0"
   },
   "devDependencies": {
+    "@playwright/test": "^1.62.1",
     "@types/node": "^26.4.0",
     "typescript": "^5.9.3",
     "vite": "^7.3.6",

playwright.config.ts

@@ -0,0 +1,107 @@
+import { defineConfig, devices } from '@playwright/test';
+
+/**
+ * A Chromium that is already on the machine, when there is one.
+ *
+ * Playwright pins an exact browser build and fetches it, which is what should
+ * happen on CI: the version in the lockfile is the version the tests ran
+ * against. Some machines already carry a Chromium of their own and cannot
+ * reach the download — a locked-down container, an offline checkout — and
+ * there the pin is the only thing standing between them and a suite that
+ * could otherwise run fine. So the path can be given, and nothing changes for
+ * anybody who does not give it.
+ */
+const ownChromium = process.env.CHROMIUM_PATH;
+
+/**
+ * The browser tests.
+ *
+ * Everything under `src/**\/*.test.ts` is a pure function measured in Node,
+ * which is the right shape for what a curve means or what a WAV header says.
+ * It is the wrong shape for the interface, and the interface is where the
+ * faults have actually been: a tool cursor that was set on the wrong element
+ * and changed nothing, a blade that matched a class name that did not exist,
+ * a drop zone that collapsed before the drop was worked out, two panel tabs
+ * sitting past the edge of a column with nothing saying so. Every one of
+ * those was found by driving a browser by hand and then forgotten, because
+ * the script that found it lived in a temporary folder.
+ *
+ * So these run against the built site rather than the dev server. The dock
+ * tab fault was found by comparing the built artifact to the design and would
+ * not have shown up any other way; testing what actually ships costs one
+ * build and removes a whole class of "worked locally".
+ */
+export default defineConfig({
+  testDir: './test/browser',
+  // A fault that only appears sometimes is still a fault, and retrying until
+  // it passes is how a flake becomes permanent. Failures are failures.
+  retries: 0,
+  fullyParallel: true,
+  // One at a time on CI, where the runner has two cores and a parallel run
+  // measuring layout gets its measurements from a machine under load.
+  workers: process.env.CI ? 1 : undefined,
+  /*
+   * On CI: annotations on the failing lines, a line per test in the log, and
+   * an HTML report kept as an artifact -- which is the only way to see what a
+   * headless run on somebody else's machine actually did.
+   */
+  reporter: process.env.CI
+    ? [['github'], ['list'], ['html', { open: 'never' }]]
+    : [['list']],
+  timeout: 30_000,
+  expect: { timeout: 10_000 },
+
+  use: {
+    baseURL: 'http://127.0.0.1:4173',
+    // Kept only for the run that failed, which is the only one anybody wants
+    // to look at.
+    trace: 'retain-on-failure',
+    screenshot: 'only-on-failure',
+  },
+
+  projects: [
+    {
+      name: 'chromium',
+      use: {
+        ...devices['Desktop Chrome'],
+        /*
+         * A fixed window, because half of what these check is measured in
+         * pixels: what is clipped at 1280, whether a tab sits past the edge
+         * of a column. A default that varies by machine would make those
+         * assertions mean different things in different places.
+         */
+        viewport: { width: 1440, height: 900 },
+        ...(ownChromium ? { launchOptions: { executablePath: ownChromium } } : {}),
+      },
+    },
+  ],
+
+  /*
+   * Built once, then served.
+   *
+   * `vite preview` refuses to start without a build, so the command does
+   * both. Reused when it is already up, so running these repeatedly while
+   * working on one does not rebuild the site every time.
+   *
+   * `--host 127.0.0.1` names the interface rather than leaving it to be
+   * worked out. Vite asks Node not to reorder what a name resolves to, so
+   * `localhost` binds to whichever address the machine's hosts file happens
+   * to list first: on a machine that has only `127.0.0.1 localhost` that is
+   * IPv4 and everything works, and on one that also has `::1 localhost` --
+   * which is the Ubuntu default, and so what CI runs on -- it binds to IPv6
+   * alone and nothing ever answers here. That cost a CI run that timed out
+   * after two minutes without starting a single test.
+   *
+   * Its output is passed through for the same reason: when the server does
+   * not come up, the timeout says only that it did not, and whatever the
+   * build or the server said about why is thrown away.
+   */
+  webServer: {
+    command: 'npm run build && npx vite preview --port 4173 --strictPort --host 127.0.0.1',
+    url: 'http://127.0.0.1:4173',
+    reuseExistingServer: !process.env.CI,
+    timeout: 120_000,
+    stdout: 'pipe',
+    stderr: 'pipe',
+  },
+});

src/sound-design-session.ts

@@ -1160,9 +1160,24 @@ export class SoundDesignSession {
       await video.play();
       const fps = await estimateFps(video);
       video.pause();
-      video.currentTime = 0;
       // Measuring the rate is not an edit, so it is not something to undo.
       this.#setProject({ ...this.project, fps }, '', false);
+
+      /*
+       * Unless somebody started playing while this was going on.
+       *
+       * The clip is announced as ready before the rate is measured, and the
+       * transport comes alive with it, so pressing play inside that half
+       * second is a fair thing to do. It was answered by this method finishing
+       * a moment later, stopping the clip and putting it back to the start --
+       * a play that turned into a jump to zero with nothing said about why.
+       * Whoever is driving wins.
+       */
+      if (this.playing) {
+        void video.play().catch(() => {});
+        return;
+      }
+      video.currentTime = 0;
       this.effects.onTime(0);
     } catch {
       // Leave the default rate in place; it can be set by hand.