From 2cf3e9a74325c62e2673bd4869c5c3a255125219 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Fri, 15 Mar 2024 10:20:06 -0700 Subject: [PATCH] Improved some tests --- .credo.exs | 2 +- .../downloading/download_option_builder.ex | 1 - lib/pinchflat/yt_dlp/media_collection.ex | 20 +++++++++++++------ mix.exs | 10 ++++++++++ .../download_option_builder_test.exs | 19 ++++++++++++------ .../fast_indexing_worker_test.exs | 10 ++++++++++ .../slow_indexing_helpers_test.exs | 17 ++++++++++++++++ .../yt_dlp/media_collection_test.exs | 9 +++++++++ 8 files changed, 74 insertions(+), 14 deletions(-) diff --git a/.credo.exs b/.credo.exs index 8b4586a..78fe1aa 100644 --- a/.credo.exs +++ b/.credo.exs @@ -128,7 +128,7 @@ {Credo.Check.Refactor.MatchInCondition, []}, {Credo.Check.Refactor.NegatedConditionsInUnless, []}, {Credo.Check.Refactor.NegatedConditionsWithElse, []}, - {Credo.Check.Refactor.Nesting, []}, + {Credo.Check.Refactor.Nesting, [max_nesting: 4]}, {Credo.Check.Refactor.RedundantWithClauseResult, []}, {Credo.Check.Refactor.RejectReject, []}, {Credo.Check.Refactor.UnlessWithElse, []}, diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 2c4a77e..49a688c 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -99,7 +99,6 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do :"480p" -> [format_sort: "res:480,#{video_codec_options}"] :"720p" -> [format_sort: "res:720,#{video_codec_options}"] :"1080p" -> [format_sort: "res:1080,#{video_codec_options}"] - :"1440p" -> [format_sort: "res:1440,#{video_codec_options}"] :"2160p" -> [format_sort: "res:2160,#{video_codec_options}"] end end diff --git a/lib/pinchflat/yt_dlp/media_collection.ex b/lib/pinchflat/yt_dlp/media_collection.ex index 465437b..5a0912d 100644 --- a/lib/pinchflat/yt_dlp/media_collection.ex +++ b/lib/pinchflat/yt_dlp/media_collection.ex @@ -6,7 +6,6 @@ defmodule Pinchflat.YtDlp.MediaCollection do require Logger - alias Pinchflat.Utils.FunctionUtils alias Pinchflat.Filesystem.FilesystemHelpers alias Pinchflat.YtDlp.Media, as: YtDlpMedia @@ -36,11 +35,20 @@ defmodule Pinchflat.YtDlp.MediaCollection do case runner.run(url, command_opts, output_template, output_filepath: output_filepath) do {:ok, output} -> - output - |> String.split("\n", trim: true) - |> Enum.map(&Phoenix.json_library().decode!/1) - |> Enum.map(&YtDlpMedia.response_to_struct/1) - |> FunctionUtils.wrap_ok() + parsed_lines = + output + |> String.split("\n", trim: true) + |> Enum.map(fn line -> + case Phoenix.json_library().decode(line) do + {:ok, parsed_json} -> + YtDlpMedia.response_to_struct(parsed_json) + + _ -> + nil + end + end) + + {:ok, Enum.filter(parsed_lines, &(&1 != nil))} res -> res diff --git a/mix.exs b/mix.exs index 455140f..ac23e35 100644 --- a/mix.exs +++ b/mix.exs @@ -13,6 +13,16 @@ defmodule Pinchflat.MixProject do preferred_cli_env: [ check: :test, credo: :test + ], + test_coverage: [ + ignore_modules: [ + Pinchflat.HTTP.HTTPClient, + PinchflatWeb.Layouts, + Pinchflat.DataCase, + Pinchflat.Release, + ~r/Fixtures/, + ~r/HTML$/ + ] ] ] end diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index bd88236..3f759b3 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -197,12 +197,19 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do end describe "build/1 when testing quality options" do - test "it includes quality options", %{media_item: media_item} do - media_item = update_media_profile_attribute(media_item, %{preferred_resolution: :"1080p"}) + test "it includes quality options" do + resolutions = ["360", "480", "720", "1080", "2160"] - assert {:ok, res} = DownloadOptionBuilder.build(media_item) + Enum.each(resolutions, fn resolution -> + resolution_atom = String.to_existing_atom(resolution <> "p") - assert {:format_sort, "res:1080,+codec:avc:m4a"} in res + media_profile = media_profile_fixture(%{preferred_resolution: resolution_atom}) + source = source_fixture(%{media_profile_id: media_profile.id}) + media_item = Repo.preload(media_item_fixture(source_id: source.id), source: :media_profile) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + assert {:format_sort, "res:#{resolution},+codec:avc:m4a"} in res + end) end test "it includes quality options for audio only", %{media_item: media_item} do @@ -218,10 +225,10 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do defp update_media_profile_attribute(media_item_with_preloads, attrs) do media_item_with_preloads.source.media_profile |> Profiles.change_media_profile(attrs) - |> Repo.update!() + |> Repo.update() media_item_with_preloads |> Repo.reload() - |> Repo.preload(source: :media_profile) + |> Repo.preload([source: :media_profile], force: true) end end diff --git a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs index da3ff50..4536fcd 100644 --- a/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs +++ b/test/pinchflat/fast_indexing/fast_indexing_worker_test.exs @@ -46,6 +46,16 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do ) end + test "does not reschedule if that would create a duplicate job" do + stub(HTTPClientMock, :get, fn _url -> {:ok, ""} end) + source = source_fixture(fast_index: true) + + perform_job(FastIndexingWorker, %{"id" => source.id}) + perform_job(FastIndexingWorker, %{"id" => source.id}) + + assert [_] = all_enqueued(worker: FastIndexingWorker) + end + test "does not call out to Youtube RSS if disabled" do expect(HTTPClientMock, :get, 0, fn _url -> {:ok, ""} end) source = source_fixture(fast_index: false) diff --git a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs index 8165c5b..f183a1e 100644 --- a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs +++ b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs @@ -290,5 +290,22 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do assert Repo.aggregate(MediaItem, :count, :id) == 3 assert [_, _, _] = all_enqueued(worker: MediaDownloadWorker) end + + test "does not blow up if the file returns invalid json", %{source: source} do + watcher_poll_interval = Application.get_env(:pinchflat, :file_watcher_poll_interval) + + stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, addl_opts -> + filepath = Keyword.get(addl_opts, :output_filepath) + File.write(filepath, "INVALID") + + # Need to add a delay to ensure the file watcher has time to read the file + :timer.sleep(watcher_poll_interval * 2) + # We know we're testing the file watcher since the syncronous call will only + # return an empty string (creating no records) + {:ok, ""} + end) + + assert [] = SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + end end end diff --git a/test/pinchflat/yt_dlp/media_collection_test.exs b/test/pinchflat/yt_dlp/media_collection_test.exs index 0a26708..1f88c53 100644 --- a/test/pinchflat/yt_dlp/media_collection_test.exs +++ b/test/pinchflat/yt_dlp/media_collection_test.exs @@ -62,6 +62,15 @@ defmodule Pinchflat.YtDlp.MediaCollectionTest do assert_receive {:handler, filename} assert String.ends_with?(filename, ".json") end + + test "gracefully handles partially failed responses" do + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_opts -> + {:ok, "INVALID\n\n" <> source_attributes_return_fixture() <> "\nINVALID\n"} + end) + + assert {:ok, [%Media{media_id: "video1"}, %Media{media_id: "video2"}, %Media{media_id: "video3"}]} = + MediaCollection.get_media_attributes_for_collection(@channel_url) + end end describe "get_source_details/1" do