Skip to content

Fix flaky remote-client tests: surface admin server bind errors and avoid ephemeral ports #214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pimterry merged 2 commits into from
Sep 22, 2026
Merged

Fix flaky remote-client tests: surface admin server bind errors and avoid ephemeral ports #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9358265
Reject admin server start when the port can't be bound
philipp-winterle Sep 21, 2026
b8f25dc
Run test admin servers on a port outside the ephemeral range
philipp-winterle Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/admin/admin-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,22 @@ export class AdminServer<Plugins extends { [key: string]: AdminPlugin<any, any>
? { port: listenOptions, host: '127.0.0.1' }
: { host: '127.0.0.1', ...listenOptions };

await new Promise<void>((resolve, reject) => {
this.server = makeDestroyable(this.app.listen(resolvedListenOptions, () => resolve()));
try {
await this.listen(resolvedListenOptions);
} catch (e) {
// Failed to bind (e.g. EADDRINUSE), so there's no running server to track:
this.server = null;
throw e;
}
}

private listen(listenOptions: { port: number, host: string }) {
return new Promise<void>((resolve, reject) => {
// Express 5 passes listen errors to this callback too, so we must reject here,
// or a failed bind would look like a successful start:
this.server = makeDestroyable(this.app.listen(listenOptions, (error?: Error) =>
error ? reject(error) : resolve()
));

this.server.on('error', reject);

Expand Down
27 changes: 16 additions & 11 deletions test/integration/plugins.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import gql from "graphql-tag";
import { PluggableAdmin, MockttpPluggableAdmin } from "../..";
import { expect, nodeOnly } from "../test-utils";
import {
expect,
nodeOnly,
TEST_ADMIN_SERVER_PORT,
TEST_ADMIN_SERVER_URL
} from "../test-utils";

nodeOnly(() => {
describe("Admin server plugins", function () {
Expand All @@ -24,9 +29,9 @@ nodeOnly(() => {
}
}
});
await adminServer.start();
await adminServer.start(TEST_ADMIN_SERVER_PORT);

adminClient = new PluggableAdmin.AdminClient();
adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL });
await adminClient.start({
myPlugin: {}
});
Expand Down Expand Up @@ -55,11 +60,11 @@ nodeOnly(() => {
}
}
});
await adminServer.start();
await adminServer.start(TEST_ADMIN_SERVER_PORT);

let client = adminClient = new PluggableAdmin.AdminClient<{
myPlugin: PluggableAdmin.AdminPlugin<{}, { aMetadataField: boolean }>
}>();
}>({ adminServerUrl: TEST_ADMIN_SERVER_URL });
const startResult = await client.start({
myPlugin: {}
});
Expand All @@ -83,9 +88,9 @@ nodeOnly(() => {
}
}
});
await adminServer.start();
await adminServer.start(TEST_ADMIN_SERVER_PORT);

adminClient = new PluggableAdmin.AdminClient();
adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL });
await adminClient.start({
myPlugin: {}
});
Expand Down Expand Up @@ -118,9 +123,9 @@ nodeOnly(() => {
http: MockttpPluggableAdmin.MockttpAdminPlugin
}
});
await adminServer.start();
await adminServer.start(TEST_ADMIN_SERVER_PORT);

const client = adminClient = new PluggableAdmin.AdminClient();
const client = adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL });
await adminClient.start({
myPlugin: {},
http: {}
Expand Down Expand Up @@ -169,9 +174,9 @@ nodeOnly(() => {
}
}
});
await adminServer.start();
await adminServer.start(TEST_ADMIN_SERVER_PORT);

adminClient = new PluggableAdmin.AdminClient();
adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL });
await adminClient.start({
myPlugin: {}
});
Expand Down
8 changes: 5 additions & 3 deletions test/integration/proxying/upstream-proxying.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import * as url from 'url';
import { getLocal, Mockttp, MockedEndpoint, getAdminServer, getRemote } from "../../..";
import {
expect,
nodeOnly
nodeOnly,
TEST_ADMIN_SERVER_PORT,
TEST_ADMIN_SERVER_URL
} from "../../test-utils";

const INITIAL_ENV = _.cloneDeep(process.env);
Expand Down Expand Up @@ -359,11 +361,11 @@ nodeOnly(() => {

const adminServer = getAdminServer();

before(() => adminServer.start());
before(() => adminServer.start(TEST_ADMIN_SERVER_PORT));
after(() => adminServer.stop());

beforeEach(async () => {
server = getRemote();
server = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });
await server.start();

// Configure Request to use the *first* server as a proxy
Expand Down
91 changes: 69 additions & 22 deletions test/integration/remote-client.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import {
browserOnly,
delay,
getDeferred,
defaultNodeConnectionHeader
defaultNodeConnectionHeader,
TEST_ADMIN_SERVER_PORT,
TEST_ADMIN_SERVER_URL
} from "../test-utils";
import type { MockttpClient } from "../../dist/client/mockttp-client";

Expand Down Expand Up @@ -55,9 +57,9 @@ nodeOnly(() => {
describe("with no configuration", () => {

const server = getAdminServer();
const remoteServer = getRemote();
const remoteServer = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });

before(() => server.start());
before(() => server.start(TEST_ADMIN_SERVER_PORT));
after(() => server.stop());

beforeEach(() => remoteServer.start());
Expand Down Expand Up @@ -560,7 +562,7 @@ nodeOnly(() => {
it("should support explicitly resetting all servers", async () => {
await remoteServer.forGet("/mocked-endpoint").thenReply(200, "mocked data");

await resetAdminServer();
await resetAdminServer({ adminServerUrl: TEST_ADMIN_SERVER_URL });

const result = await request.get(remoteServer.urlFor("/mocked-endpoint")).catch((e) => e);

Expand All @@ -571,7 +573,7 @@ nodeOnly(() => {
it("should reject multiple clients trying to control the same port", async () => {
const port = remoteServer.port!;

await expect(getRemote().start(port))
await expect(getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }).start(port))
.to.eventually.be.rejectedWith(`Failed to start mock session: listen EADDRINUSE`);
});

Expand All @@ -590,7 +592,7 @@ nodeOnly(() => {
});

it("should reject Mockttp clients trying to use that port", async () => {
await expect(getRemote().start(port))
await expect(getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }).start(port))
.to.eventually.be.rejectedWith(/Failed to start mock session: listen EADDRINUSE/);
});
});
Expand Down Expand Up @@ -619,9 +621,9 @@ nodeOnly(() => {
}
}
});
let client = getRemote();
let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });

before(() => server.start());
before(() => server.start(TEST_ADMIN_SERVER_PORT));
after(() => server.stop());

beforeEach(() => client.start());
Expand Down Expand Up @@ -651,9 +653,9 @@ nodeOnly(() => {
}
});

let client = getRemote();
let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });

before(() => server.start());
before(() => server.start(TEST_ADMIN_SERVER_PORT));
after(() => server.stop());

beforeEach(() => client.start());
Expand Down Expand Up @@ -724,9 +726,9 @@ nodeOnly(() => {
let adminServer = getAdminServer({
webSocketKeepAlive: 50
});
let client = getRemote();
let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });

before(() => adminServer.start());
before(() => adminServer.start(TEST_ADMIN_SERVER_PORT));
after(() => adminServer.stop());

beforeEach(() => client.start());
Expand Down Expand Up @@ -770,19 +772,20 @@ nodeOnly(() => {

let client: Mockttp;

before(() => server.start());
before(() => server.start(TEST_ADMIN_SERVER_PORT));
after(() => server.stop());

afterEach(() => client.stop());

it("rejects clients with no origin", async () => {
client = getRemote();
client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });

await expect(client.start()).to.be.rejectedWith('403');
});

it("rejects clients with the wrong origin", async () => {
client = getRemote({
adminServerUrl: TEST_ADMIN_SERVER_URL,
client: {
headers: {
origin: 'https://twitter.com'
Expand All @@ -795,6 +798,7 @@ nodeOnly(() => {

it("rejects clients with the wrong origin protocol", async () => {
client = getRemote({
adminServerUrl: TEST_ADMIN_SERVER_URL,
client: {
headers: {
origin: 'http://example.com'
Expand All @@ -807,6 +811,7 @@ nodeOnly(() => {

it("allows clients that specify the correct origin", async () => {
client = getRemote({
adminServerUrl: TEST_ADMIN_SERVER_URL,
client: {
headers: {
origin: 'https://example.com'
Expand All @@ -819,6 +824,7 @@ nodeOnly(() => {

it("rejects subscriptions for clients that specify no origin", async () => {
client = getRemote({
adminServerUrl: TEST_ADMIN_SERVER_URL,
client: {
headers: {
origin: 'https://example.com'
Expand All @@ -830,7 +836,7 @@ nodeOnly(() => {

// Manually send a subscription socket with no Origin (can't start an invalid client
// and test, because the client fails to start given a bad origin)
const ws = new WebSocket(`ws://localhost:45454/session/${client.port}/subscription`);
const ws = new WebSocket(`ws://localhost:${TEST_ADMIN_SERVER_PORT}/session/${client.port}/subscription`);

await expect(new Promise((resolve, reject) => {
ws.addEventListener('open', resolve);
Expand All @@ -840,6 +846,7 @@ nodeOnly(() => {

it("rejects subscriptions for clients that specify the wrong origin", async () => {
client = getRemote({
adminServerUrl: TEST_ADMIN_SERVER_URL,
client: {
headers: {
origin: 'https://example.com'
Expand All @@ -851,7 +858,7 @@ nodeOnly(() => {

// Manually send a subscription socket with the wrong Origin (can't start an invalid client
// and test, because the client fails to start given a bad origin)
const ws = new WebSocket(`ws://localhost:45454/session/${client.port}/subscription`, {
const ws = new WebSocket(`ws://localhost:${TEST_ADMIN_SERVER_PORT}/session/${client.port}/subscription`, {
headers: {
origin: 'https://twitter.com'
}
Expand All @@ -865,6 +872,7 @@ nodeOnly(() => {

it("allows subscriptions for clients that specify the correct origin", async () => {
client = getRemote({
adminServerUrl: TEST_ADMIN_SERVER_URL,
client: {
headers: {
origin: 'https://example.com'
Expand All @@ -876,7 +884,7 @@ nodeOnly(() => {

// Manually send a subscription socket with the right Origin for consistency with above
const id = getClientSessionId(client);
const ws = new WebSocket(`ws://localhost:45454/session/${id}/subscription`, {
const ws = new WebSocket(`ws://localhost:${TEST_ADMIN_SERVER_PORT}/session/${id}/subscription`, {
headers: {
origin: 'https://example.com'
}
Expand All @@ -896,15 +904,15 @@ nodeOnly(() => {

const adminServer = getAdminServer();

const client1 = getRemote();
const client2 = getRemote();
const client1 = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });
const client2 = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL });

afterEach(() => Promise.all([
client1.stop(),
client2.stop()
]));

beforeEach(() => adminServer.start());
beforeEach(() => adminServer.start(TEST_ADMIN_SERVER_PORT));
afterEach(() => adminServer.stop());

it("should expose events for mock server start & stop", async () => {
Expand Down Expand Up @@ -984,7 +992,7 @@ nodeOnly(() => {
await client1.start();
const clientPort = client1.port;

await resetAdminServer();
await resetAdminServer({ adminServerUrl: TEST_ADMIN_SERVER_URL });
await client2.start(clientPort);

// Client 1 should be broken now, because it was reset. It should _not_ try to
Expand All @@ -1004,14 +1012,53 @@ nodeOnly(() => {
});
});

describe("when the admin server port is already taken", () => {
let blockingServer: net.Server;
let port: number;

beforeEach(async () => {
port = await getPort();
blockingServer = net.createServer();
await new Promise<void>((resolve) =>
blockingServer.listen({ port, host: '127.0.0.1' }, resolve)
);
});

afterEach(() => new Promise<void>((resolve) => blockingServer.close(() => resolve())));

it("fails to start", async () => {
const adminServer = getAdminServer();

await expect(adminServer.start(port))
.to.eventually.be.rejectedWith(/EADDRINUSE/);
});

it("can still be stopped after failing to start", async () => {
const adminServer = getAdminServer();
await adminServer.start(port).catch(() => {});

await expect(adminServer.stop()).to.eventually.be.fulfilled;
});

it("can be started on a free port after failing to start", async () => {
const adminServer = getAdminServer();
await adminServer.start(port).catch(() => {});

const freePort = await getPort();
await adminServer.start(freePort);
await adminServer.stop();
});
});

describe("with message body decoding disabled", () => {

const server = getAdminServer();
const client = getRemote({
adminServerUrl: TEST_ADMIN_SERVER_URL,
messageBodyDecoding: 'none'
});

before(() => server.start());
before(() => server.start(TEST_ADMIN_SERVER_PORT));
after(() => server.stop());

beforeEach(() => client.start());
Expand Down
Loading
Loading