Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change time to display "BRD" to <60 seconds until departure at terminals #861

Merged
merged 6 commits into from
Dec 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/content/message/predictions.ex
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ defmodule Content.Message.Predictions do

{minutes, approximate?} =
cond do
prediction.stopped_at_predicted_stop? and (!terminal? or sec <= 30) -> {:boarding, false}
prediction.stopped_at_predicted_stop? and (!terminal? or sec <= 60) -> {:boarding, false}
!terminal? and sec <= 30 -> {:arriving, false}
!terminal? and sec <= 60 -> {:approaching, false}
min > 60 -> {60, true}
Expand Down
34 changes: 32 additions & 2 deletions test/content/messages/predictions_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,24 @@ defmodule Content.Message.PredictionsTest do
assert Content.Message.to_string(msg) == "Ashmont BRD"
end

test "does not put boarding on the sign too early when train is stopped at terminal" do
test "put boarding on the sign when train is stopped at terminal and departing in less than 60 seconds" do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lower level tests are fine, but they tend to be more brittle in the face of refactoring. Instead, we could add a pair of cases in realtime_test.exs to exercise this at a higher level, and it should be about the same cost code-wise. If you want to give that a try, take a look at some of the examples there. There's a @terminal_sign struct that's already set up with a terminal configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been able to get a specific test case work in realtime_test, but I've learned a lot about the Elixir test setup by trying to get this work! Will follow up on Monday to try to get it all working

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course I figured it out as soon as I got up. There are new tests in realtime_test.exs now!

prediction = %Predictions.Prediction{
seconds_until_departure: 35,
seconds_until_departure: 59,
direction_id: 1,
route_id: "Mattapan",
destination_stop_id: "70261",
stopped_at_predicted_stop?: true,
boarding_status: "Stopped at station"
}

msg = Content.Message.Predictions.new(prediction, true, nil)

assert Content.Message.to_string(msg) == "Ashmont BRD"
end

test "puts 1 min on sign when train is stopped at terminal but departure is between 60-90 seconds" do
prediction = %Predictions.Prediction{
seconds_until_departure: 61,
direction_id: 1,
route_id: "Mattapan",
destination_stop_id: "70261",
Expand All @@ -230,6 +245,21 @@ defmodule Content.Message.PredictionsTest do
assert Content.Message.to_string(msg) == "Ashmont 1 min"
end

test "puts minutes on sign when train is stopped at terminal but departure is >=1.5 minutes" do
prediction = %Predictions.Prediction{
seconds_until_departure: 92,
direction_id: 1,
route_id: "Mattapan",
destination_stop_id: "70261",
stopped_at_predicted_stop?: true,
boarding_status: "Stopped at station"
}

msg = Content.Message.Predictions.new(prediction, true, nil)

assert Content.Message.to_string(msg) == "Ashmont 2 min"
end

test "puts 1 min on the sign when train is not boarding, but is predicted to depart in less than a minute when offset" do
prediction = %Predictions.Prediction{
seconds_until_departure: 70,
Expand Down
42 changes: 42 additions & 0 deletions test/signs/realtime_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,48 @@ defmodule Signs.RealtimeTest do
Signs.Realtime.handle_info(:run_loop, @terminal_sign)
end

test "When the train is stopped at the terminal and departing in <= 60 seconds, shows BRD" do
expect(Engine.Predictions.Mock, :for_stop, fn _, _ ->
[
prediction(
destination: :mattapan,
stopped: 0,
seconds_until_arrival: -1,
seconds_until_departure: 60,
trip_id: "3"
)
]
end)

expect_messages({"Mattapan BRD", ""})

expect_audios(
[
{:canned, {"109", ["501", "21000", "4100", "21000", "864", "21000", "544"], :audio}}
],
[{"The next Mattapan train is now boarding.", nil}]
)

Signs.Realtime.handle_info(:run_loop, @terminal_sign)
end

test "When the train is stopped at the terminal and departing in more than 60 seconds, shows mins to departure" do
expect(Engine.Predictions.Mock, :for_stop, fn _, _ ->
[
prediction(
destination: :mattapan,
stopped: 0,
seconds_until_arrival: nil,
seconds_until_departure: 91,
trip_id: "3"
)
]
end)

expect_messages({"Mattapan 2 min", ""})
Signs.Realtime.handle_info(:run_loop, @terminal_sign)
end

test "When the train is stopped a long time away, shows max time instead of stopped" do
expect(Engine.Predictions.Mock, :for_stop, fn _, _ ->
[prediction(destination: :mattapan, arrival: 3700, stopped: 8)]
Expand Down
Loading