Geo: [TEST] Adds tests for GeoUtils #7510
Conversation
| // | ||
| // qNode = qNode.getSubCells(null).iterator().next(); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Is it commented out on purpose?
|
Not completely sure what the commented out test is doing but have uncommented it as it presumably served a purpose. May be superseded by the other tests now though. Also I can't work out a good way of testing the earthDiameter(double) method. |
colings86
changed the title
|
LGTM
I don't have ideas either. Let's already push these tests? |
| public void testSpecials() { | ||
| assertThat(GeoUtils.normalizeLon(Double.POSITIVE_INFINITY), equalTo(Double.NaN)); | ||
| public void testGeohashCellWidth() { | ||
| double equatorialDistance = 2 * Math.PI * 6378137.0; |
There was a problem hiding this comment.
does it make sense to use the GeoUtils constants here? might help people to understand why they are here
There was a problem hiding this comment.
@spinscale Should we not avoid using constants from the class under test since if we use those constants these tests won't pick up any changes in them?
There was a problem hiding this comment.
I am good with both options.
There was a problem hiding this comment.
fine with me as well. go ahead and push!
Also added unit tests for GeoUtils