From 85b29a7ed2ab091f414925a3184854da02ed7f15 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 @@ defmodule Restarter.MixProject do def application do [ - mod: {Restarter, []} + mod: {Restarter, []}, + extra_applications: [:logger] ] end -- 2.34.1 From 648bb0e5d391c88302e59b35712a7b55e3ea000f 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. I also fixed some bugs in the erratic tests that were introduced when removing :shout. :shout is a key where restart is needed, and was changed in the test to use :rate_limit But there was a bug in the syntax that didn't get caught because the test was tagged as erratic and therefor didn't fail. Here I fixed it. --- restarter/lib/pleroma.ex | 12 ++++ test/pleroma/config/transfer_task_test.exs | 69 ++++++++++++++++++---- 2 files changed, 68 insertions(+), 13 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 @@ defmodule Restarter.Pleroma 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 @@ defmodule Restarter.Pleroma 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 30cb92fa7..988214eb1 100644 --- a/test/pleroma/config/transfer_task_test.exs +++ b/test/pleroma/config/transfer_task_test.exs @@ -119,44 +119,87 @@ defmodule Pleroma.Config.TransferTaskTest 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([:pleroma, :rate_limit]) - insert(:config, key: {:pleroma, :rate_limit}, value: [enabled: false]) - assert capture_log(fn -> TransferTask.start_link([]) end) =~ "pleroma restarted" + clear_config(:rate_limit) + insert(:config, key: :rate_limit, value: [enabled: false]) + + # 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([:pleroma, :rate_limit]) + clear_config(:rate_limit) clear_config(Pleroma.Captcha) - insert(:config, key: {:pleroma, :rate_limit}, value: [enabled: false]) + insert(:config, key: :rate_limit, value: [enabled: false]) 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 -- 2.34.1