Fixed failing unit test

[martin-gaievski]

Signed-off-by: Martin Gaievski [email protected]

Description

Fixed flaky unit tests, narrow assertion logic and adjust for test runs with multi node mock cluster

Rolling upgrades are failing, presumable due to issue with security plugin opensearch-project/security#2228

Repro for certain seeds:

./gradlew ':test' --tests "org.opensearch.knn.index.codec.KNN940Codec.KNN940CodecTests.testKnnVectorIndex" -Dtests.seed=B5E0DC7BF07CBCC1 -Dtests.security.manager=false -Dtests.locale=el-CY -Dtests.timezone=America/Mendoza -Druntime.java=17

org.opensearch.knn.index.codec.KNN940Codec.KNN940CodecTests > testKnnVectorIndex FAILED
    org.mockito.exceptions.verification.TooManyActualInvocations:
    kNN940PerFieldKnnVectorsFormat.getKnnVectorsFormatForField(
        <any string>
    );

After change test runs are succesful:

./gradlew ':test' -Dtests.seed=B5E0DC7BF07CBCC1

BUILD SUCCESSFUL in 1m 35s
15 actionable tasks: 6 executed, 9 up-to-date

Issues Resolved

[List any issues this PR will resolve]

Check List

• All tests pass
• Commits are signed as per the DCO using --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.

[vamshin]

LGTM! Thanks

[codecov-commenter]

Codecov Report

Merging #610 (434670f) into main (120c4ce) will decrease coverage by 0.25%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #610      +/-   ##
============================================
- Coverage     84.73%   84.47%   -0.26%     
+ Complexity     1055     1050       -5     
============================================
  Files           149      149              
  Lines          4291     4291              
  Branches        379      379              
============================================
- Hits           3636     3625      -11     
- Misses          480      489       +9     
- Partials        175      177       +2     

Impacted Files	Coverage Δ
...va/org/opensearch/knn/index/KNNCircuitBreaker.java	60.00% <0.00%> (-20.00%)	⬇️
...ain/java/org/opensearch/knn/index/KNNSettings.java	80.88% <0.00%> (-2.21%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[martin-gaievski]

At first glance issue with rolling upgrades may be caused by some recent change in core transport code. I've made some test runs and logged issue for core opensearch-project/OpenSearch#5065

[martin-gaievski]

At first glance issue with rolling upgrades may be caused by some recent change in core transport code. I've made some test runs and logged issue for core opensearch-project/OpenSearch#5065

As per reply from core team 2.4 is not compatible with 3.0 builds, the plan is to port some changes in next 2.x to make next 2.x release (presumably 2.5 ) compatible with main/3.0. Until then bwc in CI will keep failing

[heemin32]

Overall, it seems we are easing the test by giving minimum number instead of specific number for a method to be invoked. Could you justify the change?

[martin-gaievski]

Overall, it seems we are easing the test by giving minimum number instead of specific number for a method to be invoked. Could you justify the change?

Right, so depending on the sequence of method execution mock cluster can be in a different states, and that may or may not trigger additional serialization/deserialization calls, and this influence the number of method calls. Particular number of calls isn't essential for this test, we just want to make sure call to the knnvector format got intercepted and out implementation executed.

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-610-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 ca49f73e8fcccc41a86e607b254ae10840aace62
# Push it to GitHub
git push --set-upstream origin backport/backport-610-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-610-to-2.x.