Add REST tests for histogram aggregation#72322
Conversation
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
nik9000
left a comment
There was a problem hiding this comment.
Wonderful! I left a few minor things. I think the new lines point I left is important for consistency with the more modern yaml tests, but the others are all your call if you want to apply them. Either way, LGTM. No need for another review pass.
|
|
||
| --- | ||
| "Show empty intervals": | ||
| - do: |
There was a problem hiding this comment.
I tend to use bulk for these just to save a bit of typing. Maybe it faster, but it makes the tests more likely to fit onto one screen. Even refreshing is just one line in the bulk.
|
|
||
| - do: | ||
| search: | ||
| rest_total_hits_as_int: true |
There was a problem hiding this comment.
I tend not to add these because I think we want to remove them from master in the end. They are useful on backport though. Either way is cool.
| - do: | ||
| search: | ||
| rest_total_hits_as_int: true | ||
| body: { "aggs" : { "histo" : { "histogram" : { "field" : "number", "interval" : 20 } } } } |
There was a problem hiding this comment.
I think it's easier to read when these are on individual lines. I tend to write them in yaml instead of json too just because we're in yaml files. But I think that is less important than the line feeds.
|
Nik thanks a lot for the feedback. I applied all changes you suggested in 7319bab |
Added more tests for histogram aggregation
Related to #26220