Skip to content

Conversation

@craigtaverner
Copy link
Contributor

@craigtaverner craigtaverner commented Mar 24, 2023

Test for geohex are doubled to support both geo_point and geo_shape, while geotile and geohash are expanded to cover same range as geohex, covering the various options supported like:

  • defaults
  • precision
  • size
  • shard_size
  • bounds

Before this PR, the test coverage was:

aggregation geo_point geo_shape
geohash 1 1
geotile 1 1
geohex 5 0

With the PR we now have:

aggregation geo_point geo_shape
geohash 5 5
geotile 5 5
geohex 5 5

Note that the tests are in two locations:

  • modules/aggregations for geo_point and geohash/geotile combinations
  • x-pack/plugin for geo_shape and for geo_point/geohex
aggregation geo_point geo_shape
geohash aggregations xpack
geotile aggregations xpack
geohex xpack xpack

For testing these two locations, different commands are used:

For modules/aggregations, eg. test geotile_grid over geo_point:

./gradlew ':modules:aggregations:yamlRestTest' --tests \
  "org.elasticsearch.aggregations.AggregationsClientYamlTestSuiteIT.test {yaml=aggregations/geotile_grid/*}"

For x-pack/plugin, eg. test geohash_grid over geo_shape:

./gradlew ':x-pack:plugin:yamlRestTest' --tests \
  "org.elasticsearch.xpack.test.rest.XPackRestIT.test {p0=spatial/40_geohash_grid/*}" 

geohex is doubled to support both geo_point and geo_shape, while
geotile and geohash are expanded to cover same range as geohex,
covering the various options supported like:
* precision
* size
* shard_size
* bounds
@craigtaverner craigtaverner added >test Issues or PRs that are addressing/adding tests :Analytics/Geo Indexing, search aggregations of geo points and shapes Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Mar 24, 2023
@craigtaverner craigtaverner requested a review from iverase March 24, 2023 17:18
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

lgtm

The tests in the aggregation module covered geo_point. We had added
additional tests in xpack for geo_shape, so we expand that coverage
here to geo_point in the aggregations module.
Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

Thanks for expanding the test

@craigtaverner craigtaverner merged commit 5070b44 into elastic:main Mar 27, 2023
@craigtaverner craigtaverner deleted the rest_tests_geo_grid branch October 20, 2023 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Geo Indexing, search aggregations of geo points and shapes Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v8.8.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants