phoenix_live_view icon indicating copy to clipboard operation
phoenix_live_view copied to clipboard

LiveViewTest render_submit raises unexpectedly with invalid upload size

Open mcrumm opened this issue 3 years ago • 3 comments

Environment

  • Elixir version (elixir -v): Elixir 1.11.1 (compiled with Erlang/OTP 23)
  • Phoenix version (mix deps): 1.5.9
  • Phoenix LiveView version (mix deps): 0.15.7
  • NodeJS version (node -v): v12.11.0
  • NPM version (npm -v): 6.11.3
  • Operating system: MacOS
  • Browsers you attempted to reproduce this bug on (the more the merrier): n/a
  • Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: n/a

Actual behavior

If I provide an invalid size for an upload entry, a subsequent call to render_submit() will raise:

test "uploads", %{conn: conn} do
  {:ok, lv, _html} = live(conn, "/upload")

  # contrived, but you get the idea
  %{size: size} = File.stat!("/path/to/foo.png")
  file = %{name: "foo.png", size: size * 2}

  assert lv
         |> file_input("#form", :file, [file])
         |> render_upload("foo.png") =~ "100%"
  
  assert lv
         |> form("#form")
         |> render_submit() =~ "upload complete"
end
1) test handle bad size (DropsWeb.BasicUploadsLiveTest)
     test/drops_web/live/basic_uploads_live_test.exs:61
     ** (EXIT from #PID<0.414.0>) an exception was raised:
         ** (RuntimeError) cannot consume uploaded file that is still in progress
             (phoenix_live_view 0.15.7) lib/phoenix_live_view/upload_channel.ex:27: Phoenix.LiveView.UploadChannel.consume/3
             (elixir 1.11.1) lib/enum.ex:1399: Enum."-map/2-lists^map/1-0-"/2
             (drops 0.1.0) lib/drops_web/live/basic_uploads_live.ex:29: DropsWeb.BasicUploadsLive.handle_event/3
             (phoenix_live_view 0.15.7) lib/phoenix_live_view/channel.ex:342: anonymous fn/3 in Phoenix.LiveView.Channel.view_handle_event/3
             (telemetry 0.4.3) /Users/mcrumm/Code/mcrumm/live_upload_example/deps/telemetry/src/telemetry.erl:272: :telemetry.span/3
             (phoenix_live_view 0.15.7) lib/phoenix_live_view/channel.ex:204: Phoenix.LiveView.Channel.handle_info/2
             (stdlib 3.13.2) gen_server.erl:680: :gen_server.try_dispatch/4
             (stdlib 3.13.2) gen_server.erl:756: :gen_server.handle_msg/6
             (stdlib 3.13.2) proc_lib.erl:226: :proc_lib.init_p_do_apply/3

Expected behavior

Under test, we calculate the real byte_size of the given content so we can simulate the chunk behaviour. If the real size differs from the given size, I propose we raise an error letting the user know that the sizes differ, as this likely indicates a problem in the user's test, and the current error does not make that clear.

// @maennchen

mcrumm avatar May 25 '21 17:05 mcrumm

Note that if you pass content we will by default set the size as the byte_size of the content, so you will most always avoid passing the size yourself. I think it it's sane to raise but I wanted to highlight how the :size option is likely not needed unless testing a bad client.

chrismccord avatar May 25 '21 18:05 chrismccord

Ah, that makes more sense :)

mcrumm avatar May 25 '21 18:05 mcrumm

@chrismccord So it would be preferred to leave out the size by default?

If yes, I think we should mention that in the docs: https://hexdocs.pm/phoenix_live_view/Phoenix.LiveViewTest.html#file_input/4

maennchen avatar May 26 '21 09:05 maennchen

Closed via bee186811dbb5b24d5ee74af5209925b33b6364e

chrismccord avatar Jan 19 '24 20:01 chrismccord