REST test for typeless APIs. #33934
REST test for typeless APIs. #33934
Conversation
This commit duplicates REST tests for the - `indices.create` - `indices.put_mapping` - `indices.get_mapping` - `index` - `get` - `delete` - `update` - `bulk` APIs, so that we both test them when used without types (include_type_name=false) and with types, mostly for mixed-version cluster tests. Given a suite called `X_test_name.yml`, I first copied it to `(X+1)_test_name_legacy.yml` and then changed `X_test_name.yml` to set `include_type_name=false` on every API that supports it. Relates elastic#15613
jpountz
added
>test
|
Pinging @elastic/es-search-aggs |
| if (parser.nextToken() != XContentParser.Token.START_OBJECT) { | ||
| throw new XContentParseException(parser.getTokenLocation(), | ||
| "expected token to be START_OBJECT but was " + parser.currentToken()); | ||
| } |
There was a problem hiding this comment.
I hit this assertion during development, which was not nice because it doesn't tell the line/offset of the problem, so I switched to a hard check that also gives the location of the parsing error.
| if (parser.nextToken() != XContentParser.Token.START_OBJECT) { | ||
| throw new XContentParseException(parser.getTokenLocation(), | ||
| "expected token to be START_OBJECT but was " + parser.currentToken()); | ||
| } |
| @@ -0,0 +1,33 @@ | |||
| --- | |||
| "Metadata Fields": | |||
There was a problem hiding this comment.
I think this is missing a corresponding change in the non-legacy version (only noticed this because it's last).
There was a problem hiding this comment.
Good catch, I missed it because it is currently disabled due to a bug.
| @@ -0,0 +1,46 @@ | |||
| --- | |||
| "No type returned": | |||
There was a problem hiding this comment.
To check I'm following -- this is our approach for the testing search API (as opposed to duplicating the REST tests, as we do for other APIs)? This test also got me thinking that we should probably deprecate/ disallow type queries.
There was a problem hiding this comment.
Sorry I wanted to mention it and then forgot.
The reason why I added this test here is that we used to have tests for a couple APIs without types under put_mapping including this test for _search, and I didn't want to lose it.
I still need to think a bit about search. Some tests would be worth migrating for sure, eg. search.highlight and search.inner_hits. I'm less sure about search.aggregations since the output is not affected by the change? I'm keeping _search for later as I felt that this PR was already pretty large and added good coverage.
For the record, there are other APIs that we still need to make typeless like _explain (it doesn't support the include_type_name flag yet) so we will need to duplicate tests for it as well.
There was a problem hiding this comment.
Does this mean that for _search, include_type_name=false means that _type should not be returned as part of the hits? Does the flag do anything else for _search?
There was a problem hiding this comment.
@javanna No, this is all it does indeed.
There was a problem hiding this comment.
This test also got me thinking that we should probably deprecate/ disallow type queries.
They are already deprecated: #29468
There was a problem hiding this comment.
Thanks for the explanation. Addressed my confusion about the type query deprecation in #34017 :)
|
@jtibshirani Thanks for the review, I rename *_legacy to *_with_types to make file names more descriptive as suggested. |
|
@elasticmachine test this please |
This commit duplicates REST tests for the - `indices.create` - `indices.put_mapping` - `indices.get_mapping` - `index` - `get` - `delete` - `update` - `bulk` APIs, so that we both test them when used without types (include_type_name=false) and with types, mostly for mixed-version cluster tests. Given a suite called `X_test_name.yml`, I first copied it to `(X+1)_test_name_with_types.yml` and then changed `X_test_name.yml` to set `include_type_name=false` on every API that supports it. Relates #15613
This commit duplicates REST tests for the
indices.createindices.put_mappingindices.get_mappingindexgetdeleteupdatebulkAPIs, so that we both test them when used without types (include_type_name=false)
and with types, mostly for mixed-version cluster tests.
Given a suite called
X_test_name.yml, I first copied it to(X+1)_test_name_legacy.ymland then changedX_test_name.ymlto setinclude_type_name=falseon every API that supports it.Relates #15613