test: Add unit tests for geo utility functions#89
Conversation
0xaboomar
left a comment
0xaboomar
left a comment
There was a problem hiding this comment.
Hi @Vishmayraj , thanks for the PR and the work you’ve put into this.
I’ve left a few comments and requested some changes that should be addressed before we proceed with merging. Please take a look and let me know once they’re updated.
| // Known great-circle distance: ~3,867 km | ||
| dist := haversineDistance(47.6062, -122.3321, 40.7128, -74.0060) | ||
| expectedMeters := 3867000.0 | ||
| marginMeters := 50000.0 // 50km tolerance |
There was a problem hiding this comment.
The current 50km margin is a bit too loose. The theoretical maximum error for spherical approximations (like Haversine) is roughly 0.5%, which equates to ~19km for this Seattle-to-NY distance.
I suggest tightening the margin to 25km. This is wide enough to cover the 0.5% model discrepancy while being significantly more precise than 50km.
Look at the "Accuracy" note in this source for more details: https://www.movable-type.co.uk/scripts/latlong.html
| // go test ./internal/geo/... -v (all tests) | ||
| // | ||
| // Tests are isolated so failure in any will stand out. | ||
|
|
There was a problem hiding this comment.
I recommend removing the header comment block (including the Author tag and test instructions). In Go, the filename and the tests themselves should be self-documenting, and Git handles the authorship tracking. This will keep the file cleaner.
| } | ||
| if bbox.MinLat != 47.60 || bbox.MaxLat != 47.60 { | ||
| t.Errorf("Expected only valid stop coordinates, got %v", bbox) | ||
| } |
There was a problem hiding this comment.
I suggest adding checks for MinLon and MaxLon as well.
| @@ -0,0 +1,199 @@ | |||
| package geo | |||
There was a problem hiding this comment.
Since these geographic tests are primarily mathematical and completely independent, I suggest adding t.Parallel() to the top of each test function and subtest (t.Run).
2 participants
geo_utils.gohad no test coverage. This PR addsgeo_utils_test.gocovering:
computeBoundingBox: empty input, nil coordinates, single stop,multiple stops, mixed valid/nil coordinates
isValidLatLon: zero coordinates, valid coordinates, boundary values,out-of-range values
BoundingBoxStore: set/get, nonexistent server, point inside/outsidebounding box
haversineDistance: same point, known real-world distanceAll existing tests continue to pass.