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

Tests: Add checks to GeoDistanceQueryBuilderTests #34273

Merged
merged 3 commits into from
Oct 23, 2018

Conversation

imotov
Copy link
Contributor

@imotov imotov commented Oct 3, 2018

Adds checks for parsed geo distance query. It is a bit hackish since it
compares with query's toString() output, but it is better than no
checks. The parsed query itself has default visibility, so we cannot
access it here unless we move the test to org.apache.lucene.document
package.

Fixes #34043

Adds checks for parsed geo distance query. It is a bit hackish since it
compares with query's toString() output, but it is better than no
checks. The parsed query itself has default visibility, so we cannot
access it here unless we move the test to org.apache.lucene.document
package.

Fixes elastic#34043
@imotov imotov added >enhancement >test Issues or PRs that are addressing/adding tests :Analytics/Geo Indexing, search aggregations of geo points and shapes v7.0.0 v6.5.0 labels Oct 3, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@cbuescher cbuescher self-assigned this Oct 8, 2018
Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

The change looks good, but I was wondering if there is a way to avoid the "toString" comparison. Could you comment if you think my suggestion works or if you disagree?

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Thanks a lot, LGTM

@imotov
Copy link
Contributor Author

imotov commented Oct 22, 2018

retest this please

@imotov imotov merged commit 123f784 into elastic:master Oct 23, 2018
imotov added a commit that referenced this pull request Oct 23, 2018
Adds checks for parsed geo distance query. It is a bit hack-ish since it
compares with query's toString() output, but it is better than no
checks. The parsed query itself has default visibility, so we cannot
access it here unless we move the test to org.apache.lucene.document
package.

Fixes #34043
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Oct 23, 2018
* master: (24 commits)
  ingest: better support for conditionals with simulate?verbose (elastic#34155)
  [Rollup] Job deletion should be invoked on the allocated task (elastic#34574)
  [DOCS] .Security index is never auto created (elastic#34589)
  CCR: Requires soft-deletes on the follower (elastic#34725)
  re-enable bwc tests (elastic#34743)
  Empty GetAliases authorization fix (elastic#34444)
  INGEST: Document Processor Conditional (elastic#33388)
  [CCR] Add total fetch time leader stat (elastic#34577)
  SQL: Support pattern against compatible indices (elastic#34718)
  [CCR] Auto follow pattern APIs adjustments (elastic#34518)
  [Test] Remove dead code from ExceptionSerializationTests (elastic#34713)
  A small typo in migration-assistance doc (elastic#34704)
  ingest: processor stats (elastic#34724)
  SQL: Implement IN(value1, value2, ...) expression. (elastic#34581)
  Tests: Add checks to GeoDistanceQueryBuilderTests (elastic#34273)
  INGEST: Rename Pipeline Processor Param. (elastic#34733)
  Core: Move IndexNameExpressionResolver to java time (elastic#34507)
  [DOCS] Force Merge: clarify execution and storage requirements (elastic#33882)
  TESTING.asciidoc fix examples using forbidden annotation (elastic#34515)
  SQL: Implement `CONVERT`, an alternative to `CAST` (elastic#34660)
  ...
kcm pushed a commit that referenced this pull request Oct 30, 2018
Adds checks for parsed geo distance query. It is a bit hack-ish since it
compares with query's toString() output, but it is better than no
checks. The parsed query itself has default visibility, so we cannot
access it here unless we move the test to org.apache.lucene.document
package.

Fixes #34043
@imotov imotov deleted the issue-34043-add-range-query-checks branch May 1, 2020 22:18
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 >enhancement >test Issues or PRs that are addressing/adding tests v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants