Skip to content

Commit

Permalink
Fix: use assert_redirect with default timeout
Browse files Browse the repository at this point in the history
What changed?
=============

Whenever our LiveViewWatcher tells our test that the LiveView process
died. We try to perform an `assert_redirect` (since a redirect is our
best guess -- otherwise, the process should've just timed out).

The problem is that we're asserting that a redirect happened in the past
(i.e. we're passing a timeout of `0`). We're doing that in an effort to
keep the `timeout` provided by the user as close to the timeout
requested as possible.

But there's a possible race condition (which I've seen in the wild)
where the LiveView dies and redirects very fast (before the watcher can
actually set the timeout), but the watcher informs the test that the
LiveView died and we check for a redirect before the redirect message is
sent. Thus, the `0` timeout in `assert_redirect` actually fails due to a
race condition.

So, we now avoid specifying the timeout in `assert_redirect`, relying on
the default (which is 100ms or whatever ExUnit timeout is configured),
to try to avoid the race condition. That causes a potential longer
timeout for some test runs, but hopefully avoids flakiness due to race
conditions.
  • Loading branch information
germsvel committed Jan 16, 2025
1 parent c9f96c1 commit 8984387
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/phoenix_test/live_view_timeout.ex
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ defmodule PhoenixTest.LiveViewTimeout do
end

defp check_for_redirect(session, action) when is_function(action) do
{path, flash} = Phoenix.LiveViewTest.assert_redirect(session.view, 0)
{path, flash} = Phoenix.LiveViewTest.assert_redirect(session.view)

session
|> PhoenixTest.Live.handle_redirect({:redirect, %{to: path, flash: flash}})
Expand Down

0 comments on commit 8984387

Please sign in to comment.