From 878c99b9bb6153e99be344f275c3a7fe8cd1c6ef Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Wed, 1 May 2024 13:44:21 -0400 Subject: [PATCH 01/12] chore: rename existing behaviour --- lib/route_patterns/mock_repo.ex | 6 +++--- lib/route_patterns/repo.ex | 4 ++-- lib/route_patterns/{repo_api.ex => repo/behaviour.ex} | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) rename lib/route_patterns/{repo_api.ex => repo/behaviour.ex} (88%) diff --git a/lib/route_patterns/mock_repo.ex b/lib/route_patterns/mock_repo.ex index 813030c3c7..14af583992 100644 --- a/lib/route_patterns/mock_repo.ex +++ b/lib/route_patterns/mock_repo.ex @@ -4,12 +4,12 @@ defmodule RoutePatterns.MockRepo do We are returning just a subset of what the actual API would return, given that this file is for testing purposes """ - @behaviour RoutePatterns.RepoApi + @behaviour RoutePatterns.Repo.Behaviour - @impl RoutePatterns.RepoApi + @impl RoutePatterns.Repo.Behaviour def by_route_id("77", _opts), do: by_route_id("77") - @impl RoutePatterns.RepoApi + @impl RoutePatterns.Repo.Behaviour def by_route_id("77") do [ %RoutePatterns.RoutePattern{ diff --git a/lib/route_patterns/repo.ex b/lib/route_patterns/repo.ex index d24090c4b8..0ef2dd660c 100644 --- a/lib/route_patterns/repo.ex +++ b/lib/route_patterns/repo.ex @@ -10,7 +10,7 @@ defmodule RoutePatterns.Repo do alias MBTA.Api.RoutePatterns, as: RoutePatternsApi alias RoutePatterns.RoutePattern - @behaviour RoutePatterns.RepoApi + @behaviour RoutePatterns.Repo.Behaviour @cache Application.compile_env!(:dotcom, :cache) @ttl :timer.hours(1) @@ -34,7 +34,7 @@ defmodule RoutePatterns.Repo do end end - @impl RoutePatterns.RepoApi + @impl RoutePatterns.Repo.Behaviour def by_route_id(route_id, opts \\ []) def by_route_id("Green", opts) do diff --git a/lib/route_patterns/repo_api.ex b/lib/route_patterns/repo/behaviour.ex similarity index 88% rename from lib/route_patterns/repo_api.ex rename to lib/route_patterns/repo/behaviour.ex index e818b525e7..42f4debcc2 100644 --- a/lib/route_patterns/repo_api.ex +++ b/lib/route_patterns/repo/behaviour.ex @@ -1,4 +1,4 @@ -defmodule RoutePatterns.RepoApi do +defmodule RoutePatterns.Repo.Behaviour do @moduledoc """ Behavior for an API client for fetching route pattern data. """ From fafd376bddf89bb7a212ae258019aa3e39a5bd13 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Wed, 1 May 2024 13:47:15 -0400 Subject: [PATCH 02/12] chore: rename existing mock --- config/dotcom/dotcom.exs | 2 +- lib/route_patterns/{mock_repo.ex => repo/mock.ex} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename lib/route_patterns/{mock_repo.ex => repo/mock.ex} (99%) diff --git a/config/dotcom/dotcom.exs b/config/dotcom/dotcom.exs index a253dd7323..7f18ce6914 100644 --- a/config/dotcom/dotcom.exs +++ b/config/dotcom/dotcom.exs @@ -34,7 +34,7 @@ config :dotcom, route_populate_caches?: config_env() == :prod routes_repo = if config_env() == :test, do: Routes.MockRepoApi, else: Routes.Repo config :dotcom, :routes_repo_api, routes_repo -repo_module = if config_env() == :test, do: RoutePatterns.MockRepo, else: RoutePatterns.Repo +repo_module = if config_env() == :test, do: RoutePatterns.Repo.Mock, else: RoutePatterns.Repo config :dotcom, :route_patterns_repo_api, repo_module diff --git a/lib/route_patterns/mock_repo.ex b/lib/route_patterns/repo/mock.ex similarity index 99% rename from lib/route_patterns/mock_repo.ex rename to lib/route_patterns/repo/mock.ex index 14af583992..533a8859c6 100644 --- a/lib/route_patterns/mock_repo.ex +++ b/lib/route_patterns/repo/mock.ex @@ -1,4 +1,4 @@ -defmodule RoutePatterns.MockRepo do +defmodule RoutePatterns.Repo.Mock do @moduledoc """ A mock RoutePatterns Repo client for testing purposes. We are returning just a subset of what the actual API would return, given that this file is for testing purposes From 1120e4a859e3556ea56c0a0b5a91c1494a39469c Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Wed, 1 May 2024 17:05:31 -0400 Subject: [PATCH 03/12] chore: fix dialyzer, race conditions was removed --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 771d093a9e..39fe6a6767 100644 --- a/mix.exs +++ b/mix.exs @@ -20,7 +20,7 @@ defmodule DotCom.Mixfile do ], dialyzer: [ plt_add_apps: [:mix, :phoenix_live_reload, :ex_aws, :ex_aws_ses], - flags: [:race_conditions, :unmatched_returns], + flags: [:unmatched_returns, :error_handling, :underspecs], ignore_warnings: ".dialyzer.ignore-warnings" ], deps: deps(), From 2a5ffc19c6da757d0fef4a12f637576e9bfcd9a3 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Wed, 1 May 2024 17:08:51 -0400 Subject: [PATCH 04/12] fix(RoutePattern): amend struct values - add new typicality value - better type for sort_order - add a few reasonable defaults - sort properties alphabetically - refactor: remove unpopulated shape_priority --- assets/ts/__v3api.d.ts | 19 ++++++------ .../schedule/components/ScheduleDirection.tsx | 4 --- assets/ts/schedule/components/__schedule.d.ts | 1 - .../__tests__/ScheduleFinderTest.tsx | 2 -- .../routePatternsByDirectionData.json | 4 --- .../direction/__tests__/BusMenuTest.tsx | 2 -- .../direction/__tests__/reducer-test.tsx | 4 --- .../controllers/schedule/line/helpers.ex | 9 +----- lib/route_patterns/route_pattern.ex | 30 +++++++++---------- .../schedule/line/helpers_test.exs | 11 ------- 10 files changed, 24 insertions(+), 62 deletions(-) diff --git a/assets/ts/__v3api.d.ts b/assets/ts/__v3api.d.ts index b04953ba1d..f4c8a3e59f 100644 --- a/assets/ts/__v3api.d.ts +++ b/assets/ts/__v3api.d.ts @@ -349,20 +349,19 @@ export interface Shape { } export interface RoutePattern { - typicality: 1 | 2 | 3 | 4; - time_desc: string | null; - route_id: string; + canonical: boolean; + direction_id: DirectionId; + headsign: string; + id: string; + name: string; representative_trip_id: string; representative_trip_polyline: string; + route_id: string; shape_id: string; - shape_priority: number; - stop_ids: string[]; - headsign: string; - name: string; - id: string; - direction_id: DirectionId; sort_order: number; - canonical: boolean; + stop_ids: string[]; + time_desc: string | null; + typicality: 0 | 1 | 2 | 3 | 4 | 5; } export interface StopHours { diff --git a/assets/ts/schedule/components/ScheduleDirection.tsx b/assets/ts/schedule/components/ScheduleDirection.tsx index e60779d330..bc215bbaed 100644 --- a/assets/ts/schedule/components/ScheduleDirection.tsx +++ b/assets/ts/schedule/components/ScheduleDirection.tsx @@ -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) - return [current]; - if (current.shape_priority < 0 && result[0].shape_priority > 0) - return result; return result.concat(current); } return result; diff --git a/assets/ts/schedule/components/__schedule.d.ts b/assets/ts/schedule/components/__schedule.d.ts index a75e1e1ef5..35f01bdbbb 100644 --- a/assets/ts/schedule/components/__schedule.d.ts +++ b/assets/ts/schedule/components/__schedule.d.ts @@ -14,7 +14,6 @@ import { export interface EnhancedRoutePattern extends RoutePattern { shape_id: string; - shape_priority: number; headsign: string; } diff --git a/assets/ts/schedule/components/__tests__/ScheduleFinderTest.tsx b/assets/ts/schedule/components/__tests__/ScheduleFinderTest.tsx index cc418b7279..9c1578b602 100644 --- a/assets/ts/schedule/components/__tests__/ScheduleFinderTest.tsx +++ b/assets/ts/schedule/components/__tests__/ScheduleFinderTest.tsx @@ -141,7 +141,6 @@ const routePatternsByDirection = { typicality: 1, time_desc: "School Trip", shape_id: "9840004", - shape_priority: 1, route_id: "CR-Fitchburg", representative_trip_id: "CR-Weekday-Spring-19-401", representative_trip_polyline: "qwerty123@777njhgb", @@ -159,7 +158,6 @@ const routePatternsByDirection = { typicality: 1, time_desc: "School Trip", shape_id: "9840003", - shape_priority: 1, route_id: "CR-Fitchburg", representative_trip_id: "CR-Weekday-Spring-19-400", representative_trip_polyline: "lkjhg987bvcxz88!", diff --git a/assets/ts/schedule/components/__tests__/test-data/routePatternsByDirectionData.json b/assets/ts/schedule/components/__tests__/test-data/routePatternsByDirectionData.json index 8a8176a52d..e784de6f9e 100644 --- a/assets/ts/schedule/components/__tests__/test-data/routePatternsByDirectionData.json +++ b/assets/ts/schedule/components/__tests__/test-data/routePatternsByDirectionData.json @@ -4,7 +4,6 @@ "typicality": 1, "time_desc": null, "shape_id": "shape-1", - "shape_priority": 1, "route_id": "route-1", "representative_trip_id": "trip-1", "representative_trip_polyline": "trip-1-polyline", @@ -25,7 +24,6 @@ "typicality": 1, "time_desc": null, "shape_id": "shape-3", - "shape_priority": 1, "route_id": "route-1", "representative_trip_id": "trip-3", "representative_trip_polyline": "trip-3-polyline", @@ -46,7 +44,6 @@ "typicality": 3, "time_desc": null, "shape_id": "shape-4", - "shape_priority": 1, "route_id": "route-1", "representative_trip_id": "trip-4", "representative_trip_polyline": "trip-4-polyline", @@ -69,7 +66,6 @@ "typicality": 2, "time_desc": null, "shape_id": "shape-2", - "shape_priority": 1, "route_id": "route-1", "representative_trip_id": "trip-1", "representative_trip_polyline": "trip-2-polyline", diff --git a/assets/ts/schedule/components/direction/__tests__/BusMenuTest.tsx b/assets/ts/schedule/components/direction/__tests__/BusMenuTest.tsx index 6d1da9309a..258496bf6b 100644 --- a/assets/ts/schedule/components/direction/__tests__/BusMenuTest.tsx +++ b/assets/ts/schedule/components/direction/__tests__/BusMenuTest.tsx @@ -21,7 +21,6 @@ const routePatterns: EnhancedRoutePattern[] = [ stop_ids: ["123", "456", "789"], route_id: "66", shape_id: "660140", - shape_priority: 1, time_desc: null, typicality: 1, sort_order: 5, @@ -30,7 +29,6 @@ const routePatterns: EnhancedRoutePattern[] = [ { typicality: 3, time_desc: "School days only", - shape_priority: 1, shape_id: "660141-2", route_id: "66", representative_trip_id: "43773700_2", diff --git a/assets/ts/schedule/components/direction/__tests__/reducer-test.tsx b/assets/ts/schedule/components/direction/__tests__/reducer-test.tsx index 74ecd9797f..7bad98ac70 100644 --- a/assets/ts/schedule/components/direction/__tests__/reducer-test.tsx +++ b/assets/ts/schedule/components/direction/__tests__/reducer-test.tsx @@ -13,7 +13,6 @@ const routePatternsForInbound: EnhancedRoutePattern[] = [ stop_ids: ["123", "456", "789"], route_id: "111", shape_id: "1110177", - shape_priority: 1, time_desc: null, typicality: 1, sort_order: 1, @@ -29,7 +28,6 @@ const routePatternsForInbound: EnhancedRoutePattern[] = [ stop_ids: ["123", "555", "789"], route_id: "111", shape_id: "1110157", - shape_priority: 1, time_desc: "Weekdays only", typicality: 2, sort_order: 2, @@ -56,7 +54,6 @@ const initialState: State = { representative_trip_polyline: "asdf444$hhhhmnb", stop_ids: ["123", "555", "777"], route_id: "111", - shape_priority: 1, shape_id: "1110180", time_desc: null, typicality: 1, @@ -90,7 +87,6 @@ it("menuReducer handles 'toggleDirection'", () => { stop_ids: ["123", "456", "789"], route_id: "111", shape_id: "1110177", - shape_priority: 1, time_desc: null, typicality: 1, sort_order: 1, diff --git a/lib/dotcom_web/controllers/schedule/line/helpers.ex b/lib/dotcom_web/controllers/schedule/line/helpers.ex index e2393dffdd..8ea55faa5e 100644 --- a/lib/dotcom_web/controllers/schedule/line/helpers.ex +++ b/lib/dotcom_web/controllers/schedule/line/helpers.ex @@ -110,19 +110,12 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do @doc """ Filters a list of route patterns down to the route patterns sharing the lowest - number for the "typicality" property, additionally removing route patterns - associated with a negative shape_priority value. + number for the "typicality" property. """ @spec filtered_by_typicality([RoutePattern.t()]) :: [RoutePattern.t()] def filtered_by_typicality(route_patterns) do route_patterns |> filter_by_min_typicality() - |> Enum.filter(fn x -> - # TODO: Deprecate our use of shape priority entirely, - # because it's no longer supported in the V3 API - # For now, be less strict if using the most typical route pattern - if x.typicality == 1, do: true, else: x.shape_priority > 0 - end) end # Filters route patterns by the smallest typicality found in the array diff --git a/lib/route_patterns/route_pattern.ex b/lib/route_patterns/route_pattern.ex index c6af7449a4..b09838d822 100644 --- a/lib/route_patterns/route_pattern.ex +++ b/lib/route_patterns/route_pattern.ex @@ -26,40 +26,38 @@ defmodule RoutePatterns.RoutePattern do defstruct [ :direction_id, + :headsign, :id, :name, :representative_trip_id, :representative_trip_polyline, - :shape_id, - :shape_priority, - :headsign, - :stop_ids, :route_id, - :time_desc, - :typicality, :service_id, - :canonical, - sort_order: 0 + :shape_id, + :time_desc, + canonical: false, + sort_order: 0, + stop_ids: [], + typicality: 5 ] @type id_t :: String.t() - @type typicality_t :: 0 | 1 | 2 | 3 | 4 + @type typicality_t :: 0 | 1 | 2 | 3 | 4 | 5 @type t :: %__MODULE__{ + canonical: boolean(), direction_id: 0 | 1, id: id_t(), + headsign: String.t(), name: String.t(), representative_trip_id: Trip.id_t(), representative_trip_polyline: String.t(), + route_id: Route.id_t(), + service_id: String.t(), shape_id: String.t(), - shape_priority: number, - headsign: String.t(), + sort_order: non_neg_integer(), stop_ids: [Stop.id_t()], - route_id: Route.id_t(), time_desc: String.t(), - typicality: typicality_t(), - sort_order: integer(), - canonical: boolean(), - service_id: String.t() + typicality: typicality_t() } def new(%Item{ diff --git a/test/dotcom_web/controllers/schedule/line/helpers_test.exs b/test/dotcom_web/controllers/schedule/line/helpers_test.exs index 081494f077..178ef7f080 100644 --- a/test/dotcom_web/controllers/schedule/line/helpers_test.exs +++ b/test/dotcom_web/controllers/schedule/line/helpers_test.exs @@ -866,17 +866,6 @@ defmodule DotcomWeb.ScheduleController.Line.HelpersTest do %RoutePatterns.RoutePattern{typicality: 5} ]) end - - test "excludes route patterns with negative shape_priority" do - assert [%RoutePatterns.RoutePattern{typicality: 2, shape_priority: 1}, _] = - Helpers.filtered_by_typicality([ - %RoutePatterns.RoutePattern{typicality: 2, shape_priority: -2}, - %RoutePatterns.RoutePattern{typicality: 2, shape_priority: 1}, - %RoutePatterns.RoutePattern{typicality: 2, shape_priority: 2}, - %RoutePatterns.RoutePattern{typicality: 4, shape_priority: 1}, - %RoutePatterns.RoutePattern{typicality: 5, shape_priority: 1} - ]) - end end describe "get_stop_tree_or_lists/2" do From 19675713ebad2c24792f88dbfdb4b0d38be79bbf Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Wed, 1 May 2024 17:09:52 -0400 Subject: [PATCH 05/12] refactor(RoutePatterns.Repo): use the mock for tests --- config/config.exs | 1 + config/dotcom/dotcom.exs | 4 - config/test.exs | 1 + .../controllers/schedule/finder_api.ex | 4 +- .../controllers/schedule/line/helpers.ex | 9 +- .../schedule/timetable_controller.ex | 4 +- lib/dotcom_web/controllers/stop_controller.ex | 4 +- lib/predictions/stream_topic.ex | 4 +- lib/route_patterns/repo.ex | 7 +- lib/route_patterns/repo/behaviour.ex | 12 + lib/route_patterns/repo/mock.ex | 215 ------------------ .../controllers/stop_controller_test.exs | 49 ++-- test/leaflet/map_data/polyline_test.exs | 25 +- test/predictions/predictions_pub_sub_test.exs | 5 +- test/predictions/stream_topic_test.exs | 30 +-- test/route_patterns/repo_test.exs | 79 +++++-- test/support/factory/mbta_api.ex | 68 ++++++ test/support/factory/route_pattern.ex | 41 ++++ test/support/mocks.ex | 1 + 19 files changed, 242 insertions(+), 321 deletions(-) delete mode 100644 lib/route_patterns/repo/mock.ex create mode 100644 test/support/factory/mbta_api.ex create mode 100644 test/support/factory/route_pattern.ex diff --git a/config/config.exs b/config/config.exs index 5a98a28c1f..e25619c672 100644 --- a/config/config.exs +++ b/config/config.exs @@ -5,6 +5,7 @@ config :elixir, ansi_enabled: true config :dotcom, :httpoison, HTTPoison config :dotcom, :mbta_api_module, MBTA.Api +config :dotcom, :repo_modules, route_patterns: RoutePatterns.Repo config :dotcom, :redis, Dotcom.Cache.Multilevel.Redis config :dotcom, :redix, Redix diff --git a/config/dotcom/dotcom.exs b/config/dotcom/dotcom.exs index 7f18ce6914..146dd21671 100644 --- a/config/dotcom/dotcom.exs +++ b/config/dotcom/dotcom.exs @@ -34,10 +34,6 @@ config :dotcom, route_populate_caches?: config_env() == :prod routes_repo = if config_env() == :test, do: Routes.MockRepoApi, else: Routes.Repo config :dotcom, :routes_repo_api, routes_repo -repo_module = if config_env() == :test, do: RoutePatterns.Repo.Mock, else: RoutePatterns.Repo - -config :dotcom, :route_patterns_repo_api, repo_module - predictions_broadcast_interval_ms = if config_env() == :test, do: 50, else: 10_000 config :dotcom, predictions_broadcast_interval_ms: predictions_broadcast_interval_ms diff --git a/config/test.exs b/config/test.exs index b4453b13e8..5344e26b3b 100644 --- a/config/test.exs +++ b/config/test.exs @@ -5,6 +5,7 @@ config :dotcom, :cache, Dotcom.Cache.TestCache config :dotcom, :httpoison, HTTPoison.Mock config :dotcom, :mbta_api_module, MBTA.Api.Mock +config :dotcom, :repo_modules, route_patterns: RoutePatterns.Repo.Mock config :dotcom, :redis, Dotcom.Redis.Mock config :dotcom, :redix, Dotcom.Redix.Mock diff --git a/lib/dotcom_web/controllers/schedule/finder_api.ex b/lib/dotcom_web/controllers/schedule/finder_api.ex index ec4e182215..bd62f7da48 100644 --- a/lib/dotcom_web/controllers/schedule/finder_api.ex +++ b/lib/dotcom_web/controllers/schedule/finder_api.ex @@ -19,6 +19,8 @@ defmodule DotcomWeb.ScheduleController.FinderApi do require Logger + @route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns] + @type react_keys :: :date | :direction | :is_current @type react_strings :: [{react_keys, String.t()}] @type converted_values :: {Date.t(), integer, boolean} @@ -508,7 +510,7 @@ defmodule DotcomWeb.ScheduleController.FinderApi do with %Schedules.Trip{route_pattern_id: route_pattern_id} when not is_nil(route_pattern_id) <- Schedules.Repo.trip(trip_id), %RoutePatterns.RoutePattern{route_id: route_id} <- - RoutePatterns.Repo.get(route_pattern_id) do + @route_patterns_repo.get(route_pattern_id) do route_id else _ -> diff --git a/lib/dotcom_web/controllers/schedule/line/helpers.ex b/lib/dotcom_web/controllers/schedule/line/helpers.ex index 8ea55faa5e..331f7d9987 100644 --- a/lib/dotcom_web/controllers/schedule/line/helpers.ex +++ b/lib/dotcom_web/controllers/schedule/line/helpers.ex @@ -3,13 +3,14 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do Helpers for the line page """ - alias RoutePatterns.Repo, as: RoutePatternsRepo alias RoutePatterns.RoutePattern alias Routes.Repo, as: RoutesRepo alias Routes.{Route, Shape} alias Stops.Repo, as: StopsRepo alias Stops.{RouteStop, RouteStops, Stop} + @route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns] + @type query_param :: String.t() | nil @type direction_id :: 0 | 1 @typep stops_by_route :: %{String.t() => [Stop.t()]} @@ -88,7 +89,7 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do def get_map_route_patterns(route_id, type) do route_id - |> RoutePatternsRepo.by_route_id( + |> @route_patterns_repo.by_route_id( include: "representative_trip.shape,representative_trip.stops" ) |> filter_map_route_patterns(type) @@ -267,12 +268,12 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do base_opts end - RoutePatternsRepo.by_route_id(route_id, opts) + @route_patterns_repo.by_route_id(route_id, opts) |> Enum.filter(&(&1.route_id == route_id)) end defp get_line_route_patterns(_route, _direction_id, route_pattern_id) do - case RoutePatternsRepo.get(route_pattern_id, + case @route_patterns_repo.get(route_pattern_id, include: "representative_trip.stops" ) do %RoutePattern{} = route_pattern -> diff --git a/lib/dotcom_web/controllers/schedule/timetable_controller.ex b/lib/dotcom_web/controllers/schedule/timetable_controller.ex index 4507adb3f1..85cbd858cc 100644 --- a/lib/dotcom_web/controllers/schedule/timetable_controller.ex +++ b/lib/dotcom_web/controllers/schedule/timetable_controller.ex @@ -7,6 +7,8 @@ defmodule DotcomWeb.ScheduleController.TimetableController do require Logger + @route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns] + plug(DotcomWeb.Plugs.Route) plug(DotcomWeb.Plugs.DateInRating) plug(:tab_name) @@ -61,7 +63,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do } = build_timetable(conn.assigns.all_stops, timetable_schedules, direction_id) canonical_rps = - RoutePatterns.Repo.by_route_id(route.id, + @route_patterns_repo.by_route_id(route.id, direction_id: direction_id, canonical: true, include: "representative_trip.stops" diff --git a/lib/dotcom_web/controllers/stop_controller.ex b/lib/dotcom_web/controllers/stop_controller.ex index 9895705ceb..96060edf6c 100644 --- a/lib/dotcom_web/controllers/stop_controller.ex +++ b/lib/dotcom_web/controllers/stop_controller.ex @@ -17,6 +17,8 @@ defmodule DotcomWeb.StopController do alias Stops.{Repo, Stop} alias Util.AndOr + @route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns] + plug(:alerts) plug(DotcomWeb.Plugs.AlertsByTimeframe) @@ -90,7 +92,7 @@ defmodule DotcomWeb.StopController do @spec route_patterns_by_route_and_headsign(Stop.id_t()) :: by_route_and_headsign() defp route_patterns_by_route_and_headsign(stop_id) do stop_id - |> RoutePatterns.Repo.by_stop_id() + |> @route_patterns_repo.by_stop_id() |> Stream.reject(&ends_at?(&1, stop_id)) |> Stream.reject(&exclusively_drop_offs?(&1, stop_id)) |> Enum.group_by(& &1.route_id) diff --git a/lib/predictions/stream_topic.ex b/lib/predictions/stream_topic.ex index fd2ec3d016..ada7a2a308 100644 --- a/lib/predictions/stream_topic.ex +++ b/lib/predictions/stream_topic.ex @@ -19,6 +19,8 @@ defmodule Predictions.StreamTopic do defstruct [:topic, :fetch_keys, :streams] + @route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns] + @type filter_params :: String.t() @type clear_keys :: Store.fetch_keys() @type t :: %__MODULE__{ @@ -56,7 +58,7 @@ defmodule Predictions.StreamTopic do @spec streams_from_fetch_keys(Store.fetch_keys()) :: [{clear_keys, filter_params}] defp streams_from_fetch_keys(stop: stop_id) do - RoutePatterns.Repo.by_stop_id(stop_id) + @route_patterns_repo.by_stop_id(stop_id) |> Enum.map(&{to_keys(&1), to_filter_name(&1)}) end diff --git a/lib/route_patterns/repo.ex b/lib/route_patterns/repo.ex index 0ef2dd660c..17ebd5eac2 100644 --- a/lib/route_patterns/repo.ex +++ b/lib/route_patterns/repo.ex @@ -15,11 +15,7 @@ defmodule RoutePatterns.Repo do @cache Application.compile_env!(:dotcom, :cache) @ttl :timer.hours(1) - @doc """ - Returns a single route pattern by ID - """ - @callback get(RoutePattern.id_t()) :: RoutePattern.t() | nil - @callback get(RoutePattern.id_t(), keyword()) :: RoutePattern.t() | nil + @impl RoutePatterns.Repo.Behaviour def get(id, opts \\ []) when is_binary(id) do case get_id(id, opts) do {:ok, route_pattern} -> route_pattern @@ -51,6 +47,7 @@ defmodule RoutePatterns.Repo do |> Enum.sort(&reorder_mrts(&1, &2, route_id)) end + @impl RoutePatterns.Repo.Behaviour def by_stop_id(stop_id) do [stop: stop_id] |> Keyword.put(:include, "representative_trip.shape,representative_trip.stops") diff --git a/lib/route_patterns/repo/behaviour.ex b/lib/route_patterns/repo/behaviour.ex index 42f4debcc2..333827bfa1 100644 --- a/lib/route_patterns/repo/behaviour.ex +++ b/lib/route_patterns/repo/behaviour.ex @@ -5,10 +5,22 @@ defmodule RoutePatterns.Repo.Behaviour do alias RoutePatterns.RoutePattern alias Routes.Route + alias Stops.Stop + + @doc """ + Return a route pattern by ID + """ + @callback get(RoutePattern.id_t()) :: RoutePattern.t() | nil + @callback get(RoutePattern.id_t(), keyword()) :: RoutePattern.t() | nil @doc """ Return all route patterns for a route ID """ @callback by_route_id(Route.id_t()) :: [RoutePattern.t()] @callback by_route_id(Route.id_t(), keyword()) :: [RoutePattern.t()] + + @doc """ + Return all route patterns for a stop ID + """ + @callback by_stop_id(Stop.id_t()) :: [RoutePattern.t()] end diff --git a/lib/route_patterns/repo/mock.ex b/lib/route_patterns/repo/mock.ex deleted file mode 100644 index 533a8859c6..0000000000 --- a/lib/route_patterns/repo/mock.ex +++ /dev/null @@ -1,215 +0,0 @@ -defmodule RoutePatterns.Repo.Mock do - @moduledoc """ - A mock RoutePatterns Repo client for testing purposes. - We are returning just a subset of what the actual API would return, given that this file is for testing purposes - """ - - @behaviour RoutePatterns.Repo.Behaviour - - @impl RoutePatterns.Repo.Behaviour - def by_route_id("77", _opts), do: by_route_id("77") - - @impl RoutePatterns.Repo.Behaviour - def by_route_id("77") do - [ - %RoutePatterns.RoutePattern{ - direction_id: 0, - headsign: "Arlington Heights", - id: "77-_-0", - name: "Harvard Station - Arlington Heights", - representative_trip_id: "47409272", - representative_trip_polyline: - "gfsaGvlaqL[Ek@Ck@A[?]BU?sAHg@DWDODKJMPM\\KNEQAOU`@c@`@EBC@[PeCn@g@HM@??[@cBG_BKmBIq@Eq@GyAGoAGcDQ??OAiDQoFYcCO_AEM?OA??yAIyAIeBK}AK_ACy@FiAPu@N??YFa@FSDaARw@\\UL[T??WT_B`Bm@r@e@n@Sb@_@|@gAxCq@hBIT]fACDGL??cAhBc@j@k@n@m@v@MNg@j@WX??q@v@oBxBiArA}@fAc@f@??STSTwAbBq@t@eAnAaAjA]b@qAxAMNuA~As@z@??MPsB~B{@jAgBbCgA~Ag@x@mA`C??IR[x@}@pCw@bCGP??y@vBGf@O\\}@tBSV??_AtAkAtAY`@mAzAMP??IJu@~@i@r@Y^cAtAa@h@w@fAe@n@KN??e@l@m@x@g@r@}@nAY\\w@hAg@p@OR??kBjCeB`CiA|AuAlBQV??Y^{AvBy@hAY`@w@fA??UZ]`@[\\k@b@u@^_ChAwBfAQH??uAr@{@b@uAv@yAz@a@`@??CBWXiApAaApAo@t@kAdBMX??a@~@[`AEPe@pAc@`BIZQr@??Sv@_@zAOfAAZAl@S`C]xECh@????]lEShCOjBMjB?H??UhCWfDOlBYxDMlAIf@Kd@s@lCg@zAEN??Uj@sAjDi@xAy@vBKRO`@??IT{@nBO^u@bBiAlCw@nBYh@??KRq@jBwA`DsBzE[b@KP??}@tAORc@l@o@~@uApBS`@IRMl@SnCEf@??AV}@`FQ`AOv@MhAEp@CvA?~AAl@???VCtBC|BC~CEbEAf@??Ad@[tDOpBAJaAPGdA", - route_id: "77", - shape_id: "770116", - shape_priority: 3, - stop_ids: [ - "place-harsq", - "2310", - "2312", - "2314", - "23151", - "2316", - "2318", - "2319", - "2320", - "2271", - "2273", - "2274", - "22751", - "2275", - "2276", - "2277", - "2278", - "2279", - "2280", - "2281", - "2282", - "2283", - "2284", - "2285", - "2286", - "2287", - "2288", - "2290", - "2291", - "2292", - "2293", - "7922" - ], - time_desc: nil, - typicality: 1 - }, - %RoutePatterns.RoutePattern{ - direction_id: 1, - headsign: "Harvard", - id: "77-_-1", - name: "Arlington Heights - Harvard Station", - representative_trip_id: "47409270", - representative_trip_polyline: - "ob}aGbinqLC`@x@NRiC@KNqBV{C??BYBmADcEB_DB}BBuB?g@??@]?_BBwADq@LiANw@PaAn@oD??Lq@F_ARoCLm@HSRa@tAqBn@_Ab@m@NShAgBJO??NSrB{EvAaDp@kBd@}@FQ??n@}AhAmCt@cBN_@z@oBNa@??HUJSx@wBh@yArAkDZ{@f@{Aj@wB??FUJe@Hg@LmAXyDNmBToC??@WTsCLkBNkBRiC\\mEBc@???E\\yERaCPy@H]??ZuAx@wC^aBp@}B^mAj@mABEv@iA??LQNUdByBbAiAV]HI??hBmBtAw@z@c@jAm@??ZOvBgA~BiAt@_@j@c@LM??LO\\a@lAcBXa@x@iAzAwBj@w@`@k@??r@aAhA}AdBaCzB_Df@q@Zc@??Ze@X]|@oAf@s@l@y@p@}@d@o@LO??h@w@`@i@bAuAX_@h@s@j@s@??HKV]lA{AXa@jAuAFK??jAaB|@uBN]XY^eAVq@??FQ`A_Dd@uAr@gBf@iA??HQVi@vAyBX_@h@y@t@cAt@_A??VYdBuBhAqAnAyAxBoCNO??zBiCjB{B\\_@RUVY??^e@z@eAr@u@dAoAvB}B??NQnA_Bx@}@n@y@P_@Vg@b@s@Na@??JYd@yAVy@JWj@yANa@Ri@`@u@HOf@u@DG@A??r@u@t@w@JIf@]\\Ur@YLEf@MPE\\I??RC`AQ|AYXCf@?h@Br@DfAD|BPd@@jBJ|@D??rCLtAH~BN`BF|@F`@BR@??hAFvAJ^BhCHbAHbCLxBNjAD`@A??VAfD}@pAa@f@QEGMSJOL]LQJKNEVEf@ErAIT?\\CZ?j@@j@B\\F??@?j@Nf@R`@b@P`@Hd@H|@F|@Fz@Bj@Ap@Dr@B|@Nr@PZRVPw@ZaBf@Z\\R", - route_id: "77", - shape_id: "770115", - shape_priority: 3, - stop_ids: [ - "7922", - "17922", - "2250", - "2251", - "2252", - "2254", - "2255", - "2256", - "2257", - "2258", - "2259", - "2260", - "2261", - "2262", - "2263", - "2264", - "2265", - "2266", - "22661", - "22671", - "2268", - "2269", - "2270", - "2296", - "2297", - "2298", - "2299", - "2300", - "12301", - "2304", - "2305", - "2307", - "place-harsq", - "32549" - ], - time_desc: nil, - typicality: 1 - }, - %RoutePatterns.RoutePattern{ - direction_id: 0, - headsign: "North Cambridge", - id: "77-4-0", - name: "Harvard Station - North Cambridge", - representative_trip_id: "47428185", - representative_trip_polyline: - "gfsaGvlaqL[Ek@Ck@A[?]BU?sAHg@DWDODKJMPM\\KNEQAOU`@c@`@EBC@[PeCn@g@HM@??[@cBG_BKmBIq@Eq@GyAGoAGcDQ??OAiDQoFYcCO_AEM?OA??yAIyAIeBK}AK_ACy@FiAPu@N??YFa@FSDaARw@\\UL[T??WT_B`Bm@r@e@n@Sb@_@|@gAxCq@hBIT]fACDGL??cAhBc@j@k@n@m@v@MNg@j@WX??q@v@oBxBiArA}@fAc@f@??STSTwAbBq@t@eAnAaAjA]b@qAxAS_@", - route_id: "77", - shape_id: "770099", - shape_priority: 0, - stop_ids: [ - "place-harsq", - "2310", - "2312", - "2314", - "23151", - "2316", - "2318", - "2319", - "2320", - "12295" - ], - time_desc: nil, - typicality: 3 - }, - %RoutePatterns.RoutePattern{ - direction_id: 1, - headsign: "Harvard", - id: "77-4-1", - name: "North Cambridge - Harvard Station", - representative_trip_id: "47428171", - representative_trip_polyline: - "krwaG~kcqLR^MNJRxBoCNO??zBiCjB{B\\_@RUVY??^e@z@eAr@u@dAoAvB}B??NQnA_Bx@}@n@y@P_@Vg@b@s@Na@??JYd@yAVy@JWj@yANa@Ri@`@u@HOf@u@DG@A??r@u@t@w@JIf@]\\Ur@YLEf@MPE\\I??RC`AQ|AYXCf@?h@Br@DfAD|BPd@@jBJ|@D??rCLtAH~BN`BF|@F`@BR@??hAFvAJ^BhCHbAHbCLxBNjAD`@A??VAfD}@pAa@f@QEGMSJOL]LQJKNEVEf@ErAIT?\\CZ?j@@j@B\\F", - route_id: "77", - shape_id: "770057", - shape_priority: 0, - stop_ids: [ - "12295", - "2296", - "2297", - "2298", - "2299", - "2300", - "12301", - "2304", - "2305", - "2307", - "place-harsq" - ], - time_desc: nil, - typicality: 3 - }, - %RoutePatterns.RoutePattern{ - direction_id: 1, - headsign: "Harvard", - id: "77-1-1", - name: "Appleton St & Massachusetts Ave - Harvard Station", - representative_trip_id: "47408856", - representative_trip_polyline: - "uz|aGlrlqLLq@F_ARoCLm@HSRa@tAqBn@_Ab@m@NShAgBJO??NSrB{EvAaDp@kBd@}@FQ??n@}AhAmCt@cBN_@z@oBNa@??HUJSx@wBh@yArAkDZ{@f@{Aj@wB??FUJe@Hg@LmAXyDNmBToC??@WTsCLkBNkBRiC\\mEBc@???E\\yERaCPy@H]??ZuAx@wC^aBp@}B^mAj@mABEv@iA??LQNUdByBbAiAV]HI??hBmBtAw@z@c@jAm@??ZOvBgA~BiAt@_@j@c@LM??LO\\a@lAcBXa@x@iAzAwBj@w@`@k@??r@aAhA}AdBaCzB_Df@q@Zc@??Ze@X]|@oAf@s@l@y@p@}@d@o@LO??h@w@`@i@bAuAX_@h@s@j@s@??HKV]lA{AXa@jAuAFK??jAaB|@uBN]XY^eAVq@??FQ`A_Dd@uAr@gBf@iA??HQVi@vAyBX_@h@y@t@cAt@_A??VYdBuBhAqAnAyAxBoCNO??zBiCjB{B\\_@RUVY??^e@z@eAr@u@dAoAvB}B??NQnA_Bx@}@n@y@P_@Vg@b@s@Na@??JYd@yAVy@JWj@yANa@Ri@`@u@HOf@u@DG@A??r@u@t@w@JIf@]\\Ur@YLEf@MPE\\I??RC`AQ|AYXCf@?h@Br@DfAD|BPd@@jBJ|@D??rCLtAH~BN`BF|@F`@BR@??hAFvAJ^BhCHbAHbCLxBNjAD`@A??VAfD}@pAa@f@QEGMSJOL]LQJKNEVEf@ErAIT?\\CZ?j@@j@B\\F??@?j@Nf@R`@b@P`@Hd@H|@F|@Fz@Bj@Ap@Dr@B|@Nr@PZRVPw@ZaBf@Z\\R", - route_id: "77", - shape_id: "770117", - shape_priority: -1, - stop_ids: [ - "2251", - "2252", - "2254", - "2255", - "2256", - "2257", - "2258", - "2259", - "2260", - "2261", - "2262", - "2263", - "2264", - "2265", - "2266", - "22661", - "22671", - "2268", - "2269", - "2270", - "2296", - "2297", - "2298", - "2299", - "2300", - "12301", - "2304", - "2305", - "2307", - "place-harsq", - "32549" - ], - time_desc: "School days only", - typicality: 3 - } - ] - end -end diff --git a/test/dotcom_web/controllers/stop_controller_test.exs b/test/dotcom_web/controllers/stop_controller_test.exs index 24482cd0a2..f183e29e35 100644 --- a/test/dotcom_web/controllers/stop_controller_test.exs +++ b/test/dotcom_web/controllers/stop_controller_test.exs @@ -1,9 +1,7 @@ defmodule DotcomWeb.StopControllerTest do - use DotcomWeb.ConnCase, async: false + use DotcomWeb.ConnCase, async: true @moduletag :external - import Mock - alias DotcomWeb.StopController alias Routes.Route alias Stops.Stop @@ -138,36 +136,23 @@ defmodule DotcomWeb.StopControllerTest do test "grouped_route_patterns returns stop's route patterns by route & headsign", %{ conn: conn } do - with_mock(RoutePatterns.Repo, - by_stop_id: fn "place-here" -> - [ - %RoutePatterns.RoutePattern{ - route_id: "Purple-A", - headsign: "Tree Hill", - name: "Here Square - Tree Hill", - direction_id: 0 - } - ] - end - ) do - response = - get(conn, stop_path(conn, :grouped_route_patterns, "place-here")) |> json_response(200) - - assert %{ - "Purple-A" => %{ - "Tree Hill" => %{ - "direction_id" => 0, - "route_patterns" => [ - %{ - "headsign" => "Tree Hill", - "name" => "Here Square - Tree Hill" - } - | _ - ] - } + response = + get(conn, stop_path(conn, :grouped_route_patterns, "place-here")) |> json_response(200) + + assert %{ + "Purple-A" => %{ + "Tree Hill" => %{ + "direction_id" => 0, + "route_patterns" => [ + %{ + "headsign" => "Tree Hill", + "name" => "Here Square - Tree Hill" + } + | _ + ] } - } = response - end + } + } = response end end end diff --git a/test/leaflet/map_data/polyline_test.exs b/test/leaflet/map_data/polyline_test.exs index 57df0da78b..d0db6994bf 100644 --- a/test/leaflet/map_data/polyline_test.exs +++ b/test/leaflet/map_data/polyline_test.exs @@ -1,15 +1,22 @@ defmodule Leaflet.MapData.PolylineTest do use ExUnit.Case, async: true + + import Test.Support.Factory.RoutePattern + alias Leaflet.MapData.Polyline - @route_patterns_repo_api Application.compile_env!(:dotcom, :route_patterns_repo_api) + setup do + route_pattern = + build(:route_pattern, + representative_trip_polyline: + "gfsaGvlaqL[Ek@Ck@A[?]BU?sAHg@DWDODKJMPM\\KNEQAOU`@c@`@EBC@[PeCn@g@HM@??[@cBG_BKmBIq@Eq@GyAGoAGcDQ??OAiDQoFYcCO_AEM?OA??yAIyAIeBK}AK_ACy@FiAPu@N??YFa@FSDaARw@\\UL[T??WT_B`Bm@r@e@n@Sb@_@|@gAxCq@hBIT]fACDGL??cAhBc@j@k@n@m@v@MNg@j@WX??q@v@oBxBiArA}@fAc@f@??STSTwAbBq@t@eAnAaAjA]b@qAxAMNuA~As@z@??MPsB~B{@jAgBbCgA~Ag@x@mA`C??IR[x@}@pCw@bCGP??y@vBGf@O\\}@tBSV??_AtAkAtAY`@mAzAMP??IJu@~@i@r@Y^cAtAa@h@w@fAe@n@KN??e@l@m@x@g@r@}@nAY\\w@hAg@p@OR??kBjCeB`CiA|AuAlBQV??Y^{AvBy@hAY`@w@fA??UZ]`@[\\k@b@u@^_ChAwBfAQH??uAr@{@b@uAv@yAz@a@`@??CBWXiApAaApAo@t@kAdBMX??a@~@[`AEPe@pAc@`BIZQr@??Sv@_@zAOfAAZAl@S`C]xECh@????]lEShCOjBMjB?H??UhCWfDOlBYxDMlAIf@Kd@s@lCg@zAEN??Uj@sAjDi@xAy@vBKRO`@??IT{@nBO^u@bBiAlCw@nBYh@??KRq@jBwA`DsBzE[b@KP??}@tAORc@l@o@~@uApBS`@IRMl@SnCEf@??AV}@`FQ`AOv@MhAEp@CvA?~AAl@???VCtBC|BC~CEbEAf@??Ad@[tDOpBAJaAPGdA" + ) - describe "new/2" do - test "turns a polyline into a struct" do - route_pattern = - @route_patterns_repo_api.by_route_id("77") - |> List.first() + %{route_pattern: route_pattern} + end + describe "new/2" do + test "turns a polyline into a struct", %{route_pattern: route_pattern} do assert %Polyline{color: color, positions: positions} = Polyline.new(route_pattern, color: "#FF0000") @@ -18,11 +25,7 @@ defmodule Leaflet.MapData.PolylineTest do assert first == [42.37428, -71.119] end - test "makes polyline with default options" do - route_pattern = - @route_patterns_repo_api.by_route_id("77") - |> List.first() - + test "makes polyline with default options", %{route_pattern: route_pattern} do assert %Polyline{color: color, positions: positions} = Polyline.new(route_pattern) assert color == "#000000" diff --git a/test/predictions/predictions_pub_sub_test.exs b/test/predictions/predictions_pub_sub_test.exs index 8a486c50b7..9405ddf026 100644 --- a/test/predictions/predictions_pub_sub_test.exs +++ b/test/predictions/predictions_pub_sub_test.exs @@ -29,10 +29,7 @@ defmodule Predictions.PredictionsPubSubTest do :ok end - setup_with_mocks([ - {RoutePatterns.Repo, [:passthrough], - [by_stop_id: fn _stop_id -> [%RoutePatterns.RoutePattern{}] end]} - ]) do + setup do subscribe_fn = fn _, _ -> :ok end {:ok, pid} = PredictionsPubSub.start_link(name: :subscribe, subscribe_fn: subscribe_fn) diff --git a/test/predictions/stream_topic_test.exs b/test/predictions/stream_topic_test.exs index f9eb481808..a32591bba2 100644 --- a/test/predictions/stream_topic_test.exs +++ b/test/predictions/stream_topic_test.exs @@ -1,24 +1,24 @@ defmodule Predictions.StreamTopicTest do @moduledoc false - use ExUnit.Case, async: false + use ExUnit.Case, async: true require Dotcom.Assertions - import Mock + import Mox import Predictions.StreamTopic + import Test.Support.Factory.RoutePattern alias Predictions.StreamTopic - alias RoutePatterns.RoutePattern - setup_with_mocks([ - {RoutePatterns.Repo, [], [by_stop_id: fn id -> mock_route_patterns(id) end]} - ]) do - :ok - end + setup :verify_on_exit! describe "new/1" do test "works for stop id with route patterns" do + expect(RoutePatterns.Repo.Mock, :by_stop_id, fn "stopId" -> + build_list(1, :route_pattern, route_id: "Route1", direction_id: 0) + end) + topic = "stop:stopId" assert %StreamTopic{ @@ -36,6 +36,10 @@ defmodule Predictions.StreamTopicTest do end test "doesn't work for stop without route patterns" do + expect(RoutePatterns.Repo.Mock, :by_stop_id, fn "unserved_stop" -> + [] + end) + assert {:error, :no_streams_found} = new("stop:unserved_stop") end @@ -52,14 +56,4 @@ defmodule Predictions.StreamTopicTest do end end end - - defp mock_route_patterns("unserved_stop"), do: [] - - defp mock_route_patterns(_id) do - [ - %RoutePattern{route_id: "Route1", direction_id: 0}, - %RoutePattern{route_id: "Route1", direction_id: 1}, - %RoutePattern{route_id: "Route2", direction_id: 1} - ] - end end diff --git a/test/route_patterns/repo_test.exs b/test/route_patterns/repo_test.exs index 9754e5aa8b..24c4d2aa6c 100644 --- a/test/route_patterns/repo_test.exs +++ b/test/route_patterns/repo_test.exs @@ -1,50 +1,81 @@ defmodule RoutePatterns.RepoTest do use ExUnit.Case, async: true - @moduletag :external - alias RoutePatterns.{Repo, RoutePattern} + import Mox + import Test.Support.Factory.MbtaApi + + alias RoutePatterns.RoutePattern + + setup :verify_on_exit! describe "get" do test "returns a single route pattern" do - assert %RoutePattern{id: "111-5-0"} = Repo.get("111-5-0") + expect(MBTA.Api.Mock, :get_json, fn url, _ -> + assert url == "/route_patterns/111-5-0" + + %JsonApi{ + data: [ + build(:route_pattern_item, id: "111-5-0") + ] + } + end) + + assert %RoutePattern{id: "111-5-0"} = RoutePatterns.Repo.get("111-5-0") 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", _ -> + {:error, :not_found} + end) + + refute RoutePatterns.Repo.get("unknown_route_pattern") end end describe "by_route_id" do test "returns route patterns for a route" do - assert [%RoutePattern{} | _] = Repo.by_route_id("Red") - end + expect(MBTA.Api.Mock, :get_json, fn "/route_patterns/", opts -> + assert Keyword.get(opts, :route) == "Red" + %JsonApi{data: build_list(3, :route_pattern_item)} + end) - test "returns route patterns for all Green line branches" do - assert [%RoutePattern{} | _] = Repo.by_route_id("Green") + assert [%RoutePattern{} | _] = RoutePatterns.Repo.by_route_id("Red") end - test "takes a direction_id param" do - all_patterns = Repo.by_route_id("Red") - assert all_patterns |> Enum.map(& &1.direction_id) |> Enum.uniq() == [0, 1] - alewife_patterns = Repo.by_route_id("Red", direction_id: 1) - assert alewife_patterns |> Enum.map(& &1.direction_id) |> Enum.uniq() == [1] - end + test "handles the Green Line" do + expect(MBTA.Api.Mock, :get_json, fn "/route_patterns/", opts -> + assert Keyword.get(opts, :route) == "Green-B,Green-C,Green-D,Green-E" + %JsonApi{data: build_list(3, :route_pattern_item)} + end) - test "takes a route_pattern_id param" do - all_patterns = Repo.by_route_id("Red") - assert all_patterns |> Enum.map(& &1.direction_id) |> Enum.uniq() == [0, 1] - alewife_patterns = Repo.by_route_id("Red", direction_id: 1) - assert alewife_patterns |> Enum.map(& &1.direction_id) |> Enum.uniq() == [1] + RoutePatterns.Repo.by_route_id("Green") end end describe "by_stop_id/1" do - test "returns route patterns for a stop, with shape and stops" do - assert [%RoutePattern{representative_trip_polyline: polyline, stop_ids: stop_ids} | _] = - Repo.by_stop_id("place-sstat") + test "requests route patterns for a stop with shape and stops" do + expect(MBTA.Api.Mock, :get_json, fn "/route_patterns/", opts -> + assert Keyword.get(opts, :include) == + "representative_trip.shape,representative_trip.stops" + + %JsonApi{data: []} + end) - assert stop_ids - assert polyline + RoutePatterns.Repo.by_stop_id("place-nonsense") end end + + test "logs API errors" do + expect(MBTA.Api.Mock, :get_json, fn "/route_patterns/", _ -> + {:error, "some API mishap"} + end) + + log = + ExUnit.CaptureLog.capture_log(fn -> + assert RoutePatterns.Repo.by_stop_id("place-sstat") == [] + end) + + assert log =~ "[warning] module=Elixir.RoutePatterns.Repo" + assert log =~ "some API mishap" + end end diff --git a/test/support/factory/mbta_api.ex b/test/support/factory/mbta_api.ex new file mode 100644 index 0000000000..dc623b1445 --- /dev/null +++ b/test/support/factory/mbta_api.ex @@ -0,0 +1,68 @@ +defmodule Test.Support.Factory.MbtaApi do + @moduledoc """ + Generated fake data for MBTA.Api + """ + use ExMachina + + alias JsonApi.Item + + def item_factory(attrs) do + item = %Item{ + id: attrs[:id] || sequence(:id, &"item-#{&1}"), + attributes: attrs[:attributes] || %{}, + relationships: attrs[:relationships] || %{} + } + + merge_attributes(item, attrs) + end + + def route_pattern_item_factory(attrs) do + item = %Item{ + id: attrs[:id] || sequence(:id, &"item-#{&1}"), + attributes: + attrs[:attributes] || + %{ + "canonical" => false, + "direction_id" => 0, + "name" => Faker.App.name(), + "sort_order" => 0, + "time_desc" => nil, + "typicality" => 1 + }, + relationships: + attrs[:relationships] || + %{ + "representative_trip" => + build_list(1, :item, %{ + id: sequence(:representative_trip_id, &"trip-#{&1}"), + attributes: %{ + "headsign" => Faker.Address.street_name() + }, + relationships: %{ + "service" => + Faker.Util.pick([ + build_list(1, :item, id: sequence(:service, &"service-#{&1}")), + [] + ]), + "stops" => + Faker.Util.pick([ + build_list(3, :item, id: sequence(:stop, &"stop-#{&1}")), + [] + ]), + "shape" => + Faker.Util.pick([ + build_list(1, :item, + id: sequence(:shape, &"shape-#{&1}"), + attributes: %{"polyline" => Faker.Lorem.characters()} + ), + [] + ]) + } + }), + "route" => build_list(1, :item, id: sequence(:route, &"route-#{&1}")) + } + } + + merge_attributes(item, attrs) + end +end diff --git a/test/support/factory/route_pattern.ex b/test/support/factory/route_pattern.ex new file mode 100644 index 0000000000..38cf214a46 --- /dev/null +++ b/test/support/factory/route_pattern.ex @@ -0,0 +1,41 @@ +defmodule Test.Support.Factory.RoutePattern do + @moduledoc """ + Generated fake data for %RoutePattern{} + """ + use ExMachina + + alias RoutePatterns.RoutePattern + + def route_pattern_factory(attrs) do + direction_id = Map.get(attrs, :direction_id, Faker.Util.pick([0, 1])) + headsign = attrs[:headsign] || Faker.Address.street_name() + id = attrs[:id] || sequence(:id, &"route-pattern-#{&1}") + name = attrs[:name] || "#{Faker.Address.street_name()}-#{Faker.Address.street_name()}" + + representative_trip_id = + attrs[:representative_trip_id] || sequence(:representative_trip_id, &"trip-#{&1}") + + representative_trip_polyline = + attrs[:representative_trip_polyline] || Faker.Lorem.characters() + + route_id = attrs[:route_id] || sequence(:route_id, &"route-#{&1}") + service_id = attrs[:service_id] || sequence(:service_id, &"service-#{&1}") + shape_id = attrs[:shape_id] || sequence(:shape_id, &"shape-#{&1}") + typicality = attrs[:typicality] || Faker.Util.pick([0, 1, 2, 3, 4, 5]) + + route_pattern = %RoutePattern{ + direction_id: direction_id, + headsign: headsign, + id: id, + name: name, + representative_trip_id: representative_trip_id, + representative_trip_polyline: representative_trip_polyline, + route_id: route_id, + service_id: service_id, + shape_id: shape_id, + typicality: typicality + } + + merge_attributes(route_pattern, attrs) + end +end diff --git a/test/support/mocks.ex b/test/support/mocks.ex index 8777111c32..e1e001f7f6 100644 --- a/test/support/mocks.ex +++ b/test/support/mocks.ex @@ -10,4 +10,5 @@ Mox.defmock(Dotcom.Redix.Mock, for: Dotcom.Redix.Behaviour) Mox.defmock(Dotcom.Redix.PubSub.Mock, for: Dotcom.Redix.PubSub.Behaviour) Mox.defmock(MBTA.Api.Mock, for: MBTA.Api.Behaviour) +Mox.defmock(RoutePatterns.Repo.Mock, for: RoutePatterns.Repo.Behaviour) Mox.defmock(OpenTripPlannerClient.Mock, for: OpenTripPlannerClient.Behaviour) From 353eb7dd1b814a50b541233828edef3ef549dbcf Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Thu, 2 May 2024 11:15:20 -0400 Subject: [PATCH 06/12] more docs --- test/support/factory/mbta_api.ex | 79 +++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/test/support/factory/mbta_api.ex b/test/support/factory/mbta_api.ex index dc623b1445..8c69f06a90 100644 --- a/test/support/factory/mbta_api.ex +++ b/test/support/factory/mbta_api.ex @@ -16,6 +16,9 @@ defmodule Test.Support.Factory.MbtaApi do merge_attributes(item, attrs) end + @doc """ + MBTA V3 API route patterns return a subset of canonical, direction_id, name, sort_order, time_desc, and typicality attributes, with representative_trip and route relationships. + """ def route_pattern_item_factory(attrs) do item = %Item{ id: attrs[:id] || sequence(:id, &"item-#{&1}"), @@ -32,37 +35,59 @@ defmodule Test.Support.Factory.MbtaApi do relationships: attrs[:relationships] || %{ - "representative_trip" => - build_list(1, :item, %{ - id: sequence(:representative_trip_id, &"trip-#{&1}"), - attributes: %{ - "headsign" => Faker.Address.street_name() - }, - relationships: %{ - "service" => - Faker.Util.pick([ - build_list(1, :item, id: sequence(:service, &"service-#{&1}")), - [] - ]), - "stops" => - Faker.Util.pick([ - build_list(3, :item, id: sequence(:stop, &"stop-#{&1}")), - [] - ]), - "shape" => - Faker.Util.pick([ - build_list(1, :item, - id: sequence(:shape, &"shape-#{&1}"), - attributes: %{"polyline" => Faker.Lorem.characters()} - ), - [] - ]) - } - }), + "representative_trip" => build_list(1, :trip_item), "route" => build_list(1, :item, id: sequence(:route, &"route-#{&1}")) } } merge_attributes(item, attrs) end + + @doc """ + MBTA V3 API trips return a subset of headsign, and optionally service, stops, and shape relationships. + """ + def trip_item_factory(attrs) do + item = %Item{ + id: attrs[:id] || sequence(:trip, &"trip-#{&1}"), + attributes: + attrs[:attributes] || + %{ + "direction_id" => Faker.Util.pick([0, 1]), + "headsign" => Faker.Address.street_name(), + "name" => Faker.Address.street_name() + }, + relationships: %{ + "service" => + Faker.Util.pick([ + build_list(1, :item, id: sequence(:service, &"service-#{&1}")), + [] + ]), + "stops" => + Faker.Util.pick([ + build_list(3, :item, id: sequence(:stop, &"stop-#{&1}")), + [] + ]), + "shape" => + Faker.Util.pick([ + build_list(1, :shape_item), + [] + ]) + } + } + + merge_attributes(item, attrs) + end + + @doc """ + MBTA V3 API shapes return a polyline attribute. + """ + def shape_item_factory(attrs) do + item = %Item{ + id: attrs[:id] || sequence(:shape, &"shape-#{&1}"), + attributes: attrs[:attributes] || %{"polyline" => Faker.Lorem.characters()}, + relationships: %{} + } + + merge_attributes(item, attrs) + end end From 5f34cd88fecad47ff2983f969378c70f89673ed6 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Thu, 2 May 2024 15:09:58 -0400 Subject: [PATCH 07/12] feedback --- test/leaflet/map_data/polyline_test.exs | 3 +-- test/route_patterns/repo_test.exs | 7 +++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/leaflet/map_data/polyline_test.exs b/test/leaflet/map_data/polyline_test.exs index d0db6994bf..f9a19afa21 100644 --- a/test/leaflet/map_data/polyline_test.exs +++ b/test/leaflet/map_data/polyline_test.exs @@ -8,8 +8,7 @@ defmodule Leaflet.MapData.PolylineTest do setup do route_pattern = build(:route_pattern, - representative_trip_polyline: - "gfsaGvlaqL[Ek@Ck@A[?]BU?sAHg@DWDODKJMPM\\KNEQAOU`@c@`@EBC@[PeCn@g@HM@??[@cBG_BKmBIq@Eq@GyAGoAGcDQ??OAiDQoFYcCO_AEM?OA??yAIyAIeBK}AK_ACy@FiAPu@N??YFa@FSDaARw@\\UL[T??WT_B`Bm@r@e@n@Sb@_@|@gAxCq@hBIT]fACDGL??cAhBc@j@k@n@m@v@MNg@j@WX??q@v@oBxBiArA}@fAc@f@??STSTwAbBq@t@eAnAaAjA]b@qAxAMNuA~As@z@??MPsB~B{@jAgBbCgA~Ag@x@mA`C??IR[x@}@pCw@bCGP??y@vBGf@O\\}@tBSV??_AtAkAtAY`@mAzAMP??IJu@~@i@r@Y^cAtAa@h@w@fAe@n@KN??e@l@m@x@g@r@}@nAY\\w@hAg@p@OR??kBjCeB`CiA|AuAlBQV??Y^{AvBy@hAY`@w@fA??UZ]`@[\\k@b@u@^_ChAwBfAQH??uAr@{@b@uAv@yAz@a@`@??CBWXiApAaApAo@t@kAdBMX??a@~@[`AEPe@pAc@`BIZQr@??Sv@_@zAOfAAZAl@S`C]xECh@????]lEShCOjBMjB?H??UhCWfDOlBYxDMlAIf@Kd@s@lCg@zAEN??Uj@sAjDi@xAy@vBKRO`@??IT{@nBO^u@bBiAlCw@nBYh@??KRq@jBwA`DsBzE[b@KP??}@tAORc@l@o@~@uApBS`@IRMl@SnCEf@??AV}@`FQ`AOv@MhAEp@CvA?~AAl@???VCtBC|BC~CEbEAf@??Ad@[tDOpBAJaAPGdA" + representative_trip_polyline: "gfsaGvlaqL[Ek@Ck@A[?]BU?sAH" ) %{route_pattern: route_pattern} diff --git a/test/route_patterns/repo_test.exs b/test/route_patterns/repo_test.exs index 24c4d2aa6c..c9f8f301f2 100644 --- a/test/route_patterns/repo_test.exs +++ b/test/route_patterns/repo_test.exs @@ -24,11 +24,14 @@ defmodule RoutePatterns.RepoTest do end test "returns nil for an unknown route pattern" do - expect(MBTA.Api.Mock, :get_json, fn "/route_patterns/unknown_route_pattern", _ -> + id = Faker.Internet.slug() + + expect(MBTA.Api.Mock, :get_json, fn path, _ -> + assert path == "/route_patterns/" <> id {:error, :not_found} end) - refute RoutePatterns.Repo.get("unknown_route_pattern") + refute RoutePatterns.Repo.get(id) end end From 5e692fb090c376c3f8cfd2bbff84f24913e796e0 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Thu, 2 May 2024 16:04:54 -0400 Subject: [PATCH 08/12] tests(StopController): add mock for test --- .../controllers/stop_controller_test.exs | 74 ++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/test/dotcom_web/controllers/stop_controller_test.exs b/test/dotcom_web/controllers/stop_controller_test.exs index f183e29e35..5aeabd5c04 100644 --- a/test/dotcom_web/controllers/stop_controller_test.exs +++ b/test/dotcom_web/controllers/stop_controller_test.exs @@ -1,17 +1,21 @@ defmodule DotcomWeb.StopControllerTest do use DotcomWeb.ConnCase, async: true - @moduletag :external + + import Mox alias DotcomWeb.StopController alias Routes.Route alias Stops.Stop alias Util.Breadcrumb + setup :verify_on_exit! + test "redirects to subway stops on index", %{conn: conn} do conn = conn |> get(stop_path(conn, :index)) assert redirected_to(conn) == stop_path(conn, :show, :subway) end + @tag :external test "shows stations by mode", %{conn: conn} do conn = conn @@ -24,6 +28,7 @@ defmodule DotcomWeb.StopControllerTest do end end + @tag :external test "assigns stop_info for each mode", %{conn: conn} do for mode <- [:subway, :ferry, "commuter-rail"] do conn = @@ -42,12 +47,14 @@ defmodule DotcomWeb.StopControllerTest do assert redirected_to(conn) == stop_path(conn, :show, "Four Corners / Geneva") end + @tag :external test "shows a stop ID", %{conn: conn} do conn = conn |> get(stop_path(conn, :show, "place-sstat")) assert conn.assigns.stop_id end + @tag :external test "sets a custom meta description for stops", %{conn: conn} do conn = conn @@ -56,6 +63,7 @@ defmodule DotcomWeb.StopControllerTest do assert conn.assigns.meta_description end + @tag :external test "redirects to a parent stop page for a child stop", %{conn: conn} do conn = conn @@ -64,6 +72,7 @@ defmodule DotcomWeb.StopControllerTest do assert redirected_to(conn) == stop_path(conn, :show, "place-harvd") end + @tag :external test "404s for an unknown stop", %{conn: conn} do conn = conn @@ -99,6 +108,7 @@ defmodule DotcomWeb.StopControllerTest do end describe "api" do + @tag :external test "returns json with departure data", %{conn: conn} do path = stop_path(conn, :api, id: "place-sstat") assert path == "/stops/api?id=place-sstat" @@ -118,6 +128,7 @@ defmodule DotcomWeb.StopControllerTest do end describe "show/2" do + @tag :external test "should set the title and meta description of the page", %{conn: conn} do conn = conn @@ -136,6 +147,67 @@ defmodule DotcomWeb.StopControllerTest do test "grouped_route_patterns returns stop's route patterns by route & headsign", %{ conn: conn } do + MBTA.Api.Mock + |> expect(:get_json, fn "/stops/place-here", _ -> + %JsonApi{ + data: [ + %JsonApi.Item{ + attributes: %{ + "description" => "", + "location_type" => 1, + "name" => "Somewhere", + "platform_code" => "", + "platform_name" => "", + "zone_number" => "" + }, + relationships: %{ + "facilities" => [], + "zone" => [] + } + } + ] + } + end) + |> expect(:get_json, fn "/schedules/", opts -> + assert opts[:stop] == "place-here" + + %JsonApi{ + data: [ + %JsonApi.Item{ + attributes: %{}, + relationships: %{} + } + ] + } + end) + |> expect(:get_json, fn "/services/", _ -> + %JsonApi{ + data: [ + %JsonApi.Item{ + id: "", + attributes: %{ + "schedule_type" => "Weekday" + }, + relationships: %{}, + type: "service" + } + ] + } + end) + + expect(RoutePatterns.Repo.Mock, :by_stop_id, fn stop_id -> + assert stop_id == "place-here" + + [ + %RoutePatterns.RoutePattern{ + route_id: "Purple-A", + headsign: "Tree Hill", + name: "Here Square - Tree Hill", + direction_id: 0 + } + ] + end) + response = get(conn, stop_path(conn, :grouped_route_patterns, "place-here")) |> json_response(200) From 3a1f56a7744f1ef86f10320ce1c140ecb4ea7f0e Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Mon, 6 May 2024 14:49:14 -0400 Subject: [PATCH 09/12] refactor factories: just do the thing --- test/support/factory/mbta_api.ex | 85 +++++---------------------- test/support/factory/route_pattern.ex | 31 +--------- 2 files changed, 17 insertions(+), 99 deletions(-) diff --git a/test/support/factory/mbta_api.ex b/test/support/factory/mbta_api.ex index 8c69f06a90..e0a100dae3 100644 --- a/test/support/factory/mbta_api.ex +++ b/test/support/factory/mbta_api.ex @@ -7,24 +7,20 @@ defmodule Test.Support.Factory.MbtaApi do alias JsonApi.Item def item_factory(attrs) do - item = %Item{ - id: attrs[:id] || sequence(:id, &"item-#{&1}"), - attributes: attrs[:attributes] || %{}, - relationships: attrs[:relationships] || %{} - } - - merge_attributes(item, attrs) + merge_attributes(%Item{}, attrs) end @doc """ - MBTA V3 API route patterns return a subset of canonical, direction_id, name, sort_order, time_desc, and typicality attributes, with representative_trip and route relationships. + MBTA V3 API route patterns return a subset of canonical, direction_id, name, + sort_order, time_desc, and typicality attributes, with representative_trip and + route relationships. """ def route_pattern_item_factory(attrs) do - item = %Item{ - id: attrs[:id] || sequence(:id, &"item-#{&1}"), - attributes: - attrs[:attributes] || - %{ + build( + :item, + Map.merge( + %{ + attributes: %{ "canonical" => false, "direction_id" => 0, "name" => Faker.App.name(), @@ -32,62 +28,13 @@ defmodule Test.Support.Factory.MbtaApi do "time_desc" => nil, "typicality" => 1 }, - relationships: - attrs[:relationships] || - %{ - "representative_trip" => build_list(1, :trip_item), - "route" => build_list(1, :item, id: sequence(:route, &"route-#{&1}")) + relationships: %{ + "representative_trip" => [build(:item)], + "route" => [build(:item)] } - } - - merge_attributes(item, attrs) - end - - @doc """ - MBTA V3 API trips return a subset of headsign, and optionally service, stops, and shape relationships. - """ - def trip_item_factory(attrs) do - item = %Item{ - id: attrs[:id] || sequence(:trip, &"trip-#{&1}"), - attributes: - attrs[:attributes] || - %{ - "direction_id" => Faker.Util.pick([0, 1]), - "headsign" => Faker.Address.street_name(), - "name" => Faker.Address.street_name() - }, - relationships: %{ - "service" => - Faker.Util.pick([ - build_list(1, :item, id: sequence(:service, &"service-#{&1}")), - [] - ]), - "stops" => - Faker.Util.pick([ - build_list(3, :item, id: sequence(:stop, &"stop-#{&1}")), - [] - ]), - "shape" => - Faker.Util.pick([ - build_list(1, :shape_item), - [] - ]) - } - } - - merge_attributes(item, attrs) - end - - @doc """ - MBTA V3 API shapes return a polyline attribute. - """ - def shape_item_factory(attrs) do - item = %Item{ - id: attrs[:id] || sequence(:shape, &"shape-#{&1}"), - attributes: attrs[:attributes] || %{"polyline" => Faker.Lorem.characters()}, - relationships: %{} - } - - merge_attributes(item, attrs) + }, + attrs + ) + ) end end diff --git a/test/support/factory/route_pattern.ex b/test/support/factory/route_pattern.ex index 38cf214a46..ca80900677 100644 --- a/test/support/factory/route_pattern.ex +++ b/test/support/factory/route_pattern.ex @@ -7,35 +7,6 @@ defmodule Test.Support.Factory.RoutePattern do alias RoutePatterns.RoutePattern def route_pattern_factory(attrs) do - direction_id = Map.get(attrs, :direction_id, Faker.Util.pick([0, 1])) - headsign = attrs[:headsign] || Faker.Address.street_name() - id = attrs[:id] || sequence(:id, &"route-pattern-#{&1}") - name = attrs[:name] || "#{Faker.Address.street_name()}-#{Faker.Address.street_name()}" - - representative_trip_id = - attrs[:representative_trip_id] || sequence(:representative_trip_id, &"trip-#{&1}") - - representative_trip_polyline = - attrs[:representative_trip_polyline] || Faker.Lorem.characters() - - route_id = attrs[:route_id] || sequence(:route_id, &"route-#{&1}") - service_id = attrs[:service_id] || sequence(:service_id, &"service-#{&1}") - shape_id = attrs[:shape_id] || sequence(:shape_id, &"shape-#{&1}") - typicality = attrs[:typicality] || Faker.Util.pick([0, 1, 2, 3, 4, 5]) - - route_pattern = %RoutePattern{ - direction_id: direction_id, - headsign: headsign, - id: id, - name: name, - representative_trip_id: representative_trip_id, - representative_trip_polyline: representative_trip_polyline, - route_id: route_id, - service_id: service_id, - shape_id: shape_id, - typicality: typicality - } - - merge_attributes(route_pattern, attrs) + merge_attributes(%RoutePattern{}, attrs) end end From 5457fd7b08c27075c81244d92eb438d286980c00 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Mon, 6 May 2024 15:10:55 -0400 Subject: [PATCH 10/12] simple item --- test/support/factory/mbta_api.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/support/factory/mbta_api.ex b/test/support/factory/mbta_api.ex index e0a100dae3..04fd7a023b 100644 --- a/test/support/factory/mbta_api.ex +++ b/test/support/factory/mbta_api.ex @@ -6,8 +6,8 @@ defmodule Test.Support.Factory.MbtaApi do alias JsonApi.Item - def item_factory(attrs) do - merge_attributes(%Item{}, attrs) + def item_factory do + %Item{} end @doc """ From 72bab3622e460991c7d95de4f321dea56e02d36f Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Mon, 6 May 2024 15:21:21 -0400 Subject: [PATCH 11/12] fixup --- test/support/factory/route_pattern.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/support/factory/route_pattern.ex b/test/support/factory/route_pattern.ex index ca80900677..4c08fabc87 100644 --- a/test/support/factory/route_pattern.ex +++ b/test/support/factory/route_pattern.ex @@ -6,7 +6,7 @@ defmodule Test.Support.Factory.RoutePattern do alias RoutePatterns.RoutePattern - def route_pattern_factory(attrs) do - merge_attributes(%RoutePattern{}, attrs) + def route_pattern_factory do + %RoutePattern{} end end From 7e204caadcfc599eae1b88733076c33df254a1c1 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Mon, 6 May 2024 15:30:15 -0400 Subject: [PATCH 12/12] remove magic number in test! --- test/route_patterns/repo_test.exs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/route_patterns/repo_test.exs b/test/route_patterns/repo_test.exs index c9f8f301f2..92697dbb6c 100644 --- a/test/route_patterns/repo_test.exs +++ b/test/route_patterns/repo_test.exs @@ -10,17 +10,19 @@ defmodule RoutePatterns.RepoTest do describe "get" do test "returns a single route pattern" do - expect(MBTA.Api.Mock, :get_json, fn url, _ -> - assert url == "/route_patterns/111-5-0" + id = Faker.Internet.slug() + + expect(MBTA.Api.Mock, :get_json, fn path, _ -> + assert path == "/route_patterns/" <> id %JsonApi{ data: [ - build(:route_pattern_item, id: "111-5-0") + build(:route_pattern_item, id: id) ] } end) - assert %RoutePattern{id: "111-5-0"} = RoutePatterns.Repo.get("111-5-0") + assert %RoutePattern{id: ^id} = RoutePatterns.Repo.get(id) end test "returns nil for an unknown route pattern" do