Fix intermittent test failures (#596)

* Bump Elixir on the CI

* Start permanent node manager in tests

* Adjust timeouts

* Make sure random node cookies are alphanumeric

.github/workflows/deploy.yaml

@@ -14,7 +14,7 @@ jobs:
         uses: erlef/setup-beam@v1
         with:
           otp-version: '24.0'
-          elixir-version: '1.12.0'
+          elixir-version: '1.12.3'
       - name: Cache Mix
         uses: actions/cache@v2
         with:

.github/workflows/test.yaml

@@ -15,7 +15,7 @@ jobs:
         uses: erlef/setup-beam@v1
         with:
           otp-version: '24.0'
-          elixir-version: '1.12.0'
+          elixir-version: '1.12.3'
       - name: Cache Mix
         uses: actions/cache@v2
         with:

lib/livebook/runtime/erl_dist/node_manager.ex

@@ -34,6 +34,9 @@ defmodule Livebook.Runtime.ErlDist.NodeManager do
       In most cases we enforce a single manager per node
       and identify it by a name, but this can be opted-out
       from by using this option. Defaults to `false`.
+
+    * `:auto_termination` - whether to terminate the manager
+      when the last runtime server terminates. Defaults to `true`.
   """
   def start(opts \\ []) do
     {opts, gen_opts} = split_opts(opts)
@@ -74,6 +77,7 @@ defmodule Livebook.Runtime.ErlDist.NodeManager do
   @impl true
   def init(opts) do
     unload_modules_on_termination = Keyword.get(opts, :unload_modules_on_termination, true)
+    auto_termination = Keyword.get(opts, :auto_termination, true)
 
     ## Initialize the node
 
@@ -97,6 +101,7 @@ defmodule Livebook.Runtime.ErlDist.NodeManager do
     {:ok,
      %{
        unload_modules_on_termination: unload_modules_on_termination,
+       auto_termination: auto_termination,
        server_supevisor: server_supevisor,
        runtime_servers: [],
        initial_ignore_module_conflict: initial_ignore_module_conflict,
@@ -124,7 +129,7 @@ defmodule Livebook.Runtime.ErlDist.NodeManager do
   def handle_info({:DOWN, _, :process, pid, _}, state) do
     if pid in state.runtime_servers do
       case update_in(state.runtime_servers, &List.delete(&1, pid)) do
-        %{runtime_servers: []} = state -> {:stop, :normal, state}
+        %{runtime_servers: [], auto_termination: true} = state -> {:stop, :normal, state}
         state -> {:noreply, state}
       end
     else

lib/livebook/utils.ex

@@ -24,7 +24,7 @@ defmodule Livebook.Utils do
   """
   @spec random_cookie() :: atom()
   def random_cookie() do
-    :crypto.strong_rand_bytes(42) |> Base.url_encode64() |> String.to_atom()
+    :crypto.strong_rand_bytes(35) |> Base.encode32(case: :lower) |> String.to_atom()
   end
 
   @doc """

test/livebook/evaluator_test.exs

@@ -109,7 +109,7 @@ defmodule Livebook.EvaluatorTest do
         assert_receive {:evaluation_response, :code_1,
                         {:error, _kind, _error, ^expected_stacktrace},
                         %{evaluation_time_ms: _time_ms}},
-                       1_000
+                       2_000
       end)
     end
 
@@ -160,12 +160,13 @@ defmodule Livebook.EvaluatorTest do
       assert_receive {:evaluation_response, :code_1, {:ok, widget_pid1},
                       %{evaluation_time_ms: _time_ms}}
 
+      ref = Process.monitor(widget_pid1)
+
       Evaluator.evaluate_code(evaluator, self(), spawn_widget_code(), :code_1)
 
       assert_receive {:evaluation_response, :code_1, {:ok, widget_pid2},
                       %{evaluation_time_ms: _time_ms}}
 
-      ref = Process.monitor(widget_pid1)
       assert_receive {:DOWN, ^ref, :process, ^widget_pid1, :shutdown}
 
       assert Process.alive?(widget_pid2)
@@ -212,9 +213,9 @@ defmodule Livebook.EvaluatorTest do
       assert_receive {:evaluation_response, :code_1, {:ok, widget_pid1},
                       %{evaluation_time_ms: _time_ms}}
 
+      ref = Process.monitor(widget_pid1)
       Evaluator.forget_evaluation(evaluator, :code_1)
 
-      ref = Process.monitor(widget_pid1)
       assert_receive {:DOWN, ^ref, :process, ^widget_pid1, :shutdown}
     end
   end
@@ -225,7 +226,7 @@ defmodule Livebook.EvaluatorTest do
       Evaluator.handle_intellisense(evaluator, self(), :ref, request)
 
       assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "version/0"}]}},
-                     1_000
+                     2_000
     end
 
     test "given evaluation reference uses its bindings and env", %{evaluator: evaluator} do
@@ -241,13 +242,13 @@ defmodule Livebook.EvaluatorTest do
       Evaluator.handle_intellisense(evaluator, self(), :ref, request, :code_1)
 
       assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "number"}]}},
-                     1_000
+                     2_000
 
       request = {:completion, "ANSI.brigh"}
       Evaluator.handle_intellisense(evaluator, self(), :ref, request, :code_1)
 
       assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "bright/0"}]}},
-                     1_000
+                     2_000
     end
   end
 

test/livebook/runtime/mix_standalone_test.exs

@@ -11,8 +11,8 @@ defmodule Livebook.Runtime.MixStandaloneTest do
 
     ref = emitter.ref
     # Wait for the Mix setup to finish and for node initialization
-    assert_receive {:emitter, ^ref, {:output, "Running mix deps.get...\n"}}, 10_000
+    assert_receive {:emitter, ^ref, {:output, "Running mix deps.get...\n"}}, 15_000
-    assert_receive {:emitter, ^ref, {:ok, runtime}}, 10_000
+    assert_receive {:emitter, ^ref, {:ok, runtime}}, 15_000
 
     Runtime.connect(runtime)
     %{node: node} = runtime

test/livebook/session_test.exs

@@ -4,12 +4,6 @@ defmodule Livebook.SessionTest do
   alias Livebook.{Session, Delta, Runtime, Utils, Notebook, FileSystem}
   alias Livebook.Notebook.{Section, Cell}
 
-  # Note: queueing evaluation in most of the tests below
-  # requires the runtime to synchronously start first,
-  # so we use a longer timeout just to make sure the tests
-  # pass reliably
-  @evaluation_wait_timeout 3_000
-
   setup do
     session = start_session()
     %{session: session}
@@ -87,7 +81,8 @@ defmodule Livebook.SessionTest do
   end
 
   describe "queue_cell_evaluation/2" do
-    test "sends a queue evaluation operation to subscribers", %{session: session} do
+    test "triggers evaluation and sends update operation once it finishes",
+         %{session: session} do
       Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session.id}")
       pid = self()
 
@@ -95,22 +90,11 @@ defmodule Livebook.SessionTest do
 
       Session.queue_cell_evaluation(session.pid, cell_id)
 
-      assert_receive {:operation, {:queue_cell_evaluation, ^pid, ^cell_id}},
+      assert_receive {:operation, {:queue_cell_evaluation, ^pid, ^cell_id}}
-                     @evaluation_wait_timeout
-    end
-
-    test "triggers evaluation and sends update operation once it finishes",
-         %{session: session} do
-      Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session.id}")
-
-      {_section_id, cell_id} = insert_section_and_cell(session.pid)
-
-      Session.queue_cell_evaluation(session.pid, cell_id)
 
       assert_receive {:operation,
                       {:add_cell_evaluation_response, _, ^cell_id, _,
-                       %{evaluation_time_ms: _time_ms}}},
+                       %{evaluation_time_ms: _time_ms}}}
-                     @evaluation_wait_timeout
     end
   end
 
@@ -124,8 +108,7 @@ defmodule Livebook.SessionTest do
 
       Session.cancel_cell_evaluation(session.pid, cell_id)
 
-      assert_receive {:operation, {:cancel_cell_evaluation, ^pid, ^cell_id}},
+      assert_receive {:operation, {:cancel_cell_evaluation, ^pid, ^cell_id}}
-                     @evaluation_wait_timeout
     end
   end
 
@@ -258,7 +241,7 @@ defmodule Livebook.SessionTest do
       Session.set_file(session.pid, file)
 
       # Wait for the session to deal with the files
-      Process.sleep(50)
+      Process.sleep(100)
 
       assert {:ok, true} =
                FileSystem.File.exists?(FileSystem.File.resolve(tmp_dir, "images/test.jpg"))
@@ -280,7 +263,7 @@ defmodule Livebook.SessionTest do
       Session.set_file(session.pid, nil)
 
       # Wait for the session to deal with the files
-      Process.sleep(50)
+      Process.sleep(200)
 
       assert {:ok, true} = FileSystem.File.exists?(image_file)
 
@@ -419,8 +402,7 @@ defmodule Livebook.SessionTest do
     # Give it a bit more time as this involves starting a system process.
     assert_receive {:operation,
                     {:add_cell_evaluation_response, _, ^cell_id, _,
-                     %{evaluation_time_ms: _time_ms}}},
+                     %{evaluation_time_ms: _time_ms}}}
-                   @evaluation_wait_timeout
   end
 
   test "if the runtime node goes down, notifies the subscribers" do
@@ -481,8 +463,7 @@ defmodule Livebook.SessionTest do
 
       assert_receive {:operation,
                       {:add_cell_evaluation_response, _, ^cell_id, {:text, text_output},
-                       %{evaluation_time_ms: _time_ms}}},
+                       %{evaluation_time_ms: _time_ms}}}
-                     @evaluation_wait_timeout
 
       assert text_output =~ "Jake Peralta"
     end
@@ -512,7 +493,7 @@ defmodule Livebook.SessionTest do
       assert_receive {:operation,
                       {:add_cell_evaluation_response, _, ^cell_id, {:text, text_output},
                        %{evaluation_time_ms: _time_ms}}},
-                     @evaluation_wait_timeout
+                     2_000
 
       assert text_output =~ "no matching Livebook input found"
     end
@@ -544,7 +525,7 @@ defmodule Livebook.SessionTest do
       assert_receive {:operation,
                       {:add_cell_evaluation_response, _, ^cell_id, {:text, text_output},
                        %{evaluation_time_ms: _time_ms}}},
-                     @evaluation_wait_timeout
+                     2_000
 
       assert text_output =~ "no matching Livebook input found"
     end
@@ -623,7 +604,7 @@ defmodule Livebook.SessionTest do
   end
 
   test "session created_at is before now", %{session: session} do
-    assert session.created_at < DateTime.utc_now()
+    assert DateTime.compare(session.created_at, DateTime.utc_now()) == :lt
   end
 
   defp start_session(opts \\ []) do

test/livebook_web/live/session_live_test.exs

@@ -96,7 +96,7 @@ defmodule LivebookWeb.SessionLiveTest do
 
     test "queueing cell evaluation", %{conn: conn, session: session} do
       section_id = insert_section(session.pid)
-      cell_id = insert_text_cell(session.pid, section_id, :elixir, "Process.sleep(10)")
+      cell_id = insert_text_cell(session.pid, section_id, :elixir, "Process.sleep(50)")
 
       {:ok, view, _} = live(conn, "/sessions/#{session.id}")
 

test/test_helper.exs

@@ -1,6 +1,14 @@
+# Start manager on the current node and configure it not to
+# terminate automatically, so there is no race condition
+# when starting/stopping Embedded runtimes in parallel
+Livebook.Runtime.ErlDist.NodeManager.start(
+  auto_termination: false,
+  unload_modules_on_termination: false
+)
+
 erl_docs_available? = Code.fetch_docs(:gen_server) != {:error, :chunk_not_found}
 
 exclude = []
 exclude = if erl_docs_available?, do: exclude, else: Keyword.put(exclude, :erl_docs, true)
 
-ExUnit.start(assert_receive_timeout: 300, exclude: exclude)
+ExUnit.start(assert_receive_timeout: 1_000, exclude: exclude)