Refactored existing tests

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, _, exit_code} when exit_code > 0 -> Media.update_media_item(media_item, %{prevent_download: true})
-        {:ok, _, _} -> Media.update_media_item(media_item, %{prevent_download: true})
+        _ -> {:ok, media_item}
       end
 
     Repo.preload(media_item, :source)

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)

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

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})

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

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

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)

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

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

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

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

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