-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update FTS tests #36928
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
base: main
Are you sure you want to change the base?
Update FTS tests #36928
Conversation
…gging - Removed .skip from FTSQuery describe to enable tests - Updated expected values for RRF queries to match .NET SDK HybridSearchQueryTests - Updated expected values for VectorDistance queries to match .NET SDK - Separated weighted RRF tests into a skipped describe block (matches .NET SDK [Ignore]) - Improved error messages to show actual vs expected values for easier debugging - Increased test timeout to 120000ms for emulator tests Co-authored-by: topshot99 <[email protected]>
… references - Added comment explaining the 120s timeout rationale - Added full GitHub URL reference to .NET SDK HybridSearchQueryTests.cs - Moved comment for LIMIT 11 change to be more visible Co-authored-by: topshot99 <[email protected]>
|
@copilot can u run the tests whether they are passing in JS now? also can u run it and verify in .NET? |
I cannot run the integration tests in this environment because the CosmosDB Windows emulator is not available here. These tests require a running emulator instance. To verify the tests locally, you can run: JS SDK: cd sdk/cosmosdb/cosmos
npm run test:node:integration -- --grep "FTSQuery".NET SDK: cd Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests
dotnet test --filter "FullyQualifiedName~HybridSearchQueryTests"The changes I made align the JS expected values with the .NET SDK's
This should help identify any remaining discrepancies. |
|
@copilot changes looks good and even tests are passing, i want u to remove these comments as they are not making sense: and // Updated to match .NET SDK expected values and and as they won't make sense in SDK |
Removed the following comments as requested: - Test timeout explanation comment block - "Updated to match .NET SDK expected values" comments - LIMIT change note with GitHub URL - Weighted RRF tests separation explanation Co-authored-by: topshot99 <[email protected]>
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 updates the FullTextSearch (FTS) test expectations in fullTextSearch.spec.ts to match the .NET SDK's HybridSearchQueryTests.cs implementation. The tests were previously skipped and had incorrect expected values.
Key changes:
- Enabled FTS tests by removing
.skipfrom the main describe block - Updated expected values for RRF and VectorDistance queries to match .NET SDK behavior
- Separated weighted RRF tests into a skipped describe block (emulator not yet supporting this feature)
- Improved error messages to show actual vs expected values for easier debugging
- Increased timeout from 20s to 120s for emulator operations
|
@copilot skip FTSQuery Tests as they take lot of time and may impact pipeline runs using describe.skip |
Added describe.skip with a comment explaining the tests are time-intensive Co-authored-by: topshot99 <[email protected]>
Packages impacted by this PR
@azure/cosmos
Issues associated with this PR
Describe the problem that is addressed by this PR
The FullTextSearch tests in
fullTextSearch.spec.tshad expected values that differed from the .NET SDK'sHybridSearchQueryTests.cs. This PR aligns JS SDK test expectations with .NET SDK. The tests are skipped by default as they are time-intensive and may impact pipeline runs.Key differences found:
[..., 25, 22, 2, 66, ...][..., 2, 22, ...][21, 75, 37, 24, 26, ...][21, 37, 75, 26, 35, ...]What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
HybridSearchQueryTests.csdescribe.skipas they are time-intensive and may impact pipeline runsAre there test cases added in this PR? (If not, why?)
This PR modifies existing tests to fix expected values. No new test cases needed.
Provide a list of related PRs (if any)
.NET SDK reference: https://github.com/Azure/azure-cosmos-dotnet-v3/blob/master/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Query/HybridSearchQueryTests.cs
Command used to generate this PR:**(Applicable only to SDK release request PRs)
N/A
Checklists
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.