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

Change EngineTestCase to not wrap the reader. #98732

Merged
merged 2 commits into from
Aug 23, 2023

Conversation

martijnvg
Copy link
Member

As part #98110, EngineTestCase now uses newSearcher() to create a new index searcher (instead of just creating a new index searcher). This is for increasing test coverage. However, this test can't deal with wrapping the index reader as it leads to assertion errors. So instead use newSearcher(leaf, false) to still keep to test benefits of wrapping the index searcher.

Closes #98716

As part elastic#98110, EngineTestCase now uses newSearcher() to create a new index searcher (instead of just creating a new index searcher). This is for increasing test coverage. However, this test can't deal with wrapping the index reader as it leads to assertion errors. So instead use `newSearcher(leaf, false)` to still keep to test benefits of wrapping the index searcher.

Closes elastic#98716
@martijnvg martijnvg added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories v8.11.0 labels Aug 22, 2023
@martijnvg martijnvg requested a review from iverase August 22, 2023 14:45
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Aug 22, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

LGTM

@martijnvg
Copy link
Member Author

@elasticmachine update branch

@martijnvg martijnvg added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 23, 2023
@elasticsearchmachine elasticsearchmachine merged commit b2ae984 into elastic:main Aug 23, 2023
11 checks passed
@martijnvg martijnvg deleted the fix_EngineTestCase branch August 23, 2023 03:38
dreamquster pushed a commit to dreamquster/elasticsearch that referenced this pull request Aug 26, 2023
As part elastic#98110, EngineTestCase now uses newSearcher() to create a new
index searcher (instead of just creating a new index searcher). This is
for increasing test coverage. However, this test can't deal with
wrapping the index reader as it leads to assertion errors. So instead
use `newSearcher(leaf, false)` to still keep to test benefits of
wrapping the index searcher.

Closes elastic#98716
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team >test Issues or PRs that are addressing/adding tests v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] InternalEngineTests testGetWithSearcherWrapper failing
4 participants