From 3d11cd585d7888274f62e8c1c728bdf41ae2b8a1 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 27 Feb 2025 13:38:06 +0000 Subject: [PATCH] Use built-in Req stub for LastFm tests --- config/test.exs | 7 ++- lib/last_fm.ex | 8 ++-- lib/last_fm/api_impl.ex | 4 ++ lib/last_fm/config.ex | 11 ++++- test/last_fm/config_test.exs | 2 +- test/last_fm_test.exs | 16 +++---- .../live/artist_live/show_test.exs | 47 +++++++++---------- test/support/fixtures/last_fm/artist.ex | 8 ++-- test/test_helper.exs | 8 ---- 9 files changed, 58 insertions(+), 53 deletions(-) diff --git a/config/test.exs b/config/test.exs index ad0d7783..bc373bfd 100644 --- a/config/test.exs +++ b/config/test.exs @@ -35,6 +35,11 @@ config :phoenix_live_view, config :music_library, monitoring_routes: true -config :music_library, LastFm, auto_refresh: false +config :music_library, LastFm, + auto_refresh: false, + req_options: [ + plug: {Req.Test, LastFm.API}, + max_retries: 0 + ] config :phoenix_test, :endpoint, MusicLibraryWeb.Endpoint diff --git a/lib/last_fm.ex b/lib/last_fm.ex index e963ca85..2ee6736c 100644 --- a/lib/last_fm.ex +++ b/lib/last_fm.ex @@ -2,7 +2,7 @@ defmodule LastFm do def get_artist_info(musicbrainz_id, name) do last_fm_config = last_fm_config() - case last_fm_config.api.get_artist_info( + case LastFm.APIImpl.get_artist_info( {:musicbrainz_id, musicbrainz_id}, last_fm_config ) do @@ -12,7 +12,7 @@ defmodule LastFm do {:error, :invalid_parameters} -> # Sometimes the artist info cannot be identified with the MusicBrainz ID, # because Last.fm doesn't have that information. In that case, we try again with the artist name. - last_fm_config.api.get_artist_info({:name, name}, last_fm_config) + LastFm.APIImpl.get_artist_info({:name, name}, last_fm_config) error -> error @@ -22,7 +22,7 @@ defmodule LastFm do def get_similar_artists(musicbrainz_id, name) do last_fm_config = last_fm_config() - case last_fm_config.api.get_similar_artists( + case LastFm.APIImpl.get_similar_artists( {:musicbrainz_id, musicbrainz_id}, last_fm_config ) do @@ -32,7 +32,7 @@ defmodule LastFm do {:error, :invalid_parameters} -> # Sometimes the artist info cannot be identified with the MusicBrainz ID, # because Last.fm doesn't have that information. In that case, we try again with the artist name. - last_fm_config.api.get_similar_artists({:name, name}, last_fm_config) + LastFm.APIImpl.get_similar_artists({:name, name}, last_fm_config) error -> error diff --git a/lib/last_fm/api_impl.ex b/lib/last_fm/api_impl.ex index 47127f0f..4892f98a 100644 --- a/lib/last_fm/api_impl.ex +++ b/lib/last_fm/api_impl.ex @@ -102,6 +102,7 @@ defmodule LastFm.APIImpl do ], user_agent: config.user_agent ) + |> Req.Request.merge_options(config.req_options) |> Req.Request.put_private(:api_key, config.api_key) |> Req.Request.append_request_steps(log_attempt: &log_attempt/1) |> Req.Request.append_response_steps(parse_error: &parse_error/1) @@ -114,6 +115,9 @@ defmodule LastFm.APIImpl do {:ok, response} -> {:ok, response.body} + + error -> + error end end diff --git a/lib/last_fm/config.ex b/lib/last_fm/config.ex index 1472ee8f..17051da3 100644 --- a/lib/last_fm/config.ex +++ b/lib/last_fm/config.ex @@ -5,7 +5,8 @@ defmodule LastFm.Config do user: String.t(), auto_refresh: boolean(), refresh_interval: pos_integer(), - user_agent: String.t() + user_agent: String.t(), + req_options: Keyword.t() } @enforce_keys [:api, :api_key, :user] @@ -14,7 +15,8 @@ defmodule LastFm.Config do user: "", auto_refresh: true, refresh_interval: 60_000, - user_agent: "change me" + user_agent: "change me", + req_options: [] @schema NimbleOptions.new!( api: [ @@ -43,6 +45,11 @@ defmodule LastFm.Config do type: :string, required: false, default: "change me" + ], + req_options: [ + type: :keyword_list, + required: false, + default: [] ] ) diff --git a/test/last_fm/config_test.exs b/test/last_fm/config_test.exs index 61e42acd..981093c7 100644 --- a/test/last_fm/config_test.exs +++ b/test/last_fm/config_test.exs @@ -4,7 +4,7 @@ defmodule LastFm.ConfigTest do describe "resolve/1" do test "reads data from application configuration" do assert %LastFm.Config{ - api: LastFm.APIMock, + api: LastFm.APIImpl, api_key: api_key, user: user, auto_refresh: false, diff --git a/test/last_fm_test.exs b/test/last_fm_test.exs index d7d1bb0e..ed8e72d9 100644 --- a/test/last_fm_test.exs +++ b/test/last_fm_test.exs @@ -1,20 +1,20 @@ defmodule LastFmTest do use ExUnit.Case, async: true - alias LastFm.APIMock - alias LastFm.Fixtures.Artist - import Mox - - setup :verify_on_exit! + alias LastFm.{Artist, Fixtures} describe "get_artist_info/1" do test "it returns the artist info" do name = "Steven Wilson" musicbrainz_id = Ecto.UUID.generate() - expected_info = Artist.get_info() - expect(APIMock, :get_artist_info, fn {:musicbrainz_id, ^musicbrainz_id}, _config -> - {:ok, expected_info} + expected_info = + Fixtures.Artist.get_info() + |> Map.get("artist") + |> Artist.from_api_response() + + Req.Test.stub(LastFm.API, fn conn -> + Req.Test.json(conn, Fixtures.Artist.get_info()) end) assert {:ok, expected_info} == LastFm.get_artist_info(musicbrainz_id, name) diff --git a/test/music_library_web/live/artist_live/show_test.exs b/test/music_library_web/live/artist_live/show_test.exs index ec0c5157..b12f7e86 100644 --- a/test/music_library_web/live/artist_live/show_test.exs +++ b/test/music_library_web/live/artist_live/show_test.exs @@ -2,12 +2,7 @@ defmodule MusicLibraryWeb.ArtistLive.ShowTest do use MusicLibraryWeb.ConnCase import MusicLibrary.Fixtures.Records - import LastFm.Fixtures.Artist - import Mox - - alias LastFm.APIMock - - setup :verify_on_exit! + alias LastFm.Fixtures defp fill_collection(_config) do collection_record = @@ -28,13 +23,14 @@ defmodule MusicLibraryWeb.ArtistLive.ShowTest do conn: conn, artist_musicbrainz_id: artist_musicbrainz_id } do - expect(APIMock, :get_artist_info, fn {:musicbrainz_id, ^artist_musicbrainz_id}, _config -> - {:ok, get_info()} - end) + Req.Test.stub(LastFm.API, fn conn -> + case Map.get(conn.params, "method") do + "artist.getInfo" -> + Req.Test.json(conn, Fixtures.Artist.get_info()) - expect(APIMock, :get_similar_artists, fn {:musicbrainz_id, ^artist_musicbrainz_id}, - _config -> - {:ok, []} + "artist.getSimilar" -> + Req.Test.json(conn, Fixtures.Artist.get_similar_artists()) + end end) conn @@ -49,13 +45,14 @@ defmodule MusicLibraryWeb.ArtistLive.ShowTest do conn: conn, artist_musicbrainz_id: artist_musicbrainz_id } do - expect(APIMock, :get_artist_info, fn {:musicbrainz_id, ^artist_musicbrainz_id}, _config -> - {:error, :timeout} - end) + Req.Test.stub(LastFm.API, fn conn -> + case Map.get(conn.params, "method") do + "artist.getInfo" -> + Req.Test.transport_error(conn, :timeout) - expect(APIMock, :get_similar_artists, fn {:musicbrainz_id, ^artist_musicbrainz_id}, - _config -> - {:ok, []} + "artist.getSimilar" -> + Req.Test.json(conn, Fixtures.Artist.get_similar_artists()) + end end) conn @@ -81,14 +78,14 @@ defmodule MusicLibraryWeb.ArtistLive.ShowTest do other_collection_record = record_with_artist("Porcupine Tree", %{purchased_at: DateTime.utc_now()}) - # for this test, we don't care about the artist info, but we mock it to avoid false test failures - expect(APIMock, :get_artist_info, fn {:musicbrainz_id, ^artist_musicbrainz_id}, _config -> - {:error, :timeout} - end) + Req.Test.stub(LastFm.API, fn conn -> + case Map.get(conn.params, "method") do + "artist.getInfo" -> + Req.Test.json(conn, Fixtures.Artist.get_info()) - expect(APIMock, :get_similar_artists, fn {:musicbrainz_id, ^artist_musicbrainz_id}, - _config -> - {:ok, []} + "artist.getSimilar" -> + Req.Test.json(conn, Fixtures.Artist.get_similar_artists()) + end end) conn diff --git a/test/support/fixtures/last_fm/artist.ex b/test/support/fixtures/last_fm/artist.ex index cee00a3f..e15d0ea8 100644 --- a/test/support/fixtures/last_fm/artist.ex +++ b/test/support/fixtures/last_fm/artist.ex @@ -1,13 +1,13 @@ defmodule LastFm.Fixtures.Artist do @fixtures_folder Path.join([File.cwd!(), "test/support/fixtures/last_fm"]) - alias LastFm.Artist - def get_info do Path.join([@fixtures_folder, "artist.getinfo.json"]) |> File.read!() |> JSON.decode!() - |> Map.get("artist") - |> Artist.from_api_response() + end + + def get_similar_artists do + %{"similarartists" => %{"artist" => []}} end end diff --git a/test/test_helper.exs b/test/test_helper.exs index 94a35444..4eac347f 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -6,13 +6,5 @@ music_brainz_config = Application.put_env(:music_library, MusicBrainz, music_brainz_config) -Mox.defmock(LastFm.APIMock, for: LastFm.APIBehaviour) - -last_fm_config = - Application.get_env(:music_library, LastFm) - |> Keyword.put(:api, LastFm.APIMock) - -Application.put_env(:music_library, LastFm, last_fm_config) - ExUnit.start() Ecto.Adapters.SQL.Sandbox.mode(MusicLibrary.Repo, :manual)