Skip to content

test (route): add table-driven tests for route IDs with multiple underscores#1039

Open
3rabiii wants to merge 1 commit into
OneBusAway:mainfrom
3rabiii:fix-route_issue_4
Open

test (route): add table-driven tests for route IDs with multiple underscores#1039
3rabiii wants to merge 1 commit into
OneBusAway:mainfrom
3rabiii:fix-route_issue_4

Conversation

@3rabiii

@3rabiii 3rabiii commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Description

The current GET /api/where/route/{id} implementation uses strings.SplitN(id, "_", 2) to parse the agency and entity IDs. While this correctly handles entity IDs containing underscores, there was no test coverage explicitly validating this edge case, leaving it vulnerable to future regressions.

Changes Made:

  • Added TestRouteHandler_EntityIDWithUnderscores to route_handler_test.go.
  • Utilized table-driven tests to verify multiple underscore scenarios (e.g., KCM_40_100479, AGENCY_part1_part2_part3).
  • Verified that the underlying utility accurately extracts the agency/entity.
  • Verified that the HTTP handler successfully parses these IDs and gracefully returns a 404 Not Found (since the dummy routes do not exist in the test DB) rather than a 400 Bad Request.

Fixes: #1010

Summary by CodeRabbit

  • Bug Fixes

    • Route identifiers containing underscores in the entity portion are now correctly validated and accepted by the API.
  • Tests

    • Added test coverage to verify proper parsing and validation of route identifiers with underscores in the entity portion.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a new test function TestRouteHandler_EntityIDWithUnderscores to verify that route IDs containing underscores in the entity portion are correctly parsed and accepted by the route handler. The test validates both the extraction logic and handler behavior across multiple test cases.

Changes

Route ID Underscore Parsing Test

Layer / File(s) Summary
Test for entity IDs with underscores
internal/restapi/route_handler_test.go
New test TestRouteHandler_EntityIDWithUnderscores validates that route IDs with underscores in the entity component are correctly extracted by utils.ExtractAgencyIDAndCodeID and accepted by the handler (returning 404 instead of 400 validation error).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

  • OneBusAway/maglev#957: Both PRs modify internal/restapi/route_handler_test.go to validate /api/where/route/{routeID}.json behavior; main PR adds underscore-containing ID parsing test, retrieved PR consolidates route not-found and invalid-ID test cases.

Suggested reviewers

  • aaronbrethorst
  • Ahmedhossamdev
  • burma-shave
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% 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
Title check ✅ Passed The title 'test (route): add table-driven tests for route IDs with multiple underscores' accurately describes the main change: a new test for handling route IDs with underscores.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit 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

sonarqubecloud Bot commented Jun 8, 2026

Copy link
Copy Markdown

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.

test: add missing edge-case coverage for route ID parsing

1 participant