req icon indicating copy to clipboard operation
req copied to clipboard

Stubbing a Req.Request in a Phoenix.LiveView.assign_async

Open davydog187 opened this issue 1 year ago • 6 comments

Disclaimer: I will note that this is an "advanced" use case which may not be solvable, but I wanted to have a discussion first.

Hello @wojtekmach, thank you for your amazing library, I love Req!

This issue is to determine if there's a possible solution for testing a Req.Request inside of async LiveView assign. Imagine the following live view (btw I can totally appreciate that this might need a change in LiveView but I'll start here if thats ok)

defmodule MyApp.Live do
  use Phoenix.LiveView
  
  def mount(_, _, _) do
    {:ok, assign_async(socket, :foo, fn -> {:ok, Req.get!("http://something.com").body} end)}
  end
  
  def render(assigns) do
    ~H"""
    <.async_result :let={foo} assign={@foo}>
      <%= inspect(foo) %>
    </.async_result>
    """
  end
end

The problem here is that in order to test this LiveView and stub out the call to Req.get!, there is a race condition, as the request to the LiveView will have already been made before you get the pid back that you can call Req.Test.allow/3` against.

e.g.

test "assync_assign with a Req.Request", %{conn: conn} do
  {:ok, live, _html} = live(conn, ~p"/my-route")
  
  # Race condition as `Req.get!` may have already run
  Req.Test.allow(MyApp.Req, self(), live.pid)
  Req.Test.stub(MyApp.Req, fn conn -> Req.Test.json(conn, %{}) end)
end

Of course, this could be solved with some sort of conditional that uses environment config, or a mocking library like Mox or Mimic.

However, I'd like to imagine there could be another way to propagate ownership so that the allow could be called prior to the LiveView creation. This may require changes to LiveView, it might be impossible, or it might not be something you want to support.

Thanks for the consideration and feel free to close if this is something you are not interested in supporting

davydog187 avatar Nov 09 '24 00:11 davydog187

I suppose this generalizes to GenServers that call Req in init/1 or a handle_continue/2 from init, so the mention of LV is maybe not necessary

davydog187 avatar Nov 09 '24 00:11 davydog187

Mox allows you to pass a function to allow in case you need to delay evaluation of the allowed pid. Could something like this work?

parent = self()

Req.Test.allow(MyApp.Req, self(), fn ->
  send(parent, {:allow, self()})
  receive(do: (pid -> pid))
end)

{:ok, live, _html} = live(conn, ...)

assert_receive {:allow, pid}
send(pid, live.pid)

This is a little clunky. It means that, if a function is passed, it needs to be run in its own process to prevent deadlocks, but perhaps a pattern like this could work? It could be wrapped in a helper to make it a bit more ergonomic.

zachallaun avatar Nov 09 '24 01:11 zachallaun

Another solution would use telemetry events to hook into the process

davydog187 avatar Nov 09 '24 11:11 davydog187

Yeah speaking of telemetry events, we have this Broadway guide: https://hexdocs.pm/req/Req.Test.html#module-broadway

I think it is fine when people do that in their own GenServers and Broadway pipelines which I’d considered intermediate level but this is too much for people using LV so we should definitely improve this. Since Ecto sandbox works in LV we should have infrastructure in place to make that happen. Or it does in LV but not in async assigns? In any case, yes, this is definitely a priority and any help is appreciated.

wojtekmach avatar Nov 09 '24 16:11 wojtekmach

Wait, this seems to work already? https://gist.github.com/wojtekmach/2ee023315dd6ea209d67b6d3ef8cd985

Could you modify the snippet to show the bug?

wojtekmach avatar Nov 09 '24 17:11 wojtekmach

For me, the problem was that my stub was defined before the render_async but after the live call, like this:

defmodule DemoLiveTest do
  use ExUnit.Case
  use PhoenixPlayground.Test, live: DemoLive

  test "it works" do
    {:ok, view, _html} = live(build_conn(), "/")

    Req.Test.stub(:foo, fn conn ->
      Req.Test.json(conn, %{foo: :bar})
    end)

    assert render_async(view) =~ "bar"
  end
end

Simply moving the stub definition up, as in your snippet @wojtekmach, fixed the issue.

Munksgaard avatar Dec 18 '24 09:12 Munksgaard