test(Predictions): Added the mox for predictions #2036
Conversation
config/test.exs
Outdated
| route_patterns: RoutePatterns.Repo.Mock, | ||
| predictions: Predictions.Repo.Mock |
There was a problem hiding this comment.
Should alphabetize this list and the one above because they are going to be quite long.
lib/dotcom/realtime_schedule.ex
Outdated
| @default_opts [ | ||
| stops_fn: &StopsRepo.get/1, | ||
| routes_fn: &RoutesRepo.by_stop_with_route_pattern/1, | ||
| predictions_fn: &PredictionsRepo.all_no_cache/1, | ||
| predictions_fn: Function.capture(@predictions_repo, :all_no_cache, 1), |
There was a problem hiding this comment.
We should just remove the dependency injection. It's pretty simple to just replace all instances of predictions_fn.(params) with a call to @predictions_repo.all_no_cache(params).
| time: ~N[2017-01-01T00:00:00], | ||
| stop: %Stops.Stop{id: "origin"}, | ||
| trip: 1234, |
There was a problem hiding this comment.
Should use Faker for all of these unless they mean something.
There was a problem hiding this comment.
Would also be super simple to turn this into a factory.
| @@ -13,3 +13,4 @@ Mox.defmock(CMS.Api.Mock, for: CMS.Api.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) | |||
| Mox.defmock(Predictions.Repo.Mock, for: Predictions.Repo.Behaviour) | |||
There was a problem hiding this comment.
Should probably separate out the Repo.Mocks and make sure they're alphabetized. I would label them too.
| @@ -578,7 +589,8 @@ defmodule DotcomWeb.ScheduleController.TripInfoTest do | |||
|
|
|||
| describe "test that wollaston station is properly inserted when expected" do | |||
| test "Does not add Wollaston to non Red line routes", %{conn: conn} do | |||
| init = init(trip_fn: &trip_fn/2, vehicle_fn: &vehicle_fn/1, prediction_fn: &prediction_fn/1) | |||
| expect(Predictions.Repo.Mock, :all, &prediction_fn/1) | |||
There was a problem hiding this comment.
Why are we still referencing prediction_fn? Should be able to remove that.
There was a problem hiding this comment.
prediction_fn/1 is a helper defined in this file. It seemed fine to keep it used here instead of redefining it for every test
There was a problem hiding this comment.
We shouldn't need it. It looks like it is being used as a kind of factory.
|
|
||
| conn = | ||
| conn | ||
| |> assign(:origin, %Stops.Stop{id: "origin"}) | ||
| |> assign(:destination, nil) | ||
| |> assign(:route, %{id: "4"}) | ||
| |> assign(:direction_id, "0") | ||
| |> call(predictions_fn: fn _ -> [prediction] end) | ||
| |> call(init()) |
There was a problem hiding this comment.
init() will just return an empty list. And call/1 will default to an empty list. In fact call/2 just ignores the options. So, I'm pretty sure you can just remove it.
test/dotcom/transit_near_me_test.exs
Outdated
| schedule_relationship: nil, | ||
| status: nil, |
There was a problem hiding this comment.
See the below comment for the factory.
| alias Predictions.Prediction | ||
|
|
||
| def prediction_factory do | ||
| %Prediction{} |
There was a problem hiding this comment.
In the above tests, you are building a prediction and passing in every single value. I'm fairly certain that not setting anything here will set them all to struct default which is probably nil. So, passing in values set to nil won't do anything. You should set reasonable defaults here and then you only have to override ones that are necessary for the tests. Makes the tests shorter and clearer.
No ticket. Adding some mox for the predictions, and updating tests that use the repo functions