refactor(RoutePatterns.Repo): use factories and mock for tests #2027
Conversation
test/support/factory/mbta_api.ex
Outdated
| id: attrs[:id] || sequence(:id, &"item-#{&1}"), | ||
| attributes: attrs[:attributes] || %{}, | ||
| relationships: attrs[:relationships] || %{} |
There was a problem hiding this comment.
I don't think you have to do this with the attrs. It should just override ones you pass in.
There was a problem hiding this comment.
Ah okay, I wasn't sure how it handled nested levels
test/support/factory/mbta_api.ex
Outdated
| def route_pattern_item_factory(attrs) do | ||
| item = %Item{ | ||
| id: attrs[:id] || sequence(:id, &"item-#{&1}"), | ||
| attributes: |
There was a problem hiding this comment.
We might want to break these out so they can be reused: attributes, relationships.
test/route_patterns/repo_test.exs
Outdated
| end | ||
|
|
||
| test "returns nil for an unknown route pattern" do | ||
| refute Repo.get("unknown_route_pattern") | ||
| expect(MBTA.Api.Mock, :get_json, fn "/route_patterns/unknown_route_pattern", _ -> |
There was a problem hiding this comment.
You could turn this into a clearer expectation and remove the hard coded checking.
test "..." do
pattern = Faker.Internet.slug()
expect(..., ..., fn path, _ ->
assert path == "/route_patterns/" <> pattern
end)
refute RoutePatterns.Repo.get(pattern)
end
| @@ -148,10 +148,6 @@ const ScheduleDirection = ({ | |||
| if (result.length === 0 || current.typicality < result[0].typicality) | |||
| return [current]; | |||
| if (current.typicality === result[0].typicality) { | |||
| if (result[0].shape_priority < 0 && current.shape_priority > 0) | |||
There was a problem hiding this comment.
I'm not sure why this is being removed in the context of this PR. A note in the PR would help.
There was a problem hiding this comment.
Thanks, totally forgot to mention it. Priority was an attribute of V3 API shapes that's been removed in more recent versions, and as far as I could tell we had already stopped reading/parsing it in our own codebase. Added a note to the main PR text too.
| response = | ||
| get(conn, stop_path(conn, :grouped_route_patterns, "place-here")) |> json_response(200) | ||
|
|
||
| assert %{ |
There was a problem hiding this comment.
Is this hard-coded somewhere? This test is a little hard to figure out.
There was a problem hiding this comment.
I totally forgot about this and because the whole module's excluded it didn't fail the tests! I'll fix this.
- add new typicality value - better type for sort_order - add a few reasonable defaults - sort properties alphabetically - refactor: remove unpopulated shape_priority
thecristen
force-pushed
the
cbj/routepatternsrepo
branch
from
cf38c51 to
3a1f56a
Compare
No ticket, just a refactor that should help me test the timetable changes that's incoming.
This also removes what was left of our code relying on shape priority. As far as I can tell, we had already stopped parsing it. This field was removed in V3 API shapes in version 2020-05-01.