chore: add integration tests for create-index

[nirinchev]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 14591543426

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 5 of 6 (83.33%) changed or added relevant lines in 2 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.6%) to 51.548%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/tools/mongodb/metadata/connect.ts	2	3	66.67%

Files with Coverage Reduction	New Missed Lines	%
src/common/atlas/apiClient.ts	2	8.0%

Totals
Change from base Build 14502491127:	0.6%
Covered Lines:	406
Relevant Lines:	692

---

💛 - Coveralls

[Copilot]

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

src/tools/mongodb/create/createIndex.ts:13

• When 'name' is not provided, ensure that the tool automatically generates a default index name to match the expectations in integration tests. If provider.createIndexes does not generate a default name, add logic to compute and assign one.

name: z.string().optional().describe("The name of the index"),

src/tools/mongodb/metadata/connect.ts:38

• The removal of the error-throwing branch for non-string connection options eliminates explicit validation for invalid connection configurations. Consider reintroducing a proper error message or validation check for non-string inputs.

if (typeof connectionStringOrClusterName === "string") {

tests/integration/tools/mongodb/metadata/listCollections.test.ts:91

• [nitpick] Using integration.randomDbName() multiple times in the same test case assumes that it will always return the same value. Ensure that randomDbName() is stable within a test run to avoid intermittent test failures.

expect(content).toEqual(`No collections found for database "${integration.randomDbName()}". To create a collection, use the "create-collection" tool.`);

[fmenezes]

?

[nirinchev]

This is unchanged code, but it's the hook where we'd want to call any atlas API that would allow us to generate a connection string based on a cluster name (if possible).

[gagik]

should we add some text to that todo then?

[nirinchev]

The text we return from the next statement is pretty much what we would write in the TODO comment, but happy to update it and add a link to the issue.

[fmenezes]

It seems you're assigning the same value, would you double check if this is needed?

[nirinchev]

connectToMongoDB - if successful - will set the session provider.

[gagik]

is there a scenario where NodeDriverServiceProvider.connect returns undefined? seems like it'd throw instead right (and not be caught by connectToMongoDB)?

[nirinchev]

Based on the types, if the promise resolves, we should get a valid NodeServiceProvider and not undefined. If the connect attempt fails, this will throw an error which will eventually be bubbled up to the error handler of whatever tool the user was calling.

[gagik]

do I understand correctly then that the second if (!this.session.serviceProvider) would never occur? We could make the connectToMongoDB return the provider and set it to the local provider variable (I guess going back to that..) to make that type logic work

[nirinchev]

I changed it to always use this.session.serviceProvider and removed the local provider var.

[gagik]

maybe we should directly use this.session.serviceProvider at all times to avoid confusion