Skip to content

Commit 03199d2

Browse files
BobDickinsonBobDickinson
committed
Addressed flaky tests. Implemented atomic storage writes (cause of some flakiness) and smart test wait helpers (cause of the rest of them). Implemented a test:repeat target that will run the entire test suite repeatedly. Verified 50 test runs with no failures.
1 parent 6c6d9cb commit 03199d2

14 files changed

Lines changed: 465 additions & 156 deletions

‎core/__tests__/auth/storage-node.test.ts‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -385,35 +385,52 @@ describe("OAuth Store (Zustand)", () => {
385385
});
386386

387387
it("should persist state to file", async () => {
388-
if (process.env.DEBUG_WAIT_FOR_STATE_FILE === "1") {
389-
console.error("[storage-node.test] state file path:", stateFilePath);
390-
}
391-
const store = getOAuthStore(testStatePath);
392-
const serverUrl = "http://localhost:3000";
393-
const clientInfo: OAuthClientInformation = {
394-
client_id: "test-client-id",
395-
};
388+
// Use a unique path so no other test (e.g. "should overwrite..." with second-id) can
389+
// write to the same file; Zustand persist is async and can race with shared paths.
390+
const persistTestPath = path.join(
391+
os.tmpdir(),
392+
`mcp-inspector-oauth-persist-${Date.now()}-${Math.random().toString(36).slice(2)}.json`,
393+
);
394+
try {
395+
if (process.env.DEBUG_WAIT_FOR_STATE_FILE === "1") {
396+
console.error("[storage-node.test] state file path:", persistTestPath);
397+
}
398+
const store = getOAuthStore(persistTestPath);
399+
const serverUrl = "http://localhost:3000";
400+
const clientInfo: OAuthClientInformation = {
401+
client_id: "test-client-id",
402+
};
396403

397-
store.getState().setServerState(serverUrl, {
398-
clientInformation: clientInfo,
399-
});
404+
store.getState().setServerState(serverUrl, {
405+
clientInformation: clientInfo,
406+
});
400407

401-
type StateShape = {
402-
state: {
403-
servers: Record<string, { clientInformation?: OAuthClientInformation }>;
408+
type StateShape = {
409+
state: {
410+
servers: Record<
411+
string,
412+
{ clientInformation?: OAuthClientInformation }
413+
>;
414+
};
404415
};
405-
};
406-
const parsed = await waitForStateFile<StateShape>(
407-
stateFilePath,
408-
(p) => {
409-
const s = (p as StateShape)?.state?.servers?.[serverUrl];
410-
return !!s?.clientInformation;
411-
},
412-
{ timeout: 2000, interval: 50 },
413-
);
414-
expect(parsed.state.servers[serverUrl]?.clientInformation).toEqual(
415-
clientInfo,
416-
);
416+
const parsed = await waitForStateFile<StateShape>(
417+
persistTestPath,
418+
(p) => {
419+
const s = (p as StateShape)?.state?.servers?.[serverUrl];
420+
return !!s?.clientInformation;
421+
},
422+
{ timeout: 2000, interval: 50 },
423+
);
424+
expect(parsed.state.servers[serverUrl]?.clientInformation).toEqual(
425+
clientInfo,
426+
);
427+
} finally {
428+
try {
429+
await fs.unlink(persistTestPath);
430+
} catch {
431+
/* ignore */
432+
}
433+
}
417434
});
418435
});
419436

‎core/__tests__/inspectorClient-oauth-e2e.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { createTransportNode } from "../mcp/node/transport.js";
2121
import {
2222
TestServerHttp,
2323
waitForStateFile,
24+
waitForOAuthWellKnown,
2425
getDefaultServerConfig,
2526
createOAuthTestServerConfig,
2627
clearOAuthTestData,
@@ -135,6 +136,8 @@ describe("InspectorClient OAuth E2E", () => {
135136
server = new TestServerHttp(serverConfig);
136137
const port = await server.start();
137138
const serverUrl = `http://localhost:${port}`;
139+
await waitForOAuthWellKnown(serverUrl);
140+
await waitForOAuthWellKnown(serverUrl);
138141

139142
// Create client with static OAuth config
140143
const oauthConfig = createTestOAuthConfig({
@@ -209,6 +212,8 @@ describe("InspectorClient OAuth E2E", () => {
209212
server = new TestServerHttp(serverConfig);
210213
const port = await server.start();
211214
const serverUrl = `http://localhost:${port}`;
215+
await waitForOAuthWellKnown(serverUrl);
216+
await waitForOAuthWellKnown(serverUrl);
212217

213218
const oauthConfig = createTestOAuthConfig({
214219
mode: "static",
@@ -293,6 +298,8 @@ describe("InspectorClient OAuth E2E", () => {
293298
server = new TestServerHttp(serverConfig);
294299
const port = await server.start();
295300
const serverUrl = `http://localhost:${port}`;
301+
await waitForOAuthWellKnown(serverUrl);
302+
await waitForOAuthWellKnown(serverUrl);
296303

297304
const oauthConfig = createTestOAuthConfig({
298305
mode: "static",
@@ -383,6 +390,8 @@ describe("InspectorClient OAuth E2E", () => {
383390
server = new TestServerHttp(serverConfig);
384391
const port = await server.start();
385392
const serverUrl = `http://localhost:${port}`;
393+
await waitForOAuthWellKnown(serverUrl);
394+
await waitForOAuthWellKnown(serverUrl);
386395

387396
// Create client with CIMD config
388397
const oauthConfig = createTestOAuthConfig({
@@ -461,6 +470,8 @@ describe("InspectorClient OAuth E2E", () => {
461470
server = new TestServerHttp(serverConfig);
462471
const port = await server.start();
463472
const serverUrl = `http://localhost:${port}`;
473+
await waitForOAuthWellKnown(serverUrl);
474+
await waitForOAuthWellKnown(serverUrl);
464475

465476
const oauthConfig = createTestOAuthConfig({
466477
mode: "cimd",
@@ -521,6 +532,8 @@ describe("InspectorClient OAuth E2E", () => {
521532
server = new TestServerHttp(serverConfig);
522533
const port = await server.start();
523534
const serverUrl = `http://localhost:${port}`;
535+
await waitForOAuthWellKnown(serverUrl);
536+
await waitForOAuthWellKnown(serverUrl);
524537

525538
const oauthConfig = createTestOAuthConfig({
526539
mode: "dcr",
@@ -576,6 +589,8 @@ describe("InspectorClient OAuth E2E", () => {
576589
server = new TestServerHttp(serverConfig);
577590
const port = await server.start();
578591
const serverUrl = `http://localhost:${port}`;
592+
await waitForOAuthWellKnown(serverUrl);
593+
await waitForOAuthWellKnown(serverUrl);
579594

580595
const oauthConfig = createTestOAuthConfig({
581596
mode: "dcr",
@@ -645,6 +660,8 @@ describe("InspectorClient OAuth E2E", () => {
645660
server = new TestServerHttp(serverConfig);
646661
const port = await server.start();
647662
const serverUrl = `http://localhost:${port}`;
663+
await waitForOAuthWellKnown(serverUrl);
664+
await waitForOAuthWellKnown(serverUrl);
648665

649666
const oauthConfig = createTestOAuthConfig({
650667
mode: "dcr",
@@ -716,6 +733,8 @@ describe("InspectorClient OAuth E2E", () => {
716733
server = new TestServerHttp(serverConfig);
717734
const port = await server.start();
718735
const serverUrl = `http://localhost:${port}`;
736+
await waitForOAuthWellKnown(serverUrl);
737+
await waitForOAuthWellKnown(serverUrl);
719738

720739
const oauthConfig = createTestOAuthConfig({
721740
mode: "static",
@@ -802,6 +821,8 @@ describe("InspectorClient OAuth E2E", () => {
802821
server = new TestServerHttp(serverConfig);
803822
const port = await server.start();
804823
const serverUrl = `http://localhost:${port}`;
824+
await waitForOAuthWellKnown(serverUrl);
825+
await waitForOAuthWellKnown(serverUrl);
805826

806827
const oauthConfig = createTestOAuthConfig({
807828
mode: "static",
@@ -905,6 +926,8 @@ describe("InspectorClient OAuth E2E", () => {
905926
server = new TestServerHttp(serverConfig);
906927
const port = await server.start();
907928
const serverUrl = `http://localhost:${port}`;
929+
await waitForOAuthWellKnown(serverUrl);
930+
await waitForOAuthWellKnown(serverUrl);
908931

909932
const oauthConfig = createTestOAuthConfig({
910933
mode: "static",
@@ -1001,6 +1024,8 @@ describe("InspectorClient OAuth E2E", () => {
10011024
server = new TestServerHttp(serverConfig);
10021025
const port = await server.start();
10031026
const serverUrl = `http://localhost:${port}`;
1027+
await waitForOAuthWellKnown(serverUrl);
1028+
await waitForOAuthWellKnown(serverUrl);
10041029

10051030
const oauthConfig = createTestOAuthConfig({
10061031
mode: "static",
@@ -1076,6 +1101,8 @@ describe("InspectorClient OAuth E2E", () => {
10761101
server = new TestServerHttp(serverConfig);
10771102
const port = await server.start();
10781103
const serverUrl = `http://localhost:${port}`;
1104+
await waitForOAuthWellKnown(serverUrl);
1105+
await waitForOAuthWellKnown(serverUrl);
10791106

10801107
const oauthConfig = createTestOAuthConfig({
10811108
mode: "static",
@@ -1140,6 +1167,8 @@ describe("InspectorClient OAuth E2E", () => {
11401167
server = new TestServerHttp(serverConfig);
11411168
const port = await server.start();
11421169
const serverUrl = `http://localhost:${port}`;
1170+
await waitForOAuthWellKnown(serverUrl);
1171+
await waitForOAuthWellKnown(serverUrl);
11431172

11441173
const oauthConfig = createTestOAuthConfig({
11451174
mode: "dcr",
@@ -1194,6 +1223,8 @@ describe("InspectorClient OAuth E2E", () => {
11941223
server = new TestServerHttp(serverConfig);
11951224
const port = await server.start();
11961225
const serverUrl = `http://localhost:${port}`;
1226+
await waitForOAuthWellKnown(serverUrl);
1227+
await waitForOAuthWellKnown(serverUrl);
11971228

11981229
const oauthConfig = createTestOAuthConfig({
11991230
mode: "dcr",
@@ -1266,6 +1297,7 @@ describe("InspectorClient OAuth E2E", () => {
12661297
server = new TestServerHttp(serverConfig);
12671298
const port = await server.start();
12681299
const serverUrl = `http://localhost:${port}`;
1300+
await waitForOAuthWellKnown(serverUrl);
12691301

12701302
const oauthConfig = createTestOAuthConfig({
12711303
mode: "static",
@@ -1334,6 +1366,8 @@ describe("InspectorClient OAuth E2E", () => {
13341366
server = new TestServerHttp(serverConfig);
13351367
const port = await server.start();
13361368
const serverUrl = `http://localhost:${port}`;
1369+
await waitForOAuthWellKnown(serverUrl);
1370+
await waitForOAuthWellKnown(serverUrl);
13371371

13381372
const oauthConfig = createTestOAuthConfig({
13391373
mode: "static",
@@ -1404,6 +1438,8 @@ describe("InspectorClient OAuth E2E", () => {
14041438
server = new TestServerHttp(serverConfig);
14051439
const port = await server.start();
14061440
const serverUrl = `http://localhost:${port}`;
1441+
await waitForOAuthWellKnown(serverUrl);
1442+
await waitForOAuthWellKnown(serverUrl);
14071443

14081444
const oauthConfig = createTestOAuthConfig({
14091445
mode: "static",
@@ -1514,6 +1550,8 @@ describe("InspectorClient OAuth E2E", () => {
15141550
server = new TestServerHttp(serverConfig);
15151551
const port = await server.start();
15161552
const serverUrl = `http://localhost:${port}`;
1553+
await waitForOAuthWellKnown(serverUrl);
1554+
await waitForOAuthWellKnown(serverUrl);
15171555

15181556
const oauthConfig = createTestOAuthConfig({
15191557
mode: "static",
@@ -1592,6 +1630,7 @@ describe("InspectorClient OAuth E2E", () => {
15921630
server = new TestServerHttp(serverConfig);
15931631
const port = await server.start();
15941632
const serverUrl = `http://localhost:${port}`;
1633+
await waitForOAuthWellKnown(serverUrl);
15951634

15961635
const oauthConfig = createTestOAuthConfig({
15971636
mode: "static",
@@ -1668,6 +1707,7 @@ describe("InspectorClient OAuth E2E", () => {
16681707
server = new TestServerHttp(serverConfig);
16691708
const port = await server.start();
16701709
const serverUrl = `http://localhost:${port}`;
1710+
await waitForOAuthWellKnown(serverUrl);
16711711

16721712
const oauthConfig = createTestOAuthConfig({
16731713
mode: "static",
@@ -1774,6 +1814,7 @@ describe("InspectorClient OAuth E2E", () => {
17741814
server = new TestServerHttp(serverConfig);
17751815
const port = await server.start();
17761816
const serverUrl = `http://localhost:${port}`;
1817+
await waitForOAuthWellKnown(serverUrl);
17771818

17781819
const oauthConfig = createTestOAuthConfig({
17791820
mode: "static",

‎core/__tests__/inspectorClient-oauth-remote-storage-e2e.test.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import { NodeOAuthStorage } from "../auth/node/storage-node.js";
1818
import { createRemoteApp } from "../mcp/remote/node/server.js";
1919
import {
2020
TestServerHttp,
21+
waitForOAuthWellKnown,
22+
waitForRemoteStore,
2123
getDefaultServerConfig,
2224
createOAuthTestServerConfig,
2325
clearOAuthTestData,
@@ -190,6 +192,7 @@ describe("InspectorClient OAuth E2E with Remote Storage", () => {
190192
mcpServer = new TestServerHttp(serverConfig);
191193
const port = await mcpServer.start();
192194
const serverUrl = `http://localhost:${port}`;
195+
await waitForOAuthWellKnown(serverUrl);
193196

194197
// Create client with remote transport and remote OAuth storage
195198
const createTransport = createRemoteTransport({
@@ -280,6 +283,7 @@ describe("InspectorClient OAuth E2E with Remote Storage", () => {
280283
mcpServer = new TestServerHttp(serverConfig);
281284
const port = await mcpServer.start();
282285
const serverUrl = `http://localhost:${port}`;
286+
await waitForOAuthWellKnown(serverUrl);
283287

284288
const createTransport = createRemoteTransport({
285289
baseUrl: remoteBaseUrl!,
@@ -338,8 +342,28 @@ describe("InspectorClient OAuth E2E with Remote Storage", () => {
338342
expect(tokens1).toBeDefined();
339343
await client1.disconnect();
340344

341-
// Wait for persistence
342-
await new Promise((resolve) => setTimeout(resolve, 200));
345+
// Wait until remote server has persisted state before creating second client
346+
await waitForRemoteStore(
347+
remoteBaseUrl!,
348+
"oauth",
349+
remoteAuthToken!,
350+
(body) => {
351+
const b = body as {
352+
state?: {
353+
servers?: Record<
354+
string,
355+
{ tokens?: { access_token?: string } }
356+
>;
357+
};
358+
};
359+
return !!(
360+
b?.state?.servers &&
361+
Object.values(b.state.servers).some(
362+
(s) => s?.tokens?.access_token,
363+
)
364+
);
365+
},
366+
);
343367

344368
// Second client: should load persisted state
345369
const remoteStorage2 = new RemoteOAuthStorage({
@@ -422,6 +446,7 @@ describe("InspectorClient OAuth E2E with Remote Storage", () => {
422446
mcpServer = new TestServerHttp(serverConfig);
423447
const port = await mcpServer.start();
424448
const serverUrl = `http://localhost:${port}`;
449+
await waitForOAuthWellKnown(serverUrl);
425450

426451
const createTransport = createRemoteTransport({
427452
baseUrl: remoteBaseUrl!,

0 commit comments

Comments
 (0)