Skip to content

Break up field collapsing tests #69753

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

Merged
merged 2 commits into from
Mar 8, 2021

Conversation

mayya-sharipova
Copy link
Contributor

Break up field collapsting tests into 2:

  1. simple indexing and 2) indexingwith versions
    to check if a problem of documents missing could be because of versions.

Relates to #52416

Break up field collapsting tests into 2:
1) simple indexing and 2) indexingwith versions
to check if a problem of documents missing could be because of versions.

Relates to elastic#52416
@mayya-sharipova mayya-sharipova added the :Search/Search Search-related issues that do not fall into other categories label Mar 1, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Mar 1, 2021
@elasticmachine
Copy link
Collaborator

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

@mayya-sharipova mayya-sharipova added v7.13.0 and removed Team:Search Meta label for search team labels Mar 1, 2021
@mayya-sharipova
Copy link
Contributor Author

mayya-sharipova commented Mar 1, 2021

I had a discussion with @ywelsch about this test failure. We initially hypothesize that typeless indexing API could have some problems when using for 6.8 nodes. But after talking to @jtibshirani about this, she pointed out that this should not be a problem, as in our test framework in ClientYamlTestExecutionContext#adaptRequestForOlderVersion we add types to requests run agains 6.8 nodes.

So this PR introduces another hypothesis that a problem with missing documents could be related to version parameter. To test this, the original test is broken into two tests: 1) simple indexing and 2) indexing with versions
to check if a problem of missing documents could be because of version.

Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

I guess we will keep a close eye on the tests since we've unmuted them.

@mayya-sharipova
Copy link
Contributor Author

mayya-sharipova commented Mar 4, 2021

I've chatted with @dnhatn offline, and he suggested to remove the test with versions entirely, as we should have other tests that verify versioning already.

Addressed in c66dd71

@mayya-sharipova mayya-sharipova requested a review from dnhatn March 4, 2021 19:10
Copy link
Member

@dnhatn dnhatn 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 Mayya!

@mayya-sharipova mayya-sharipova merged commit ca71564 into elastic:7.x Mar 8, 2021
@mayya-sharipova mayya-sharipova deleted the field-collapsing branch March 8, 2021 14:20
pgomulka added a commit that referenced this pull request Mar 8, 2021
'search/110_field_collapsing/field collapsing and inner_hits', is
enabled in 7.x (#69753 )and the compatible api is not implemented yet
@pugnascotia pugnascotia added the >test Issues or PRs that are addressing/adding tests label Apr 20, 2021
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 >test Issues or PRs that are addressing/adding tests v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants