Let random_score yaml test explicitly fail on _id field #125230
Conversation
tteofili
added
:Search Relevance/Search
elasticsearchmachine
added
v9.1.0
Team:Search Relevance
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
tteofili
added
v9.0.0
and removed
Team:Search Relevance
elasticsearchmachine
added
the
Team:Search Relevance
|
Hi @tteofili, I've created a changelog YAML for you. |
| - match: { hits.total.value: 2 } | ||
|
|
||
| - do: | ||
| catch: /Fielddata access on the _id field is disallowed/ |
There was a problem hiding this comment.
I don't see how this improves things? I think we should block bwc testing. field is no longer necessary for v9: https://github.com/elastic/elasticsearch/pull/118671/files
I am not sure why this is failing at all. Is it failing in a mixed cluster? If so, we should adjust the assertions when hitting an older cluster vs v9.
But I do not like changing the test behavior like this. We are purposefully testing that not providing a field works. Let's continue to do that.
There was a problem hiding this comment.
Should this test pass if we provide rest api compatibility?
There was a problem hiding this comment.
the thing is that the actual field being used underneath has changed from 8.x to 9.x (_id to _seqno), so the original idea was to make sure that this works across versions even when not supplying any field.
having an explicit test with _id should fail both on 8.x and 9.x (without explicitly enabling fielddata for _id), but I am fine with your suggestion of constraining the test to 9.x versions only.
There was a problem hiding this comment.
The failure is that there is a deprecation warning indicating that "Hey, we will require you to provide a field in the future", which isn't the case any longer.
The user's API experience is largely unchanged, except now they do NOT get a deprecation warning.
There was a problem hiding this comment.
AH, I see the test was added here: #124893
If we want this test to run in 8.x, we should have a separate one that is designed for 8.x that exercises the paths we expect there. But given that this test only exists in v9.0, maybe we only have it run there.
benwtrent
added
>test
💔 Backport failed
You can use sqren/backport to manually backport by running |
* constrain the no-field scenario to 9.x
* constrain the no-field scenario to 9.x
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
* constrain the no-field scenario to 9.x (cherry picked from commit 6d3dac3) # Conflicts: # muted-tests.yml
* constrain the no-field scenario to 9.x
4 participants
fixes #125010