Skip to content

chore: add integration tests for count #78

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.

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ export default defineConfig([
// TODO: Configure tests and scripts to work with this.
ignores: ["eslint.config.js", "jest.config.js", "tests/**/*.ts", "scripts/**/*.ts"],
}),
globalIgnores(["node_modules", "dist", "src/common/atlas/openapi.d.ts"]),
globalIgnores(["node_modules", "dist", "src/common/atlas/openapi.d.ts", "coverage"]),
eslintConfigPrettier,
]);
2 changes: 1 addition & 1 deletion src/tools/mongodb/read/count.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class CountTool extends MongoDBToolBase {
return {
content: [
{
text: `Found ${count} documents in the collection \`${collection}\``,
text: `Found ${count} documents in the collection "${collection}"`,
type: "text",
},
],
Expand Down
7 changes: 7 additions & 0 deletions tests/integration/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface ParameterInfo {
name: string;
type: string;
description: string;
required: boolean;
}

type ToolInfo = Awaited<ReturnType<Client["listTools"]>>["tools"][number];
Expand Down Expand Up @@ -180,10 +181,16 @@ export function getParameters(tool: ToolInfo): ParameterInfo[] {
name: key,
type: typedValue.type,
description: typedValue.description,
required: (tool.inputSchema.required as string[])?.includes(key) ?? false,
};
});
}

export const dbOperationParameters: ParameterInfo[] = [
{ name: "database", type: "string", description: "Database name", required: true },
{ name: "collection", type: "string", description: "Collection name", required: true },
];

export function validateParameters(tool: ToolInfo, parameters: ParameterInfo[]): void {
const toolParameters = getParameters(tool);
expect(toolParameters).toHaveLength(parameters.length);
Expand Down
14 changes: 2 additions & 12 deletions tests/integration/tools/mongodb/create/createCollection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
jestTestMCPClient,
getResponseContent,
validateParameters,
dbOperationParameters,
} from "../../../helpers.js";
import { toIncludeSameMembers } from "jest-extended";
import { McpError } from "@modelcontextprotocol/sdk/types.js";
Expand All @@ -21,18 +22,7 @@ describe("createCollection tool", () => {
"Creates a new collection in a database. If the database doesn't exist, it will be created automatically."
);

validateParameters(listCollections, [
{
name: "database",
description: "Database name",
type: "string",
},
{
name: "collection",
description: "Collection name",
type: "string",
},
]);
validateParameters(listCollections, dbOperationParameters);
});

describe("with invalid arguments", () => {
Expand Down
1 change: 1 addition & 0 deletions tests/integration/tools/mongodb/metadata/connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe("Connect tool", () => {
name: "connectionStringOrClusterName",
description: "MongoDB connection string (in the mongodb:// or mongodb+srv:// format) or cluster name",
type: "string",
required: false,
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
jestTestCluster,
jestTestMCPClient,
getResponseContent,
getParameters,
validateParameters,
} from "../../../helpers.js";
import { toIncludeSameMembers } from "jest-extended";
Expand All @@ -20,7 +19,9 @@ describe("listCollections tool", () => {
expect(listCollections).toBeDefined();
expect(listCollections.description).toBe("List all collections for a given database");

validateParameters(listCollections, [{ name: "database", description: "Database name", type: "string" }]);
validateParameters(listCollections, [
{ name: "database", description: "Database name", type: "string", required: true },
]);
});

describe("with invalid arguments", () => {
Expand Down
118 changes: 118 additions & 0 deletions tests/integration/tools/mongodb/read/count.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import {
connect,
jestTestCluster,
jestTestMCPClient,
getResponseContent,
validateParameters,
dbOperationParameters,
} from "../../../helpers.js";
import { toIncludeSameMembers } from "jest-extended";
import { McpError } from "@modelcontextprotocol/sdk/types.js";
import { ObjectId } from "mongodb";

describe("count tool", () => {
const client = jestTestMCPClient();
const cluster = jestTestCluster();

let randomDbName: string;
beforeEach(() => {
randomDbName = new ObjectId().toString();
});

it("should have correct metadata", async () => {
const { tools } = await client().listTools();
const listCollections = tools.find((tool) => tool.name === "count")!;
expect(listCollections).toBeDefined();
expect(listCollections.description).toBe("Gets the number of documents in a MongoDB collection");

validateParameters(listCollections, [
{
name: "query",
description:
"The query filter to count documents. Matches the syntax of the filter argument of db.collection.count()",
type: "object",
required: false,
},
...dbOperationParameters,
]);
});

describe("with invalid arguments", () => {
const args = [
{},
{ database: 123, collection: "bar" },
{ foo: "bar", database: "test", collection: "bar" },
{ collection: [], database: "test" },
{ collection: "bar", database: "test", query: "{ $gt: { foo: 5 } }" },
];
for (const arg of args) {
Comment on lines +41 to +48
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const args = [
{},
{ database: 123, collection: "bar" },
{ foo: "bar", database: "test", collection: "bar" },
{ collection: [], database: "test" },
{ collection: "bar", database: "test", query: "{ $gt: { foo: 5 } }" },
];
for (const arg of args) {
for (const arg of [
{},
{ database: 123, collection: "bar" },
{ foo: "bar", database: "test", collection: "bar" },
{ collection: [], database: "test" },
{ collection: "bar", database: "test", query: "{ $gt: { foo: 5 } }" },
]) {

it(`throws a schema error for: ${JSON.stringify(arg)}`, async () => {
await connect(client(), cluster());
try {
await client().callTool({ name: "count", arguments: arg });
expect.fail("Expected an error to be thrown");
} catch (error) {
expect(error).toBeInstanceOf(McpError);
const mcpError = error as McpError;
expect(mcpError.code).toEqual(-32602);
expect(mcpError.message).toContain("Invalid arguments for tool count");
}
});
}
});

it("returns 0 when database doesn't exist", async () => {
await connect(client(), cluster());
const response = await client().callTool({
name: "count",
arguments: { database: "non-existent", collection: "foos" },
});
const content = getResponseContent(response.content);
expect(content).toEqual('Found 0 documents in the collection "foos"');
});

it("returns 0 when collection doesn't exist", async () => {
await connect(client(), cluster());
const mongoClient = cluster().getClient();
await mongoClient.db(randomDbName).collection("bar").insertOne({});
const response = await client().callTool({
name: "count",
arguments: { database: randomDbName, collection: "non-existent" },
});
const content = getResponseContent(response.content);
expect(content).toEqual('Found 0 documents in the collection "non-existent"');
});

describe("with existing database", () => {
beforeEach(async () => {
const mongoClient = cluster().getClient();
await mongoClient
.db(randomDbName)
.collection("foo")
.insertMany([
{ name: "Peter", age: 5 },
{ name: "Parker", age: 10 },
{ name: "George", age: 15 },
]);
});

const testCases = [
{ filter: undefined, expectedCount: 3 },
{ filter: {}, expectedCount: 3 },
{ filter: { age: { $lt: 15 } }, expectedCount: 2 },
{ filter: { age: { $gt: 5 }, name: { $regex: "^P" } }, expectedCount: 1 },
];
for (const testCase of testCases) {
Comment on lines +99 to +105
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const testCases = [
{ filter: undefined, expectedCount: 3 },
{ filter: {}, expectedCount: 3 },
{ filter: { age: { $lt: 15 } }, expectedCount: 2 },
{ filter: { age: { $gt: 5 }, name: { $regex: "^P" } }, expectedCount: 1 },
];
for (const testCase of testCases) {
for (const testCase of [
{ filter: undefined, expectedCount: 3 },
{ filter: {}, expectedCount: 3 },
{ filter: { age: { $lt: 15 } }, expectedCount: 2 },
{ filter: { age: { $gt: 5 }, name: { $regex: "^P" } }, expectedCount: 1 },
]) {

nit: I think this is quicker to read

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially had it like that, but then moved it out because I thought it's getting cluttered with the test case definition inlined into the for loop.

it(`returns ${testCase.expectedCount} documents for filter ${JSON.stringify(testCase.filter)}`, async () => {
await connect(client(), cluster());
const response = await client().callTool({
name: "count",
arguments: { database: randomDbName, collection: "foo", query: testCase.filter },
});

const content = getResponseContent(response.content);
expect(content).toEqual(`Found ${testCase.expectedCount} documents in the collection "foo"`);
});
}
});
});
Loading