From a0a3c88179717e5c064fa732226fcbab72b099d5 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 17 Apr 2024 12:47:29 -0700 Subject: [PATCH] Added some random test coverage for fun --- lib/pinchflat/notifications/command_runner.ex | 3 +-- lib/pinchflat/utils/cli_utils.ex | 4 ++++ lib/pinchflat/yt_dlp/command_runner.ex | 6 +----- lib/pinchflat/yt_dlp/media.ex | 4 ++-- lib/pinchflat/yt_dlp/media_collection.ex | 4 ++-- .../fast_indexing_helpers_test.exs | 20 +++++++++++++++++++ test/pinchflat/sources_test.exs | 12 +++++++++++ 7 files changed, 42 insertions(+), 11 deletions(-) diff --git a/lib/pinchflat/notifications/command_runner.ex b/lib/pinchflat/notifications/command_runner.ex index 5f51823..782b1a9 100644 --- a/lib/pinchflat/notifications/command_runner.ex +++ b/lib/pinchflat/notifications/command_runner.ex @@ -28,8 +28,7 @@ defmodule Pinchflat.Notifications.CommandRunner do default_opts = [:verbose] parsed_opts = CliUtils.parse_options(default_opts ++ command_opts) - Logger.info("[apprise] called with: #{Enum.join(parsed_opts ++ endpoints, " ")}") - {output, return_code} = System.cmd(backend_executable(), parsed_opts ++ endpoints) + {output, return_code} = CliUtils.wrap_cmd(backend_executable(), parsed_opts ++ endpoints) Logger.info("[apprise] response: #{output}") case return_code do diff --git a/lib/pinchflat/utils/cli_utils.ex b/lib/pinchflat/utils/cli_utils.ex index 8d6c1d4..1426a1b 100644 --- a/lib/pinchflat/utils/cli_utils.ex +++ b/lib/pinchflat/utils/cli_utils.ex @@ -3,6 +3,8 @@ defmodule Pinchflat.Utils.CliUtils do Utility methods for working with CLI executables """ + require Logger + alias Pinchflat.Utils.StringUtils @doc """ @@ -19,6 +21,8 @@ defmodule Pinchflat.Utils.CliUtils do wrapper_command = Path.join(:code.priv_dir(:pinchflat), "cmd_wrapper.sh") actual_command = [command] ++ args + Logger.info("[command_wrapper]: #{command} called with: #{Enum.join(args, " ")}") + System.cmd(wrapper_command, actual_command, opts) end diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex index d16cbea..7a03df7 100644 --- a/lib/pinchflat/yt_dlp/command_runner.ex +++ b/lib/pinchflat/yt_dlp/command_runner.ex @@ -3,8 +3,6 @@ defmodule Pinchflat.YtDlp.CommandRunner do Runs yt-dlp commands using the `System.cmd/3` function """ - require Logger - alias Pinchflat.Utils.CliUtils alias Pinchflat.YtDlp.YtDlpCommandRunner alias Pinchflat.Utils.FilesystemUtils, as: FSUtils @@ -34,7 +32,6 @@ defmodule Pinchflat.YtDlp.CommandRunner do # These must stay in exactly this order, hence why I'm giving it its own variable. all_opts = command_opts ++ print_to_file_opts ++ user_configured_opts formatted_command_opts = [url] ++ CliUtils.parse_options(all_opts) - Logger.info("[yt-dlp] called with: #{Enum.join(formatted_command_opts, " ")}") case CliUtils.wrap_cmd(command, formatted_command_opts, stderr_to_stdout: true) do {_, 0} -> @@ -57,8 +54,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do def version do command = backend_executable() - # TODO: fix to use CliUtils.wrap_cmd (and look at apprise too) - case System.cmd(command, ["--version"]) do + case CliUtils.wrap_cmd(command, ["--version"]) do {output, 0} -> {:ok, String.trim(output)} diff --git a/lib/pinchflat/yt_dlp/media.ex b/lib/pinchflat/yt_dlp/media.ex index c6b03fa..7a6c1c5 100644 --- a/lib/pinchflat/yt_dlp/media.ex +++ b/lib/pinchflat/yt_dlp/media.ex @@ -63,8 +63,8 @@ defmodule Pinchflat.YtDlp.Media do |> response_to_struct() |> FunctionUtils.wrap_ok() - res -> - res + err -> + err end end diff --git a/lib/pinchflat/yt_dlp/media_collection.ex b/lib/pinchflat/yt_dlp/media_collection.ex index 7ef28dd..d4b99a1 100644 --- a/lib/pinchflat/yt_dlp/media_collection.ex +++ b/lib/pinchflat/yt_dlp/media_collection.ex @@ -50,8 +50,8 @@ defmodule Pinchflat.YtDlp.MediaCollection do {:ok, Enum.filter(parsed_lines, &(&1 != nil))} - res -> - res + err -> + err end end diff --git a/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs b/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs index f065b7e..de51abb 100644 --- a/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs +++ b/test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs @@ -89,5 +89,25 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do refute_enqueued(worker: MediaDownloadWorker) end + + test "does not blow up if a media item cannot be created", %{source: source} do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + + stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> + {:ok, "{}"} + end) + + assert [] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) + end + + test "does not blow up if a media item causes a yt-dlp error", %{source: source} do + expect(HTTPClientMock, :get, fn _url -> {:ok, "test_1"} end) + + stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> + {:error, "message", 1} + end) + + assert [] = FastIndexingHelpers.kickoff_download_tasks_from_youtube_rss_feed(source) + end end end diff --git a/test/pinchflat/sources_test.exs b/test/pinchflat/sources_test.exs index d6589ff..3e4a1da 100644 --- a/test/pinchflat/sources_test.exs +++ b/test/pinchflat/sources_test.exs @@ -131,6 +131,18 @@ defmodule Pinchflat.SourcesTest do assert String.starts_with?(source.collection_id, "some_playlist_id_") end + test "adds an error if the runner fails" do + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "some error", 1} end) + + valid_attrs = %{ + media_profile_id: media_profile_fixture().id, + original_url: "https://www.youtube.com/channel/abc123" + } + + assert {:error, %Ecto.Changeset{} = changeset} = Sources.create_source(valid_attrs) + assert "could not fetch source details from URL" in errors_on(changeset).original_url + end + test "you can specify a custom custom_name" do expect(YtDlpRunnerMock, :run, &channel_mock/3)