From 92ff0e517723d02ef2c5324d6781540c4fb62873 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Mon, 22 Jul 2024 10:20:18 -0700 Subject: [PATCH] Refactored existing tests --- .../downloading/media_download_worker.ex | 4 ++-- .../lifecycle/user_scripts/command_runner.ex | 2 +- lib/pinchflat/media/media.ex | 4 ++-- .../downloading/media_download_worker_test.exs | 4 ++-- .../media_retention_worker_test.exs | 2 +- .../user_scripts/command_runner_test.exs | 18 ++++++++++++------ test/pinchflat/media_test.exs | 8 ++++---- test/pinchflat/profiles_test.exs | 2 +- test/pinchflat/sources_test.exs | 2 +- .../controllers/media_item_controller_test.exs | 2 +- .../media_profile_controller_test.exs | 2 +- .../controllers/source_controller_test.exs | 2 +- 12 files changed, 29 insertions(+), 23 deletions(-) diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index b33029a..370ed53 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/lib/pinchflat/downloading/media_download_worker.ex @@ -64,8 +64,8 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do {:ok, media_item} = case run_user_script(:media_pre_download, media_item) do - {:ok, _, 0} -> {:ok, media_item} - {:ok, _, _} -> Media.update_media_item(media_item, %{prevent_download: true}) + {:ok, _, exit_code} when exit_code > 0 -> Media.update_media_item(media_item, %{prevent_download: true}) + _ -> {:ok, media_item} end Repo.preload(media_item, :source) diff --git a/lib/pinchflat/lifecycle/user_scripts/command_runner.ex b/lib/pinchflat/lifecycle/user_scripts/command_runner.ex index 61ef0e5..2bdc739 100644 --- a/lib/pinchflat/lifecycle/user_scripts/command_runner.ex +++ b/lib/pinchflat/lifecycle/user_scripts/command_runner.ex @@ -35,7 +35,7 @@ defmodule Pinchflat.Lifecycle.UserScripts.CommandRunner do def run(event_type, encodable_data) when event_type in @event_types do case backend_executable() do {:ok, :no_executable} -> - :ok + {:ok, :no_executable} {:ok, executable_path} -> {:ok, encoded_data} = Phoenix.json_library().encode(encodable_data) diff --git a/lib/pinchflat/media/media.ex b/lib/pinchflat/media/media.ex index ba46e7c..013bf09 100644 --- a/lib/pinchflat/media/media.ex +++ b/lib/pinchflat/media/media.ex @@ -171,7 +171,7 @@ defmodule Pinchflat.Media do if delete_files do {:ok, _} = do_delete_media_files(media_item) - :ok = run_user_script(:media_deleted, media_item) + run_user_script(:media_deleted, media_item) end # Should delete these no matter what @@ -194,7 +194,7 @@ defmodule Pinchflat.Media do Tasks.delete_tasks_for(media_item) {:ok, _} = do_delete_media_files(media_item) - :ok = run_user_script(:media_deleted, media_item) + run_user_script(:media_deleted, media_item) update_media_item(media_item, Map.merge(filepath_attrs, addl_attrs)) end diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index 754f838..f7dfc23 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -10,7 +10,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do setup do stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, ""} end) - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end) media_item = @@ -170,7 +170,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do expect(UserScriptRunnerMock, :run, fn :media_downloaded, data -> assert data.id == media_item.id - :ok + {:ok, "", 0} end) perform_job(MediaDownloadWorker, %{id: media_item.id}) diff --git a/test/pinchflat/downloading/media_retention_worker_test.exs b/test/pinchflat/downloading/media_retention_worker_test.exs index e49bb39..f0a59ce 100644 --- a/test/pinchflat/downloading/media_retention_worker_test.exs +++ b/test/pinchflat/downloading/media_retention_worker_test.exs @@ -8,7 +8,7 @@ defmodule Pinchflat.Downloading.MediaRetentionWorkerTest do alias Pinchflat.Downloading.MediaRetentionWorker setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) :ok end diff --git a/test/pinchflat/lifecycle/user_scripts/command_runner_test.exs b/test/pinchflat/lifecycle/user_scripts/command_runner_test.exs index 351f4c4..5a35faf 100644 --- a/test/pinchflat/lifecycle/user_scripts/command_runner_test.exs +++ b/test/pinchflat/lifecycle/user_scripts/command_runner_test.exs @@ -19,7 +19,7 @@ defmodule Pinchflat.Lifecycle.UserScripts.CommandRunnerTest do File.write(filepath(), "#!/bin/bash\ntouch #{filename}\n") refute File.exists?(filename) - assert :ok = Runner.run(:media_downloaded, %{}) + assert {:ok, _, _} = Runner.run(:media_downloaded, %{}) assert File.exists?(filename) end @@ -27,7 +27,7 @@ defmodule Pinchflat.Lifecycle.UserScripts.CommandRunnerTest do tmp_dir = Application.get_env(:pinchflat, :tmpfile_directory) File.write(filepath(), "#!/bin/bash\necho $1 > #{tmp_dir}/event_name\n") - assert :ok = Runner.run(:media_downloaded, %{}) + assert {:ok, _, _} = Runner.run(:media_downloaded, %{}) assert File.read!("#{tmp_dir}/event_name") == "media_downloaded\n" end @@ -35,26 +35,32 @@ defmodule Pinchflat.Lifecycle.UserScripts.CommandRunnerTest do tmp_dir = Application.get_env(:pinchflat, :tmpfile_directory) File.write(filepath(), "#!/bin/bash\necho $2 > #{tmp_dir}/encoded_data\n") - assert :ok = Runner.run(:media_downloaded, %{foo: "bar"}) + assert {:ok, _, _} = Runner.run(:media_downloaded, %{foo: "bar"}) assert File.read!("#{tmp_dir}/encoded_data") == "{\"foo\":\"bar\"}\n" end test "does nothing if the lifecycle file is not present" do :ok = File.rm(filepath()) - assert :ok = Runner.run(:media_downloaded, %{}) + assert {:ok, :no_executable} = Runner.run(:media_downloaded, %{}) end test "does nothing if the lifecycle file is empty" do File.write(filepath(), "") - assert :ok = Runner.run(:media_downloaded, %{}) + assert {:ok, :no_executable} = Runner.run(:media_downloaded, %{}) end test "returns :ok if the command exits with a non-zero status" do File.write(filepath(), "#!/bin/bash\nexit 1\n") - assert :ok = Runner.run(:media_downloaded, %{}) + assert {:ok, _, 1} = Runner.run(:media_downloaded, %{}) + end + + test "returns the output of the command" do + File.write(filepath(), "#!/bin/bash\necho 'hello'\n") + + assert {:ok, "hello\n", 0} = Runner.run(:media_downloaded, %{}) end test "gets upset if you pass an invalid event type" do diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index 5f1efb5..597309f 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -684,7 +684,7 @@ defmodule Pinchflat.MediaTest do describe "delete_media_item/2 when testing file deletion" do setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) :ok end @@ -745,7 +745,7 @@ defmodule Pinchflat.MediaTest do expect(UserScriptRunnerMock, :run, fn :media_deleted, data -> assert data.id == media_item.id - :ok + {:ok, "", 0} end) assert {:ok, _} = Media.delete_media_item(media_item, delete_files: true) @@ -754,7 +754,7 @@ defmodule Pinchflat.MediaTest do describe "delete_media_files/2" do setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) :ok end @@ -817,7 +817,7 @@ defmodule Pinchflat.MediaTest do expect(UserScriptRunnerMock, :run, fn :media_deleted, data -> assert data.id == media_item.id - :ok + {:ok, "", 0} end) assert {:ok, _} = Media.delete_media_files(media_item) diff --git a/test/pinchflat/profiles_test.exs b/test/pinchflat/profiles_test.exs index 4cf6437..6f78e7f 100644 --- a/test/pinchflat/profiles_test.exs +++ b/test/pinchflat/profiles_test.exs @@ -113,7 +113,7 @@ defmodule Pinchflat.ProfilesTest do describe "delete_media_profile/2 when deleting files" do setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) :ok end diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index 65f48e0..59e004b 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -617,7 +617,7 @@ defmodule Pinchflat.SourcesTest do describe "delete_source/2 when deleting files" do setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) :ok end diff --git a/test/pinchflat_web/controllers/media_item_controller_test.exs b/test/pinchflat_web/controllers/media_item_controller_test.exs index 4070938..34457d5 100644 --- a/test/pinchflat_web/controllers/media_item_controller_test.exs +++ b/test/pinchflat_web/controllers/media_item_controller_test.exs @@ -56,7 +56,7 @@ defmodule PinchflatWeb.MediaItemControllerTest do describe "delete media" do setup do media_item = media_item_with_attachments() - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) %{media_item: media_item} end diff --git a/test/pinchflat_web/controllers/media_profile_controller_test.exs b/test/pinchflat_web/controllers/media_profile_controller_test.exs index af174ca..03e9a8f 100644 --- a/test/pinchflat_web/controllers/media_profile_controller_test.exs +++ b/test/pinchflat_web/controllers/media_profile_controller_test.exs @@ -137,7 +137,7 @@ defmodule PinchflatWeb.MediaProfileControllerTest do setup [:create_media_profile] setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) :ok end diff --git a/test/pinchflat_web/controllers/source_controller_test.exs b/test/pinchflat_web/controllers/source_controller_test.exs index a4d18dc..011b5eb 100644 --- a/test/pinchflat_web/controllers/source_controller_test.exs +++ b/test/pinchflat_web/controllers/source_controller_test.exs @@ -152,7 +152,7 @@ defmodule PinchflatWeb.SourceControllerTest do setup [:create_source] setup do - stub(UserScriptRunnerMock, :run, fn _event_type, _data -> :ok end) + stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end) :ok end