From 9a6280cdb9a39e85ea1434dec51cb4781eacb379 Mon Sep 17 00:00:00 2001 From: Ilja Date: Wed, 20 Jul 2022 13:17:44 +0200 Subject: [PATCH 1/2] Fix warnings ":logger is used by the current application but the current application does not depend on :logger" During compilation, we had the following warning which is now fixed ``` ==> restarter Compiling 1 file (.ex) warning: Logger.__do_log__/4 defined in application :logger is used by the current application but the current application does not depend on :logger. To fix this, you must do one of: 1. If :logger is part of Erlang/Elixir, you must include it under :extra_applications inside "def application" in your mix.exs 2. If :logger is a dependency, make sure it is listed under "def deps" in your mix.exs 3. In case you don't want to add a requirement to :logger, you may optionally skip this warning by adding [xref: [exclude: [Logger]]] to your "def project" in mix.exs Invalid call found at 2 locations: lib/pleroma.ex:65: Restarter.Pleroma.handle_cast/2 lib/pleroma.ex:78: Restarter.Pleroma.handle_cast/2 warning: Logger.__should_log__/2 defined in application :logger is used by the current application but the current application does not depend on :logger. To fix this, you must do one of: 1. If :logger is part of Erlang/Elixir, you must include it under :extra_applications inside "def application" in your mix.exs 2. If :logger is a dependency, make sure it is listed under "def deps" in your mix.exs 3. In case you don't want to add a requirement to :logger, you may optionally skip this warning by adding [xref: [exclude: [Logger]]] to your "def project" in mix.exs Invalid call found at 2 locations: lib/pleroma.ex:65: Restarter.Pleroma.handle_cast/2 lib/pleroma.ex:78: Restarter.Pleroma.handle_cast/2 warning: Logger.debug/1 defined in application :logger is used by the current application but the current application does not depend on :logger. To fix this, you must do one of: 1. If :logger is part of Erlang/Elixir, you must include it under :extra_applications inside "def application" in your mix.exs 2. If :logger is a dependency, make sure it is listed under "def deps" in your mix.exs 3. In case you don't want to add a requirement to :logger, you may optionally skip this warning by adding [xref: [exclude: [Logger]]] to your "def project" in mix.exs Invalid call found at 2 locations: lib/pleroma.ex:65: Restarter.Pleroma.handle_cast/2 lib/pleroma.ex:78: Restarter.Pleroma.handle_cast/2 ``` --- restarter/mix.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/restarter/mix.exs b/restarter/mix.exs index b0908aece..9f26f5f64 100644 --- a/restarter/mix.exs +++ b/restarter/mix.exs @@ -13,7 +13,8 @@ def project do def application do [ - mod: {Restarter, []} + mod: {Restarter, []}, + extra_applications: [:logger] ] end From ba31af021ca82c6eb9685e967749620d8b8a44d9 Mon Sep 17 00:00:00 2001 From: Ilja Date: Wed, 20 Jul 2022 11:53:54 +0200 Subject: [PATCH 2/2] Fix flaky/erratic tests in Pleroma.Config.TransferTaskTest There were async calls happening, so they weren't always finished when assert happened. --- restarter/lib/pleroma.ex | 12 +++++ test/pleroma/config/transfer_task_test.exs | 61 ++++++++++++++++++---- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/restarter/lib/pleroma.ex b/restarter/lib/pleroma.ex index 149a569ce..a7186cec4 100644 --- a/restarter/lib/pleroma.ex +++ b/restarter/lib/pleroma.ex @@ -61,6 +61,12 @@ def handle_cast(:refresh, _state) do {:noreply, @init_state} end + # Don't actually restart during tests. + # We just check if the correct call has been done. + # If we actually restart, we get errors during the tests like + # (RuntimeError) could not lookup Ecto repo Pleroma.Repo because it was not started or + # it does not exist + # See tests in Pleroma.Config.TransferTaskTest def handle_cast({:restart, :test, _}, state) do Logger.debug("pleroma manually restarted") {:noreply, Map.put(state, :need_reboot, false)} @@ -74,6 +80,12 @@ def handle_cast({:restart, _, delay}, state) do def handle_cast({:after_boot, _}, %{after_boot: true} = state), do: {:noreply, state} + # Don't actually restart during tests. + # We just check if the correct call has been done. + # If we actually restart, we get errors during the tests like + # (RuntimeError) could not lookup Ecto repo Pleroma.Repo because it was not started or + # it does not exist + # See tests in Pleroma.Config.TransferTaskTest def handle_cast({:after_boot, :test}, state) do Logger.debug("pleroma restarted after boot") state = %{state | after_boot: true, rebooted: true} diff --git a/test/pleroma/config/transfer_task_test.exs b/test/pleroma/config/transfer_task_test.exs index 927744add..3dc917362 100644 --- a/test/pleroma/config/transfer_task_test.exs +++ b/test/pleroma/config/transfer_task_test.exs @@ -79,35 +79,70 @@ test "transfer config values with full subkey update" do describe "pleroma restart" do setup do - on_exit(fn -> Restarter.Pleroma.refresh() end) + on_exit(fn -> + Restarter.Pleroma.refresh() + + # Restarter.Pleroma.refresh/0 is an asynchronous call. + # A GenServer will first finish the previous call before starting a new one. + # Here we do a synchronous call. + # That way we are sure that the previous call has finished before we continue. + # See https://stackoverflow.com/questions/51361856/how-to-use-task-await-with-genserver + Restarter.Pleroma.rebooted?() + end) end - @tag :erratic test "don't restart if no reboot time settings were changed" do clear_config(:emoji) insert(:config, key: :emoji, value: [groups: [a: 1, b: 2]]) refute String.contains?( - capture_log(fn -> TransferTask.start_link([]) end), + capture_log(fn -> + TransferTask.start_link([]) + + # TransferTask.start_link/1 is an asynchronous call. + # A GenServer will first finish the previous call before starting a new one. + # Here we do a synchronous call. + # That way we are sure that the previous call has finished before we continue. + Restarter.Pleroma.rebooted?() + end), "pleroma restarted" ) end - @tag :erratic test "on reboot time key" do clear_config(:shout) insert(:config, key: :shout, value: [enabled: false]) - assert capture_log(fn -> TransferTask.start_link([]) end) =~ "pleroma restarted" + + # Note that we don't actually restart Pleroma. + # See module Restarter.Pleroma + assert capture_log(fn -> + TransferTask.start_link([]) + + # TransferTask.start_link/1 is an asynchronous call. + # A GenServer will first finish the previous call before starting a new one. + # Here we do a synchronous call. + # That way we are sure that the previous call has finished before we continue. + Restarter.Pleroma.rebooted?() + end) =~ "pleroma restarted" end - @tag :erratic test "on reboot time subkey" do clear_config(Pleroma.Captcha) insert(:config, key: Pleroma.Captcha, value: [seconds_valid: 60]) - assert capture_log(fn -> TransferTask.start_link([]) end) =~ "pleroma restarted" + + # Note that we don't actually restart Pleroma. + # See module Restarter.Pleroma + assert capture_log(fn -> + TransferTask.start_link([]) + + # TransferTask.start_link/1 is an asynchronous call. + # A GenServer will first finish the previous call before starting a new one. + # Here we do a synchronous call. + # That way we are sure that the previous call has finished before we continue. + Restarter.Pleroma.rebooted?() + end) =~ "pleroma restarted" end - @tag :erratic test "don't restart pleroma on reboot time key and subkey if there is false flag" do clear_config(:shout) clear_config(Pleroma.Captcha) @@ -116,7 +151,15 @@ test "don't restart pleroma on reboot time key and subkey if there is false flag insert(:config, key: Pleroma.Captcha, value: [seconds_valid: 60]) refute String.contains?( - capture_log(fn -> TransferTask.load_and_update_env([], false) end), + capture_log(fn -> + TransferTask.load_and_update_env([], false) + + # TransferTask.start_link/1 is an asynchronous call. + # A GenServer will first finish the previous call before starting a new one. + # Here we do a synchronous call. + # That way we are sure that the previous call has finished before we continue. + Restarter.Pleroma.rebooted?() + end), "pleroma restarted" ) end