Skip to content
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

Refactored GeoHashGrid unit tests #37832

Merged
merged 3 commits into from
Jan 25, 2019
Merged

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented Jan 24, 2019

This change allows other grid aggregations to reuse the same tests.

The change mostly just moves code to the base classes, trying to
keep changes to a bare minimum. A follow up refactoring might
want to rename some variables to remove explicit "geohash" from the base.

@nyurik nyurik requested review from talevy and imotov January 24, 2019 18:44
@nyurik nyurik force-pushed the geohashgrid_tests-v2 branch from 3a719c8 to d9df172 Compare January 24, 2019 19:40
Copy link
Contributor

@talevy talevy left a comment

Choose a reason for hiding this comment

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

overall looks good! left a few comments

Copy link
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

Left some comments.

@nyurik nyurik force-pushed the geohashgrid_tests-v2 branch 2 times, most recently from 5ecbd07 to 48e25e8 Compare January 24, 2019 23:08
@nyurik nyurik requested a review from talevy January 25, 2019 00:24
@nyurik nyurik added :Analytics/Geo Indexing, search aggregations of geo points and shapes >refactoring labels Jan 25, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

This change allows other grid aggregations to reuse the same tests.

The change mostly just moves code to the base classes, trying to
keep changes to a bare minimum.
@nyurik nyurik force-pushed the geohashgrid_tests-v2 branch from da7c7af to 8848778 Compare January 25, 2019 05:07
@nyurik nyurik requested a review from imotov January 25, 2019 16:39
Copy link
Contributor

@talevy talevy left a comment

Choose a reason for hiding this comment

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

thanks @nyurik LGTM

@nyurik nyurik merged commit f1e71be into elastic:master Jan 25, 2019
@nyurik nyurik deleted the geohashgrid_tests-v2 branch January 25, 2019 18:37
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 25, 2019
* elastic/master: (68 commits)
  Fix potential IllegalCapacityException in LLRC when selecting nodes (elastic#37821)
  Mute CcrRepositoryIT#testFollowerMappingIsUpdated
  Fix S3 Repository ITs When Docker is not Available (elastic#37878)
  Pass distribution type through to docs tests (elastic#37885)
  Mute SharedClusterSnapshotRestoreIT#testSnapshotCanceledOnRemovedShard
  SQL: Fix casting from date to numeric type to use millis (elastic#37869)
  Migrate o.e.i.r.RecoveryState to Writeable (elastic#37380)
  ML: removing unnecessary upgrade code (elastic#37879)
  Relax cluster metadata version check (elastic#37834)
  Mute TransformIntegrationTests#testSearchTransform
  Refactored GeoHashGrid unit tests (elastic#37832)
  Fixes for a few randomized agg tests that fail hasValue() checks
  Geo: replace intermediate geo objects with libs/geo (elastic#37721)
  Remove NOREPLACE for /etc/elasticsearch in rpm and deb (elastic#37839)
  Remove "reinstall" packaging tests (elastic#37851)
  Add unit tests for ShardStateAction's ShardStartedClusterStateTaskExecutor (elastic#37756)
  Exit batch files explictly using ERRORLEVEL (elastic#29583)
  TransportUnfollowAction should increase settings version (elastic#37859)
  AsyncTwoPhaseIndexerTests race condition fixed (elastic#37830)
  Do not allow negative variances (elastic#37384)
  ...
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 >refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants