[#3251] Fixed wrong test-env config setting for [Pleroma.Upload]. Refactoring. Added warning to clear_config/_ to minimize such issues in future.

lib/pleroma/upload/filter.ex

@@ -43,6 +43,4 @@ defmodule Pleroma.Upload.Filter do
         error
     end
   end
-
-  def filter(nil, upload), do: filter([], upload)
 end

test/pleroma/object_test.exs

@@ -78,8 +78,8 @@ defmodule Pleroma.ObjectTest do
     setup do: clear_config([:instance, :cleanup_attachments])
 
     test "Disabled via config" do
-      Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+      clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
-      Pleroma.Config.put([:instance, :cleanup_attachments], false)
+      clear_config([:instance, :cleanup_attachments], false)
 
       file = %Plug.Upload{
         content_type: "image/jpeg",
@@ -112,8 +112,8 @@ defmodule Pleroma.ObjectTest do
     end
 
     test "in subdirectories" do
-      Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+      clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
-      Pleroma.Config.put([:instance, :cleanup_attachments], true)
+      clear_config([:instance, :cleanup_attachments], true)
 
       file = %Plug.Upload{
         content_type: "image/jpeg",
@@ -146,9 +146,9 @@ defmodule Pleroma.ObjectTest do
     end
 
     test "with dedupe enabled" do
-      Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+      clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
-      Pleroma.Config.put([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe])
+      clear_config([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe])
-      Pleroma.Config.put([:instance, :cleanup_attachments], true)
+      clear_config([:instance, :cleanup_attachments], true)
 
       uploads_dir = Pleroma.Config.get!([Pleroma.Uploaders.Local, :uploads])
 
@@ -184,8 +184,8 @@ defmodule Pleroma.ObjectTest do
     end
 
     test "with objects that have legacy data.url attribute" do
-      Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+      clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
-      Pleroma.Config.put([:instance, :cleanup_attachments], true)
+      clear_config([:instance, :cleanup_attachments], true)
 
       file = %Plug.Upload{
         content_type: "image/jpeg",
@@ -220,9 +220,9 @@ defmodule Pleroma.ObjectTest do
     end
 
     test "With custom base_url" do
-      Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+      clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
-      Pleroma.Config.put([Pleroma.Upload, :base_url], "https://sub.domain.tld/dir/")
+      clear_config([Pleroma.Upload, :base_url], "https://sub.domain.tld/dir/")
-      Pleroma.Config.put([:instance, :cleanup_attachments], true)
+      clear_config([:instance, :cleanup_attachments], true)
 
       file = %Plug.Upload{
         content_type: "image/jpeg",

test/pleroma/scheduled_activity_test.exs

@@ -4,15 +4,14 @@
 
 defmodule Pleroma.ScheduledActivityTest do
   use Pleroma.DataCase
-  alias Pleroma.DataCase
+
   alias Pleroma.ScheduledActivity
+
   import Pleroma.Factory
 
   setup do: clear_config([ScheduledActivity, :enabled])
 
-  setup context do
+  setup [:ensure_local_uploader]
-    DataCase.ensure_local_uploader(context)
-  end
 
   describe "creation" do
     test "scheduled activities with jobs when ScheduledActivity enabled" do

test/pleroma/uploaders/s3_test.exs

@@ -14,10 +14,8 @@ defmodule Pleroma.Uploaders.S3Test do
   setup do
     clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.S3)
     clear_config([Pleroma.Upload, :base_url], "https://s3.amazonaws.com")
-
+    clear_config([Pleroma.Uploaders.S3])
-    clear_config(Pleroma.Uploaders.S3,
+    clear_config([Pleroma.Uploaders.S3, :bucket], "test_bucket")
-      bucket: "test_bucket"
-    )
   end
 
   describe "get_file/1" do
@@ -34,7 +32,7 @@ defmodule Pleroma.Uploaders.S3Test do
         truncated_namespace: ""
       )
 
-      Config.put([Pleroma.Upload], base_url: "https://s3.amazonaws.com")
+      Config.put([Pleroma.Upload, :base_url], "https://s3.amazonaws.com")
 
       assert S3.get_file("test_image.jpg") == {
                :ok,

test/support/data_case.ex

@@ -18,6 +18,8 @@ defmodule Pleroma.DataCase do
 
   use ExUnit.CaseTemplate
 
+  import Pleroma.Tests.Helpers, only: [clear_config: 2]
+
   using do
     quote do
       alias Pleroma.Repo
@@ -105,17 +107,10 @@ defmodule Pleroma.DataCase do
   end
 
   def ensure_local_uploader(context) do
-    test_uploader = Map.get(context, :uploader, Pleroma.Uploaders.Local)
+    test_uploader = Map.get(context, :uploader) || Pleroma.Uploaders.Local
-    uploader = Pleroma.Config.get([Pleroma.Upload, :uploader])
-    filters = Pleroma.Config.get([Pleroma.Upload, :filters]) || []
 
-    Pleroma.Config.put([Pleroma.Upload, :uploader], test_uploader)
+    clear_config([Pleroma.Upload, :uploader], test_uploader)
-    Pleroma.Config.put([Pleroma.Upload, :filters], [])
+    clear_config([Pleroma.Upload, :filters], [])
-
-    on_exit(fn ->
-      Pleroma.Config.put([Pleroma.Upload, :uploader], uploader)
-      Pleroma.Config.put([Pleroma.Upload, :filters], filters)
-    end)
 
     :ok
   end

test/support/helpers.ex

@@ -8,6 +8,8 @@ defmodule Pleroma.Tests.Helpers do
   """
   alias Pleroma.Config
 
+  require Logger
+
   defmacro clear_config(config_path) do
     quote do
       clear_config(unquote(config_path)) do
@@ -18,6 +20,7 @@ defmodule Pleroma.Tests.Helpers do
   defmacro clear_config(config_path, do: yield) do
     quote do
       initial_setting = Config.fetch(unquote(config_path))
+
       unquote(yield)
 
       on_exit(fn ->
@@ -35,6 +38,15 @@ defmodule Pleroma.Tests.Helpers do
   end
 
   defmacro clear_config(config_path, temp_setting) do
+    # NOTE: `clear_config([section, key], value)` != `clear_config([section], key: value)` (!)
+    # Displaying a warning to prevent unintentional clearing of all but one keys in section
+    if Keyword.keyword?(temp_setting) and length(temp_setting) == 1 do
+      Logger.warn(
+        "Please change to `clear_config([section]); clear_config([section, key], value)`: " <>
+          "#{inspect(config_path)}, #{inspect(temp_setting)}"
+      )
+    end
+
     quote do
       clear_config(unquote(config_path)) do
         Config.put(unquote(config_path), unquote(temp_setting))