test(comfyui): refuse a real-host port another run holds (#310)
The real-host config waited on the /system_stats URL, which any server on
the port answers. When two runs started on 8189 within a few seconds of
each other, the second run's ComfyUI failed to bind ("Port 8189 is already
in use"), but Playwright had already seen the first run's host answer and
ran the second run's tests against it. When the first run finished and
stopped its host, the second run failed mid-test with ECONNREFUSED,
ECONNRESET or ERR_CONNECTION_REFUSED, and passed when rerun alone.
The launcher now refuses a port that is in use, and reports ready only
when /system_stats names its own base directory. The config waits for
that line instead of the URL, and every test checks again that the host
answering the port is the one its run started.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
Vendored
+14
@@ -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);
|
||||
});
|
||||
|
||||
Vendored
+50
-1
@@ -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/);
|
||||
});
|
||||
|
||||
Vendored
+58
-3
@@ -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; }
|
||||
}
|
||||
|
||||
Vendored
+6
-1
@@ -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=(?<pm_comfy_host_base>.+?)\r?$`, 'm') },
|
||||
timeout: 120000, reuseExistingServer: false,
|
||||
env: { PM_COMFY_PORT: host.port },
|
||||
gracefulShutdown: { signal: 'SIGTERM', timeout: 10000 },
|
||||
}];
|
||||
|
||||
Reference in New Issue
Block a user