From e1c6082e495ca0ec28022cb669e11ac4701cb2a4 Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Mon, 11 Dec 2023 17:22:07 -0800 Subject: [PATCH 1/5] Handle compilation exiting It is possible for compilation to exit, and we were not handling it properly. This was complicated by the fact that builds happen inside a GenServer, so wrapping things in a try/rescue with an exit handler wasn't enough. Instead, i launched another process that's monitored and awaited by the build genserver. Then any DOWN messages are turned into diagnostics, and reported. Fixes #504 --- .../lexical/remote_control/build/document.ex | 27 +++++++++++++- .../lexical/remote_control/build/project.ex | 37 ++++++++++++++++++- .../.formatter.exs | 4 ++ .../compilation_callback_errors/.gitignore | 26 +++++++++++++ .../lib/compile_callback_error.ex | 6 +++ .../compilation_callback_errors/mix.exs | 25 +++++++++++++ .../lexical/remote_control/build_test.exs | 36 ++++++++++++++++++ 7 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 apps/remote_control/test/fixtures/compilation_callback_errors/.formatter.exs create mode 100644 apps/remote_control/test/fixtures/compilation_callback_errors/.gitignore create mode 100644 apps/remote_control/test/fixtures/compilation_callback_errors/lib/compile_callback_error.ex create mode 100644 apps/remote_control/test/fixtures/compilation_callback_errors/mix.exs diff --git a/apps/remote_control/lib/lexical/remote_control/build/document.ex b/apps/remote_control/lib/lexical/remote_control/build/document.ex index d79be46a9..728e145d4 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/document.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/document.ex @@ -1,11 +1,36 @@ defmodule Lexical.RemoteControl.Build.Document do alias Lexical.Document + alias Lexical.RemoteControl.Build alias Lexical.RemoteControl.Build.Document.Compilers @compilers [Compilers.Config, Compilers.Elixir, Compilers.EEx, Compilers.HEEx, Compilers.NoOp] def compile(%Document{} = document) do compiler = Enum.find(@compilers, & &1.recognizes?(document)) - compiler.compile(document) + me = self() + + {pid, ref} = + spawn_monitor(fn -> + result = compiler.compile(document) + send(me, {:result, result}) + end) + + receive do + {:result, result} -> + flush_normal_down(ref, pid) + result + + {:DOWN, ^ref, :process, ^pid, {exception, stack}} -> + diagnostic = Build.Error.error_to_diagnostic(document, exception, stack, nil) + diagnostics = Build.Error.refine_diagnostics([diagnostic]) + {:error, diagnostics} + end + end + + defp flush_normal_down(ref, pid) do + receive do + {:DOWN, ^ref, :process, ^pid, :normal} -> + :ok + end end end diff --git a/apps/remote_control/lib/lexical/remote_control/build/project.ex b/apps/remote_control/lib/lexical/remote_control/build/project.ex index 399f78eb9..37a467e1b 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/project.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/project.ex @@ -3,6 +3,7 @@ defmodule Lexical.RemoteControl.Build.Project do alias Lexical.RemoteControl alias Lexical.RemoteControl.Build alias Lexical.RemoteControl.Plugin + alias Mix.Task.Compiler.Diagnostic use Build.Progress require Logger @@ -17,7 +18,7 @@ defmodule Lexical.RemoteControl.Build.Project do Mix.Task.clear() with_progress building_label(project), fn -> - result = Mix.Task.run(:compile, mix_compile_opts(force?)) + result = compile_in_monitor(force?) Mix.Task.run(:loadpaths) result end @@ -44,6 +45,40 @@ defmodule Lexical.RemoteControl.Build.Project do end) end + defp compile_in_monitor(force?) do + me = self() + + {pid, ref} = + spawn_monitor(fn -> + result = Mix.Task.run(:compile, mix_compile_opts(force?)) + send(me, {:result, result}) + end) + + receive do + {:result, result} -> + flush_normal_down(ref, pid) + result + + {:DOWN, ^ref, _, ^pid, {exception, [{_module, _function, _arity, meta} | _]}} -> + diagnostic = %Diagnostic{ + file: Keyword.get(meta, :file), + severity: :error, + message: Exception.message(exception), + compiler_name: "Elixir", + position: Keyword.get(meta, :line, 1) + } + + {:error, [diagnostic]} + end + end + + defp flush_normal_down(ref, pid) do + receive do + {:DOWN, ^ref, _, ^pid, :normal} -> + :ok + end + end + defp prepare_for_project_build(false = _force?) do :ok end diff --git a/apps/remote_control/test/fixtures/compilation_callback_errors/.formatter.exs b/apps/remote_control/test/fixtures/compilation_callback_errors/.formatter.exs new file mode 100644 index 000000000..d2cda26ed --- /dev/null +++ b/apps/remote_control/test/fixtures/compilation_callback_errors/.formatter.exs @@ -0,0 +1,4 @@ +# Used by "mix format" +[ + inputs: ["{mix,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}"] +] diff --git a/apps/remote_control/test/fixtures/compilation_callback_errors/.gitignore b/apps/remote_control/test/fixtures/compilation_callback_errors/.gitignore new file mode 100644 index 000000000..5b5619e04 --- /dev/null +++ b/apps/remote_control/test/fixtures/compilation_callback_errors/.gitignore @@ -0,0 +1,26 @@ +# The directory Mix will write compiled artifacts to. +/_build/ + +# If you run "mix test --cover", coverage assets end up here. +/cover/ + +# The directory Mix downloads your dependencies sources to. +/deps/ + +# Where third-party dependencies like ExDoc output generated docs. +/doc/ + +# Ignore .fetch files in case you like to edit your project deps locally. +/.fetch + +# If the VM crashes, it generates a dump, let's ignore it too. +erl_crash.dump + +# Also ignore archive artifacts (built via "mix archive.build"). +*.ez + +# Ignore package tarball (built via "mix hex.build"). +compilation_errors-*.tar + +# Temporary files, for example, from tests. +/tmp/ diff --git a/apps/remote_control/test/fixtures/compilation_callback_errors/lib/compile_callback_error.ex b/apps/remote_control/test/fixtures/compilation_callback_errors/lib/compile_callback_error.ex new file mode 100644 index 000000000..de7ffd508 --- /dev/null +++ b/apps/remote_control/test/fixtures/compilation_callback_errors/lib/compile_callback_error.ex @@ -0,0 +1,6 @@ +defmodule CompileCallbackError do + @after_verify __MODULE__ + def __after_verify__(_) do + raise "boom" + end +end diff --git a/apps/remote_control/test/fixtures/compilation_callback_errors/mix.exs b/apps/remote_control/test/fixtures/compilation_callback_errors/mix.exs new file mode 100644 index 000000000..bda846f67 --- /dev/null +++ b/apps/remote_control/test/fixtures/compilation_callback_errors/mix.exs @@ -0,0 +1,25 @@ +defmodule CompilationCallbackErrors.MixProject do + use Mix.Project + + def project do + Code.put_compiler_option(:ignore_module_conflict, true) + + [ + app: :compilation_callback_errors, + version: "0.1.0", + elixir: "~> 1.13", + start_permanent: Mix.env() == :prod, + deps: deps() + ] + end + + def application do + [ + extra_applications: [:logger] + ] + end + + defp deps do + [] + end +end diff --git a/apps/remote_control/test/lexical/remote_control/build_test.exs b/apps/remote_control/test/lexical/remote_control/build_test.exs index 648ea3410..715541bb0 100644 --- a/apps/remote_control/test/lexical/remote_control/build_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build_test.exs @@ -617,4 +617,40 @@ defmodule Lexical.BuildTest do assert loaded?(project, WithAType) end end + + describe "exceptions during compilation" do + test "compiling a project with callback errors" do + {:ok, project} = with_project(:compilation_callback_errors) + Build.schedule_compile(project) + assert_receive project_compiled(status: :error), 500 + assert_receive project_diagnostics(diagnostics: [diagnostic]), 500 + + file_name = + diagnostic.uri + |> Document.Path.ensure_path() + |> Path.basename() + + assert file_name == "compile_callback_error.ex" + assert diagnostic.position == 4 + assert diagnostic.severity == :error + assert diagnostic.message == "boom" + end + + test "compiling a file with callback errors" do + document = ~S[ + defmodule WithCallbackErrors do + @after_verify __MODULE__ + def __after_verify__(_) do + raise "boom" + end + end + ] + {:ok, project} = with_project(:project) + compile_document(project, document) + assert_receive file_compiled(status: :error), 500 + assert_receive file_diagnostics(diagnostics: [diagnostic]) + assert diagnostic.message == "boom" + assert diagnostic.position == 1 + end + end end From b994e12f373fa1d60424b321372daec44f7d5ec4 Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Mon, 11 Dec 2023 17:34:52 -0800 Subject: [PATCH 2/5] If the position is nil, default to the first line --- .../lexical/convertibles/lexical.plugin.diagnostic.result.ex | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex b/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex index 14517473d..71e18923b 100644 --- a/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex +++ b/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex @@ -75,6 +75,10 @@ defimpl Lexical.Convertible, for: Lexical.Plugin.V1.Diagnostic.Result do end end + defp position_to_range(document, nil) do + position_to_range(document, 1) + end + defp to_lexical_range(%Document{} = document, line_number, column) do line_number = Math.clamp(line_number, 1, Document.size(document) + 1) From 8a3b69f7562fd6c31beebf0abce3f1e79fb46e89 Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Tue, 12 Dec 2023 07:59:12 -0800 Subject: [PATCH 3/5] Moved isolation to its own module --- .../lexical/remote_control/build/document.ex | 23 +++----------- .../lexical/remote_control/build/isolation.ex | 31 +++++++++++++++++++ .../lexical/remote_control/build/project.ex | 27 +++++----------- 3 files changed, 43 insertions(+), 38 deletions(-) create mode 100644 apps/remote_control/lib/lexical/remote_control/build/isolation.ex diff --git a/apps/remote_control/lib/lexical/remote_control/build/document.ex b/apps/remote_control/lib/lexical/remote_control/build/document.ex index 728e145d4..d1e3a41f2 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/document.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/document.ex @@ -2,35 +2,22 @@ defmodule Lexical.RemoteControl.Build.Document do alias Lexical.Document alias Lexical.RemoteControl.Build alias Lexical.RemoteControl.Build.Document.Compilers + alias Lexical.RemoteControl.Build.Isolation @compilers [Compilers.Config, Compilers.Elixir, Compilers.EEx, Compilers.HEEx, Compilers.NoOp] def compile(%Document{} = document) do compiler = Enum.find(@compilers, & &1.recognizes?(document)) - me = self() + compile_fun = fn -> compiler.compile(document) end - {pid, ref} = - spawn_monitor(fn -> - result = compiler.compile(document) - send(me, {:result, result}) - end) - - receive do - {:result, result} -> - flush_normal_down(ref, pid) + case Isolation.invoke(compile_fun) do + {:ok, result} -> result - {:DOWN, ^ref, :process, ^pid, {exception, stack}} -> + {:error, {exception, stack}} -> diagnostic = Build.Error.error_to_diagnostic(document, exception, stack, nil) diagnostics = Build.Error.refine_diagnostics([diagnostic]) {:error, diagnostics} end end - - defp flush_normal_down(ref, pid) do - receive do - {:DOWN, ^ref, :process, ^pid, :normal} -> - :ok - end - end end diff --git a/apps/remote_control/lib/lexical/remote_control/build/isolation.ex b/apps/remote_control/lib/lexical/remote_control/build/isolation.ex new file mode 100644 index 000000000..42b25171a --- /dev/null +++ b/apps/remote_control/lib/lexical/remote_control/build/isolation.ex @@ -0,0 +1,31 @@ +defmodule Lexical.RemoteControl.Build.Isolation do + @moduledoc """ + Runs functions in an isolated, monitored process + """ + + @spec invoke((() -> term())) :: {:ok, term()} | {:error, term()} + def invoke(function) when is_function(function, 0) do + me = self() + + {pid, ref} = + spawn_monitor(fn -> + send(me, {:result, function.()}) + end) + + receive do + {:result, result} -> + flush_normal_down(ref, pid) + {:ok, result} + + {:DOWN, ^ref, :process, ^pid, reason} -> + {:error, reason} + end + end + + defp flush_normal_down(ref, pid) do + receive do + {:DOWN, ^ref, :process, ^pid, :normal} -> + :ok + end + end +end diff --git a/apps/remote_control/lib/lexical/remote_control/build/project.ex b/apps/remote_control/lib/lexical/remote_control/build/project.ex index 37a467e1b..6a8f90ea0 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/project.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/project.ex @@ -2,6 +2,7 @@ defmodule Lexical.RemoteControl.Build.Project do alias Lexical.Project alias Lexical.RemoteControl alias Lexical.RemoteControl.Build + alias Lexical.RemoteControl.Build.Isolation alias Lexical.RemoteControl.Plugin alias Mix.Task.Compiler.Diagnostic @@ -18,7 +19,7 @@ defmodule Lexical.RemoteControl.Build.Project do Mix.Task.clear() with_progress building_label(project), fn -> - result = compile_in_monitor(force?) + result = compile_in_isolation(force?) Mix.Task.run(:loadpaths) result end @@ -45,21 +46,14 @@ defmodule Lexical.RemoteControl.Build.Project do end) end - defp compile_in_monitor(force?) do - me = self() + defp compile_in_isolation(force?) do + compile_fun = fn -> Mix.Task.run(:compile, mix_compile_opts(force?)) end - {pid, ref} = - spawn_monitor(fn -> - result = Mix.Task.run(:compile, mix_compile_opts(force?)) - send(me, {:result, result}) - end) - - receive do - {:result, result} -> - flush_normal_down(ref, pid) + case Isolation.invoke(compile_fun) do + {:ok, result} -> result - {:DOWN, ^ref, _, ^pid, {exception, [{_module, _function, _arity, meta} | _]}} -> + {:error, {exception, [{_mod, _fun, _arity, meta} | _]}} -> diagnostic = %Diagnostic{ file: Keyword.get(meta, :file), severity: :error, @@ -72,13 +66,6 @@ defmodule Lexical.RemoteControl.Build.Project do end end - defp flush_normal_down(ref, pid) do - receive do - {:DOWN, ^ref, _, ^pid, :normal} -> - :ok - end - end - defp prepare_for_project_build(false = _force?) do :ok end From 6e6d35f009d4fb13a4b8086c83bafb49339963c5 Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Tue, 12 Dec 2023 08:15:38 -0800 Subject: [PATCH 4/5] PR suggestions Implemented some of the PR suggestions, and increased the global timeout for assert_receive to 1 second to see if that fixes errors in CI --- .../lexical/remote_control/build/isolation.ex | 3 + .../lexical/remote_control/build_test.exs | 124 +++++++++--------- apps/remote_control/test/test_helper.exs | 2 + 3 files changed, 67 insertions(+), 62 deletions(-) diff --git a/apps/remote_control/lib/lexical/remote_control/build/isolation.ex b/apps/remote_control/lib/lexical/remote_control/build/isolation.ex index 42b25171a..7a0d8b51a 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/isolation.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/isolation.ex @@ -26,6 +26,9 @@ defmodule Lexical.RemoteControl.Build.Isolation do receive do {:DOWN, ^ref, :process, ^pid, :normal} -> :ok + after + 50 -> + :ok end end end diff --git a/apps/remote_control/test/lexical/remote_control/build_test.exs b/apps/remote_control/test/lexical/remote_control/build_test.exs index 715541bb0..22ad141dc 100644 --- a/apps/remote_control/test/lexical/remote_control/build_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build_test.exs @@ -50,7 +50,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, module) - assert_receive file_compiled(), 500 + assert_receive file_compiled() :ok end @@ -69,8 +69,8 @@ defmodule Lexical.BuildTest do {:ok, project} = with_project(:project_metadata) Build.schedule_compile(project, true) - assert_receive project_compiled(status: :success), 500 - assert_receive project_progress(label: "Building " <> project_name), 500 + assert_receive project_compiled(status: :success) + assert_receive project_progress(label: "Building " <> project_name) assert project_name == "project_metadata" end @@ -78,7 +78,7 @@ defmodule Lexical.BuildTest do {:ok, project} = with_project(:project_metadata) Build.schedule_compile(project, true) - assert_receive module_updated(name: ProjectMetadata, functions: functions), 500 + assert_receive module_updated(name: ProjectMetadata, functions: functions) assert {:zero_arity, 0} in functions assert {:one_arity, 1} in functions @@ -91,22 +91,22 @@ defmodule Lexical.BuildTest do {:ok, project} = with_project(:umbrella) Build.schedule_compile(project, true) - assert_receive project_compiled(status: :success), 500 - assert_receive project_diagnostics(diagnostics: []), 500 + assert_receive project_compiled(status: :success) + assert_receive project_diagnostics(diagnostics: []) - assert_receive module_updated(name: Umbrella.First, functions: functions), 500 + assert_receive module_updated(name: Umbrella.First, functions: functions) assert {:arity_0, 0} in functions assert {:arity_1, 1} in functions assert {:arity_2, 2} in functions - assert_receive module_updated(name: Umbrella.Second, functions: functions), 500 + assert_receive module_updated(name: Umbrella.Second, functions: functions) assert {:arity_0, 0} in functions assert {:arity_1, 1} in functions assert {:arity_2, 2} in functions - assert_receive project_progress(label: "Building " <> project_name), 500 + assert_receive project_progress(label: "Building " <> project_name) assert project_name == "umbrella" end end @@ -116,8 +116,8 @@ defmodule Lexical.BuildTest do {:ok, project} = with_project(:compilation_errors) Build.schedule_compile(project, true) - assert_receive project_compiled(status: :error), 500 - assert_receive project_diagnostics(diagnostics: [%Diagnostic.Result{}]), 500 + assert_receive project_compiled(status: :error) + assert_receive project_diagnostics(diagnostics: [%Diagnostic.Result{}]) end end @@ -127,8 +127,8 @@ defmodule Lexical.BuildTest do test "stuff", %{project: project} do Build.schedule_compile(project, true) - assert_receive project_compiled(status: :error), 500 - assert_receive project_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic]), 500 + assert_receive project_compiled(status: :error) + assert_receive project_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic]) assert diagnostic.uri assert diagnostic.message =~ "SyntaxError" @@ -140,8 +140,8 @@ defmodule Lexical.BuildTest do {:ok, project} = with_project(:compilation_warnings) Build.schedule_compile(project, true) - assert_receive project_compiled(status: :success), 500 - assert_receive project_diagnostics(diagnostics: diagnostics), 500 + assert_receive project_compiled(status: :success) + assert_receive project_diagnostics(diagnostics: diagnostics) assert [%Diagnostic.Result{}, %Diagnostic.Result{}] = diagnostics @@ -170,8 +170,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive file_compiled(status: :error), 500 - assert_receive file_diagnostics(diagnostics: [diagnostic]), 500 + assert_receive file_compiled(status: :error) + assert_receive file_diagnostics(diagnostics: [diagnostic]) assert %Diagnostic.Result{} = diagnostic assert diagnostic.uri @@ -184,8 +184,8 @@ defmodule Lexical.BuildTest do source = ~S[%{foo: 3] compile_document(project, source) - assert_receive file_compiled(status: :error), 500 - assert_receive file_diagnostics(diagnostics: [diagnostic]), 500 + assert_receive file_compiled(status: :error) + assert_receive file_diagnostics(diagnostics: [diagnostic]) assert %Diagnostic.Result{} = diagnostic assert diagnostic.uri @@ -200,8 +200,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive file_compiled(status: :error), 500 - assert_receive file_diagnostics(diagnostics: [diagnostic]), 500 + assert_receive file_compiled(status: :error) + assert_receive file_diagnostics(diagnostics: [diagnostic]) assert %Diagnostic.Result{} = diagnostic assert diagnostic.uri @@ -222,8 +222,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive file_compiled(status: :error), 500 - assert_receive file_diagnostics(diagnostics: [diagnostic]), 500 + assert_receive file_compiled(status: :error) + assert_receive file_diagnostics(diagnostics: [diagnostic]) assert %Diagnostic.Result{} = diagnostic assert diagnostic.uri @@ -239,8 +239,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, "my_test.ex", source) - assert_receive file_compiled(status: :error), 500 - assert_receive file_diagnostics(diagnostics: [diagnostic]), 500 + assert_receive file_compiled(status: :error) + assert_receive file_diagnostics(diagnostics: [diagnostic]) assert diagnostic.severity == :error assert diagnostic.uri =~ "my_test.ex" assert diagnostic.message =~ "function IO.ins/0 is undefined or private" @@ -257,8 +257,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive file_compiled(status: :success), 500 - assert_receive file_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic]), 500 + assert_receive file_compiled(status: :success) + assert_receive file_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic]) assert diagnostic.uri assert diagnostic.severity == :warning @@ -286,7 +286,7 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive file_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic | _]), 500 + assert_receive file_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic | _]) assert diagnostic.uri if Features.with_diagnostics?() do @@ -297,7 +297,7 @@ defmodule Lexical.BuildTest do assert diagnostic.position == {4, 13} else - assert_receive file_compiled(status: :success), 500 + assert_receive file_compiled(status: :success) assert diagnostic.severity == :warning assert diagnostic.details == {WithWarnings, :error, 0} assert diagnostic.position == 4 @@ -316,8 +316,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive file_compiled(status: :success), 500 - assert_receive file_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic]), 500 + assert_receive file_compiled(status: :success) + assert_receive file_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic]) assert diagnostic.uri assert diagnostic.severity == :warning @@ -336,8 +336,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive file_compiled(status: :error), 500 - assert_receive file_diagnostics(diagnostics: [diagnostic]), 500 + assert_receive file_compiled(status: :error) + assert_receive file_diagnostics(diagnostics: [diagnostic]) assert diagnostic.uri assert diagnostic.severity == :error @@ -358,13 +358,13 @@ defmodule Lexical.BuildTest do compile_document(project, source) - assert_receive file_compiled(status: :error), 500 + assert_receive file_compiled(status: :error) if Features.with_diagnostics?() do - assert_receive file_diagnostics(diagnostics: [_, _, _] = diagnostics), 500 + assert_receive file_diagnostics(diagnostics: [_, _, _] = diagnostics) assert length(diagnostics) == 3 else - assert_receive file_diagnostics(diagnostics: [_, _, _, _, _] = diagnostics), 500 + assert_receive file_diagnostics(diagnostics: [_, _, _, _, _] = diagnostics) assert length(diagnostics) == 5 end end @@ -376,7 +376,7 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive module_updated(name: NewModule, functions: []), 500 + assert_receive module_updated(name: NewModule, functions: []) end test "adding a non-loaded module notifies the listener", %{project: project} do @@ -388,7 +388,7 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive module_updated(name: NotLoaded, struct: fields), 500 + assert_receive module_updated(name: NotLoaded, struct: fields) assert [%{field: :loaded, required?: true}] = fields end @@ -405,9 +405,9 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive module_updated(name: FirstModule), 500 - assert_receive module_updated(name: SecondModule), 500 - assert_receive module_updated(name: ThirdModule), 500 + assert_receive module_updated(name: FirstModule) + assert_receive module_updated(name: SecondModule) + assert_receive module_updated(name: ThirdModule) end test "adding a function notifies the listener", %{project: project} do @@ -420,7 +420,7 @@ defmodule Lexical.BuildTest do ] compile_document(project, source) - assert_receive module_updated(name: UnderTest, functions: [added_function: 2]), 500 + assert_receive module_updated(name: UnderTest, functions: [added_function: 2]) end test "removing a function notifies the listener", %{project: project} do @@ -437,10 +437,10 @@ defmodule Lexical.BuildTest do ] compile_document(project, initial) - assert_receive module_updated(), 500 + assert_receive module_updated() compile_document(project, removed) - assert_receive module_updated(name: Remove, functions: []), 500 + assert_receive module_updated(name: Remove, functions: []) end test "changing a function's arity notifies the listener", %{project: project} do @@ -451,7 +451,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, initial) - assert_receive module_updated(name: ArityChange, functions: [arity: 1]), 500 + assert_receive module_updated(name: ArityChange, functions: [arity: 1]) changed = ~S[ defmodule ArityChange do @@ -460,7 +460,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, changed) - assert_receive module_updated(name: ArityChange, functions: [arity: 2]), 500 + assert_receive module_updated(name: ArityChange, functions: [arity: 2]) end test "adding a macro notifies the listener", %{project: project} do @@ -474,7 +474,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, changed) - assert_receive module_updated(name: UnderTest, macros: [something: 1]), 500 + assert_receive module_updated(name: UnderTest, macros: [something: 1]) end test "removing a macro notifies the listener", %{project: project} do @@ -491,10 +491,10 @@ defmodule Lexical.BuildTest do ] compile_document(project, initial) - assert_receive module_updated(), 500 + assert_receive module_updated() compile_document(project, removed) - assert_receive module_updated(name: RemoveMacro, macros: []), 500 + assert_receive module_updated(name: RemoveMacro, macros: []) end test "changing a macro's arity notifies the listener", %{project: project} do @@ -505,7 +505,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, initial) - assert_receive module_updated(name: ArityChange, macros: [arity: 1]), 500 + assert_receive module_updated(name: ArityChange, macros: [arity: 1]) changed = ~S[ defmodule ArityChange do @@ -514,7 +514,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, changed) - assert_receive module_updated(name: ArityChange, macros: [arity: 2]), 500 + assert_receive module_updated(name: ArityChange, macros: [arity: 2]) end end @@ -530,8 +530,8 @@ defmodule Lexical.BuildTest do ] compile_document(project, initial) - assert_receive file_compile_requested(uri: file_uri), 500 - assert_receive file_diagnostics(uri: ^file_uri, diagnostics: []), 500 + assert_receive file_compile_requested(uri: file_uri) + assert_receive file_diagnostics(uri: ^file_uri, diagnostics: []) end end @@ -555,7 +555,7 @@ defmodule Lexical.BuildTest do module_name = Module.concat(["Module", submodule, "Submodule"]) do compile_document(project, __ENV__.file, source) - assert_receive module_updated(name: ^module_name), 500 + assert_receive module_updated(name: ^module_name) end refute loaded?(project, Module.S.Submodule) @@ -569,7 +569,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, source) - assert_receive file_compiled(status: :success), 500 + assert_receive file_compiled(status: :success) assert loaded?(project, EmptyModule) end @@ -580,7 +580,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, source) - assert_receive file_compiled(status: :success), 500 + assert_receive file_compiled(status: :success) assert loaded?(project, WithAFunction) end @@ -591,7 +591,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, source) - assert_receive file_compiled(status: :success), 500 + assert_receive file_compiled(status: :success) assert loaded?(project, WithAMacro) end @@ -602,7 +602,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, source) - assert_receive file_compiled(status: :success), 500 + assert_receive file_compiled(status: :success) assert loaded?(project, WithAStruct) end @@ -613,7 +613,7 @@ defmodule Lexical.BuildTest do end ] compile_document(project, source) - assert_receive file_compiled(status: :success), 500 + assert_receive file_compiled(status: :success) assert loaded?(project, WithAType) end end @@ -622,8 +622,8 @@ defmodule Lexical.BuildTest do test "compiling a project with callback errors" do {:ok, project} = with_project(:compilation_callback_errors) Build.schedule_compile(project) - assert_receive project_compiled(status: :error), 500 - assert_receive project_diagnostics(diagnostics: [diagnostic]), 500 + assert_receive project_compiled(status: :error) + assert_receive project_diagnostics(diagnostics: [diagnostic]) file_name = diagnostic.uri @@ -647,7 +647,7 @@ defmodule Lexical.BuildTest do ] {:ok, project} = with_project(:project) compile_document(project, document) - assert_receive file_compiled(status: :error), 500 + assert_receive file_compiled(status: :error) assert_receive file_diagnostics(diagnostics: [diagnostic]) assert diagnostic.message == "boom" assert diagnostic.position == 1 diff --git a/apps/remote_control/test/test_helper.exs b/apps/remote_control/test/test_helper.exs index 727d2b2b3..966e6e86b 100644 --- a/apps/remote_control/test/test_helper.exs +++ b/apps/remote_control/test/test_helper.exs @@ -5,4 +5,6 @@ with :nonode@nohost <- Node.self() do {:ok, _pid} = :net_kernel.start([:"testing-#{random_number}@127.0.0.1"]) end +ExUnit.configure(assert_receive_timeout: 1000) + ExUnit.start(exclude: [:skip]) From 446303c44efcb2ee706e924bf81223b4f29cc54a Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Tue, 12 Dec 2023 08:31:21 -0800 Subject: [PATCH 5/5] Further improvements * Excluded a test that relied on elixir >= 1.14.0 on elixir 1.13 * Used Process.demonitor rather than a receive block to flush down message --- apps/common/lib/elixir/features.ex | 4 ++ .../lexical/remote_control/build/isolation.ex | 13 +---- .../lexical/remote_control/build_test.exs | 52 ++++++++++--------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/apps/common/lib/elixir/features.ex b/apps/common/lib/elixir/features.ex index 7b4c8d42c..2a0bb7def 100644 --- a/apps/common/lib/elixir/features.ex +++ b/apps/common/lib/elixir/features.ex @@ -10,4 +10,8 @@ defmodule Elixir.Features do def config_reader? do Version.match?(System.version(), ">= 1.11.0") end + + def after_verify? do + Version.match?(System.version(), ">= 1.14.0") + end end diff --git a/apps/remote_control/lib/lexical/remote_control/build/isolation.ex b/apps/remote_control/lib/lexical/remote_control/build/isolation.ex index 7a0d8b51a..edd2abdb9 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/isolation.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/isolation.ex @@ -14,21 +14,12 @@ defmodule Lexical.RemoteControl.Build.Isolation do receive do {:result, result} -> - flush_normal_down(ref, pid) + # clean up the DOWN message from the above process in the mailbox. + Process.demonitor(ref, [:flush]) {:ok, result} {:DOWN, ^ref, :process, ^pid, reason} -> {:error, reason} end end - - defp flush_normal_down(ref, pid) do - receive do - {:DOWN, ^ref, :process, ^pid, :normal} -> - :ok - after - 50 -> - :ok - end - end end diff --git a/apps/remote_control/test/lexical/remote_control/build_test.exs b/apps/remote_control/test/lexical/remote_control/build_test.exs index 22ad141dc..8d1bd0279 100644 --- a/apps/remote_control/test/lexical/remote_control/build_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build_test.exs @@ -618,26 +618,27 @@ defmodule Lexical.BuildTest do end end - describe "exceptions during compilation" do - test "compiling a project with callback errors" do - {:ok, project} = with_project(:compilation_callback_errors) - Build.schedule_compile(project) - assert_receive project_compiled(status: :error) - assert_receive project_diagnostics(diagnostics: [diagnostic]) - - file_name = - diagnostic.uri - |> Document.Path.ensure_path() - |> Path.basename() - - assert file_name == "compile_callback_error.ex" - assert diagnostic.position == 4 - assert diagnostic.severity == :error - assert diagnostic.message == "boom" - end + if Features.after_verify?() do + describe "exceptions during compilation" do + test "compiling a project with callback errors" do + {:ok, project} = with_project(:compilation_callback_errors) + Build.schedule_compile(project) + assert_receive project_compiled(status: :error) + assert_receive project_diagnostics(diagnostics: [diagnostic]) + + file_name = + diagnostic.uri + |> Document.Path.ensure_path() + |> Path.basename() + + assert file_name == "compile_callback_error.ex" + assert diagnostic.position == 4 + assert diagnostic.severity == :error + assert diagnostic.message == "boom" + end - test "compiling a file with callback errors" do - document = ~S[ + test "compiling a file with callback errors" do + document = ~S[ defmodule WithCallbackErrors do @after_verify __MODULE__ def __after_verify__(_) do @@ -645,12 +646,13 @@ defmodule Lexical.BuildTest do end end ] - {:ok, project} = with_project(:project) - compile_document(project, document) - assert_receive file_compiled(status: :error) - assert_receive file_diagnostics(diagnostics: [diagnostic]) - assert diagnostic.message == "boom" - assert diagnostic.position == 1 + {:ok, project} = with_project(:project) + compile_document(project, document) + assert_receive file_compiled(status: :error) + assert_receive file_diagnostics(diagnostics: [diagnostic]) + assert diagnostic.message == "boom" + assert diagnostic.position == 1 + end end end end