diff --git a/TESTING.md b/TESTING.md index 21aaa788..369283f1 100644 --- a/TESTING.md +++ b/TESTING.md @@ -39,9 +39,12 @@ npm run --prefix tools/vendor test:coverage npm run --prefix tools/vendor test:workflows -- --project firefox --project webkit ``` -`PM_TEST_PYTHON` selects the Python executable; `PM_TEST_PORT` selects the local -port (default 8765). Set these environment variables when Python is outside PATH -or the port is occupied. Test servers are never reused. On unsupported Linux +`PM_TEST_PYTHON` selects the Python executable; set it when Python is outside +PATH. Every Playwright worker starts its own `tests/browser/server.py` on a port the +OS picks, with its own temp root and database (`tests/browser/fixtures.mjs`), so +parallel workers and concurrent runs never share state. `PM_TEST_WORKERS` sets the +worker count (default 1 under `CI`, otherwise a quarter of the CPUs, at most 8). +Test servers are never reused across runs. On unsupported Linux distributions, browser system dependencies may need manual installation. For a quick browser run, use `test:workflows -- --project chromium`. @@ -1556,7 +1559,7 @@ bytes at the same path without changing the immutable receipt. The focused acceptance command retains all four projects: ```sh -PM_TEST_PORT=8766 npm run --prefix tools/vendor test:workflows -- \ +npm run --prefix tools/vendor test:workflows -- \ workspace-history.spec.mjs workspace-visual.spec.mjs \ --project chromium --project touch --project firefox --project webkit npm run --prefix tools/vendor test:executions @@ -1649,7 +1652,7 @@ acceptance; a baseline is added only after the maintainer accepts them. Focused commands: ```sh -PM_TEST_PORT=8766 npm run --prefix tools/vendor test:workflows -- \ +npm run --prefix tools/vendor test:workflows -- \ workspace-library-editor.spec.mjs library-editor-visual.spec.mjs \ --project chromium --project touch --project firefox --project webkit npm run --prefix tools/vendor test:comfyui # library-sender.spec.mjs host regression @@ -1749,7 +1752,7 @@ mobile, the concept geometry/token/control comparison and its mutation check), ```sh node --test tests/js/workspace-vocabulary-state.test.js tests/js/workspace-state.test.js -PM_TEST_PORT=8766 npm run --prefix tools/vendor test:workflows -- \ +npm run --prefix tools/vendor test:workflows -- \ workspace-vocabulary.spec.mjs vocabulary-visual.spec.mjs focused-node.spec.mjs \ --project chromium --project touch --project firefox --project webkit ``` diff --git a/tests/browser/activity-concept.spec.mjs b/tests/browser/activity-concept.spec.mjs index e3c8f9f7..2f4ef4dd 100644 --- a/tests/browser/activity-concept.spec.mjs +++ b/tests/browser/activity-concept.spec.mjs @@ -1,5 +1,5 @@ import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; import { THEMES, seedConcept, openConcept, openProduction, diffInventory, mutate } from './concept-inventory.mjs'; // Activity-1 Task 3. The review dialog is compared with the concept's own `scan-review` ("Review metadata scan", diff --git a/tests/browser/classic-node.spec.mjs b/tests/browser/classic-node.spec.mjs index a915acd8..9eb66832 100644 --- a/tests/browser/classic-node.spec.mjs +++ b/tests/browser/classic-node.spec.mjs @@ -1,5 +1,5 @@ import { readFileSync } from 'node:fs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; test.beforeEach(async ({page, browserName}) => { diff --git a/tests/browser/concept-inventory.mjs b/tests/browser/concept-inventory.mjs index 966bd649..0bc9dfbe 100644 --- a/tests/browser/concept-inventory.mjs +++ b/tests/browser/concept-inventory.mjs @@ -2,7 +2,7 @@ // same concept selectors in the immutable concept (/__test__/canonical/workspace) and in production, which now renders // the concept's classes. The concept is never rewritten: each page's mapConcept(reference) applies only the // recorded spec-§8 deviations, each commented with its D-number, and diffInventory must then report nothing. -import {expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {expect} from './fixtures.mjs'; export {diffInventory} from './library-editor-canonical-comparison.mjs'; export const CONCEPT_URL='/__test__/canonical/workspace'; diff --git a/tests/browser/fixtures.mjs b/tests/browser/fixtures.mjs new file mode 100644 index 00000000..084d41b4 --- /dev/null +++ b/tests/browser/fixtures.mjs @@ -0,0 +1,30 @@ +// Shared `test` for the page workflows: every Playwright worker owns one +// tests/browser/server.py process on an OS-chosen port, with its own temp root +// and database. `baseURL` (and so `page`, `request` and `browser.newContext`) +// points at that worker's server. A server that dies fails the running test +// with its stderr tail, and the next test in the worker gets a fresh server. +import { test as base, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { freePort } from '../../tools/vendor/free-port.mjs'; +import { createServerSupervisor, startBrowserServer } from '../../tools/vendor/start-browser-server.mjs'; + +const SERVER_FIXTURE_TIMEOUT_MS = 90_000; + +export const test = base.extend({ + testServer: [async ({}, use) => { + const supervisor = createServerSupervisor({ start: async () => startBrowserServer({ port: await freePort() }) }); + try { + await use(supervisor); + } finally { + await supervisor.stop(); + } + }, { scope: 'worker', timeout: SERVER_FIXTURE_TIMEOUT_MS }], + workerServer: [async ({ testServer }, use) => { + const server = await testServer.ensure(); + await use(server); + const crash = testServer.crash(server); + if (crash) throw crash; + }, { auto: true, timeout: SERVER_FIXTURE_TIMEOUT_MS }], + baseURL: async ({ workerServer }, use) => use(workerServer.url), +}); + +export { expect }; diff --git a/tests/browser/focused-node.spec.mjs b/tests/browser/focused-node.spec.mjs index c99e9b38..6f9eb316 100644 --- a/tests/browser/focused-node.spec.mjs +++ b/tests/browser/focused-node.spec.mjs @@ -1,5 +1,5 @@ import {readFileSync} from 'node:fs'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; for (const variant of ['Focused', 'Split']) { diff --git a/tests/browser/full-inspector.spec.mjs b/tests/browser/full-inspector.spec.mjs index 6c6e2f9f..5c82d3e8 100644 --- a/tests/browser/full-inspector.spec.mjs +++ b/tests/browser/full-inspector.spec.mjs @@ -1,5 +1,5 @@ import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; import { seedConcept, openConcept, openProduction, SIZES } from './concept-inventory.mjs'; // D10/D20: the heading is the linked positive's title (its confirmed first line here), never the pair's // "Inspector recipe", which names the negative variant. diff --git a/tests/browser/history-canonical-comparison.mjs b/tests/browser/history-canonical-comparison.mjs index 0433d914..e7c829ca 100644 --- a/tests/browser/history-canonical-comparison.mjs +++ b/tests/browser/history-canonical-comparison.mjs @@ -1,5 +1,5 @@ import { writeFile } from 'node:fs/promises'; -import { expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { expect } from './fixtures.mjs'; // Content, row counts, and image bytes come from independent synthetic datasets. // Saved choices and actual executions are separate production sections. The concept's diff --git a/tests/browser/history-concept.spec.mjs b/tests/browser/history-concept.spec.mjs index bac5df4e..353b25d4 100644 --- a/tests/browser/history-concept.spec.mjs +++ b/tests/browser/history-concept.spec.mjs @@ -1,5 +1,5 @@ import {writeFile} from 'node:fs/promises'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; import {SIZES,THEMES,MOTIONS,seedConcept,openConcept,openProduction,inventoryOf,diffInventory,mutate,stripExact} from './concept-inventory.mjs'; diff --git a/tests/browser/images-concept.spec.mjs b/tests/browser/images-concept.spec.mjs index 6d3ffbe0..309badc5 100644 --- a/tests/browser/images-concept.spec.mjs +++ b/tests/browser/images-concept.spec.mjs @@ -1,5 +1,5 @@ import {writeFile} from 'node:fs/promises'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; import {SIZES,THEMES,MOTIONS,seedConcept,openConcept,openProduction,inventoryOf,diffInventory,mutate,stripExact,countCopy,reflow,withoutItems} from './concept-inventory.mjs'; diff --git a/tests/browser/inspector-concept.spec.mjs b/tests/browser/inspector-concept.spec.mjs index 6a7f32f0..951c833a 100644 --- a/tests/browser/inspector-concept.spec.mjs +++ b/tests/browser/inspector-concept.spec.mjs @@ -1,5 +1,5 @@ import {writeFile} from 'node:fs/promises'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; import {SIZES,THEMES,seedConcept,openConcept,openProduction,inspectorOf,diffInventory,mutate,countCopy} from './concept-inventory.mjs'; diff --git a/tests/browser/library-concept.spec.mjs b/tests/browser/library-concept.spec.mjs index afc73aee..eeab9b92 100644 --- a/tests/browser/library-concept.spec.mjs +++ b/tests/browser/library-concept.spec.mjs @@ -1,5 +1,5 @@ import {writeFile} from 'node:fs/promises'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; import {SIZES,THEMES,MOTIONS,seedConcept,openConcept,openProduction,inventoryOf,diffInventory,mutate,stripExact,countCopy,reflow} from './concept-inventory.mjs'; diff --git a/tests/browser/library-editor-visual.spec.mjs b/tests/browser/library-editor-visual.spec.mjs index e277f8ec..8b92c225 100644 --- a/tests/browser/library-editor-visual.spec.mjs +++ b/tests/browser/library-editor-visual.spec.mjs @@ -1,4 +1,4 @@ -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; // Artifact-only captures of the editor states that have no concept counterpart (archive confirmation, Show archived, diff --git a/tests/browser/pair-node.spec.mjs b/tests/browser/pair-node.spec.mjs index d1245741..1d0e4bd8 100644 --- a/tests/browser/pair-node.spec.mjs +++ b/tests/browser/pair-node.spec.mjs @@ -1,5 +1,5 @@ import {readFileSync} from 'node:fs'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; const pair={id:4,positive_text:'Library positive',negative_text:'Library negative',category:'Portrait',tags:['studio']}; diff --git a/tests/browser/prompt-bridge.spec.mjs b/tests/browser/prompt-bridge.spec.mjs index 0219dfc5..f6cdb910 100644 --- a/tests/browser/prompt-bridge.spec.mjs +++ b/tests/browser/prompt-bridge.spec.mjs @@ -1,4 +1,4 @@ -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; const moduleUrl = '/prompt_manager/js/shared/toolkit/prompt-bridge.js'; diff --git a/tests/browser/prompt-inspector.spec.mjs b/tests/browser/prompt-inspector.spec.mjs index 9ebca0ea..8841598a 100644 --- a/tests/browser/prompt-inspector.spec.mjs +++ b/tests/browser/prompt-inspector.spec.mjs @@ -1,5 +1,5 @@ import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; const workspace='/prompt_manager/workspace#/images'; const details=page=>page.getByRole('complementary',{name:'Selected image details',exact:true}); diff --git a/tests/browser/shell-concept.spec.mjs b/tests/browser/shell-concept.spec.mjs index 509bdc4a..82493110 100644 --- a/tests/browser/shell-concept.spec.mjs +++ b/tests/browser/shell-concept.spec.mjs @@ -1,5 +1,5 @@ import {writeFile} from 'node:fs/promises'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; import {SIZES,THEMES,MOTIONS,seedConcept,openConcept as openConceptPage,openProduction,inventoryOf,diffInventory,focusRingOf,hoverStateOf,mutate} from './concept-inventory.mjs'; diff --git a/tests/browser/vocabulary-visual.spec.mjs b/tests/browser/vocabulary-visual.spec.mjs index 9d354ddc..2d837952 100644 --- a/tests/browser/vocabulary-visual.spec.mjs +++ b/tests/browser/vocabulary-visual.spec.mjs @@ -1,4 +1,4 @@ -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; // Artifact-only captures of Tags & categories states that have no concept counterpart: the Manage categories diff --git a/tests/browser/worker-server.spec.mjs b/tests/browser/worker-server.spec.mjs new file mode 100644 index 00000000..2ed41360 --- /dev/null +++ b/tests/browser/worker-server.spec.mjs @@ -0,0 +1,36 @@ +// Each Playwright worker owns its server: `request`, `page` and manual +// `browser.newContext()` all reach it through `baseURL`, and another server +// process never sees its database. +import { test, expect } from './fixtures.mjs'; +import { freePort } from '../../tools/vendor/free-port.mjs'; +import { startBrowserServer } from '../../tools/vendor/start-browser-server.mjs'; + +const search = text => `/prompt_manager/search?text=${encodeURIComponent(text)}`; + +test('every fixture reaches this worker\'s own server and another server keeps its own database', async ({ request, page, browser, baseURL, workerServer }, testInfo) => { + expect(baseURL).toBe(workerServer.url); + await expect(await request.post('/__test__/reset')).toBeOK(); + const marker = `worker ${testInfo.workerIndex} isolation ${testInfo.project.name} ${testInfo.repeatEachIndex}`; + await expect(await request.post('/prompt_manager/save', { data: { text: marker } })).toBeOK(); + expect((await (await request.get(search(marker))).json()).count).toBe(1); + + const viaPage = await page.request.get(search(marker)); + expect(viaPage.url().startsWith(`${baseURL}/`)).toBe(true); + expect((await viaPage.json()).count).toBe(1); + const context = await browser.newContext(); + try { + const response = await (await context.newPage()).goto(search(marker)); + expect(response.url().startsWith(`${baseURL}/`)).toBe(true); + expect((await response.json()).count).toBe(1); + } finally { + await context.close(); + } + + const other = await startBrowserServer({ port: await freePort() }); + try { + expect(other.url).not.toBe(baseURL); + expect((await (await fetch(`${other.url}${search(marker)}`)).json()).count).toBe(0); + } finally { + await other.stop(); + } +}); diff --git a/tests/browser/workflows.spec.mjs b/tests/browser/workflows.spec.mjs index 091bfb6f..453a3ac2 100644 --- a/tests/browser/workflows.spec.mjs +++ b/tests/browser/workflows.spec.mjs @@ -1,5 +1,5 @@ import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; test.beforeEach(async ({ request, page, browserName }) => { await startCoverage(page, browserName); diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index 3970de06..05cb2d75 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -1,5 +1,5 @@ import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; import { seedConcept, openConcept, openProduction } from './concept-inventory.mjs'; // Activity (#249 Activity-1): the concept's Activity page (app.js renderActivity 732–738), its top-bar button diff --git a/tests/browser/workspace-history.spec.mjs b/tests/browser/workspace-history.spec.mjs index c28e591e..405718c8 100644 --- a/tests/browser/workspace-history.spec.mjs +++ b/tests/browser/workspace-history.spec.mjs @@ -1,5 +1,5 @@ import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {readFile,writeFile} from 'node:fs/promises'; import {createHash} from 'node:crypto'; diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 99dbbba5..c6007593 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1,5 +1,5 @@ import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; import { seedConcept, openConcept, openProduction } from './concept-inventory.mjs'; const workspace = '/prompt_manager/workspace#/images'; diff --git a/tests/browser/workspace-library-editor.spec.mjs b/tests/browser/workspace-library-editor.spec.mjs index 7560fbfa..cac57771 100644 --- a/tests/browser/workspace-library-editor.spec.mjs +++ b/tests/browser/workspace-library-editor.spec.mjs @@ -1,6 +1,6 @@ import {writeFile} from 'node:fs/promises'; import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; import {inventoryOf,mapConcept,diffInventory} from './library-editor-canonical-comparison.mjs'; diff --git a/tests/browser/workspace-library.spec.mjs b/tests/browser/workspace-library.spec.mjs index bca40b45..0b35746d 100644 --- a/tests/browser/workspace-library.spec.mjs +++ b/tests/browser/workspace-library.spec.mjs @@ -1,5 +1,5 @@ import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; const route='/prompt_manager/workspace#/library'; const cards=page=>page.locator('article[data-prompt-id]'); diff --git a/tests/browser/workspace-saved-views.spec.mjs b/tests/browser/workspace-saved-views.spec.mjs index e8ad3661..ce8f09fc 100644 --- a/tests/browser/workspace-saved-views.spec.mjs +++ b/tests/browser/workspace-saved-views.spec.mjs @@ -1,5 +1,5 @@ import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; // Saved views (#249 D16, maintainer ruling 2026-10-21): the toolbar's Save view (concept app.js a359, a1330–1346), // the sidebar SAVED VIEWS rows (a331–335) and `#/saved/` (a1158–1162), on Images and the Prompt library. diff --git a/tests/browser/workspace-visual.spec.mjs b/tests/browser/workspace-visual.spec.mjs index a9e2e281..8896505c 100644 --- a/tests/browser/workspace-visual.spec.mjs +++ b/tests/browser/workspace-visual.spec.mjs @@ -3,7 +3,7 @@ import { readFile } from 'node:fs/promises'; import { createHash } from 'node:crypto'; import { captureCanonicalState, historyGeometry, compareCanonicalHistory, nativeDialogState, exerciseCurrentPairSend, assertRecordedHistory } from './history-canonical-comparison.mjs'; import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; test.use({ locale: 'en-US', timezoneId: 'UTC', reducedMotion: 'reduce' }); const workspace = '/prompt_manager/workspace'; diff --git a/tests/browser/workspace-vocabulary.spec.mjs b/tests/browser/workspace-vocabulary.spec.mjs index 913c6808..9cc07427 100644 --- a/tests/browser/workspace-vocabulary.spec.mjs +++ b/tests/browser/workspace-vocabulary.spec.mjs @@ -1,6 +1,6 @@ import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; import {writeFile} from 'node:fs/promises'; -import {test,expect} from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import {test,expect} from './fixtures.mjs'; import {captureCanonicalState} from './history-canonical-comparison.mjs'; import {pageInventoryOf,inventoryOf,diffInventory,mapVocabularyConcept,mapVocabularyDialog} from './vocabulary-canonical-comparison.mjs'; diff --git a/tests/browser/workspace.spec.mjs b/tests/browser/workspace.spec.mjs index e353121b..c8ed5054 100644 --- a/tests/browser/workspace.spec.mjs +++ b/tests/browser/workspace.spec.mjs @@ -1,5 +1,5 @@ import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; -import { test, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, expect } from './fixtures.mjs'; const workspace = '/prompt_manager/workspace'; const statistic = (page, label) => page.locator('#workspace-main dt') diff --git a/tests/vendor/browser-server-fixture.test.mjs b/tests/vendor/browser-server-fixture.test.mjs new file mode 100644 index 00000000..ee42e693 --- /dev/null +++ b/tests/vendor/browser-server-fixture.test.mjs @@ -0,0 +1,110 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createServer } from 'node:net'; +import { fileURLToPath } from 'node:url'; +import { freePort } from '../../tools/vendor/free-port.mjs'; +import { startBrowserServer, createServerSupervisor } from '../../tools/vendor/start-browser-server.mjs'; +import { workflowWorkers } from '../../tools/vendor/test-workers.mjs'; + +const fakeServer = fileURLToPath(new URL('./fake-browser-server.mjs', import.meta.url)); +const fake = (...extra) => ({ command: process.execPath, args: port => [fakeServer, '--port', String(port), ...extra] }); + +function listen(port = 0) { + return new Promise((resolve, reject) => { + const server = createServer(); + server.once('error', reject); + server.listen(port, '127.0.0.1', () => resolve(server)); + }); +} + +test('free port allocation never reuses a bound port', async t => { + const held = await listen(); + t.after(() => held.close()); + const heldPort = held.address().port; + const seen = []; + for (let index = 0; index < 20; index += 1) { + const port = await freePort(); + assert.ok(Number.isInteger(port) && port > 0 && port < 65536, `port ${port} is a TCP port`); + assert.notEqual(port, heldPort, 'a port another process holds is never handed out'); + const probe = await listen(port); + seen.push(port); + await new Promise(resolve => probe.close(resolve)); + } + assert.ok(new Set(seen).size > 1, 'the OS hands out more than one port'); +}); + +test('startBrowserServer serves the readiness page on the given port and stops cleanly', async () => { + const port = await freePort(); + const server = await startBrowserServer({ port, ...fake() }); + assert.equal(server.port, port); + assert.equal(server.url, `http://127.0.0.1:${port}`); + assert.equal(await (await fetch(`${server.url}/prompt_manager/test`)).text(), 'ready'); + await server.stop(); + await assert.rejects(fetch(`${server.url}/prompt_manager/test`)); +}); + +test('startBrowserServer reports a startup failure with the server\'s stderr tail', async () => { + const port = await freePort(); + await assert.rejects(startBrowserServer({ port, ...fake('--fail') }), error => { + assert.match(error.message, /exited with code 4 before it was ready/); + assert.match(error.message, /last fake stderr line/); + return true; + }); +}); + +test('startBrowserServer gives up on a server that never becomes ready', async () => { + const port = await freePort(); + const silent = { command: process.execPath, args: () => ['-e', 'setInterval(() => {}, 1000)'] }; + await assert.rejects(startBrowserServer({ port, ...silent, timeoutMs: 500 }), /not ready .* within 500 ms/); +}); + +test('worker server restarts after an unexpected exit', async t => { + const supervisor = createServerSupervisor({ start: async () => startBrowserServer({ port: await freePort(), ...fake() }) }); + t.after(() => supervisor.stop()); + const first = await supervisor.ensure(); + assert.equal(supervisor.crash(first), null, 'a running server reports no crash'); + await fetch(`${first.url}/__exit`); + await first.exited; + const crash = supervisor.crash(first); + assert.match(crash.message, /test server on port \d+ exited unexpectedly \(code 3\)/); + assert.match(crash.message, /crashed on purpose/); + const second = await supervisor.ensure(); + assert.notEqual(second, first, 'the next test gets a fresh server'); + assert.equal(await (await fetch(`${second.url}/prompt_manager/test`)).text(), 'ready'); + assert.equal(await supervisor.ensure(), second, 'a healthy server is reused'); +}); + +test('supervisor retries a port that was taken between reservation and bind', async t => { + const ports = []; + let attempts = 0; + const supervisor = createServerSupervisor({ + start: async () => { + attempts += 1; + if (attempts === 1) throw new Error('OSError: [Errno 98] error while attempting to bind: address already in use'); + const port = await freePort(); + ports.push(port); + return startBrowserServer({ port, ...fake() }); + }, + }); + t.after(() => supervisor.stop()); + const server = await supervisor.ensure(); + assert.equal(attempts, 2); + assert.equal(server.port, ports[0]); +}); + +test('supervisor does not retry a startup failure that is not a port collision', async () => { + let attempts = 0; + const supervisor = createServerSupervisor({ start: async () => { attempts += 1; throw new Error('ImportError: no module'); } }); + await assert.rejects(supervisor.ensure(), /ImportError/); + assert.equal(attempts, 1); +}); + +test('worker count follows PM_TEST_WORKERS, then one in CI, then a quarter of the CPUs up to eight', () => { + assert.equal(workflowWorkers({ env: { PM_TEST_WORKERS: '3', CI: '1' }, cpus: 32 }), 3); + assert.equal(workflowWorkers({ env: { CI: '1' }, cpus: 32 }), 1); + assert.equal(workflowWorkers({ env: {}, cpus: 32 }), 8); + assert.equal(workflowWorkers({ env: {}, cpus: 12 }), 3); + assert.equal(workflowWorkers({ env: {}, cpus: 2 }), 1); + assert.throws(() => workflowWorkers({ env: { PM_TEST_WORKERS: 'many' }, cpus: 8 }), /PM_TEST_WORKERS must be a positive integer/); + assert.throws(() => workflowWorkers({ env: { PM_TEST_WORKERS: '0' }, cpus: 8 }), /PM_TEST_WORKERS must be a positive integer/); +}); diff --git a/tests/vendor/fake-browser-server.mjs b/tests/vendor/fake-browser-server.mjs new file mode 100644 index 00000000..1a4f7004 --- /dev/null +++ b/tests/vendor/fake-browser-server.mjs @@ -0,0 +1,21 @@ +// A stand-in for tests/browser/server.py: `--port N` serves the readiness page, +// `--fail` writes to stderr and exits before listening, and GET /__exit makes +// the process die unexpectedly after writing a last stderr line. +import { createServer } from 'node:http'; + +const args = process.argv.slice(2); +const port = Number(args[args.indexOf('--port') + 1]); +if (args.includes('--fail')) { + process.stderr.write('fake server: refusing to start\nlast fake stderr line\n'); + process.exit(4); +} +const server = createServer((request, response) => { + if (request.url === '/__exit') { + process.stderr.write('fake server: crashed on purpose\n'); + response.end('bye', () => process.exit(3)); + return; + } + response.end(request.url === '/prompt_manager/test' ? 'ready' : `pid ${process.pid}`); +}); +server.listen(port, '127.0.0.1'); +for (const signal of ['SIGTERM', 'SIGINT']) process.once(signal, () => server.close(() => process.exit(0))); diff --git a/tools/vendor/free-port.mjs b/tools/vendor/free-port.mjs new file mode 100644 index 00000000..88a2d345 --- /dev/null +++ b/tools/vendor/free-port.mjs @@ -0,0 +1,17 @@ +import { createServer } from 'node:net'; + +// Ask the OS for a port nobody holds: bind port 0 on loopback, read it back and +// release it. A port another process has bound is never returned. The caller +// binds it next; the short window in between is covered by retrying a startup +// that fails with "address already in use" (see createServerSupervisor). +export function freePort(host = '127.0.0.1') { + return new Promise((resolve, reject) => { + const server = createServer(); + server.unref(); + server.once('error', reject); + server.listen(0, host, () => { + const { port } = server.address(); + server.close(error => (error ? reject(error) : resolve(port))); + }); + }); +} diff --git a/tools/vendor/playwright.workflows.config.mjs b/tools/vendor/playwright.workflows.config.mjs index 40226f1e..67e7c21b 100644 --- a/tools/vendor/playwright.workflows.config.mjs +++ b/tools/vendor/playwright.workflows.config.mjs @@ -1,23 +1,18 @@ import { defineConfig, devices } from '@playwright/test'; -import { repositoryRoot } from './vendor-viewerjs.mjs'; -const port = Number(process.env.PM_TEST_PORT || 8765); +import { workflowWorkers } from './test-workers.mjs'; + +// No global webServer: tests/browser/fixtures.mjs starts one server per worker +// on an OS-chosen port. Files run in parallel; tests inside a file keep their order. export default defineConfig({ testDir: '../../tests/browser', outputDir: './test-results/workflows', testMatch: '*.spec.mjs', - workers: 1, + workers: workflowWorkers(), fullyParallel: false, forbidOnly: Boolean(process.env.CI), retries: 0, reporter: 'list', - use: { baseURL: `http://127.0.0.1:${port}`, trace: 'retain-on-failure', screenshot: 'only-on-failure' }, - webServer: { - command: `node tools/vendor/start-browser-server.mjs --port ${port}`, - cwd: repositoryRoot, - url: `http://127.0.0.1:${port}/prompt_manager/test`, - reuseExistingServer: false, - gracefulShutdown: { signal: 'SIGTERM', timeout: 5000 }, - }, + use: { trace: 'retain-on-failure', screenshot: 'only-on-failure' }, projects: [ { name: 'chromium', outputDir: './test-results/workflows/chromium', use: { browserName: 'chromium' } }, { name: 'firefox', outputDir: './test-results/workflows/firefox', use: { browserName: 'firefox' } }, diff --git a/tools/vendor/start-browser-server.mjs b/tools/vendor/start-browser-server.mjs index f41dc627..358fd574 100644 --- a/tools/vendor/start-browser-server.mjs +++ b/tools/vendor/start-browser-server.mjs @@ -1,8 +1,136 @@ import { spawn } from 'node:child_process'; -const child = spawn(process.env.PM_TEST_PYTHON || 'python', - ['-m', 'tests.browser.server', ...process.argv.slice(2)], { stdio: 'inherit' }); -child.on('error', error => { console.error(error.message); process.exitCode = 1; }); -child.on('exit', code => { process.exitCode = code ?? 1; }); -for (const signal of ['SIGTERM', 'SIGINT']) { - process.once(signal, () => child.kill(signal)); +import { resolve } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { setTimeout as delay } from 'node:timers/promises'; +import { repositoryRoot } from './vendor-viewerjs.mjs'; + +const READY_PATH = '/prompt_manager/test'; +const STARTUP_TIMEOUT_MS = 60_000; +const STOP_TIMEOUT_MS = 5_000; +const POLL_INTERVAL_MS = 100; +const PROBE_TIMEOUT_MS = 1_000; +const STDERR_TAIL_BYTES = 8_192; +const START_ATTEMPTS = 3; +const PORT_COLLISION = /address already in use|errno 98\b|errno 48\b|10048|EADDRINUSE/i; + +const pythonServer = port => ['-m', 'tests.browser.server', '--port', String(port)]; + +function describeExit({ code, signal }) { + return signal ? `signal ${signal}` : `code ${code}`; +} + +// Start one disposable tests/browser/server.py on `port` and wait until it +// serves the readiness page. Resolves to {port, url, exited, exitStatus, +// stderrTail(), stop()}; rejects with the server's stderr tail when it exits +// or stays unready. +export async function startBrowserServer({ + port, + command = process.env.PM_TEST_PYTHON || 'python', + args = pythonServer, + cwd = repositoryRoot, + timeoutMs = STARTUP_TIMEOUT_MS, +} = {}) { + if (!Number.isInteger(port) || port <= 0) throw new Error(`startBrowserServer needs a port, got ${port}`); + const url = `http://127.0.0.1:${port}`; + const child = spawn(command, args(port), { cwd, stdio: ['ignore', 'ignore', 'pipe'] }); + let stderr = ''; + child.stderr.setEncoding('utf8'); + child.stderr.on('data', chunk => { stderr = (stderr + chunk).slice(-STDERR_TAIL_BYTES); }); + const killOnExit = () => child.kill('SIGKILL'); + process.once('exit', killOnExit); + const server = { + port, + url, + exitStatus: null, + stopping: false, + stderrTail: () => stderr.trimEnd(), + exited: new Promise(done => { + child.once('error', error => { + stderr += `\n${error.message}`; + server.exitStatus = { code: null, signal: null, error }; + done(server.exitStatus); + }); + child.once('exit', (code, signal) => { + server.exitStatus = { code, signal }; + done(server.exitStatus); + }); + }), + async stop() { + server.stopping = true; + if (server.exitStatus === null) { + child.kill('SIGTERM'); + const stopped = await Promise.race([server.exited, delay(STOP_TIMEOUT_MS).then(() => false)]); + if (!stopped) { + child.kill('SIGKILL'); + await server.exited; + } + } + process.removeListener('exit', killOnExit); + }, + }; + server.exited.then(() => process.removeListener('exit', killOnExit)); + const failure = async reason => { + await server.stop(); + const tail = server.stderrTail(); + return new Error(`test server on port ${port} ${reason}${tail ? `\n--- server stderr tail ---\n${tail}` : ''}`); + }; + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (server.exitStatus !== null) { + throw await failure(`exited with ${describeExit(server.exitStatus)} before it was ready`); + } + try { + const response = await fetch(`${url}${READY_PATH}`, { signal: AbortSignal.timeout(PROBE_TIMEOUT_MS) }); + await response.arrayBuffer(); + if (response.ok) return server; + } catch { + // Not listening yet. + } + await delay(POLL_INTERVAL_MS); + } + throw await failure(`was not ready at ${READY_PATH} within ${timeoutMs} ms`); +} + +// Keep one live server for a Playwright worker. ensure() returns the running +// server, or starts a fresh one when the last one died; crash(server) turns an +// unexpected exit into an error carrying the stderr tail; a startup that lost +// its port to another process between reservation and bind is retried. +export function createServerSupervisor({ start, attempts = START_ATTEMPTS }) { + let current = null; + return { + async ensure() { + if (current && current.exitStatus === null) return current; + current = null; + for (let attempt = 1; ; attempt += 1) { + try { + current = await start(); + return current; + } catch (error) { + if (attempt >= attempts || !PORT_COLLISION.test(error.message)) throw error; + } + } + }, + crash(server) { + if (!server || server.stopping || server.exitStatus === null) return null; + const tail = server.stderrTail(); + return new Error(`test server on port ${server.port} exited unexpectedly (${describeExit(server.exitStatus)})` + + (tail ? `\n--- server stderr tail ---\n${tail}` : '')); + }, + async stop() { + const server = current; + current = null; + await server?.stop(); + }, + }; +} + +// CLI: `node start-browser-server.mjs --port N` runs one server in the foreground. +if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) { + const child = spawn(process.env.PM_TEST_PYTHON || 'python', + ['-m', 'tests.browser.server', ...process.argv.slice(2)], { stdio: 'inherit' }); + child.on('error', error => { console.error(error.message); process.exitCode = 1; }); + child.on('exit', code => { process.exitCode = code ?? 1; }); + for (const signal of ['SIGTERM', 'SIGINT']) { + process.once(signal, () => child.kill(signal)); + } } diff --git a/tools/vendor/test-workers.mjs b/tools/vendor/test-workers.mjs new file mode 100644 index 00000000..92dff984 --- /dev/null +++ b/tools/vendor/test-workers.mjs @@ -0,0 +1,18 @@ +import { availableParallelism } from 'node:os'; + +const MAX_LOCAL_WORKERS = 8; +const CPUS_PER_WORKER = 4; + +// PM_TEST_WORKERS wins; CI without it keeps one worker, so CI only changes +// through explicit workflow edits; locally a quarter of the CPUs, at most eight. +export function workflowWorkers({ env = process.env, cpus = availableParallelism() } = {}) { + if (env.PM_TEST_WORKERS !== undefined) { + const workers = Number(env.PM_TEST_WORKERS); + if (!Number.isInteger(workers) || workers < 1) { + throw new Error(`PM_TEST_WORKERS must be a positive integer, got "${env.PM_TEST_WORKERS}"`); + } + return workers; + } + if (env.CI) return 1; + return Math.max(1, Math.min(MAX_LOCAL_WORKERS, Math.floor(cpus / CPUS_PER_WORKER))); +}