diff --git a/TESTING.md b/TESTING.md index 16cf4cc..2b29c97 100644 --- a/TESTING.md +++ b/TESTING.md @@ -121,6 +121,13 @@ npm run --prefix tools/vendor test:comfyui CI performs this setup automatically. `PM_COMFY_URL` can select another `http://127.0.0.1:PORT/` address (default port 8189). This is an owned managed server: a running host is never reused. Do not point it at your live checkout. +One port serves one run. The launcher refuses a port that is already in use and +reports ready only when `/system_stats` names its own base directory, so two runs +started together on 8189 fail at startup ("already in use" or "answered by a +ComfyUI host this run did not start") instead of one run testing the other's +host and losing it mid-test (`ECONNREFUSED`, `ERR_CONNECTION_REFUSED`, Network +Error). Every test checks the answering host again. Agents sharing a machine +give each run its own `PM_COMFY_URL` port. The launcher verifies the upstream SHA and clean tracked files, snapshots only tracked PromptManager files, and creates temporary user/input/output/database/ configuration/log directories. Personal untracked files are excluded. It runs diff --git a/tests/comfyui/host-fixture.mjs b/tests/comfyui/host-fixture.mjs index 8246ad2..a88cb77 100644 --- a/tests/comfyui/host-fixture.mjs +++ b/tests/comfyui/host-fixture.mjs @@ -1,11 +1,20 @@ import { expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; import runtime from '../../tools/vendor/comfyui-runtime.json' with { type: 'json' }; +import { hostBaseDirectory } from '../../tools/vendor/comfyui-host.mjs'; export function installHostHooks(test, expectedTypes = ['PromptManager', 'PromptManagerText', 'PromptSearchList']) { test.beforeEach(async ({ page, context, request }, testInfo) => { const response = await request.get('/system_stats'); expect(response.ok()).toBe(true); - const info = (await response.json()).system; + const stats = await response.json(); + // comfyui.config.mjs's launcher reports the host it owns. The execution suite has no + // web server: it starts its own host per worker behind a port refusal. + if (testInfo.config.webServer) { + const ownedBase = process.env.PM_COMFY_HOST_BASE; + expect(ownedBase, 'the launcher reported the host it started').toBeTruthy(); + expect(hostBaseDirectory(stats), 'the host answering the port is the one this run started').toBe(ownedBase); + } + const info = stats.system; expect(info.comfyui_version).toBe(runtime.serverVersion); expect(info.pytorch_version).toBe(`${runtime.torchVersion}+cpu`); expect(info.required_frontend_version).toBe(runtime.frontendVersion); diff --git a/tests/vendor/comfyui-config.test.mjs b/tests/vendor/comfyui-config.test.mjs index 971dee7..e13e505 100644 --- a/tests/vendor/comfyui-config.test.mjs +++ b/tests/vendor/comfyui-config.test.mjs @@ -51,3 +51,17 @@ test('CI retains stock, execution and pack suites in order with uploaded executi assert.deepEqual(commands, ['test:comfyui', 'test:executions', 'test:compat']); assert.match(workflow, /path: tools\/vendor\/test-results\//); }); + +test('real-host config waits for its own launcher, never for whichever server holds the port', async t => { + const previous = process.env.PM_COMFY_ROOT; + process.env.PM_COMFY_ROOT = '/unused-pinned-checkout'; + t.after(() => { if (previous === undefined) delete process.env.PM_COMFY_ROOT; else process.env.PM_COMFY_ROOT = previous; }); + const { default: config } = await import('../../tools/vendor/comfyui.config.mjs'); + const [server] = config.webServer; + assert.equal(server.url, undefined); + assert.equal(server.port, undefined); + assert.equal(server.reuseExistingServer, false); + const ready = server.wait.stdout.exec('Starting server\nPM_COMFY_HOST_READY base-directory=/tmp/comfyassets-host-a b\r\nlater'); + assert.equal(ready.groups.pm_comfy_host_base, '/tmp/comfyassets-host-a b'); + assert.equal(server.wait.stdout.exec('To see the GUI go to: http://127.0.0.1:8189'), null); +}); diff --git a/tests/vendor/comfyui-host.test.mjs b/tests/vendor/comfyui-host.test.mjs index a39a289..010f069 100644 --- a/tests/vendor/comfyui-host.test.mjs +++ b/tests/vendor/comfyui-host.test.mjs @@ -6,7 +6,8 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { once } from 'node:events'; import { createServer } from 'node:net'; -import { launchComfyHost } from '../../tools/vendor/comfyui-host.mjs'; +import { createServer as createHttpServer } from 'node:http'; +import { launchComfyHost, assertPortFree, waitForOwnHost, hostBaseDirectory } from '../../tools/vendor/comfyui-host.mjs'; async function fixture(t, program) { const directory = await mkdtemp(join(tmpdir(), 'pm-host-test-')); @@ -232,3 +233,51 @@ test('execution launcher refuses an occupied port without touching an existing s await new Promise(resolve => server.close(resolve)); } }); + +// A stand-in ComfyUI that answers /system_stats with its own argv, as the real host does. +const answersOwnArgv = `const args = process.argv.slice(1); +require('node:http').createServer((request, response) => { + response.setHeader('content-type', 'application/json'); + response.end(JSON.stringify({ system: { argv: args } })); +}).listen(Number(args[args.indexOf('--port') + 1]), '127.0.0.1');`; + +async function listen(t, port, handler = (request, response) => response.end()) { + const server = createHttpServer(handler); + await new Promise((resolve, reject) => { server.once('error', reject); server.listen(port, '127.0.0.1', resolve); }); + t.after(() => new Promise(resolve => server.close(resolve))); + return server; +} + +test('real-host launcher refuses a port another run already holds and leaves that server running', async t => { + const server = await listen(t, 8199); + await assert.rejects(assertPortFree(8199), /8199 is already in use.*never shared/s); + assert.equal(server.listening, true); +}); + +test('real-host launcher is ready only when its own ComfyUI answers the port', async t => { + const options = await fixture(t, answersOwnArgv); + await assertPortFree(options.port); + const host = await launchComfyHost(options); + t.after(() => host.stop()); + await waitForOwnHost(host, options.port); + const stats = await (await fetch(`http://127.0.0.1:${options.port}/system_stats`)).json(); + assert.equal(hostBaseDirectory(stats), host.stateDirectory); +}); + +test('real-host launcher refuses a host it did not start that answers before its own binds', async t => { + await listen(t, 8199, (request, response) => { + response.setHeader('content-type', 'application/json'); + response.end(JSON.stringify({ system: { argv: ['main.py', '--base-directory', '/other-run-state'] } })); + }); + const options = await fixture(t, idle); + const host = await launchComfyHost(options); + t.after(() => host.stop()); + await assert.rejects(waitForOwnHost(host, options.port), /did not start \(base directory \/other-run-state\)/); +}); + +test('real-host launcher reports a host that exits before it answers', async t => { + const options = await fixture(t, 'process.exit(3);'); + const host = await launchComfyHost(options); + t.after(() => host.stop()); + await assert.rejects(waitForOwnHost(host, options.port), /exited before it answered on port 8199 \(exit=3/); +}); diff --git a/tools/vendor/comfyui-host.mjs b/tools/vendor/comfyui-host.mjs index a545487..49f264e 100644 --- a/tools/vendor/comfyui-host.mjs +++ b/tools/vendor/comfyui-host.mjs @@ -1,4 +1,5 @@ import { writeFile } from 'node:fs/promises'; +import { createServer } from 'node:net'; import { join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { launchComfyHost as launchSharedHost } from '@comfyassets/toolkit/testing/host'; @@ -7,6 +8,9 @@ import runtime from './comfyui-runtime.json' with { type: 'json' }; export { launchSharedHost }; const repositoryRoot = fileURLToPath(new URL('../../', import.meta.url)); +// comfyui.config.mjs waits for this line; its named group becomes PM_COMFY_HOST_BASE in the test workers. +export const HOST_READY_MARKER = 'PM_COMFY_HOST_READY'; +const READY_POLL_MS = 250; export async function preparePromptManagerPlugin({ installed, stateDirectory }) { const configPath = join(installed, 'config.json'); @@ -27,17 +31,68 @@ export function launchComfyHost({ checkout, repository = repositoryRoot, }); } +export async function assertPortFree(port) { + await new Promise((resolve, reject) => { + const probe = createServer(); + probe.once('error', error => reject(new Error(`ComfyUI host port ${port} is already in use, often by another ` + + 'real-host run. A managed host is never shared: stop that run or set PM_COMFY_URL to a free ' + + 'http://127.0.0.1:PORT/ address.', { cause: error }))); + probe.listen({ host: '127.0.0.1', port, exclusive: true }, () => probe.close(error => error ? reject(error) : resolve())); + }); +} + +export function hostBaseDirectory(stats) { + const argv = stats?.system?.argv; + if (!Array.isArray(argv)) return null; + const index = argv.indexOf('--base-directory'); + return index >= 0 ? argv[index + 1] ?? null : null; +} + +// Ready means this launcher's own ComfyUI answers: /system_stats reports its base directory. +// Any other answer on the port comes from a host this run did not start, so the run must not use it. +export async function waitForOwnHost(host, port, { fetchImpl = fetch } = {}) { + let exited = null; + host.finished.then(result => { exited = result; }); + const url = `http://127.0.0.1:${port}/system_stats`; + for (;;) { + let stats = null; + try { stats = await (await fetchImpl(url)).json(); } + catch { stats = null; } + if (stats) { + const base = hostBaseDirectory(stats); + if (base === host.stateDirectory) return; + throw new Error(`Port ${port} is answered by a ComfyUI host this run did not start ` + + `(base directory ${base ?? 'unknown'}); another run is using the port.`); + } + if (exited) { + throw new Error(`ComfyUI host exited before it answered on port ${port} ` + + `(exit=${exited.code}, signal=${exited.signal || 'none'}).`); + } + await new Promise(resolve => setTimeout(resolve, READY_POLL_MS)); + } +} + if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) { try { - const host = await launchComfyHost({ checkout: process.env.PM_COMFY_ROOT, - port: Number(process.env.PM_COMFY_PORT || 8189) }); + const port = Number(process.env.PM_COMFY_PORT || 8189); + await assertPortFree(port); + const host = await launchComfyHost({ checkout: process.env.PM_COMFY_ROOT, port }); const stop = () => { host.stop().catch(error => { console.error(error.message); process.exitCode = 1; }); }; process.once('SIGINT', stop); process.once('SIGTERM', stop); + let refused = false; + try { + await waitForOwnHost(host, port); + console.log(`${HOST_READY_MARKER} base-directory=${host.stateDirectory}`); + } catch (error) { + refused = true; + console.error(error.message); + await host.stop(); + } const result = await host.finished; process.removeListener('SIGINT', stop); process.removeListener('SIGTERM', stop); if (result.code) console.error(`ComfyUI host exited with status ${result.code}; see tools/vendor/test-results/comfyui-host.log`); - process.exitCode = result.code; + process.exitCode = refused ? 1 : result.code; } catch (error) { console.error(error.message); process.exitCode = 1; } } diff --git a/tools/vendor/comfyui.config.mjs b/tools/vendor/comfyui.config.mjs index 6d7dc76..0c121a4 100644 --- a/tools/vendor/comfyui.config.mjs +++ b/tools/vendor/comfyui.config.mjs @@ -1,5 +1,6 @@ import { defineConfig } from './node_modules/@playwright/test/index.mjs'; import { fileURLToPath } from 'node:url'; +import { HOST_READY_MARKER } from './comfyui-host.mjs'; if (!process.env.PM_COMFY_ROOT) throw new Error('Set PM_COMFY_ROOT to a pinned disposable ComfyUI checkout'); const host = new URL(process.env.PM_COMFY_URL || 'http://127.0.0.1:8189/'); @@ -7,7 +8,11 @@ if (host.protocol !== 'http:' || host.hostname !== '127.0.0.1' || !host.port) th const servers = [{ command: 'node tools/vendor/comfyui-host.mjs', cwd: fileURLToPath(new URL('../../', import.meta.url)), - url: new URL('system_stats', host).href, timeout: 120000, reuseExistingServer: false, + // No URL wait: any server on the port would satisfy it, including another run's host that + // then stops mid-test. The launcher refuses a used port and prints this line only once its + // own ComfyUI answers; the group reaches the tests as PM_COMFY_HOST_BASE. + wait: { stdout: new RegExp(`^${HOST_READY_MARKER} base-directory=(?.+?)\r?$`, 'm') }, + timeout: 120000, reuseExistingServer: false, env: { PM_COMFY_PORT: host.port }, gracefulShutdown: { signal: 'SIGTERM', timeout: 10000 }, }];