-
Notifications
You must be signed in to change notification settings - Fork 70
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
Add integration and unit tests for missing RRF coverage #997
Add integration and unit tests for missing RRF coverage #997
Conversation
…pensearch-project#874) * initial commit of RRF Signed-off-by: Isaac Johnson <[email protected]> Co-authored-by: Varun Jain <[email protected]> Signed-off-by: Martin Gaievski <[email protected]>
…pensearch-project#874) * initial commit of RRF Signed-off-by: Isaac Johnson <[email protected]> Co-authored-by: Varun Jain <[email protected]> Signed-off-by: Martin Gaievski <[email protected]>
Please rebase to see the codecov comments. Thanks. |
…nsearch-project#999) Signed-off-by: Peter Zhu <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
5b613ca
to
24c7fa8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/rrf-score-normalization-v2 #997 +/- ##
========================================================================
+ Coverage 79.80% 80.59% +0.79%
- Complexity 1027 1043 +16
========================================================================
Files 83 83
Lines 3536 3536
Branches 592 592
========================================================================
+ Hits 2822 2850 +28
+ Misses 465 434 -31
- Partials 249 252 +3 ☔ View full report in Codecov by Sentry. |
@ryanbogan I've rebased on latest main out of curiosity, code coverage report/bot works. Please make sure to pull latest changes before making any edits locally. |
src/test/java/org/opensearch/neuralsearch/processor/RRFProcessorIT.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/neuralsearch/processor/RRFProcessorIT.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/neuralsearch/processor/RRFProcessorIT.java
Outdated
Show resolved
Hide resolved
a957fd3
to
29fafd6
Compare
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Code looks good to me. Just one ask - can you please add the BWC test, similar to what is there for normalization |
Signed-off-by: Ryan Bogan <[email protected]>
This reverts commit 2c00025. Signed-off-by: Ryan Bogan <[email protected]>
As discussed offline, let's hold off on BWC tests, we can do it after RRF is merged to main from feature branch, otherwise it will keep failing for 2.19 to main/3.0 |
c64f09b
into
opensearch-project:feature/rrf-score-normalization-v2
Description
This PR adds a successful end to end integration test using RRF, as well as unit tests for the RRFProcessor class, which was the last remaining RRF class without tests.
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.