chore: add integration tests for insert-many #85
Conversation
| expect(content).toContain('Inserted `1` document(s) into collection "coll1"'); | ||
| }); | ||
|
|
||
| it("throw an error if connection string is not configured", async () => { |
There was a problem hiding this comment.
[nitpick] Consider updating the test description to 'throws an error if connection string is not configured' to maintain grammatical consistency.
| it("throw an error if connection string is not configured", async () => { | |
| it("throws an error if connection string is not configured", async () => { |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
| await integration.connectMcpClient(); | ||
| const response = await integration.mcpClient().callTool({ | ||
| name: "insert-many", | ||
| arguments: { | ||
| database: integration.randomDbName(), |
There was a problem hiding this comment.
Store the random database name in a variable and reuse it for both the tool call and the subsequent validation to ensure consistency between the created and verified namespaces.
| await integration.connectMcpClient(); | |
| const response = await integration.mcpClient().callTool({ | |
| name: "insert-many", | |
| arguments: { | |
| database: integration.randomDbName(), | |
| const dbName = integration.randomDbName(); | |
| await integration.connectMcpClient(); | |
| const response = await integration.mcpClient().callTool({ | |
| name: "insert-many", | |
| arguments: { | |
| database: dbName, |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Pull Request Test Coverage Report for Build 14591816093Details
💛 - Coveralls |
| @@ -27,7 +27,7 @@ export class InsertManyTool extends MongoDBToolBase { | |||
| return { | |||
| content: [ | |||
| { | |||
| text: `Inserted \`${result.insertedCount}\` documents into collection \`${collection}\``, | |||
| text: `Inserted \`${result.insertedCount}\` document(s) into collection "${collection}"`, | |||
There was a problem hiding this comment.
is there a reason for the change of the `?
There was a problem hiding this comment.
Mostly consistency - it doesn't really matter as in my experience, the model will not return the result as-is, but rather reformat it. But we've been using quotes in other places, so figured I should standardize around it.
| const args = [ | ||
| {}, | ||
| { collection: "bar", database: 123, documents: [] }, | ||
| { collection: [], database: "test", documents: [] }, | ||
| { collection: "bar", database: "test", documents: "my-document" }, | ||
| { collection: "bar", database: "test", documents: { name: "Peter" } }, | ||
| ]; | ||
| for (const arg of args) { |
There was a problem hiding this comment.
| const args = [ | |
| {}, | |
| { collection: "bar", database: 123, documents: [] }, | |
| { collection: [], database: "test", documents: [] }, | |
| { collection: "bar", database: "test", documents: "my-document" }, | |
| { collection: "bar", database: "test", documents: { name: "Peter" } }, | |
| ]; | |
| for (const arg of args) { | |
| for (const arg of [ | |
| {}, | |
| { collection: "bar", database: 123, documents: [] }, | |
| { collection: [], database: "test", documents: [] }, | |
| { collection: "bar", database: "test", documents: "my-document" }, | |
| { collection: "bar", database: "test", documents: { name: "Peter" } }, | |
| ]) { |
nirinchev
force-pushed
the
ni/integration-insert-many
branch
from
3084bc0 to
e5b67cf
Compare
Also, removes insertOne as it seems the model is good enough to figure out the usage of insert-many even with a single doc.