verifiy-arrivals-and-departures-for-stop-94

[Ahmedhossamdev]

Fix: #94

[aaronbrethorst]

nice work. I see a couple things to address before this can be merged.

---

Issues to Fix

1. Branch needs rebasing onto latest main

The test TestScheduleForRouteHandler/Valid_route is failing, but this isn't caused by your changes. The main branch received a fix in PR #173 that adds a required date parameter to this test. You'll need to rebase your branch onto the latest main:

git fetch origin main
git rebase origin/main

---

2. Missing test coverage for the new functionality

The linked issue #94 specifically asks for strengthening unit tests when coverage is missing. The changes add logic for:

• distanceFromStop calculation via getBlockDistanceToStop()
• numberOfStopsAway calculation via getNumberOfStopsAway()
• Stop Direction field now using calculateStopDirection()

However, the existing tests don't verify these values are calculated correctly. Consider adding test assertions that check:

// Example additions to TestArrivalsAndDeparturesForStopHandlerEndToEnd
if distanceFromStop, ok := firstArrival["distanceFromStop"].(float64); ok {
    // Verify it's a reasonable value when vehicle data is present
    assert.GreaterOrEqual(t, distanceFromStop, 0.0)
}

if numberOfStopsAway, ok := firstArrival["numberOfStopsAway"].(float64); ok {
    // -1 is valid (unknown), otherwise should be non-negative
    assert.GreaterOrEqual(t, numberOfStopsAway, -1.0)
}

And for stop references:

for _, stop := range stops_ref {
    stopMap := stop.(map[string]interface{})
    direction := stopMap["direction"].(string)
    // Direction should be a valid cardinal direction or "UNKNOWN"
    validDirections := []string{"N", "NE", "E", "SE", "S", "SW", "W", "NW", "UNKNOWN"}
    assert.Contains(t, validDirections, direction)
}

---

Suggestions

Code organization consideration

The distance and stops-away calculation logic is now inside the if status != nil block. This means these metrics will only be populated when we have trip status data. If a vehicle has position data but we couldn't build a trip status (e.g., BuildTripStatus returns nil), these metrics won't be calculated.

Looking at the code flow, this seems intentional and matches the single-arrival handler behavior at arrival_and_departure_for_stop_handler.go:257-266. Just wanted to confirm this is the expected behavior.

---

Once you rebase onto main and the tests pass, this PR is good to go!