Skip to content

Fix half-float range in SupportedTypeTests#55409

Merged
polyfractal merged 3 commits intoelastic:masterfrom
polyfractal:fix_half_float_tests
Apr 23, 2020
Merged

Fix half-float range in SupportedTypeTests#55409
polyfractal merged 3 commits intoelastic:masterfrom
polyfractal:fix_half_float_tests

Conversation

@polyfractal
Copy link
Copy Markdown
Contributor

We used a half-float mapper test as the template for how to generate half-floats in the supported-type tests (#55325), but didn't realize the particular test we were looking at was purposefully generating out-of-range half-floats. These are rounded to Infinity, which breaks the assumptions of our agg tests.

Instead, we need to generate floats < 65504. This PR also adds a comment to the mapper tests in case someone else misreads it in the future :)

Fixes #55360

Also adds a comment to the half-float number field type tests indicating
why 70000 is used instead of 65504
@polyfractal polyfractal added >test Issues or PRs that are addressing/adding tests :Analytics/Aggregations Aggregations v8.0.0 v7.8.0 labels Apr 17, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
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.

LGTM

Copy link
Copy Markdown
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM

@polyfractal
Copy link
Copy Markdown
Contributor Author

@elasticmachine run elasticsearch-ci/2

@polyfractal
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@polyfractal
Copy link
Copy Markdown
Contributor Author

@elasticmachine run elasticsearch-ci/2

@polyfractal polyfractal merged commit cbc9c7a into elastic:master Apr 23, 2020
polyfractal pushed a commit that referenced this pull request Apr 23, 2020
Also adds a comment to the half-float number field type tests indicating
why 70000 is used instead of 65504
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations >test Issues or PRs that are addressing/adding tests v7.8.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AggregatorTestCase#testSupportedFieldTypes fails with some generated half-floats

6 participants