From 3f88e33a71ce02cdea722c322f1e86672aa5ff69 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 16 Jan 2021 23:05:31 +0300 Subject: [PATCH] [#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 | 2 -- test/pleroma/object_test.exs | 24 ++++++++++++------------ test/pleroma/scheduled_activity_test.exs | 7 +++---- test/pleroma/uploaders/s3_test.exs | 8 +++----- test/support/data_case.ex | 15 +++++---------- test/support/helpers.ex | 12 ++++++++++++ 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/lib/pleroma/upload/filter.ex b/lib/pleroma/upload/filter.ex index 367acd214..661135634 100644 --- a/lib/pleroma/upload/filter.ex +++ b/lib/pleroma/upload/filter.ex @@ -43,6 +43,4 @@ def filter([filter | rest], upload) do error end end - - def filter(nil, upload), do: filter([], upload) end diff --git a/test/pleroma/object_test.exs b/test/pleroma/object_test.exs index fe7f37e7c..3150c8e01 100644 --- a/test/pleroma/object_test.exs +++ b/test/pleroma/object_test.exs @@ -78,8 +78,8 @@ test "ensures cache is cleared for the object" do setup do: clear_config([:instance, :cleanup_attachments]) test "Disabled via config" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([:instance, :cleanup_attachments], false) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([:instance, :cleanup_attachments], false) file = %Plug.Upload{ content_type: "image/jpeg", @@ -112,8 +112,8 @@ test "Disabled via config" do end test "in subdirectories" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpeg", @@ -146,9 +146,9 @@ test "in subdirectories" do end test "with dedupe enabled" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe]) - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe]) + clear_config([:instance, :cleanup_attachments], true) uploads_dir = Pleroma.Config.get!([Pleroma.Uploaders.Local, :uploads]) @@ -184,8 +184,8 @@ test "with dedupe enabled" do end test "with objects that have legacy data.url attribute" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpeg", @@ -220,9 +220,9 @@ test "with objects that have legacy data.url attribute" do end test "With custom base_url" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([Pleroma.Upload, :base_url], "https://sub.domain.tld/dir/") - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([Pleroma.Upload, :base_url], "https://sub.domain.tld/dir/") + clear_config([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpeg", diff --git a/test/pleroma/scheduled_activity_test.exs b/test/pleroma/scheduled_activity_test.exs index 7faa5660d..902d1d99c 100644 --- a/test/pleroma/scheduled_activity_test.exs +++ b/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 - DataCase.ensure_local_uploader(context) - end + setup [:ensure_local_uploader] describe "creation" do test "scheduled activities with jobs when ScheduledActivity enabled" do diff --git a/test/pleroma/uploaders/s3_test.exs b/test/pleroma/uploaders/s3_test.exs index 30653aad2..242dc0d50 100644 --- a/test/pleroma/uploaders/s3_test.exs +++ b/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, - bucket: "test_bucket" - ) + clear_config([Pleroma.Uploaders.S3]) + clear_config([Pleroma.Uploaders.S3, :bucket], "test_bucket") end describe "get_file/1" do @@ -34,7 +32,7 @@ test "it returns path without bucket when truncated_namespace set to ''" 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, diff --git a/test/support/data_case.ex b/test/support/data_case.ex index 23c858d2a..0427682a2 100644 --- a/test/support/data_case.ex +++ b/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 @@ def stub_pipeline do end def ensure_local_uploader(context) do - test_uploader = Map.get(context, :uploader, Pleroma.Uploaders.Local) - uploader = Pleroma.Config.get([Pleroma.Upload, :uploader]) - filters = Pleroma.Config.get([Pleroma.Upload, :filters]) || [] + test_uploader = Map.get(context, :uploader) || Pleroma.Uploaders.Local - Pleroma.Config.put([Pleroma.Upload, :uploader], test_uploader) - Pleroma.Config.put([Pleroma.Upload, :filters], []) - - on_exit(fn -> - Pleroma.Config.put([Pleroma.Upload, :uploader], uploader) - Pleroma.Config.put([Pleroma.Upload, :filters], filters) - end) + clear_config([Pleroma.Upload, :uploader], test_uploader) + clear_config([Pleroma.Upload, :filters], []) :ok end diff --git a/test/support/helpers.ex b/test/support/helpers.ex index 15e8cbd9d..db38a1e81 100644 --- a/test/support/helpers.ex +++ b/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 @@ defmacro clear_config(config_path) 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 @@ defmacro clear_config(config_path, do: yield) 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))