From 4d70e5cceb1fdf78eab21bc9a0e46a7605338b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Sat, 29 Jan 2022 16:39:41 +0100 Subject: [PATCH] Test and typing improvements (#949) * Minimize race condition in the frame update test * Use defmacrop for building intellisense context * Remove unnecessary cell view computation * Fix nested assets resolution * Fix typing errors * Add missing async attribute to test suites * Improve rendering synchronization * Up --- lib/livebook/application.ex | 1 + lib/livebook/notebook.ex | 6 ++--- lib/livebook/session.ex | 2 +- lib/livebook/session/data.ex | 4 ++-- .../controllers/session_controller.ex | 2 +- lib/livebook_web/live/home_live.ex | 2 +- lib/livebook_web/live/session_live.ex | 19 +++------------- test/livebook/intellisense_test.exs | 22 ++++++------------- test/livebook/sessions_test.exs | 2 +- test/livebook_web/live/explore_live_test.exs | 2 +- test/livebook_web/live/home_live_test.exs | 4 ++-- test/livebook_web/live/session_live_test.exs | 12 ++++++---- 12 files changed, 31 insertions(+), 47 deletions(-) diff --git a/lib/livebook/application.ex b/lib/livebook/application.ex index 5c7a2d614..e1f7ab637 100644 --- a/lib/livebook/application.ex +++ b/lib/livebook/application.ex @@ -119,6 +119,7 @@ defmodule Livebook.Application do end end + @spec invalid_hostname!(String.t()) :: no_return() defp invalid_hostname!(prelude) do Livebook.Config.abort!(""" #{prelude}, which indicates something wrong in your OS configuration. diff --git a/lib/livebook/notebook.ex b/lib/livebook/notebook.ex index 4ddf3e7e1..e8ce1c0f7 100644 --- a/lib/livebook/notebook.ex +++ b/lib/livebook/notebook.ex @@ -226,7 +226,7 @@ defmodule Livebook.Notebook do @doc """ Updates cells as `update_cells/2`, but carries an accumulator. """ - @spec update_reduce_cells(t(), acc, ({Cell.t(), acc} -> {Cell.t(), acc})) :: t() + @spec update_reduce_cells(t(), acc, (Cell.t(), acc -> {Cell.t(), acc})) :: {t(), acc} when acc: term() def update_reduce_cells(notebook, acc, fun) do {sections, acc} = @@ -441,8 +441,8 @@ defmodule Livebook.Notebook do The cells are not ordered in any secific way. """ @spec cell_ids_with_children(t(), list(Cell.id())) :: list(Cell.id()) - def cell_ids_with_children(data, parent_cell_ids) do - graph = cell_dependency_graph(data.notebook) + def cell_ids_with_children(notebook, parent_cell_ids) do + graph = cell_dependency_graph(notebook) for parent_id <- parent_cell_ids, leaf_id <- Graph.leaves(graph), diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 952110d6c..e23e84eac 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -1266,7 +1266,7 @@ defmodule Livebook.Session do end defp extract_archive!(binary, path) do - :ok = :erl_tar.extract({:binary, binary}, [:compressed, {:cwd, path}]) + :ok = :erl_tar.extract({:binary, binary}, [:compressed, {:cwd, String.to_charlist(path)}]) end @doc """ diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 37d784913..4cf015685 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -847,7 +847,7 @@ defmodule Livebook.Session.Data do |> set!( notebook: Notebook.add_cell_output(data.notebook, cell.id, output), input_values: - {-1, output} + {0, output} |> Cell.Elixir.find_inputs_in_output() |> Map.new(fn attrs -> {attrs.id, attrs.default} end) |> Map.merge(data.input_values) @@ -1521,7 +1521,7 @@ defmodule Livebook.Session.Data do uniq: true, do: cell.id - cell_ids = Notebook.cell_ids_with_children(data, evaluable_cell_ids) + cell_ids = Notebook.cell_ids_with_children(data.notebook, evaluable_cell_ids) for {cell, _} <- elixir_cells_with_section, cell.id in cell_ids, diff --git a/lib/livebook_web/controllers/session_controller.ex b/lib/livebook_web/controllers/session_controller.ex index 7eeb65ea8..f1f7049c7 100644 --- a/lib/livebook_web/controllers/session_controller.ex +++ b/lib/livebook_web/controllers/session_controller.ex @@ -114,7 +114,7 @@ defmodule LivebookWeb.SessionController do # The request comes from a cross-origin iframe conn = allow_cors(conn) - case lookup_asset(hash, file_parts) do + case lookup_asset(hash, asset_path) do {:ok, local_asset_path} -> conn |> put_content_type(asset_path) diff --git a/lib/livebook_web/live/home_live.ex b/lib/livebook_web/live/home_live.ex index 9e0c4a7f1..3a06b2b17 100644 --- a/lib/livebook_web/live/home_live.ex +++ b/lib/livebook_web/live/home_live.ex @@ -85,7 +85,7 @@ defmodule LivebookWeb.HomeLive do -
+

Explore diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 3c3d98b51..314ed2e2c 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -3,7 +3,7 @@ defmodule LivebookWeb.SessionLive do import LivebookWeb.UserHelpers import LivebookWeb.SessionHelpers - import Livebook.Utils, only: [access_by_id: 1, format_bytes: 1] + import Livebook.Utils, only: [format_bytes: 1] alias LivebookWeb.SidebarHelpers alias Livebook.{Sessions, Session, Delta, Notebook, Runtime, LiveMarkdown} @@ -1453,10 +1453,8 @@ defmodule LivebookWeb.SessionLive do {:report_cell_revision, _pid, _cell_id, _revision} -> data_view - {:apply_cell_delta, _pid, cell_id, _delta, _revision} -> - data_view - |> update_cell_view(data, cell_id) - |> update_dirty_status(data) + {:apply_cell_delta, _pid, _cell_id, _delta, _revision} -> + update_dirty_status(data_view, data) # For outputs that update existing outputs we send the update directly # to the corresponding component, so the DOM patch is isolated and fast. @@ -1490,17 +1488,6 @@ defmodule LivebookWeb.SessionLive do end end - defp update_cell_view(data_view, data, cell_id) do - {:ok, cell, section} = Notebook.fetch_cell_and_section(data.notebook, cell_id) - cell_view = cell_to_view(cell, data) - - put_in( - data_view, - [:section_views, access_by_id(section.id), :cell_views, access_by_id(cell.id)], - cell_view - ) - end - defp prune_outputs(%{private: %{data: data}} = socket) do assign_private( socket, diff --git a/test/livebook/intellisense_test.exs b/test/livebook/intellisense_test.exs index d50533268..9251346c6 100644 --- a/test/livebook/intellisense_test.exs +++ b/test/livebook/intellisense_test.exs @@ -1,11 +1,11 @@ -defmodule Livebook.IntellisenseTest.Utils do - @moduledoc false +defmodule Livebook.IntellisenseTest do + use ExUnit.Case, async: true - @doc """ - Returns intellisense context resulting from evaluating - the given block of code in a fresh context. - """ - defmacro eval(do: block) do + alias Livebook.Intellisense + + # Returns intellisense context resulting from evaluating + # the given block of code in a fresh context. + defmacrop eval(do: block) do quote do block = unquote(Macro.escape(block)) binding = [] @@ -21,14 +21,6 @@ defmodule Livebook.IntellisenseTest.Utils do } end end -end - -defmodule Livebook.IntellisenseTest do - use ExUnit.Case, async: true - - import Livebook.IntellisenseTest.Utils - - alias Livebook.Intellisense describe "format_code/1" do test "formats valid code" do diff --git a/test/livebook/sessions_test.exs b/test/livebook/sessions_test.exs index b58e9a594..ac8777f84 100644 --- a/test/livebook/sessions_test.exs +++ b/test/livebook/sessions_test.exs @@ -1,5 +1,5 @@ defmodule Livebook.SessionsTest do - use ExUnit.Case + use ExUnit.Case, async: true alias Livebook.Sessions diff --git a/test/livebook_web/live/explore_live_test.exs b/test/livebook_web/live/explore_live_test.exs index e91c78006..88c03732f 100644 --- a/test/livebook_web/live/explore_live_test.exs +++ b/test/livebook_web/live/explore_live_test.exs @@ -1,5 +1,5 @@ defmodule LivebookWeb.ExploreLiveTest do - use LivebookWeb.ConnCase + use LivebookWeb.ConnCase, async: true import Phoenix.LiveViewTest diff --git a/test/livebook_web/live/home_live_test.exs b/test/livebook_web/live/home_live_test.exs index f34abb386..d68fd5c8a 100644 --- a/test/livebook_web/live/home_live_test.exs +++ b/test/livebook_web/live/home_live_test.exs @@ -1,5 +1,5 @@ defmodule LivebookWeb.HomeLiveTest do - use LivebookWeb.ConnCase + use LivebookWeb.ConnCase, async: true import Phoenix.LiveViewTest @@ -206,7 +206,7 @@ defmodule LivebookWeb.HomeLiveTest do assert {:error, {:live_redirect, %{to: to}}} = view - |> element(~s{a}, "Welcome to Livebook") + |> element(~s{[data-element="explore-section"] a}, "Welcome to Livebook") |> render_click() |> follow_redirect(conn) diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index 7ea8836ae..143fb8141 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -1,5 +1,5 @@ defmodule LivebookWeb.SessionLiveTest do - use LivebookWeb.ConnCase + use LivebookWeb.ConnCase, async: true import Phoenix.LiveViewTest @@ -37,7 +37,7 @@ defmodule LivebookWeb.SessionLiveTest do wait_for_session_update(session.pid) # Wait for LV to update - render(view) + _ = render(view) assert page_title(view) =~ "My notebook" end @@ -319,8 +319,12 @@ defmodule LivebookWeb.SessionLiveTest do wait_for_session_update(session.pid) - assert render(view) =~ "Updated frame" - refute render(view) =~ "In frame" + # Render once, so that frame send_update is processed + _ = render(view) + + content = render(view) + assert content =~ "Updated frame" + refute content =~ "In frame" end end