Skip to content

Commit 3eba906

Browse files
chore: change interface of describeWithMongoDB
The number of parameters were increasing as I wanted to add elicitInput as another argument. This commit compacts the rest of the config parameters in an object and modifies the usage to call the method correctly. Additionally, in places where we were just doing the defaults, this commit modifies those calls to not pass anything and rely on default.
1 parent 8e3c6a6 commit 3eba906

File tree

10 files changed

+165
-100
lines changed

10 files changed

+165
-100
lines changed

tests/integration/common/connectionManager.oidc.test.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import { describe, beforeEach, afterAll, it, expect, vi } from "vitest";
33
import semver from "semver";
44
import process from "process";
55
import type { MongoDBIntegrationTestCase } from "../tools/mongodb/mongodbHelpers.js";
6-
import { describeWithMongoDB, isCommunityServer, getServerVersion } from "../tools/mongodb/mongodbHelpers.js";
6+
import {
7+
describeWithMongoDB,
8+
isCommunityServer,
9+
getServerVersion,
10+
defaultTestSuiteConfig,
11+
} from "../tools/mongodb/mongodbHelpers.js";
712
import { defaultTestConfig, responseAsText, timeout, waitUntil } from "../helpers.js";
813
import type { ConnectionStateConnected, ConnectionStateConnecting } from "../../../src/common/connectionManager.js";
914
import type { UserConfig } from "../../../src/common/config.js";
@@ -137,14 +142,20 @@ describe.skipIf(process.platform !== "linux")("ConnectionManager OIDC Tests", as
137142

138143
addCb?.(oidcIt);
139144
},
140-
() => oidcConfig,
141-
() => ({
142-
...setupDriverConfig({
143-
config: oidcConfig,
144-
defaults: {},
145-
}),
146-
}),
147-
{ runner: true, downloadOptions: { enterprise: true, version: mongodbVersion }, serverArgs }
145+
{
146+
...defaultTestSuiteConfig,
147+
getUserConfig: () => oidcConfig,
148+
getDriverOptions: () =>
149+
setupDriverConfig({
150+
config: oidcConfig,
151+
defaults: {},
152+
}),
153+
downloadOptions: {
154+
runner: true,
155+
downloadOptions: { enterprise: true, version: mongodbVersion },
156+
serverArgs,
157+
},
158+
}
148159
);
149160
}
150161

tests/integration/indexCheck.test.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { defaultTestConfig, getResponseContent } from "./helpers.js";
2-
import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js";
2+
import { defaultTestSuiteConfig, describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js";
33
import { beforeEach, describe, expect, it } from "vitest";
44

55
describe("IndexCheck integration tests", () => {
@@ -313,10 +313,13 @@ describe("IndexCheck integration tests", () => {
313313
});
314314
});
315315
},
316-
() => ({
317-
...defaultTestConfig,
318-
indexCheck: true, // Enable indexCheck
319-
})
316+
{
317+
...defaultTestSuiteConfig,
318+
getUserConfig: () => ({
319+
...defaultTestConfig,
320+
indexCheck: true, // Enable indexCheck
321+
}),
322+
}
320323
);
321324
});
322325

@@ -424,10 +427,13 @@ describe("IndexCheck integration tests", () => {
424427
expect(content).not.toContain("Index check failed");
425428
});
426429
},
427-
() => ({
428-
...defaultTestConfig,
429-
indexCheck: false, // Disable indexCheck
430-
})
430+
{
431+
...defaultTestSuiteConfig,
432+
getUserConfig: () => ({
433+
...defaultTestConfig,
434+
indexCheck: false, // Disable indexCheck
435+
}),
436+
}
431437
);
432438
});
433439

@@ -456,10 +462,13 @@ describe("IndexCheck integration tests", () => {
456462
expect(response.isError).toBeFalsy();
457463
});
458464
},
459-
() => ({
460-
...defaultTestConfig,
461-
// indexCheck not specified, should default to false
462-
})
465+
{
466+
...defaultTestSuiteConfig,
467+
getUserConfig: () => ({
468+
...defaultTestConfig,
469+
// indexCheck not specified, should default to false
470+
}),
471+
}
463472
);
464473
});
465474
});

tests/integration/resources/exportedData.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { EJSON, Long, ObjectId } from "bson";
44
import { describe, expect, it, beforeEach, afterAll } from "vitest";
55
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
66
import { defaultTestConfig, getDataFromUntrustedContent, resourceChangedNotification, timeout } from "../helpers.js";
7-
import { describeWithMongoDB } from "../tools/mongodb/mongodbHelpers.js";
7+
import { defaultTestSuiteConfig, describeWithMongoDB } from "../tools/mongodb/mongodbHelpers.js";
88
import { contentWithResourceURILink } from "../tools/mongodb/read/export.test.js";
99
import type { UserConfig } from "../../../src/lib.js";
1010

@@ -173,5 +173,8 @@ describeWithMongoDB(
173173
});
174174
});
175175
},
176-
() => userConfig
176+
{
177+
...defaultTestSuiteConfig,
178+
getUserConfig: () => userConfig,
179+
}
177180
);

tests/integration/server.test.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { defaultDriverOptions, defaultTestConfig, expectDefined, setupIntegrationTest } from "./helpers.js";
2-
import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js";
2+
import { defaultTestSuiteConfig, describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js";
33
import { describe, expect, it } from "vitest";
44

55
describe("Server integration test", () => {
@@ -15,12 +15,14 @@ describe("Server integration test", () => {
1515
expect(atlasTools.length).toBeLessThanOrEqual(0);
1616
});
1717
},
18-
() => ({
19-
...defaultTestConfig,
20-
apiClientId: undefined,
21-
apiClientSecret: undefined,
22-
}),
23-
() => defaultDriverOptions
18+
{
19+
...defaultTestSuiteConfig,
20+
getUserConfig: () => ({
21+
...defaultTestConfig,
22+
apiClientId: undefined,
23+
apiClientSecret: undefined,
24+
}),
25+
}
2426
);
2527

2628
describe("with atlas", () => {

tests/integration/tools/mongodb/connect/connect.test.ts

Lines changed: 50 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import { describeWithMongoDB } from "../mongodbHelpers.js";
1+
import { defaultTestSuiteConfig, describeWithMongoDB } from "../mongodbHelpers.js";
22
import {
33
defaultDriverOptions,
44
getResponseContent,
55
getResponseElements,
66
validateThrowsForInvalidArguments,
77
validateToolMetadata,
88
} from "../../../helpers.js";
9-
import { config } from "../../../../../src/common/config.js";
109
import { defaultTestConfig, setupIntegrationTest } from "../../../helpers.js";
1110
import { beforeEach, describe, expect, it } from "vitest";
1211

@@ -82,66 +81,65 @@ describeWithMongoDB(
8281
});
8382
});
8483
},
85-
(mdbIntegration) => ({
86-
...config,
87-
connectionString: mdbIntegration.connectionString(),
88-
})
84+
{
85+
...defaultTestSuiteConfig,
86+
getUserConfig: (mdbIntegration) => ({
87+
...defaultTestConfig,
88+
connectionString: mdbIntegration.connectionString(),
89+
}),
90+
}
8991
);
9092

91-
describeWithMongoDB(
92-
"Connect tool",
93-
(integration) => {
94-
validateToolMetadata(
95-
integration,
96-
"connect",
97-
"Connect to a MongoDB instance. The config resource captures if the server is already connected to a MongoDB cluster. If the user has configured a connection string or has previously called the connect tool, a connection is already established and there's no need to call this tool unless the user has explicitly requested to switch to a new MongoDB cluster.",
98-
[
99-
{
100-
name: "connectionString",
101-
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format)",
102-
type: "string",
103-
required: true,
104-
},
105-
]
106-
);
93+
describeWithMongoDB("Connect tool", (integration) => {
94+
validateToolMetadata(
95+
integration,
96+
"connect",
97+
"Connect to a MongoDB instance. The config resource captures if the server is already connected to a MongoDB cluster. If the user has configured a connection string or has previously called the connect tool, a connection is already established and there's no need to call this tool unless the user has explicitly requested to switch to a new MongoDB cluster.",
98+
[
99+
{
100+
name: "connectionString",
101+
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format)",
102+
type: "string",
103+
required: true,
104+
},
105+
]
106+
);
107107

108-
validateThrowsForInvalidArguments(integration, "connect", [{}, { connectionString: 123 }]);
108+
validateThrowsForInvalidArguments(integration, "connect", [{}, { connectionString: 123 }]);
109109

110-
it("doesn't have the switch-connection tool registered", async () => {
111-
const { tools } = await integration.mcpClient().listTools();
112-
const tool = tools.find((tool) => tool.name === "switch-connection");
113-
expect(tool).toBeUndefined();
114-
});
110+
it("doesn't have the switch-connection tool registered", async () => {
111+
const { tools } = await integration.mcpClient().listTools();
112+
const tool = tools.find((tool) => tool.name === "switch-connection");
113+
expect(tool).toBeUndefined();
114+
});
115115

116-
describe("with connection string", () => {
117-
it("connects to the database", async () => {
118-
const response = await integration.mcpClient().callTool({
119-
name: "connect",
120-
arguments: {
121-
connectionString: integration.connectionString(),
122-
},
123-
});
124-
const content = getResponseContent(response.content);
125-
expect(content).toContain("Successfully connected");
116+
describe("with connection string", () => {
117+
it("connects to the database", async () => {
118+
const response = await integration.mcpClient().callTool({
119+
name: "connect",
120+
arguments: {
121+
connectionString: integration.connectionString(),
122+
},
126123
});
124+
const content = getResponseContent(response.content);
125+
expect(content).toContain("Successfully connected");
127126
});
127+
});
128128

129-
describe("with invalid connection string", () => {
130-
it("returns error message", async () => {
131-
const response = await integration.mcpClient().callTool({
132-
name: "connect",
133-
arguments: { connectionString: "mangodb://localhost:12345" },
134-
});
135-
const content = getResponseContent(response.content);
136-
expect(content).toContain("The configured connection string is not valid.");
137-
138-
// Should not suggest using the config connection string (because we don't have one)
139-
expect(content).not.toContain("Your config lists a different connection string");
129+
describe("with invalid connection string", () => {
130+
it("returns error message", async () => {
131+
const response = await integration.mcpClient().callTool({
132+
name: "connect",
133+
arguments: { connectionString: "mangodb://localhost:12345" },
140134
});
135+
const content = getResponseContent(response.content);
136+
expect(content).toContain("The configured connection string is not valid.");
137+
138+
// Should not suggest using the config connection string (because we don't have one)
139+
expect(content).not.toContain("Your config lists a different connection string");
141140
});
142-
},
143-
() => config
144-
);
141+
});
142+
});
145143

146144
describe("Connect tool when disabled", () => {
147145
const integration = setupIntegrationTest(

tests/integration/tools/mongodb/mongodbHelpers.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,22 @@ export type MongoDBIntegrationTestCase = IntegrationTest &
6666

6767
export type MongoSearchConfiguration = { search: true; image?: string };
6868

69+
export type TestSuiteConfig = {
70+
getUserConfig: (mdbIntegration: MongoDBIntegrationTest) => UserConfig;
71+
getDriverOptions: (mdbIntegration: MongoDBIntegrationTest) => DriverOptions;
72+
downloadOptions: MongoClusterConfiguration;
73+
};
74+
75+
export const defaultTestSuiteConfig: TestSuiteConfig = {
76+
getUserConfig: () => defaultTestConfig,
77+
getDriverOptions: () => defaultDriverOptions,
78+
downloadOptions: DEFAULT_MONGODB_PROCESS_OPTIONS,
79+
};
80+
6981
export function describeWithMongoDB(
7082
name: string,
7183
fn: (integration: MongoDBIntegrationTestCase) => void,
72-
getUserConfig: (mdbIntegration: MongoDBIntegrationTest) => UserConfig = () => defaultTestConfig,
73-
getDriverOptions: (mdbIntegration: MongoDBIntegrationTest) => DriverOptions = () => defaultDriverOptions,
74-
downloadOptions: MongoClusterConfiguration = DEFAULT_MONGODB_PROCESS_OPTIONS
84+
{ getUserConfig, getDriverOptions, downloadOptions }: TestSuiteConfig = defaultTestSuiteConfig
7585
): void {
7686
describe.skipIf(!MongoDBClusterProcess.isConfigurationSupportedInCurrentEnv(downloadOptions))(name, () => {
7787
const mdbIntegration = setupMongoDBIntegrationTest(downloadOptions);

tests/integration/tools/mongodb/read/aggregate.test.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ import {
66
defaultTestConfig,
77
} from "../../../helpers.js";
88
import { beforeEach, describe, expect, it, vi, afterEach } from "vitest";
9-
import { describeWithMongoDB, getDocsFromUntrustedContent, validateAutoConnectBehavior } from "../mongodbHelpers.js";
9+
import {
10+
defaultTestSuiteConfig,
11+
describeWithMongoDB,
12+
getDocsFromUntrustedContent,
13+
validateAutoConnectBehavior,
14+
} from "../mongodbHelpers.js";
1015
import * as constants from "../../../../../src/helpers/constants.js";
1116
import { freshInsertDocuments } from "./find.test.js";
1217

@@ -282,7 +287,10 @@ describeWithMongoDB(
282287
);
283288
});
284289
},
285-
() => ({ ...defaultTestConfig, maxDocumentsPerQuery: 20 })
290+
{
291+
...defaultTestSuiteConfig,
292+
getUserConfig: () => ({ ...defaultTestConfig, maxDocumentsPerQuery: 20 }),
293+
}
286294
);
287295

288296
describeWithMongoDB(
@@ -339,7 +347,10 @@ describeWithMongoDB(
339347
);
340348
});
341349
},
342-
() => ({ ...defaultTestConfig, maxBytesPerQuery: 200 })
350+
{
351+
...defaultTestSuiteConfig,
352+
getUserConfig: () => ({ ...defaultTestConfig, maxBytesPerQuery: 200 }),
353+
}
343354
);
344355

345356
describeWithMongoDB(
@@ -369,5 +380,8 @@ describeWithMongoDB(
369380
expect(content).toContain(`Returning 990 documents.`);
370381
});
371382
},
372-
() => ({ ...defaultTestConfig, maxDocumentsPerQuery: -1, maxBytesPerQuery: -1 })
383+
{
384+
...defaultTestSuiteConfig,
385+
getUserConfig: () => ({ ...defaultTestConfig, maxDocumentsPerQuery: -1, maxBytesPerQuery: -1 }),
386+
}
373387
);

tests/integration/tools/mongodb/read/export.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
validateThrowsForInvalidArguments,
1111
validateToolMetadata,
1212
} from "../../../helpers.js";
13-
import { describeWithMongoDB } from "../mongodbHelpers.js";
13+
import { defaultTestSuiteConfig, describeWithMongoDB } from "../mongodbHelpers.js";
1414
import type { UserConfig } from "../../../../../src/lib.js";
1515

1616
const userConfig: UserConfig = {
@@ -458,5 +458,8 @@ describeWithMongoDB(
458458
});
459459
});
460460
},
461-
() => userConfig
461+
{
462+
...defaultTestSuiteConfig,
463+
getUserConfig: () => userConfig,
464+
}
462465
);

0 commit comments

Comments
 (0)