From ec6980eea09d5e0c3dd9162386058d640fefe225 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Thu, 21 May 2026 10:45:29 +0100 Subject: [PATCH] ML-193: add secondary parser, search, and API coverage tests Add ~80 focused tests across 10 files covering: - Record set search by name, description, contained record title, artist name - Navigation events for records, artists, record sets, links, and view-all actions - MusicBrainz.ReleaseGroup parser (artist credits, included RGs, release IDs, types) - MusicBrainz.ReleaseSearchResult edge cases (missing RG, unknown formats) - LastFm.Session XML edge cases (non-subscriber, missing nodes) - MusicLibrary.HttpError default status-kind mapping - Discogs.API.ErrorResponse fallback message and retry-delay behavior - ArtistRefreshWikipediaData {:cancel, :no_english_wikipedia} worker path - StatsComponents grouped on-this-day records and anniversary labels --- ...econdary-parser-search-and-API-coverage.md | 50 ++-- test/discogs_test.exs | 69 ++++++ test/last_fm/session_test.exs | 94 ++++++++ test/music_brainz/release_group_test.exs | 220 ++++++++++++++++++ .../release_search_result_test.exs | 118 ++++++++++ test/music_library/http_error_test.exs | 41 ++++ test/music_library/search_test.exs | 66 +++++- .../artist_refresh_wikipedia_data_test.exs | 9 + .../live/stats_live/index_test.exs | 126 ++++++++++ .../live/universal_search_live/index_test.exs | 184 ++++++++++++++- 10 files changed, 961 insertions(+), 16 deletions(-) create mode 100644 test/music_brainz/release_group_test.exs create mode 100644 test/music_library/http_error_test.exs diff --git a/backlog/tasks/ml-193 - Add-secondary-parser-search-and-API-coverage.md b/backlog/tasks/ml-193 - Add-secondary-parser-search-and-API-coverage.md index 7a1d1b0e..39e444c5 100644 --- a/backlog/tasks/ml-193 - Add-secondary-parser-search-and-API-coverage.md +++ b/backlog/tasks/ml-193 - Add-secondary-parser-search-and-API-coverage.md @@ -1,10 +1,10 @@ --- id: ML-193 title: Add secondary parser search and API coverage -status: To Do +status: Done assignee: [] created_date: "2026-05-20 17:19" -updated_date: "2026-05-20 17:20" +updated_date: "2026-05-21 09:12" labels: - testing - coverage @@ -31,13 +31,13 @@ Add focused tests for low-coverage modules where direct tests would be more diag -- [ ] #1 Universal search tests cover record-set results and counts when record sets match by name, description, contained record title, or artist name. -- [ ] #2 Universal search tests cover navigation events for collection records, wishlist records, artists, record sets, navigation links, and view-all collection/wishlist/record-set actions. -- [ ] #3 MusicBrainz parser tests cover ReleaseGroup artist credits with joinphrases, included release-group filtering, release ID extraction, parse_record_type precedence for live and compilation albums, and ReleaseSearchResult handling of missing release groups or unknown media formats. -- [ ] #4 LastFm.Session tests cover non-subscriber XML, missing subscriber, and missing name/key nodes with explicit expected struct values. -- [ ] #5 HTTP/API error tests cover MusicLibrary.HttpError default status-kind mapping and Discogs.API.ErrorResponse fallback message and retry-delay behavior. -- [ ] #6 ArtistRefreshWikipediaData worker tests cover the {:cancel, :no_english_wikipedia} branch when Wikipedia reports no English article. -- [ ] #7 StatsComponents or page-level tests cover grouped on-this-day records and anniversary labels for Today, normal years, 5-year, and 10-year milestones. +- [x] #1 Universal search tests cover record-set results and counts when record sets match by name, description, contained record title, or artist name. +- [x] #2 Universal search tests cover navigation events for collection records, wishlist records, artists, record sets, navigation links, and view-all collection/wishlist/record-set actions. +- [x] #3 MusicBrainz parser tests cover ReleaseGroup artist credits with joinphrases, included release-group filtering, release ID extraction, parse_record_type precedence for live and compilation albums, and ReleaseSearchResult handling of missing release groups or unknown media formats. +- [x] #4 LastFm.Session tests cover non-subscriber XML, missing subscriber, and missing name/key nodes with explicit expected struct values. +- [x] #5 HTTP/API error tests cover MusicLibrary.HttpError default status-kind mapping and Discogs.API.ErrorResponse fallback message and retry-delay behavior. +- [x] #6 ArtistRefreshWikipediaData worker tests cover the {:cancel, :no_english_wikipedia} branch when Wikipedia reports no English article. +- [x] #7 StatsComponents or page-level tests cover grouped on-this-day records and anniversary labels for Today, normal years, 5-year, and 10-year milestones. ## Implementation Plan @@ -46,10 +46,32 @@ Add focused tests for low-coverage modules where direct tests would be more diag 1. Read docs/architecture.md, docs/project-conventions.md, .agents/skills/testing/SKILL.md, and the UI/external API skill guidance before editing tests. 2. Extend universal search tests with record set fixtures and navigation assertions; prefer PhoenixTest and unwrap only where direct LiveView event interaction is needed. -3. Add direct parser tests for MusicBrainz.ReleaseGroup and MusicBrainz.ReleaseSearchResult using small explicit maps plus existing fixtures where helpful. +3. Create MusicBrainz.ReleaseGroup direct parser tests using small explicit maps plus existing fixtures where helpful. 4. Expand LastFm.Session tests beyond doctests with explicit XML examples for non-subscriber and missing-node cases. -5. Add table-style tests for MusicLibrary.HttpError and Discogs.API.ErrorResponse that assert concrete kinds, messages, retryability, and retry delays. -6. Add the ArtistRefreshWikipediaData no-English-Wikipedia worker branch using a Wikipedia stub or context setup that produces {:error, :no_english_wikipedia}. -7. Cover StatsComponents on-this-day grouped and anniversary rendering through the page if practical, otherwise use focused component rendering consistent with local patterns. -8. Run the new/modified targeted tests and any broader affected test files. +5. Create table-style tests for MusicLibrary.HttpError and add Discogs.API.ErrorResponse fallback message and retry-delay tests. +6. Add the ArtistRefreshWikipediaData no-English-Wikipedia worker branch test using Wikipedia stub. +7. Cover StatsComponents on-this-day grouped and anniversary rendering via the page-level stats test. +8. Run tests to verify. + +## Implementation Notes + + + +Added ~70 focused tests across 9 test files: + +**#1 Record set search** (`test/music_library/search_test.exs`): Tests for universal_search returning record sets by name, description, contained record title, artist name; search_counts includes record_sets_count; search_record_sets basic behavior. + +**#2 Navigation events** (`test/music_library_web/live/universal_search_live/index_test.exs`): Tests for navigating to collection records, wishlist records, artists, record sets, navigation links, and verifying record sets appear in search results. + +**#3 MusicBrainz parsers** (`test/music_brainz/release_group_test.exs` new, `test/music_brainz/release_search_result_test.exs` extended): Tests for parse_artist_credits with joinphrases, included_release_groups filtering, release_ids extraction, parse_record_type precedence (Live/Compilation override), and ReleaseSearchResult handling of missing release groups, unknown media formats. + +**#4 LastFm.Session** (`test/last_fm/session_test.exs`): Tests for non-subscriber XML (pro: false), missing subscriber/missing name/missing key nodes with explicit struct assertions. + +**#5 HTTP/API errors** (`test/music_library/http_error_test.exs` new, `test/discogs_test.exs` extended): Table-style HttpError.default_kind tests for all status ranges; Discogs ErrorResponse fallback message, retryable? and retry_delay_seconds for rate_limit/server_error/timeout/default. + +**#6 Wikipedia worker** (`test/music_library/worker/artist_refresh_wikipedia_data_test.exs`): Test for {:cancel, :no_english_wikipedia} branch when Wikipedia stub returns no_enwiki fixture. + +**#7 StatsComponents** (`test/music_library_web/live/stats_live/index_test.exs`): Grouped on-this-day records test (same musicbrainz_id → details/summary); anniversary label tests for Today, 5 years, 10 years, and normal year labels. + + diff --git a/test/discogs_test.exs b/test/discogs_test.exs index b99c5493..36bc4929 100644 --- a/test/discogs_test.exs +++ b/test/discogs_test.exs @@ -4,6 +4,75 @@ defmodule DiscogsTest do alias Discogs.API.ErrorResponse alias Discogs.Fixtures + describe "ErrorResponse.from_response/1" do + test "extracts message from body" do + err = ErrorResponse.from_response(%{status: 429, body: %{"message" => "rate limited"}}) + assert err.message == "rate limited" + assert err.kind == :rate_limit + end + + test "fallback message when body has no message key" do + err = ErrorResponse.from_response(%{status: 500, body: %{"error" => "boom"}}) + assert err.message == nil + assert err.kind == :server_error + end + + test "fallback message when body is not a map" do + err = ErrorResponse.from_response(%{status: 404, body: "not found"}) + assert err.message == nil + assert err.kind == :not_found + end + end + + describe "retry_delay_seconds/1" do + test "returns 60 for rate_limit" do + err = ErrorResponse.from_response(%{status: 429, body: %{}}) + assert ErrorResponse.retry_delay_seconds(err) == 60 + end + + test "returns 30 for server_error" do + err = ErrorResponse.from_response(%{status: 500, body: %{}}) + assert ErrorResponse.retry_delay_seconds(err) == 30 + end + + test "returns 10 for timeout" do + err = %ErrorResponse{status: nil, message: nil, kind: :timeout, body: nil} + assert ErrorResponse.retry_delay_seconds(err) == 10 + end + + test "returns 30 as default for non-retryable kinds" do + err = ErrorResponse.from_response(%{status: 404, body: %{}}) + assert ErrorResponse.retry_delay_seconds(err) == 30 + end + end + + describe "retryable?/1" do + test "returns true for rate_limit" do + err = ErrorResponse.from_response(%{status: 429, body: %{}}) + assert ErrorResponse.retryable?(err) + end + + test "returns true for server_error" do + err = ErrorResponse.from_response(%{status: 503, body: %{}}) + assert ErrorResponse.retryable?(err) + end + + test "returns true for timeout" do + err = %ErrorResponse{status: nil, message: nil, kind: :timeout, body: nil} + assert ErrorResponse.retryable?(err) + end + + test "returns false for not_found" do + err = ErrorResponse.from_response(%{status: 404, body: %{}}) + refute ErrorResponse.retryable?(err) + end + + test "returns false for auth_error" do + err = ErrorResponse.from_response(%{status: 401, body: %{}}) + refute ErrorResponse.retryable?(err) + end + end + describe "get_artist/1" do test "returns the artist" do discogs_id = "discogs_id" diff --git a/test/last_fm/session_test.exs b/test/last_fm/session_test.exs index 98f7c649..5e010bb8 100644 --- a/test/last_fm/session_test.exs +++ b/test/last_fm/session_test.exs @@ -2,4 +2,98 @@ defmodule LastFm.SessionTest do use ExUnit.Case, async: true doctest LastFm.Session + + alias LastFm.Session + + describe "parse/1" do + test "non-subscriber XML" do + xml = """ + + + + freeuser + free-key-123 + 0 + + + """ + + assert Session.parse(xml) == %Session{ + name: "freeuser", + key: "free-key-123", + pro: false + } + end + + test "missing subscriber node defaults pro to false" do + xml = """ + + + + basicuser + basic-key-456 + + + """ + + assert Session.parse(xml) == %Session{ + name: "basicuser", + key: "basic-key-456", + pro: false + } + end + + test "missing name node returns nil name" do + xml = """ + + + + some-key + 1 + + + """ + + assert Session.parse(xml) == %Session{ + name: nil, + key: "some-key", + pro: true + } + end + + test "missing key node returns nil key" do + xml = """ + + + + mysteryuser + 0 + + + """ + + assert Session.parse(xml) == %Session{ + name: "mysteryuser", + key: nil, + pro: false + } + end + + test "missing both name and key nodes returns nil values" do + xml = """ + + + + 1 + + + """ + + assert Session.parse(xml) == %Session{ + name: nil, + key: nil, + pro: true + } + end + end end diff --git a/test/music_brainz/release_group_test.exs b/test/music_brainz/release_group_test.exs new file mode 100644 index 00000000..d7cd83f8 --- /dev/null +++ b/test/music_brainz/release_group_test.exs @@ -0,0 +1,220 @@ +defmodule MusicBrainz.ReleaseGroupTest do + use ExUnit.Case, async: true + + alias MusicBrainz.ReleaseGroup + + describe "parse_artist_credits/1" do + test "extracts artist credits with joinphrases" do + musicbrainz_data = %{ + "artist-credit" => [ + %{ + "joinphrase" => " & ", + "artist" => %{ + "id" => "mbid-1", + "name" => "Steven Wilson", + "sort-name" => "Wilson, Steven", + "disambiguation" => "English musician" + } + }, + %{ + "joinphrase" => "", + "artist" => %{ + "id" => "mbid-2", + "name" => "Mikael Åkerfeldt", + "sort-name" => "Åkerfeldt, Mikael", + "disambiguation" => "Swedish musician" + } + } + ] + } + + assert ReleaseGroup.parse_artist_credits(musicbrainz_data) == [ + %{ + name: "Steven Wilson", + musicbrainz_id: "mbid-1", + sort_name: "Wilson, Steven", + disambiguation: "English musician", + joinphrase: " & " + }, + %{ + name: "Mikael Åkerfeldt", + musicbrainz_id: "mbid-2", + sort_name: "Åkerfeldt, Mikael", + disambiguation: "Swedish musician", + joinphrase: "" + } + ] + end + + test "returns empty list when artist-credit key is empty list" do + assert ReleaseGroup.parse_artist_credits(%{"artist-credit" => []}) == [] + end + + test "returns empty list when artist-credit has no artists" do + assert ReleaseGroup.parse_artist_credits(%{"artist-credit" => []}) == [] + end + end + + describe "included_release_groups/1" do + test "filters related release groups with correct target-type and direction" do + release_group = %{ + "relations" => [ + %{ + "target-type" => "release_group", + "type" => "included in", + "direction" => "backward", + "release_group" => %{ + "id" => "rg-included-1", + "primary-type" => "EP", + "title" => "Bonus Disc", + "artist-credit" => [ + %{"artist" => %{"name" => "Test Artist"}, "joinphrase" => ""} + ], + "first-release-date" => "2020-01-01" + } + }, + %{ + "target-type" => "url", + "type" => "other databases", + "direction" => "forward", + "release_group" => nil + }, + %{ + "target-type" => "release_group", + "type" => "included in", + "direction" => "forward", + "release_group" => %{ + "id" => "rg-forward", + "primary-type" => "Album", + "title" => "Forward RG", + "artist-credit" => [], + "first-release-date" => "2019-01-01" + } + } + ] + } + + result = ReleaseGroup.included_release_groups(release_group) + assert length(result) == 1 + assert hd(result).id == "rg-included-1" + assert hd(result).type == :ep + end + + test "returns empty list when no relations key" do + assert ReleaseGroup.included_release_groups(%{}) == [] + end + + test "returns empty list when no matching relations" do + release_group = %{ + "relations" => [ + %{ + "target-type" => "url", + "type" => "wikipedia", + "direction" => "forward" + } + ] + } + + assert ReleaseGroup.included_release_groups(release_group) == [] + end + end + + describe "release_ids/1" do + test "extracts IDs from releases" do + release_group = %{ + "releases" => [ + %{"id" => "rel-1"}, + %{"id" => "rel-2"}, + %{"id" => "rel-3"} + ] + } + + assert ReleaseGroup.release_ids(release_group) == ["rel-1", "rel-2", "rel-3"] + end + + test "returns empty list when no releases" do + assert ReleaseGroup.release_ids(%{}) == [] + assert ReleaseGroup.release_ids(%{"releases" => []}) == [] + end + end + + describe "included_release_group_ids/1" do + test "extracts IDs from included release groups" do + release_group = %{ + "relations" => [ + %{ + "target-type" => "release_group", + "type" => "included in", + "direction" => "backward", + "release_group" => %{ + "id" => "rg-sub-1", + "primary-type" => "EP", + "title" => "Bonus", + "artist-credit" => [], + "first-release-date" => "2020-01-01" + } + } + ] + } + + assert ReleaseGroup.included_release_group_ids(release_group) == ["rg-sub-1"] + end + end + + describe "parse_record_type/2" do + test "Album without special secondary types is :album" do + assert ReleaseGroup.parse_record_type("Album", ["Soundtrack"]) == :album + end + + test "Album with Live secondary type is :live" do + assert ReleaseGroup.parse_record_type("Album", ["Live"]) == :live + end + + test "Album with Compilation secondary type is :compilation" do + assert ReleaseGroup.parse_record_type("Album", ["Compilation"]) == :compilation + end + + test "Live overrides Compilation when both are in secondary types" do + assert ReleaseGroup.parse_record_type("Album", ["Compilation", "Live"]) == :live + end + + test "Album with nil secondary types defaults to :album" do + assert ReleaseGroup.parse_record_type("Album", nil) == :album + end + + test "EP maps to :ep" do + assert ReleaseGroup.parse_record_type("EP", ["Live"]) == :ep + end + + test "Single maps to :single" do + assert ReleaseGroup.parse_record_type("Single", nil) == :single + end + + test "Unknown primary type maps to :other" do + assert ReleaseGroup.parse_record_type("Broadcast", nil) == :other + assert ReleaseGroup.parse_record_type(nil, nil) == :other + end + end + + describe "parse_type/1" do + test "maps known types to atoms" do + assert ReleaseGroup.parse_type("Album") == :album + assert ReleaseGroup.parse_type("EP") == :ep + assert ReleaseGroup.parse_type("Live") == :live + assert ReleaseGroup.parse_type("Compilation") == :compilation + assert ReleaseGroup.parse_type("Single") == :single + end + + test "maps unknown types to :other" do + assert ReleaseGroup.parse_type("Broadcast") == :other + assert ReleaseGroup.parse_type(nil) == :other + assert ReleaseGroup.parse_type("") == :other + end + end + + describe "url/1" do + test "generates MusicBrainz URL" do + assert ReleaseGroup.url("mbid-123") == "https://musicbrainz.org/release-group/mbid-123" + end + end +end diff --git a/test/music_brainz/release_search_result_test.exs b/test/music_brainz/release_search_result_test.exs index 1792fdaf..571651e1 100644 --- a/test/music_brainz/release_search_result_test.exs +++ b/test/music_brainz/release_search_result_test.exs @@ -2,4 +2,122 @@ defmodule MusicBrainz.ReleaseSearchResultTest do use ExUnit.Case, async: true doctest MusicBrainz.ReleaseSearchResult + + alias MusicBrainz.ReleaseSearchResult + + describe "from_api_response/1" do + test "handles missing release-group" do + r = %{ + "id" => "rel-1", + "title" => "No Release Group", + "artist-credit" => [ + %{"artist" => %{"name" => "Artist Name"}} + ], + "date" => "2020-01-01", + "barcode" => "1234", + "media" => [%{"format" => "CD", "track-count" => 10, "disc-count" => 1}] + } + + result = ReleaseSearchResult.from_api_response(r) + assert result.id == "rel-1" + assert result.release_group == nil + assert result.artists == "Artist Name" + end + + test "handles missing artist-credit" do + r = %{ + "id" => "rel-2", + "title" => "No Artists", + "release-group" => %{ + "id" => "rg-1", + "primary-type" => "Album", + "title" => "The Album" + }, + "date" => nil, + "barcode" => nil, + "media" => [] + } + + result = ReleaseSearchResult.from_api_response(r) + assert result.artists == "" + assert result.release_group.type == :album + end + end + + describe "format/1" do + test "returns :unknown for nil media format" do + result = %ReleaseSearchResult{ + id: "1", + title: "T", + release_group: nil, + artists: "A", + date: "2000", + barcode: "0", + media: [%{format: nil, disc_count: 0, track_count: 5}] + } + + assert ReleaseSearchResult.format(result) == :unknown + end + + test "returns :unknown for completely unknown format string" do + result = %ReleaseSearchResult{ + id: "1", + title: "T", + release_group: nil, + artists: "A", + date: "2000", + barcode: "0", + media: [%{format: "Wax Cylinder", disc_count: 0, track_count: 4}] + } + + assert ReleaseSearchResult.format(result) == :unknown + end + + test "returns :multi for empty media list" do + result = %ReleaseSearchResult{ + id: "1", + title: "T", + release_group: nil, + artists: "A", + date: "2000", + barcode: "0", + media: [] + } + + # Empty media list has no frequencies, so the catch-all returns :multi + assert ReleaseSearchResult.format(result) == :multi + end + + test "returns :multi for mixed format types" do + result = %ReleaseSearchResult{ + id: "1", + title: "T", + release_group: nil, + artists: "A", + date: "2000", + barcode: "0", + media: [ + %{format: "CD", disc_count: 1, track_count: 10}, + %{format: "12\" Vinyl", disc_count: 1, track_count: 6}, + %{format: "Digital Media", disc_count: 0, track_count: 12} + ] + } + + assert ReleaseSearchResult.format(result) == :multi + end + end + + describe "parse_media/1" do + test "handles empty media list" do + assert ReleaseSearchResult.parse_media([]) == [] + end + + test "handles missing fields gracefully" do + media = [%{}] + parsed = ReleaseSearchResult.parse_media(media) + assert hd(parsed).format == nil + assert hd(parsed).track_count == nil + assert hd(parsed).disc_count == nil + end + end end diff --git a/test/music_library/http_error_test.exs b/test/music_library/http_error_test.exs new file mode 100644 index 00000000..50bbb3a6 --- /dev/null +++ b/test/music_library/http_error_test.exs @@ -0,0 +1,41 @@ +defmodule MusicLibrary.HttpErrorTest do + use ExUnit.Case, async: true + + alias MusicLibrary.HttpError + + describe "default_kind/1" do + test "maps 429 to :rate_limit" do + assert HttpError.default_kind(429) == :rate_limit + end + + test "maps 5xx to :server_error" do + for status <- [500, 502, 503, 504, 599] do + assert HttpError.default_kind(status) == :server_error, + "expected #{status} to be :server_error" + end + end + + test "maps 401 and 403 to :auth_error" do + assert HttpError.default_kind(401) == :auth_error + assert HttpError.default_kind(403) == :auth_error + end + + test "maps 404 to :not_found" do + assert HttpError.default_kind(404) == :not_found + end + + test "maps other 4xx to :client_error" do + for status <- [400, 402, 405, 422, 499] do + assert HttpError.default_kind(status) == :client_error, + "expected #{status} to be :client_error" + end + end + + test "maps unexpected statuses to :unknown" do + for status <- [200, 302, 600, 999] do + assert HttpError.default_kind(status) == :unknown, + "expected #{status} to be :unknown" + end + end + end +end diff --git a/test/music_library/search_test.exs b/test/music_library/search_test.exs index d486e484..bdf0e82f 100644 --- a/test/music_library/search_test.exs +++ b/test/music_library/search_test.exs @@ -2,6 +2,7 @@ defmodule MusicLibrary.SearchTest do use MusicLibrary.DataCase import MusicLibrary.Fixtures.Records + import MusicLibrary.Fixtures.RecordSets alias MusicLibrary.Search @@ -38,6 +39,49 @@ defmodule MusicLibrary.SearchTest do assert "Marillion" in artist_names end + test "returns record sets matching by name" do + record_set(%{name: "My Jazz Collection"}) + record_set(%{name: "Rock Favorites"}) + + results = Search.universal_search("Jazz") + assert results.record_sets != [] + names = Enum.map(results.record_sets, & &1.name) + assert Enum.any?(names, &String.contains?(&1, "Jazz")) + end + + test "returns record sets matching by description" do + record_set(%{name: "Favorites", description: "All the prog rock classics"}) + record_set(%{name: "Other", description: "Some other set"}) + + results = Search.universal_search("prog rock") + assert results.record_sets != [] + names = Enum.map(results.record_sets, & &1.name) + assert Enum.any?(names, &(&1 == "Favorites")) + end + + test "returns record sets matching by contained record title" do + {set, _records} = record_set_with_records(1, %{name: "Prog", description: "Prog"}) + contained_title = set.items |> List.first() |> Map.get(:record) |> Map.get(:title) + + results = Search.universal_search(contained_title) + assert results.record_sets != [] + end + + test "returns record sets matching by contained artist name" do + # Create a record set with a record that has a unique artist + {_, [record]} = record_set_with_records(1, %{name: "Artist Set", description: "Testing"}) + artist = hd(record.artists) + + results = Search.universal_search(artist.name) + record_set_ids = Enum.map(results.record_sets, & &1.id) + assert record_set_ids != [] + end + + test "returns empty record sets for no match" do + results = Search.universal_search("zzz_nonexistent_record_set_zzz") + assert results.record_sets == [] + end + test "returns empty results for no matches" do results = Search.universal_search("zzz_nonexistent_zzz") @@ -52,18 +96,22 @@ defmodule MusicLibrary.SearchTest do assert length(results.collection) <= 1 assert length(results.wishlist) <= 1 assert length(results.artists) <= 1 + assert length(results.record_sets) <= 1 end end describe "search_counts/1" do setup [:create_records] - test "returns counts per category" do + test "returns counts per category including record sets" do + {_set, _records} = record_set_with_records(1, %{name: "Marillion Favorites"}) + counts = Search.search_counts("Marillion") assert counts.collection_count >= 1 assert counts.wishlist_count >= 1 assert counts.artists_count >= 1 + assert counts.record_sets_count >= 1 end test "returns zero counts for no matches" do @@ -72,6 +120,22 @@ defmodule MusicLibrary.SearchTest do assert counts.collection_count == 0 assert counts.wishlist_count == 0 assert counts.artists_count == 0 + assert counts.record_sets_count == 0 + end + end + + describe "search_record_sets/2" do + test "returns record sets matching by name" do + record_set(%{name: "Best of 2020", description: "Top albums of 2020"}) + + results = Search.search_record_sets("2020") + names = Enum.map(results, & &1.name) + assert "Best of 2020" in names + end + + test "returns empty for non-matching query" do + results = Search.search_record_sets("zzz_nonexistent_zzz") + assert results == [] end end end diff --git a/test/music_library/worker/artist_refresh_wikipedia_data_test.exs b/test/music_library/worker/artist_refresh_wikipedia_data_test.exs index 48f54650..a87e8bb4 100644 --- a/test/music_library/worker/artist_refresh_wikipedia_data_test.exs +++ b/test/music_library/worker/artist_refresh_wikipedia_data_test.exs @@ -39,6 +39,15 @@ defmodule MusicLibrary.Worker.ArtistRefreshWikipediaDataTest do assert Map.has_key?(updated.wikipedia_data, "intro_html") end + test "cancels when no English Wikipedia article exists", %{artist_info: artist_info} do + Req.Test.stub(Wikipedia.API, fn conn -> + Req.Test.json(conn, Wikipedia.Fixtures.wikidata_response_no_enwiki()) + end) + + assert {:cancel, :no_english_wikipedia} = + perform_job(ArtistRefreshWikipediaData, %{"id" => artist_info.id}) + end + test "discards job when no wikidata_id exists in musicbrainz_data" do artist_info = artist_info_fixture(%{musicbrainz_data: %{"name" => "No Wikipedia Artist"}}) diff --git a/test/music_library_web/live/stats_live/index_test.exs b/test/music_library_web/live/stats_live/index_test.exs index 7356645f..50a7fb44 100644 --- a/test/music_library_web/live/stats_live/index_test.exs +++ b/test/music_library_web/live/stats_live/index_test.exs @@ -143,6 +143,132 @@ defmodule MusicLibraryWeb.StatsLive.IndexTest do assert_has(session, "##{record_yesterday.id} h2", escape(record_yesterday.title)) end + + test "groups records with the same musicbrainz_id in the 'On This Day' section", %{ + conn: conn, + collection: collection + } do + today = Date.utc_today() + + # Create a record and a duplicate with the same musicbrainz_id + base_record = + List.first(collection) + |> Records.change_record(%{ + release_date: Date.to_iso8601(today), + purchased_at: DateTime.utc_now() + }) + |> Repo.update!() + + # Create another record with the SAME musicbrainz_id (different purchase) + # Convert Artist structs to plain maps for create_record + artist_maps = Enum.map(base_record.artists, &Map.from_struct/1) + + dup_attrs = %{ + title: base_record.title, + musicbrainz_id: base_record.musicbrainz_id, + release_date: Date.to_iso8601(today), + purchased_at: DateTime.utc_now() |> DateTime.add(1, :second), + artists: artist_maps, + genres: base_record.genres, + format: base_record.format, + type: base_record.type, + cover_url: base_record.cover_url, + cover_hash: base_record.cover_hash, + musicbrainz_data: base_record.musicbrainz_data + } + + {:ok, _dup} = MusicLibrary.Records.create_record(dup_attrs) + + session = conn |> visit("/") + + # The grouped record should appear with the group ID + assert_has(session, "#group-#{base_record.musicbrainz_id}") + # It should show the release count + assert_has(session, "#group-#{base_record.musicbrainz_id}", "2 releases") + end + + test "shows 'Today' label for records released on the current date", %{ + conn: conn, + collection: collection + } do + today = Date.utc_today() + + _record = + List.first(collection) + |> Records.change_record(%{ + release_date: Date.to_iso8601(today), + purchased_at: DateTime.utc_now() + }) + |> Repo.update!() + + session = conn |> visit("/") + + assert_has(session, "span", "Today") + end + + test "shows anniversary label for records released exactly 5 years ago", %{ + conn: conn, + collection: collection + } do + today = Date.utc_today() + # Use year-based adjustment so month-day stays the same + five_years_ago = %{today | year: today.year - 5} + + _record = + List.first(collection) + |> Records.change_record(%{ + release_date: Date.to_iso8601(five_years_ago), + purchased_at: DateTime.utc_now() + }) + |> Repo.update!() + + session = conn |> visit("/") + + assert_has(session, "span", "5 years ago") + end + + test "shows anniversary label for records released exactly 10 years ago", %{ + conn: conn, + collection: collection + } do + today = Date.utc_today() + # Use year-based adjustment so month-day stays the same + ten_years_ago = %{today | year: today.year - 10} + + _record = + List.first(collection) + |> Records.change_record(%{ + release_date: Date.to_iso8601(ten_years_ago), + purchased_at: DateTime.utc_now() + }) + |> Repo.update!() + + session = conn |> visit("/") + + assert_has(session, "span", "10 years ago") + end + + test "shows normal year label for records not on milestone anniversary", %{ + conn: conn, + collection: collection + } do + today = Date.utc_today() + # Use year-based adjustment so month-day stays the same + three_years_ago = %{today | year: today.year - 3} + + _record = + List.first(collection) + |> Records.change_record(%{ + release_date: Date.to_iso8601(three_years_ago), + purchased_at: DateTime.utc_now() + }) + |> Repo.update!() + + session = conn |> visit("/") + + # 3 years is not a milestone (not divisible by 5 or 10) + assert_has(session, "span", "3 years ago") + end end describe "Scrobble activity" do diff --git a/test/music_library_web/live/universal_search_live/index_test.exs b/test/music_library_web/live/universal_search_live/index_test.exs index f26f158c..db07c7a0 100644 --- a/test/music_library_web/live/universal_search_live/index_test.exs +++ b/test/music_library_web/live/universal_search_live/index_test.exs @@ -2,6 +2,13 @@ defmodule MusicLibraryWeb.UniversalSearchLive.IndexTest do use MusicLibraryWeb.ConnCase import MusicLibrary.Fixtures.Records + import MusicLibrary.Fixtures.RecordSets + + import Phoenix.LiveViewTest, + only: [ + render_click: 1, + element: 2 + ] setup do collection_record = record(%{title: "Dark Side of the Moon"}) @@ -12,10 +19,18 @@ defmodule MusicLibraryWeb.UniversalSearchLive.IndexTest do artist_info(artist_record.artists |> List.first() |> Map.get(:musicbrainz_id), %{}) + # Create a record set for navigation testing + {:ok, record_set} = + MusicLibrary.RecordSets.create_record_set(%{ + name: "Prog Favorites", + description: "Best prog albums" + }) + %{ collection_record: collection_record, wishlist_record: wishlist_record, - artist_record: artist_record + artist_record: artist_record, + record_set: record_set } end @@ -125,6 +140,173 @@ defmodule MusicLibraryWeb.UniversalSearchLive.IndexTest do end end + describe "Navigation events" do + setup do + Req.Test.set_req_test_to_shared() + + # Return valid-enough shapes so async task parsers don't crash + Req.Test.stub(LastFm.API, fn conn -> + Req.Test.json(conn, %{ + "artist" => %{ + "name" => "test", + "mbid" => "", + "bio" => %{"summary" => "", "content" => ""}, + "image" => [%{"#text" => "", "size" => "large"}], + "url" => "" + }, + "similarartists" => %{"artist" => []} + }) + end) + + Req.Test.stub(MusicBrainz.API, fn conn -> + Req.Test.json(conn, %{ + "media" => [], + "track-count" => 0 + }) + end) + + on_exit(fn -> + Req.Test.stub(MusicBrainz.API, nil) + Req.Test.stub(LastFm.API, nil) + end) + + :ok + end + + test "navigates to collection record on click", %{ + conn: conn, + collection_record: collection_record + } do + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: collection_record.title) + |> unwrap(fn view -> + view + |> element("div[phx-click='navigate_to_record'][phx-value-type='collection']") + |> render_click() + end) + |> assert_path(~p"/collection/#{collection_record.id}") + end + + test "navigates to wishlist record on click", %{ + conn: conn, + wishlist_record: wishlist_record + } do + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: wishlist_record.title) + |> unwrap(fn view -> + view + |> element("div[phx-click='navigate_to_record'][phx-value-type='wishlist']") + |> render_click() + end) + |> assert_path(~p"/wishlist/#{wishlist_record.id}") + end + + test "navigates to artist on click", %{ + conn: conn, + artist_record: artist_record + } do + artist = List.first(artist_record.artists) + + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: artist.name) + |> unwrap(fn view -> + view + |> element("div[phx-click='navigate_to_artist']") + |> render_click() + end) + |> assert_path(~p"/artists/#{artist.musicbrainz_id}") + end + + test "navigates to record set on click", %{ + conn: conn, + record_set: record_set + } do + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: record_set.name) + |> unwrap(fn view -> + view + |> element("div[phx-click='navigate_to_record_set']") + |> render_click() + end) + |> assert_path(~p"/record-sets/#{record_set.id}") + end + + test "navigates via navigation link to collection chat", %{conn: conn} do + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: "chat") + |> unwrap(fn view -> + view + |> element("div[phx-click='navigate_to_link']") + |> render_click() + end) + |> assert_path(~p"/collection", query_params: %{"chat" => "open"}) + end + + test "record sets appear in search results", %{ + conn: conn, + record_set: record_set + } do + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: record_set.name) + |> assert_has("p", record_set.name) + |> assert_has("h3", "Record Sets") + end + + test "view-all-collection navigates to collection with query param", %{conn: conn} do + # Create 7 records sharing a common prefix so > 5 results exist + Enum.each(1..7, fn i -> + record(%{title: "ViewAllTest #{i}"}) + end) + + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: "ViewAllTest") + |> click_button("button[phx-click='navigate_to_collection']", "View all") + |> assert_path(~p"/collection", query_params: %{"query" => "ViewAllTest"}) + end + + test "view-all-wishlist navigates to wishlist with query param", %{conn: conn} do + # Create 7 wishlist records with a common prefix + Enum.each(1..7, fn i -> + record(%{title: "WishlistAll #{i}", purchased_at: nil}) + end) + + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: "WishlistAll") + |> click_button("button[phx-click='navigate_to_wishlist']", "View all") + |> assert_path(~p"/wishlist", query_params: %{"query" => "WishlistAll"}) + end + + test "view-all-record-sets navigates to record sets with query param", %{conn: conn} do + # Create 7 record sets with a common prefix + Enum.each(1..7, fn i -> + record_set(%{name: "SetAll #{i}"}) + end) + + conn + |> visit(~p"/collection") + |> click_button("Search (Cmd/Ctrl+K)") + |> fill_in("Universal Search", with: "SetAll") + |> click_button("button[phx-click='navigate_to_record_sets']", "View all") + |> assert_path(~p"/record-sets", query_params: %{"query" => "SetAll"}) + end + end + describe "Keyboard navigation" do test "displays keyboard navigation hints when results are present", %{ conn: conn,