-
Notifications
You must be signed in to change notification settings - Fork 139
chore: use describeWithMongoDB to setup test suites instead of custom wiring #650
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the describeWithMongoDB
helper function interface to accept a configuration object instead of individual parameters, and adds support for elicitation-related options. The change improves maintainability as the number of parameters was increasing.
Key changes:
- Refactored
describeWithMongoDB
to accept aTestSuiteConfig
object instead of separate function parameters - Added elicitation support with
getMockElicitationInput
andgetClientCapabilities
options - Updated all test files to use the new interface with
defaultTestSuiteConfig
as the base configuration
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/integration/tools/mongodb/mongodbHelpers.ts |
Defines new TestSuiteConfig interface and updates describeWithMongoDB function signature |
tests/integration/elicitation.test.ts |
Converts elicitation tests to use describeWithMongoDB with new elicitation options |
Multiple test files (search, read, connect, etc.) | Updates all existing describeWithMongoDB calls to use new object-based configuration |
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.
Additionally modifies elicitation tests to use describeWithMongoDB
d8fea3a
to
475ea09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense - just a minor ergonomics suggestion from me.
Proposed changes
In some places we were doing the same wiring as the one that is already done in
describeWithMongoDB
and the reason for that was we needed some extra configuration to be passed down tosetupIntegrationTest
or tosetupMongoDBIntegrationTest
. This PR modifiesdescribeWithMongoDB
to accept the additional parameters and pass them down tosetupIntegrationTest
andsetupMongoDBIntegrationTest
and unify the usage in tests to usedescribeWithMongoDB
instead of manual wiring.The motivation is to have one place to have checks like skipping test on specific environments.
Notes for reviewers
Review individual commits.
Checklist