diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b67732f0..197caf39 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,9 +120,14 @@ jobs: - name: Install the harness working-directory: harness run: npm ci - - name: Install Chromium + # All three, because improvise-xf1's acceptance criterion names Chrome, + # Firefox and Safari and `createSyncAccessHandle` is implemented + # separately by each engine. WebKit is installed even though its OPFS is + # absent today: the spec skips on a runtime probe, so the day that build + # gains OPFS the check starts running without a CI change. + - name: Install browsers working-directory: harness - run: npx playwright install --with-deps chromium + run: npx playwright install --with-deps chromium firefox webkit # Builds both site bundles, the harness's own wasm bundle, and assembles # `harness/serve` — the same tree `pages-build` publishes, plus /opfs/. - name: Build the harness diff --git a/harness/playwright.config.js b/harness/playwright.config.js index 130867c3..907ea7e7 100644 --- a/harness/playwright.config.js +++ b/harness/playwright.config.js @@ -38,7 +38,36 @@ export default defineConfig({ baseURL: "http://127.0.0.1:8099", trace: "retain-on-failure", }, - projects: [{ name: "chromium", use: { ...devices["Desktop Chrome"] } }], + // improvise-xf1's acceptance criterion names Chrome, Firefox and Safari, + // and the OPFS spec is the reason: `createSyncAccessHandle` is the one API + // this whole design rests on (gotcha 28), its sync read/write/truncate + // signatures were added later than the handle itself, and each engine + // implemented it separately. Chromium alone would be one engine's opinion. + // + // The shell spec is Chromium-only on purpose — it is about `web/app.js`'s + // own logic, which is engine-independent, and `routeWebSocket` plus the + // worker handshake are slower and flakier to no benefit across three. + // + // Result today: Chromium and Firefox run the whole battery and pass. + // **Playwright's WebKit provides no OPFS at all** — `getDirectory()` itself + // rejects with `UnknownError` — so Safari is NOT covered and xf1's + // criterion is met for two engines of three. The project is kept rather + // than deleted so that it starts running the day that build gains OPFS; + // `opfs.spec.js` skips on a runtime probe, and an unexpected skip on any + // other engine is a failure. + projects: [ + { name: "chromium", use: { ...devices["Desktop Chrome"] } }, + { + name: "firefox", + use: { ...devices["Desktop Firefox"] }, + testMatch: /opfs\.spec\.js/, + }, + { + name: "webkit", + use: { ...devices["Desktop Safari"] }, + testMatch: /opfs\.spec\.js/, + }, + ], webServer: { command: "python3 -m http.server 8099 --bind 127.0.0.1 --directory serve", url: "http://127.0.0.1:8099/index.html", diff --git a/harness/tests/opfs.spec.js b/harness/tests/opfs.spec.js index 2fe6be7e..2d675af5 100644 --- a/harness/tests/opfs.spec.js +++ b/harness/tests/opfs.spec.js @@ -32,8 +32,36 @@ const EXPECTED = [ ]; test.describe("OpfsFs in a real browser", () => { - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ page, browserName }, testInfo) => { await page.goto("/opfs/index.html"); + + // improvise-xf1's acceptance criterion names Chrome, Firefox and Safari. + // Chromium and Firefox run the whole battery. Playwright's WebKit build + // provides **no OPFS at all** — `navigator.storage.getDirectory()` itself + // rejects with `UnknownError` — so there is nothing there for `OpfsFs` to + // be right or wrong about, and Safari stays uncovered. See the hand-back + // and `playwright.config.js`. + // + // The skip is conditional on the PROBE, not on the browser name, so it + // cannot quietly swallow a real regression: if Chromium or Firefox ever + // stops providing OPFS, this fails rather than skipping. A silently + // skipped check is exactly the failure this harness exists to prevent, so + // an unexpected skip also fails. + const opfs = await page.evaluate(async () => { + try { + await navigator.storage.getDirectory(); + return "yes"; + } catch (err) { + return String(err?.name ?? err); + } + }); + if (opfs !== "yes") { + expect( + browserName, + `${browserName} lost OPFS (${opfs}) — that is a regression, not a known gap`, + ).toBe("webkit"); + testInfo.skip(true, `this WebKit build provides no OPFS (${opfs})`); + } }); // Check the checker, and do it first: a green run against a worker that diff --git a/harness/tests/shell.spec.js b/harness/tests/shell.spec.js index 0e014d87..af75a1e7 100644 --- a/harness/tests/shell.spec.js +++ b/harness/tests/shell.spec.js @@ -88,14 +88,20 @@ test.describe("the shell reports what its peer is doing", () => { // starts passing, Playwright reports "expected to fail but passed", and the // `test.fail()` line has to be deleted. It cannot land silently either way. test("close before open leaves the precise message standing", async ({ page }) => { - test.fail(true, "improvise-m18f: the close handler is unconditional on this branch"); await page.goto("/index.html?ws=127.0.0.1:9"); - // Wait for the status to settle on SOMETHING rather than for a particular - // message: waiting for "cannot reach" would retry against the value the - // page has already replaced, and waiting for "disconnected" would time - // out for the wrong reason once the fix lands. + // Everything up to here is SETUP, and it is asserted BEFORE `test.fail` + // is applied — otherwise a page that failed to load, or a status that + // never updated at all, would also be an "expected" failure and CI would + // stay green through it (Copilot, PR #24). Waiting for the status to + // settle on SOMETHING rather than on a particular message is what makes + // this arm work under both implementations: "cannot reach" would retry + // against a value the page has already replaced, and "disconnected" + // would time out for the wrong reason once the fix lands. await expect(status(page)).not.toHaveText("", { timeout: 15_000 }); await page.waitForTimeout(1_000); + + // From here on the known bug owns the outcome. + test.fail(true, "improvise-m18f: the close handler is unconditional on this branch"); await expect(status(page)).toContainText("cannot reach", { timeout: 1_000 }); }); @@ -124,13 +130,17 @@ test.describe("the shell reports what its peer is doing", () => { // timer still running. Filed rather than fixed here: `web/app.js` is not // this unit's file. test("the watchdog does not overwrite a mid-session disconnect", async ({ page }) => { - test.fail(true, "the close handler does not disarm the handshake watchdog"); await page.routeWebSocket("ws://127.0.0.1:8791/", (ws) => { setTimeout(() => ws.close(), 300); }); await page.goto("/index.html?ws=127.0.0.1:8791"); + // Setup, asserted before `test.fail` for the reason above: a socket that + // never opened would otherwise make this pass as an expected failure + // while testing nothing. await expect(status(page)).toContainText("disconnected", { timeout: 15_000 }); await page.waitForTimeout(HANDSHAKE_TIMEOUT_MS + 1_000); + + test.fail(true, "the close handler does not disarm the handshake watchdog"); await expect(status(page)).toContainText("disconnected", { timeout: 1_000 }); });