pleroma

commit: ba31af021ca82c6eb9685e967749620d8b8a44d9
parent 9a6280cdb9a39e85ea1434dec51cb4781eacb379
Author: Ilja <ilja@ilja.space>
Date:   Wed, 20 Jul 2022 11:53:54 +0200

Fix flaky/erratic tests in Pleroma.Config.TransferTaskTest

There were async calls happening, so they weren't always finished when assert happened.

Diffstat:
M
restarter/lib/pleroma.ex
12
++++++++++++
M
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
@@ -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
@@ -79,35 +79,70 @@ 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(: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 @@ defmodule Pleroma.Config.TransferTaskTest do
       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