fix(runtime): don't call on_initialized after init

fix(runtime): don't crash if compile is called before ready
test(runtime): rgroup related tests in describe blocks

Author: zachallaun

lib/next_ls/runtime.ex

@@ -136,8 +136,11 @@ defmodule NextLS.Runtime do
           |> Path.join("monkey/_next_ls_private_compiler.ex")
           |> then(&:rpc.call(node, Code, :compile_file, [&1]))
           |> tap(fn
-            {:badrpc, error} -> NextLS.Logger.error(logger, "Bad RPC call: #{inspect(error)}")
-            _ -> :ok
+            {:badrpc, error} ->
+              NextLS.Logger.error(logger, "Bad RPC call to node #{node}: #{inspect(error)}")
+
+            _ ->
+              :ok
           end)
 
           :rpc.call(node, Code, :put_compiler_option, [:parser_options, [columns: true, token_metadata: true]])
@@ -190,7 +193,7 @@ defmodule NextLS.Runtime do
       Task.Supervisor.async_nolink(state.task_supervisor, fn ->
         case :rpc.call(node, :_next_ls_private_compiler, :compile, []) do
           {:badrpc, error} ->
-            NextLS.Logger.error(state.logger, "Bad RPC call: #{inspect(error)}")
+            NextLS.Logger.error(state.logger, "Bad RPC call to node #{node}: #{inspect(error)}")
             []
 
           {_, diagnostics} when is_list(diagnostics) ->
@@ -209,6 +212,10 @@ defmodule NextLS.Runtime do
     {:noreply, %{state | compiler_ref: %{task.ref => from}}}
   end
 
+  def handle_call(:compile, _from, state) do
+    {:reply, {:error, :not_ready}, state}
+  end
+
   @impl GenServer
   def handle_info({ref, errors}, %{compiler_ref: compiler_ref} = state) when is_map_key(compiler_ref, ref) do
     Process.demonitor(ref, [:flush])
@@ -225,7 +232,11 @@ defmodule NextLS.Runtime do
 
   def handle_info({:DOWN, _, :port, port, reason}, %{port: port} = state) do
     error = {:port_down, reason}
-    state.on_initialized.({:error, error})
+
+    unless is_map_key(state, :node) do
+      state.on_initialized.({:error, error})
+    end
+
     {:stop, {:shutdown, error}, state}
   end
 

test/next_ls/runtime_test.exs

@@ -42,37 +42,122 @@ defmodule NextLs.RuntimeTest do
     [logger: logger, cwd: Path.absname(tmp_dir), on_init: on_init]
   end
 
-  test "returns the response in an ok tuple", %{logger: logger, cwd: cwd, on_init: on_init} do
-    start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
-    tvisor = start_supervised!(Task.Supervisor)
+  describe "call/2" do
+    test "responds with an ok tuple", %{logger: logger, cwd: cwd, on_init: on_init} do
+      start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
+      tvisor = start_supervised!(Task.Supervisor)
+
+      pid =
+        start_supervised!(
+          {Runtime,
+           name: "my_proj",
+           on_initialized: on_init,
+           task_supervisor: tvisor,
+           working_dir: cwd,
+           uri: "file://#{cwd}",
+           parent: self(),
+           logger: logger,
+           db: :some_db,
+           registry: RuntimeTest.Registry}
+        )
+
+      Process.link(pid)
+
+      assert_receive :ready
+
+      assert {:ok, "\"hi\""} = Runtime.call(pid, {Kernel, :inspect, ["hi"]})
+    end
 
-    pid =
-      start_supervised!(
-        {Runtime,
-         name: "my_proj",
-         on_initialized: on_init,
-         task_supervisor: tvisor,
-         working_dir: cwd,
-         uri: "file://#{cwd}",
-         parent: self(),
-         logger: logger,
-         db: :some_db,
-         registry: RuntimeTest.Registry}
-      )
+    test "responds with an error when the runtime isn't ready",
+         %{logger: logger, cwd: cwd, on_init: on_init} do
+      start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
+
+      tvisor = start_supervised!(Task.Supervisor)
+
+      pid =
+        start_supervised!(
+          {Runtime,
+           task_supervisor: tvisor,
+           name: "my_proj",
+           on_initialized: on_init,
+           working_dir: cwd,
+           uri: "file://#{cwd}",
+           parent: self(),
+           logger: logger,
+           db: :some_db,
+           registry: RuntimeTest.Registry}
+        )
+
+      Process.link(pid)
+
+      assert {:error, :not_ready} = Runtime.call(pid, {IO, :puts, ["hi"]})
+    end
+  end
 
-    Process.link(pid)
+  describe "compile/1" do
+    test "compiles the project and returns diagnostics",
+         %{logger: logger, cwd: cwd, on_init: on_init} do
+      start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
+
+      tvisor = start_supervised!(Task.Supervisor)
+
+      pid =
+        start_link_supervised!(
+          {Runtime,
+           name: "my_proj",
+           on_initialized: on_init,
+           task_supervisor: tvisor,
+           working_dir: cwd,
+           uri: "file://#{cwd}",
+           parent: self(),
+           logger: logger,
+           db: :some_db,
+           registry: RuntimeTest.Registry}
+        )
+
+      assert_receive :ready
+
+      file = Path.join(cwd, "lib/bar.ex")
+
+      assert [
+               %Mix.Task.Compiler.Diagnostic{
+                 file: ^file,
+                 severity: :warning,
+                 message:
+                   "variable \"arg1\" is unused (if the variable is not meant to be used, prefix it with an underscore)",
+                 position: position,
+                 compiler_name: "Elixir",
+                 details: nil
+               }
+             ] = Runtime.compile(pid)
+
+      if Version.match?(System.version(), ">= 1.15.0") do
+        assert position == {4, 11}
+      else
+        assert position == 4
+      end
 
-    assert_receive :ready
+      File.write!(file, """
+      defmodule Bar do
+        def foo(arg1) do
+          arg1
+        end
+      end
+      """)
 
-    assert {:ok, "\"hi\""} = Runtime.call(pid, {Kernel, :inspect, ["hi"]})
-  end
+      assert [] == Runtime.compile(pid)
+    end
+
+    test "emits errors when runtime compilation fails",
+         %{tmp_dir: tmp_dir, logger: logger, cwd: cwd, on_init: on_init} do
+      # obvious syntax error
+      bad_mix_exs = String.replace(mix_exs(), "defmodule", "")
+      File.write!(Path.join(tmp_dir, "mix.exs"), bad_mix_exs)
 
-  test "call returns an error when the runtime is not ready", %{logger: logger, cwd: cwd, on_init: on_init} do
-    start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
+      start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
 
-    tvisor = start_supervised!(Task.Supervisor)
+      tvisor = start_supervised!(Task.Supervisor)
 
-    pid =
       start_supervised!(
         {Runtime,
          task_supervisor: tvisor,
@@ -83,95 +168,42 @@ defmodule NextLs.RuntimeTest do
          parent: self(),
          logger: logger,
          db: :some_db,
-         registry: RuntimeTest.Registry}
+         registry: RuntimeTest.Registry},
+        restart: :temporary
       )
 
-    Process.link(pid)
-
-    assert {:error, :not_ready} = Runtime.call(pid, {IO, :puts, ["hi"]})
-  end
-
-  test "compiles the code and returns diagnostics", %{logger: logger, cwd: cwd, on_init: on_init} do
-    start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
+      assert_receive {:error, {:port_down, :normal}}
 
-    tvisor = start_supervised!(Task.Supervisor)
+      assert_receive {:log, :log, log_msg}
+      assert log_msg =~ "syntax error"
 
-    pid =
-      start_link_supervised!(
-        {Runtime,
-         name: "my_proj",
-         on_initialized: on_init,
-         task_supervisor: tvisor,
-         working_dir: cwd,
-         uri: "file://#{cwd}",
-         parent: self(),
-         logger: logger,
-         db: :some_db,
-         registry: RuntimeTest.Registry}
-      )
-
-    assert_receive :ready
-
-    file = Path.join(cwd, "lib/bar.ex")
-
-    assert [
-             %Mix.Task.Compiler.Diagnostic{
-               file: ^file,
-               severity: :warning,
-               message:
-                 "variable \"arg1\" is unused (if the variable is not meant to be used, prefix it with an underscore)",
-               position: position,
-               compiler_name: "Elixir",
-               details: nil
-             }
-           ] = Runtime.compile(pid)
-
-    if Version.match?(System.version(), ">= 1.15.0") do
-      assert position == {4, 11}
-    else
-      assert position == 4
+      assert_receive {:log, :error, error_msg}
+      assert error_msg =~ "{:shutdown, {:port_down, :normal}}"
     end
 
-    File.write!(file, """
-    defmodule Bar do
-      def foo(arg1) do
-        arg1
-      end
+    test "responds with an error when the runtime isn't ready",
+         %{logger: logger, cwd: cwd, on_init: on_init} do
+      start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
+
+      tvisor = start_supervised!(Task.Supervisor)
+
+      pid =
+        start_supervised!(
+          {Runtime,
+           task_supervisor: tvisor,
+           name: "my_proj",
+           on_initialized: on_init,
+           working_dir: cwd,
+           uri: "file://#{cwd}",
+           parent: self(),
+           logger: logger,
+           db: :some_db,
+           registry: RuntimeTest.Registry}
+        )
+
+      Process.link(pid)
+
+      assert {:error, :not_ready} = Runtime.compile(pid)
     end
-    """)
-
-    assert [] == Runtime.compile(pid)
-  end
-
-  test "emits errors when runtime compilation fails", %{tmp_dir: tmp_dir, logger: logger, cwd: cwd, on_init: on_init} do
-    # obvious syntax error
-    bad_mix_exs = String.replace(mix_exs(), "defmodule", "")
-    File.write!(Path.join(tmp_dir, "mix.exs"), bad_mix_exs)
-
-    start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
-
-    tvisor = start_supervised!(Task.Supervisor)
-
-    start_supervised!(
-      {Runtime,
-       task_supervisor: tvisor,
-       name: "my_proj",
-       on_initialized: on_init,
-       working_dir: cwd,
-       uri: "file://#{cwd}",
-       parent: self(),
-       logger: logger,
-       db: :some_db,
-       registry: RuntimeTest.Registry},
-      restart: :temporary
-    )
-
-    assert_receive {:error, {:port_down, :normal}}
-
-    assert_receive {:log, :log, log_msg}
-    assert log_msg =~ "syntax error"
-
-    assert_receive {:log, :error, error_msg}
-    assert error_msg =~ "{:shutdown, {:port_down, :normal}}"
   end
 end