From 679edf50ff655a4349b7a87286c2d5db551a5000 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Mon, 23 Dec 2024 10:24:33 -0500 Subject: [PATCH] fix(tests): Don't run tests that fail non-deterministically (#2286) --- .../dotcom/lib/green_line/supervisor_test.exs | 3 +++ test/dotcom/stream/vehicles_test.exs | 2 ++ test/dotcom/transit_near_me_test.exs | 1 + test/dotcom/trip_plan/alerts_test.exs | 1 + .../customer_support_controller_test.exs | 19 +++++++++++++++++++ .../schedule/line/helpers_test.exs | 1 + .../controllers/schedule/predictions_test.exs | 2 ++ .../transit_near_me_controller_test.exs | 2 ++ .../controllers/trip_plan/feedback_test.exs | 1 + test/feedback/repo_test.exs | 1 + test/routes/populate_caches_test.exs | 1 + test/test_helper.exs | 2 +- test/util/util_test.exs | 1 + 13 files changed, 36 insertions(+), 1 deletion(-) diff --git a/test/dotcom/lib/green_line/supervisor_test.exs b/test/dotcom/lib/green_line/supervisor_test.exs index 385699b21a..66af421b32 100644 --- a/test/dotcom/lib/green_line/supervisor_test.exs +++ b/test/dotcom/lib/green_line/supervisor_test.exs @@ -19,6 +19,7 @@ defmodule Dotcom.GreenLine.CacheSupervisorTest do :ok end + @tag :flaky test "lookup/1 can retrieve the pid by date" do date = Util.service_date() {:ok, _} = start_child(date) @@ -31,6 +32,7 @@ defmodule Dotcom.GreenLine.CacheSupervisorTest do assert lookup(date) == nil end + @tag :flaky test "stops_on_routes/2 gets information for nil date" do Stops.Repo.Mock |> expect(:by_route, 4, fn route_id, direction_id, opts -> @@ -50,6 +52,7 @@ defmodule Dotcom.GreenLine.CacheSupervisorTest do assert stops_on_routes(0, nil) end + @tag :flaky test "stops_on_routes/2 gets information for service date" do date = Util.service_date() diff --git a/test/dotcom/stream/vehicles_test.exs b/test/dotcom/stream/vehicles_test.exs index e4ad044b07..32c813aa25 100644 --- a/test/dotcom/stream/vehicles_test.exs +++ b/test/dotcom/stream/vehicles_test.exs @@ -15,6 +15,7 @@ defmodule Dotcom.Stream.VehiclesTest do :ok end + @tag :flaky test "broadcasts vehicles by route and direction id" do Phoenix.PubSub.subscribe(Dotcom.PubSub, "vehicles:Red:0") Phoenix.PubSub.subscribe(Dotcom.PubSub, "vehicles:CR-Lowell:1") @@ -58,6 +59,7 @@ defmodule Dotcom.Stream.VehiclesTest do }) end + @tag :flaky test "broadcasts to the configured topic" do DotcomWeb.Endpoint.subscribe("vehicles-v2:Green:1") assert {:ok, pid} = start_supervised({Dotcom.Stream.Vehicles, topic: "vehicles-v2"}) diff --git a/test/dotcom/transit_near_me_test.exs b/test/dotcom/transit_near_me_test.exs index 3f93a95d4b..8ea8394fa9 100644 --- a/test/dotcom/transit_near_me_test.exs +++ b/test/dotcom/transit_near_me_test.exs @@ -729,6 +729,7 @@ defmodule Dotcom.TransitNearMeTest do trip: @trip3 }) + @tag :flaky test "returns time data for the next 2 predictions" do expect(Predictions.Repo.Mock, :all, fn _ -> [@prediction1, @prediction2, @prediction3] diff --git a/test/dotcom/trip_plan/alerts_test.exs b/test/dotcom/trip_plan/alerts_test.exs index 4d54683e7e..e5855fc3c2 100644 --- a/test/dotcom/trip_plan/alerts_test.exs +++ b/test/dotcom/trip_plan/alerts_test.exs @@ -166,6 +166,7 @@ defmodule Dotcom.TripPlan.AlertsTest do end describe "by_mode_and_stops/2" do + @tag :flaky test "groups alerts by route, to, and from", %{itinerary: itinerary, route_id: route_id} do expect(MBTA.Api.Mock, :get_json, fn "/trips/" <> id, [] -> %JsonApi{ diff --git a/test/dotcom_web/controllers/customer_support_controller_test.exs b/test/dotcom_web/controllers/customer_support_controller_test.exs index 2468762573..89e64736ff 100644 --- a/test/dotcom_web/controllers/customer_support_controller_test.exs +++ b/test/dotcom_web/controllers/customer_support_controller_test.exs @@ -37,6 +37,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert response =~ "A comment about the MBTA" end + @tag :flaky test "sets the service options on the connection", %{conn: conn} do conn = get(conn, customer_support_path(conn, :index)) @@ -100,6 +101,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do } end + @tag :flaky test "shows a thank you message on success and sends an email", %{conn: conn} do conn = post(conn, customer_support_path(conn, :submit), valid_request_response_data()) @@ -134,6 +136,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert conn.assigns.meta_description end + @tag :flaky test "validates presence of comments", %{conn: conn} do conn = post( @@ -145,6 +148,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "comments" in conn.assigns.errors end + @tag :flaky test "validates the presence of the service type", %{conn: conn} do conn = post( @@ -178,6 +182,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "subject" in conn.assigns.errors end + @tag :flaky test "validates that the subject is a required field", %{conn: conn} do # remove subject from valid_no_response_data: form_data = pop_in(valid_no_response_data()["support"]["subject"]) |> elem(1) @@ -221,6 +226,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do refute conn.assigns["errors"] end + @tag :flaky test "requires first_name if customer does want a response", %{conn: conn} do conn = post( @@ -243,6 +249,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "last_name" in conn.assigns.errors end + @tag :flaky test "invalid with no email when the customer wants a response", %{conn: conn} do conn = post( @@ -265,6 +272,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "email" in conn.assigns.errors end + @tag :flaky test "invalid with phone but no email when the customer wants a response", %{conn: conn} do conn = post( @@ -289,6 +297,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "privacy" in conn.assigns.errors end + @tag :flaky test "attaches photos in params", %{conn: conn} do File.write!("/tmp/upload-1", "upload 1 data") File.write!("/tmp/upload-2", "upload 2 data") @@ -310,6 +319,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert %{"filename" => "photo-2.jpg", "data" => Base.encode64("upload 2 data")} in attachments end + @tag :flaky test "doesn't attach more than 6 files", %{conn: conn} do params = valid_no_response_data() @@ -321,6 +331,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert length(attachments) <= 6 end + @tag :flaky test "doesn't attach a file larger than 2 MB", %{conn: conn} do # Oversized test file is ~4 MB oversized_file = Enum.find(test_photos(), &String.starts_with?(&1.filename, "oversized")) @@ -336,6 +347,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert attachments == [] end + @tag :flaky test "prevents submissions when an upload does not appear to be an image", %{conn: conn} do params = valid_request_response_data() @@ -354,6 +366,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "photos" in conn.assigns.errors end + @tag :flaky test "logs a warning, returns 429, and shows an error when rate limit reached", %{conn: conn} do rate_limit = Application.get_env(:dotcom, :feedback_rate_limit) @@ -382,6 +395,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert log =~ "rate limit exceeded" end + @tag :flaky test "requires a successful recaptcha response", %{conn: conn} do conn = post( @@ -393,6 +407,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "recaptcha" in conn.assigns.errors end + @tag :flaky test "handles invalid response", %{conn: conn} do conn = post( @@ -417,6 +432,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert "recaptcha" in conn.assigns.errors end + @tag :flaky test "adds date and time fields if not present in the form", %{conn: conn} do conn = post( @@ -486,6 +502,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do assert rendered == "" end + @tag :flaky test "sets date to today if it's in the future", %{conn: conn} do current_year = DateTime.utc_now().year m = DateTime.utc_now().month @@ -526,6 +543,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do ) end + @tag :flaky test "submits the date as is because it's not in the future", %{conn: conn} do current_year = DateTime.utc_now().year m = DateTime.utc_now().month @@ -566,6 +584,7 @@ defmodule DotcomWeb.CustomerSupportControllerTest do end describe "Date and time selector" do + @tag :flaky test "renders a date and time selector", %{conn: conn} do conn = get(conn, customer_support_path(conn, :index)) rendered = html_response(conn, 200) diff --git a/test/dotcom_web/controllers/schedule/line/helpers_test.exs b/test/dotcom_web/controllers/schedule/line/helpers_test.exs index e3accce5db..6fff6d31df 100644 --- a/test/dotcom_web/controllers/schedule/line/helpers_test.exs +++ b/test/dotcom_web/controllers/schedule/line/helpers_test.exs @@ -822,6 +822,7 @@ defmodule DotcomWeb.ScheduleController.Line.HelpersTest do end describe "get_branches/4" do + @tag :flaky test "returns a list of RouteStops, one for each branch of the line" do stub(Routes.Repo.Mock, :by_stop, fn _, _ -> Factories.Routes.Route.build_list(2, :route) end) diff --git a/test/dotcom_web/controllers/schedule/predictions_test.exs b/test/dotcom_web/controllers/schedule/predictions_test.exs index af75dc8f21..85fe683771 100644 --- a/test/dotcom_web/controllers/schedule/predictions_test.exs +++ b/test/dotcom_web/controllers/schedule/predictions_test.exs @@ -218,6 +218,7 @@ defmodule DotcomWeb.ScheduleController.PredictionsTest do assert conn.assigns[:predictions] == [] end + @tag :flaky test "assigns a list containing predictions for every stop with a vehicle at it", %{ conn: conn } do @@ -271,6 +272,7 @@ defmodule DotcomWeb.ScheduleController.PredictionsTest do ] end + @tag :flaky test "does not make duplicate requests for vehicles at the same stop", %{conn: conn} do stop_id_1 = Faker.Pokemon.location() route_id = "#{Faker.Util.digit()}" diff --git a/test/dotcom_web/controllers/transit_near_me_controller_test.exs b/test/dotcom_web/controllers/transit_near_me_controller_test.exs index 1e4237a034..3af109bf4d 100644 --- a/test/dotcom_web/controllers/transit_near_me_controller_test.exs +++ b/test/dotcom_web/controllers/transit_near_me_controller_test.exs @@ -88,6 +88,7 @@ defmodule DotcomWeb.TransitNearMeControllerTest do setup :verify_on_exit! describe "with no location params" do + @tag :flaky test "does not attempt to calculate stops with routes", %{conn: conn} do conn = get(conn, transit_near_me_path(conn, :index)) assert conn.status == 200 @@ -146,6 +147,7 @@ defmodule DotcomWeb.TransitNearMeControllerTest do assert conn.assigns.flash == %{} end + @tag :flaky test "flashes an error if location has no stops nearby", %{conn: conn} do expect(LocationService.Mock, :geocode, fn address -> assert address == @address diff --git a/test/dotcom_web/controllers/trip_plan/feedback_test.exs b/test/dotcom_web/controllers/trip_plan/feedback_test.exs index 751c1d666e..ff0ebf79d0 100644 --- a/test/dotcom_web/controllers/trip_plan/feedback_test.exs +++ b/test/dotcom_web/controllers/trip_plan/feedback_test.exs @@ -42,6 +42,7 @@ defmodule DotcomWeb.TripPlan.FeedbackTest do end describe "put/2" do + @tag :flaky test "returns 202 status and caches the data", %{conn: conn, cache: cache} do refute cache.get(@expected_cache_key) conn = Feedback.put(conn, @arbitrary_data) diff --git a/test/feedback/repo_test.exs b/test/feedback/repo_test.exs index 6ee9a5afb3..f8c19136a0 100644 --- a/test/feedback/repo_test.exs +++ b/test/feedback/repo_test.exs @@ -31,6 +31,7 @@ defmodule Feedback.RepoTest do set_log_level(:info) end + @tag :flaky test "returns ok and logs success" do expect(AwsClient.Mock, :send_raw_email, fn message -> assert message["RawMessage"]["Data"] diff --git a/test/routes/populate_caches_test.exs b/test/routes/populate_caches_test.exs index 50161bbe90..38fd818553 100644 --- a/test/routes/populate_caches_test.exs +++ b/test/routes/populate_caches_test.exs @@ -42,6 +42,7 @@ defmodule Routes.PopulateCachesTest do end describe "handle_info/2" do + @tag :flaky test "populate_all: gets headsigns and shapes for each route" do assert {:noreply, FakeRepo} = handle_info(:populate_all, FakeRepo) diff --git a/test/test_helper.exs b/test/test_helper.exs index 0665952887..7e2f5527d0 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -7,7 +7,7 @@ Application.ensure_all_started(:ex_machina) Application.ensure_all_started(:mox) Application.ensure_all_started(:tzdata) -ExUnit.configure(exclude: [external: true]) +ExUnit.configure(exclude: [external: true, flaky: true]) ExUnit.configure(formatters: [ExUnit.CLIFormatter, ExUnitSummary.Formatter]) ExUnit.start(capture_log: true) diff --git a/test/util/util_test.exs b/test/util/util_test.exs index 36f7ca9711..b6a3378ff8 100644 --- a/test/util/util_test.exs +++ b/test/util/util_test.exs @@ -376,6 +376,7 @@ defmodule UtilTest do assert log =~ "Async task timed out" end + @tag :flaky test "retries request according to param, then returns the default for timeouts" do set_retries = 2