Skip to content

refactor: migrate the trip_for_vehicle_tests to the new response type model#955

Open
Ahmedhossamdev wants to merge 1 commit into
mainfrom
refactor/typed-trip-for-vehicle-tests
Open

refactor: migrate the trip_for_vehicle_tests to the new response type model#955
Ahmedhossamdev wants to merge 1 commit into
mainfrom
refactor/typed-trip-for-vehicle-tests

Conversation

@Ahmedhossamdev
Copy link
Copy Markdown
Member

@Ahmedhossamdev Ahmedhossamdev commented May 16, 2026

Update the trip-for-vehicle-tests to use new response type models for parsing the JSON responses.

Summary by CodeRabbit

Release Notes

No user-visible changes in this release. This update contains internal testing infrastructure improvements to enhance code maintainability and test reliability.

Review Change Stack

@Ahmedhossamdev Ahmedhossamdev requested a review from fletcherw May 16, 2026 10:59
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cb75db4-a919-4cff-b1ba-d35000f5539a

📥 Commits

Reviewing files that changed from the base of the PR and between 995781a and 9f6ef82.

📒 Files selected for processing (1)
  • internal/restapi/trip_for_vehicle_handler_test.go

📝 Walkthrough

Walkthrough

This PR refactors the trip-for-vehicle handler test file to use typed response models via callAPIHandler[TripDetailsResponse], replacing manual HTTP server setup and untyped JSON decoding. Test helpers are consolidated, test cases restructured with table-driven variants, and parameter validation moved to consistent typed assertions.

Changes

Trip-for-vehicle handler test refactoring

Layer / File(s) Summary
Imports and test helpers
internal/restapi/trip_for_vehicle_handler_test.go (lines 4–49)
Updated imports; added tripForVehicleURL builder for deterministic endpoint construction; refactored setupTestApiWithMockVehicle to use testdata and return (api *RestAPI, vehicleCombinedID string).
API key validation and end-to-end success
internal/restapi/trip_for_vehicle_handler_test.go (lines 51–115)
Reworked main success path to use callAPIHandler[TripDetailsResponse] with typed assertions: HTTP status, model code/text, version/timestamp, serviceDate normalized to agency-local midnight, and reference arrays (agencies/routes/trips/stops).
Not found scenarios
internal/restapi/trip_for_vehicle_handler_test.go (lines 117–150)
Consolidated multiple 404 cases into single table-driven test covering unknown vehicle, idle vehicle (empty trip ID), ghost trip, and unknown agency; asserts typed error codes.
Response filtering via include toggles
internal/restapi/trip_for_vehicle_handler_test.go (lines 151–194)
Tests includeStatus, includeTrip, includeSchedule query params; validates that response omits/empties fields (nil status/schedule, empty trip references) for disabled toggles.
Parameter validation and error handling
internal/restapi/trip_for_vehicle_handler_test.go (lines 196–257)
Tests serviceDate normalization to agency timezone, time UTC parsing, malformed vehicle ID detection (400), and invalid query-parameter format checks; uses typed handler assertions.
Unit tests for parameter parsing
internal/restapi/trip_for_vehicle_handler_test.go (lines 258–292)
Direct unit tests for api.parseTripParams using httptest.NewRequest; validates default include flags and field-error messages for invalid date/time inputs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • OneBusAway/maglev#897: Refactors REST API handler unit tests to use shared typed response models via callAPIHandler[...] with new generic response types across handler tests.
  • OneBusAway/maglev#913: Refactors handler tests to typed callAPIHandler[TripDetailsResponse] pattern, reusing the same TripDetailsResponse type in trip handler test suites.
  • OneBusAway/maglev#905: Refactors REST API handler tests to typed callAPIHandler[...] approach with different response types (e.g., ScheduleForRouteResponse for schedule-for-route tests).

Suggested reviewers

  • fletcherw
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: migrating trip_for_vehicle tests to use the new typed response model approach.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/typed-trip-for-vehicle-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown

Performance Smoke Test Results

Status: PASSED

Metric Value
p(95) latency 1.9 ms
Error rate 0.00%
Total requests 344
Req/sec 11.2

Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%.

Full results uploaded as workflow artifact: k6-smoke-summary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant