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

[BUG] 30_max_analyzed_offset failing #686

Closed
dblock opened this issue Mar 4, 2024 · 7 comments
Closed

[BUG] 30_max_analyzed_offset failing #686

dblock opened this issue Mar 4, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@dblock
Copy link
Member

dblock commented Mar 4, 2024

What is the bug?

FAILED test_opensearchpy/test_server/test_rest_api_spec.py::test_rest_api_spec[OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/search/highlight/30_max_analyzed_offset[5]]

How can one reproduce the bug?

Latest builds are failing in this REST spec.

@saimedhi
Copy link
Collaborator

saimedhi commented Mar 13, 2024

  • The failing test link is here.

  • This failure is related to this issue.

  • The fix is present in opensearch 2.x, but it is unreleased. Ideally, the fix may be included in version 2.13.0 when it is released.

  • I believe the skip version in this test should be set to " - 2.12.99". @Xtansia, what do you think? Did I miss anything?

Additional info:

  • I debugged the opensearch-net client to check if this test is executed and noticed that it is skipped for all currently released opensearch versions. It is only running for the unreleased version, where it is working fine. @Xtansia, I have one more question: How is the opensearch-net client correctly interpreting the skip version as " - 2.12.99" even though it is currently specified as " - 2.1.99"

  • Regarding opensearch-rs, similar to opensearch-net yaml test is skipped for released opensearch versions and is running fine for unreleased versions.

@saimedhi
Copy link
Collaborator

  • How is the opensearch-net client correctly interpreting the skip version as " - 2.12.99" even though it is currently specified as " - 2.1.99"

Got it @Xtansia! In opensearch-net's YAML test runner, tests against each opensearch version are taken from the corresponding opensearch version, such as 2.x branch when tests are run against 2.x.

On the other hand, opensearch-py uses only yaml tests from the main branch to test all opensearch versions.

Thomas, what do you think is the correct way to test? I'm inclined towards testing all opensearch versions against the opensearch main branch yaml tests. This should be the place where we'll have all the tests correctly, including skip tests that determine whether the test should be run on a particular branch or not.

@dblock
Copy link
Member Author

dblock commented Mar 13, 2024

If I understand correctly OpenSearch/main and 2.x are eventually consistent and should be identical? So it shouldn't matter where we take them from. However, I think it's best if we standardized on one way of doing it.

@dblock
Copy link
Member Author

dblock commented Mar 13, 2024

Another thought. If OpenSearch 3.x (main) removes an API, would it be deleted from main? In that case we want to take tests from 2.x.

@saimedhi
Copy link
Collaborator

saimedhi commented Mar 13, 2024

Listing other failing tests against the unreleased OpenSearch main branch. Working on fixing them.

  • FAILED test_opensearchpy/test_server/test_rest_api_spec.py::test_rest_api_spec[OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/indices/forcemerge/10_basic[2]]
  • FAILED test_opensearchpy/test_server/test_rest_api_spec.py::test_rest_api_spec[OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/search/highlight/30_max_analyzed_offset[5]]
  • FAILED test_opensearchpy/test_server/test_rest_api_spec.py::test_rest_api_spec[OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/search/350_matched_queries[1]]

@Xtansia
Copy link
Contributor

Xtansia commented Mar 13, 2024

@dblock @saimedhi For both .NET & Rust this behaviour of picking the tests from the matching branch/revision of the OS cluster was inherited in the fork from Elastic. Even if main is eventually consistent, it seems evident that taking the tests intended for a given version will be more reliable.

@dblock
Copy link
Member Author

dblock commented Mar 16, 2024

Fixed in #696.

@dblock dblock closed this as completed Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants