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

WIP: Move LegacyGeoShapeWithDocValuesQueryTests to N-2 compatibility tests #119771

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cbuescher
Copy link
Member

This change moves LegacyGeoShapeWithDocValuesQueryTests to the new
N-2 index version compatibility QA project so we can leverage the test infrastructure
to create indices in v7 and test them after the cluster is upgraded to v8 and v9.

The test in question is currently disabled because v7 indices are not supported in v9.
With this change the two most important test cases are moved to the new full cluster
restart framework, restoring the v7 indices via searchable snapshots in v9. When we also
support reading directly from v7 indices, this can be added easily into this test infrastructure.

Currently this PR is still marked WIP because I only moved to test cases over, but the test inherits many more (~30) via GeoShapeQueryTestCase, so adding all of these would require some additional work.
Maybe we can cherry-pick the most important ones here.

@cbuescher cbuescher added >non-issue WIP :Search/Search Search-related issues that do not fall into other categories v9.0.0 labels Jan 8, 2025
@cbuescher cbuescher force-pushed the LegacyGeoShapeWithDocValuesQueryCompatibilityIT branch from 5ea85b8 to 2d23c6d Compare January 8, 2025 16:00
@cbuescher cbuescher requested review from javanna and drempapis January 9, 2025 13:04
Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

I looked a bit at whether these can be replaced with unit tests. I think it's possible, and doing so would simplify tests as well as not require full blown IT tests.

The problem I see is that it is not entirely clear what bit of the compatibility these tests are testing - where are the conditional checks that need to be covered with each test?

In general though, I'd expect them all to be in the mappings code. It is possible to build a mapper service providing a specific index version, see MapperServiceTestCase#createMapperService(IndexVersion version, XContentBuilder mapping) for an example. From there MapperService.documentParser().parseDocument should allow to simulate parsing of the document, which I expect to be the area where the compatibility is, without requiring to index the doc entirely. I wonder if the search is needed as well, if it is, we could still retrieve the mapped field type and simulate querying, but we first need to answer the first question about what these tests are actually testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories v9.0.0 WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants