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

Don't use an asserting searcher at all in MatchingDirectoryReader #100668

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

romseygeek
Copy link
Contributor

@romseygeek romseygeek commented Oct 11, 2023

Follow up to #100527

We are not testing anything to do with searching with this searcher, and so
there is no point in using LuceneTestCase.newSearcher() which will wrap
it with all sorts of extra checks that may access the underlying reader in
ways that are not anticipated by tests.

Fixes #100460
Fixes #99024

@romseygeek romseygeek added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories auto-backport-and-merge v8.11.1 v8.12.0 labels Oct 11, 2023
@romseygeek romseygeek requested review from javanna and ywangd October 11, 2023 11:19
@romseygeek romseygeek self-assigned this Oct 11, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Oct 11, 2023
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks Alan. I won't pretend that I know how search code works. But I can confirm that the change does fix the failure.

@romseygeek romseygeek merged commit 769c3f3 into elastic:main Oct 11, 2023
romseygeek added a commit to romseygeek/elasticsearch that referenced this pull request Oct 11, 2023
…astic#100668)

Follow up to elastic#100527

We are not testing anything to do with searching with this searcher, and so
there is no point in using LuceneTestCase.newSearcher() which will wrap
it with all sorts of extra checks that may access the underlying reader in
ways that are not anticipated by tests.

Fixes elastic#100460
Fixes elastic#99024
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.11

elasticsearchmachine pushed a commit that referenced this pull request Oct 11, 2023
…00668) (#100685)

Follow up to #100527

We are not testing anything to do with searching with this searcher, and so
there is no point in using LuceneTestCase.newSearcher() which will wrap
it with all sorts of extra checks that may access the underlying reader in
ways that are not anticipated by tests.

Fixes #100460
Fixes #99024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
: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.1 v8.12.0
Projects
None yet
3 participants