Skip to content
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

feat: adding full text search support #746

Merged
merged 4 commits into from
Jan 24, 2024
Merged

feat: adding full text search support #746

merged 4 commits into from
Jan 24, 2024

Conversation

gsi-alejandro
Copy link
Collaborator

Closes: #745

Copy link

codecov bot commented Dec 12, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (ddb0d12) 93.36% compared to head (fed3e00) 93.20%.

Files Patch % Lines
src/ottoman/ottoman.ts 40.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #746      +/-   ##
==========================================
- Coverage   93.36%   93.20%   -0.17%     
==========================================
  Files          96       96              
  Lines        2667     2677      +10     
  Branches      641      642       +1     
==========================================
+ Hits         2490     2495       +5     
- Misses        177      182       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -0,0 +1,33 @@
import { searchQuery, SearchQuery } from '../src';

const maybe = process.env.CI ? test.skip : test;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this mean we skip these tests on CI? Is that due to the cb server version not supporting FTS?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests were marked to be skipped in the CI due to the data being stored in an FTS index and we don't know the time for they to be ready and return the correct values, so we tried adding some delays but it is uncertain and only worked a few times with 15 sec of delay which is too much delay and even with that fails most the times.

Note: Testing indexed data in a CI can be tricky

docusaurus/docs/advanced/fts.md Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

This docs page looks really good 🙂 nice work!

@ejscribner
Copy link
Collaborator

This all looks awesome! Just a few small nits, but otherwise looks great!

The Couchbase server version used in GitHub workflows (main.yml and pull_request.yml) has been updated. The new version 7.1.3 replaces the previous version 7.0.2. This ensures our testing and integration environment uses the latest stable release of Couchbase server.
Added a delay function call in the beforeAll hook in the `fts.spec.ts` file to ensure preconditions are met before test execution. This delay is particularly important in cases where asynchronous setup operations have to be completed before the tests run.
The `maybe` function has been introduced in the `fts.spec.ts` test file to skip tests when in a continuous integration environment. The beforeAll hook using delay has been removed, since test cases are either run or skipped based on the process environment, thereby improving test execution efficiency
@ejscribner ejscribner merged commit c9af0b1 into master Jan 24, 2024
3 checks passed
@ejscribner ejscribner deleted the task/745 branch January 24, 2024 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Full Text Search support implementation
2 participants