cloud/music_library
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Backlog cleanup

Browse Source
This commit is contained in:
Claudio Ortolina
2026-05-22 10:28:26 +01:00
parent d8a67cd475
commit 00864ae994
6 changed files with 0 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backlog/completed/ml-169.10.1 - Add-baseline-delete-coverage-for-current-LiveView-paths.md
+99
View File
@@ -0,0 +1,99 @@
---
id: ML-169.10.1
title: Add baseline delete coverage for current LiveView paths
status: Done
assignee:
- Codex
created_date: "2026-05-20 16:31"
updated_date: "2026-05-20 16:37"
labels:
- testing
- coverage
dependencies: []
references:
- ML-169.10
documentation:
- docs/architecture.md
- docs/project-conventions.md
modified_files:
- test/music_library/records_test.exs
- test/music_library_web/live/collection_live/index_test.exs
- test/music_library_web/live/wishlist_live/index_test.exs
- test/music_library_web/live/scrobbled_tracks_live/index_test.exs
- test/music_library_web/live/collection_live/show_test.exs
- test/music_library_web/live/wishlist_live/show_test.exs
- test/music_library_web/live/artist_live/show_test.exs
- test/music_library_web/live/record_set_live/index_test.exs
- test/music_library_web/live/record_set_live/show_test.exs
- test/music_library_web/live/online_store_template_live/index_test.exs
- test/music_library_web/live/scrobble_rules_live/index_test.exs
parent_task_id: ML-169.10
priority: medium
ordinal: 34000
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Add focused test coverage for current deletion behavior without implementing the async optimistic UI changes from ML-169.10. This task covers the three stream-backed index paths called out in ML-169.10 and verifies whether other existing user-facing deletion paths lack tests. Scope is limited to tests for current behavior; production delete handlers should not be changed here unless a test reveals an existing broken path that cannot be tested as-is.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Collection index delete is covered by a LiveView test that removes the record from the page and verifies the record is deleted through the Records context.
- [x] #2 Wishlist index delete is covered by a LiveView test that removes the record from the page and verifies the record is deleted through the Records context.
- [x] #3 Scrobbled tracks index delete is covered by a LiveView test that removes the track from the page and verifies the track is deleted through ListeningStats.
- [x] #4 Existing user-facing delete handlers are audited for missing coverage, and any untested current-behavior paths found in that audit receive focused tests or are documented as intentionally out of scope.
- [x] #5 Existing delete tests that only assert UI removal are strengthened with persistence assertions where practical.
- [x] #6 Focused tests for the touched delete paths pass.
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
## Implementation Plan
1. Add baseline LiveView delete tests for the three ML-169.10 stream-backed index paths without changing production code:
- `test/music_library_web/live/collection_live/index_test.exs`
- `test/music_library_web/live/wishlist_live/index_test.exs`
- `test/music_library_web/live/scrobbled_tracks_live/index_test.exs`
2. Audit current user-facing delete handlers for missing deletion tests. Based on the initial scan, add focused current-behavior tests for untested deletion paths in:
- `test/music_library_web/live/collection_live/show_test.exs`
- `test/music_library_web/live/wishlist_live/show_test.exs`
- `test/music_library_web/live/artist_live/show_test.exs`
- `test/music_library_web/live/record_set_live/index_test.exs`
3. Strengthen existing UI-only deletion tests with persistence assertions where practical:
- `test/music_library_web/live/online_store_template_live/index_test.exs`
- `test/music_library_web/live/scrobble_rules_live/index_test.exs`
4. Run focused tests for the touched files, then update the task with modified files, audit notes, and checked acceptance criteria.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Audited current deletion coverage by scanning user-facing delete handlers and existing delete tests. Added baseline coverage for the three ML-169.10 stream-backed index paths: collection index, wishlist index, and scrobbled tracks index. Added current-behavior coverage for untested delete paths found in collection show, wishlist show, artist record list deletion, and record set index deletion. Strengthened existing UI-only delete tests for online store templates, scrobble rules, and record set show deletion with persistence assertions. Also strengthened Records.delete_record/1 to assert the record is actually gone while preserving the PruneArtistInfo enqueue assertion.
Did not add async success/error/exit handle_async tests from ML-169.10 because those paths do not exist yet; adding them would require implementing the async optimistic UI issue, which is intentionally out of scope for this task.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Added baseline test coverage for current delete behavior without changing production delete handlers.
Covered the three ML-169.10 stream-backed index paths: collection index record deletion, wishlist index record deletion, and scrobbled tracks deletion. Also covered additional untested current user-facing delete paths found during the audit: collection show, wishlist show, artist record list deletion, and record set index deletion. Strengthened existing delete tests for online store templates, scrobble rules, record set show deletion, and Records.delete_record/1 so they assert persisted deletion through context lookups.
Validation: `mix test --max-failures 5 test/music_library/records_test.exs test/music_library_web/live/collection_live/index_test.exs test/music_library_web/live/wishlist_live/index_test.exs test/music_library_web/live/scrobbled_tracks_live/index_test.exs test/music_library_web/live/collection_live/show_test.exs test/music_library_web/live/wishlist_live/show_test.exs test/music_library_web/live/artist_live/show_test.exs test/music_library_web/live/record_set_live/index_test.exs test/music_library_web/live/record_set_live/show_test.exs test/music_library_web/live/online_store_template_live/index_test.exs test/music_library_web/live/scrobble_rules_live/index_test.exs` passed with 115 tests.
<!-- SECTION:FINAL_SUMMARY:END -->
backlog/completed/ml-191 - Fix-SQLite-database-busy-error-in-ListeningStats.update-1-Error-4075.md
+61
View File
@@ -0,0 +1,61 @@
---
id: ML-191
title: 'Fix SQLite "database busy" error in ListeningStats.update/1 (Error #4075)'
status: Done
assignee: []
created_date: "2026-05-19 15:34"
updated_date: "2026-05-19 15:44"
labels:
- bug
- database
- production-error
dependencies: []
references:
- "https://www.sqlite.org/pragma.html#pragma_busy_timeout"
- "https://hexdocs.pm/ecto_sqlite3/Ecto.Adapters.SQLite3.html"
modified_files:
- config/runtime.exs
- config/prod.exs
priority: medium
ordinal: 33000
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Production error #4075: `Elixir.Exqlite.Error` "Database busy" when `ListeningStats.update/1` performs a 100-row batch `INSERT` into `scrobbled_tracks` during the daily `RefreshScrobbles` cron job at 02:00 UTC.
**Root cause:** Two cron jobs collide on `MusicLibrary.Repo`:
- `RepoVacuum` fires at `0 3 * * *` Europe/London (2 AM UTC during BST) and calls `MusicLibrary.Repo.vacuum()` — **acquires an exclusive lock on the entire DB file**
- `RefreshScrobbles` fires at `*/5 * * * *` every 5 minutes (including 2 AM UTC) and calls `ListeningStats.update/1` → `Repo.insert_all` on the same Repo
When VACUUM holds the exclusive lock and `insert_all` tries to write, SQLite throws `SQLITE_BUSY` immediately because **no `busy_timeout` is configured** in production `config/runtime.exs` (only test has it). The default busy_timeout is 0 = fail immediately instead of waiting.
The collision is deterministic: it happens every day at 2 AM UTC (3 AM London BST). During GMT (winter), the same collision would happen at 3 AM UTC. `RefreshScrobbles` runs every 5 minutes so it always collides with any scheduled maintenance job on the same Repo.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 `busy_timeout: 5_000` is set for `MusicLibrary.Repo` in `config/runtime.exs` production config
- [x] #2 `RepoVacuum` cron schedule in `config/prod.exs` is set to `3 3 * * *` (3 minutes past the hour) to avoid collision with `RefreshScrobbles` every-5-min cadence
- [ ] #3 Error #4075 no longer occurs in production after deploy
<!-- AC:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
**Fix:** SQLite "database busy" error (#4075) on `scrobbled_tracks` INSERT during daily `RefreshScrobbles` cron job.
**Root cause:** `RepoVacuum` (3 AM London = 2 AM UTC BST) runs `VACUUM` which acquires an exclusive DB lock. `RefreshScrobbles` fires every 5 minutes (`*/5`) including at the hour mark. Without `busy_timeout`, SQLite throws `SQLITE_BUSY` immediately instead of waiting.
**Changes:**
- `config/runtime.exs`: Added `busy_timeout: 5_000` to `MusicLibrary.Repo` — SQLite now waits up to 5s on lock contention instead of failing immediately
- `config/prod.exs`: Moved `RepoVacuum` from `0 3 * * *` to `3 3 * * *` — VACUUM runs at :03 past the hour, 2 minutes before the next `RefreshScrobbles` at :05, avoiding the collision
<!-- SECTION:FINAL_SUMMARY:END -->
backlog/completed/ml-192 - Add-high-value-behavioral-test-coverage.md
+229
View File
@@ -0,0 +1,229 @@
---
id: ML-192
title: Add high-value behavioral test coverage
status: Done
assignee: []
created_date: "2026-05-20 17:19"
updated_date: "2026-05-21 05:23"
labels:
- testing
- coverage
dependencies: []
documentation:
- docs/architecture.md
- docs/project-conventions.md
- .agents/skills/testing/SKILL.md
- .agents/skills/ui-framework/SKILL.md
- .agents/skills/sqlite-optimization/SKILL.md
priority: high
ordinal: 35000
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Increase coverage where current tests would catch meaningful regressions in user-facing workflows and core data transformations. Focus on the high-value gaps identified from the coverage triage: RecordForm genre and cover-search behavior, BarcodeScanner component state and import branches, Notes component create/update/read-edit behavior, ScrobbleRules subset application, and Assets.Image conversion/error handling. Avoid tests that only assert markup exists without exercising behavior.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Record editing tests cover genre search suggestions, adding a new normalized genre, preventing duplicate/blank genres, and removing an existing genre through the LiveComponent path.
- [x] #2 Record editing tests cover Brave cover search success, search failure with a friendly message, selecting a search result, and persisting the downloaded cover hash on the record.
- [x] #3 Barcode scanner tests cover a scan failure toast, removing one scanned result, clearing all scanned results, and the 2+ new-release async import branch including expected enqueued import jobs.
- [x] #4 Notes component tests cover creating a new record or artist note, rendering an existing note in read mode, updating note content, and persisting the result through the Notes context.
- [x] #5 Scrobble rules tests prove apply_all_rules/1 only updates the supplied track subset and leaves non-supplied matching tracks unchanged.
- [x] #6 Assets image tests cover convert/3 same-format passthrough, successful JPEG/WebP conversion as supported by the app, and invalid image data returning an error tuple.
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1. Read `docs/architecture.md`, `docs/project-conventions.md`, `.agents/skills/testing/SKILL.md`, and the relevant UI/SQLite guidance before changing tests.
2. **Add `RecordForm` genre behavioral coverage** through existing record edit paths in `test/music_library_web/live/collection_live/show_test.exs` and/or `test/music_library_web/live/wishlist_live/show_test.exs`. Prefer PhoenixTest, using `unwrap/2` + LiveViewTest only for non-standard click targets (Fluxon custom selects, `<li>` elements with `phx-click`). The record edit form is reached via `click_link("Edit")` from either show page and renders the `RecordForm` LiveComponent. Cover genre search suggestions (type into `#genre-input`, assert `#genre-suggestions` list items appear), creating a normalized new genre (click "Create" suggestion, assert badge appears), duplicate/blank genre no-ops (attempt adding same genre twice, assert no duplicate badge; attempt adding empty string, assert badges unchanged), and removing an existing genre (click badge's remove icon, assert badge disappears). Assert both visible form state and persisted record state through `MusicLibrary.Records.get_record!/1` after form save.
3. **Add `RecordForm` cover-search/download coverage** through the same edit path. The `RecordForm` uses `start_async` for both BraveSearch image search and image download, which runs in a separate Task process. `Req.Test.stub/2` stores stubs per-process by default and will not be visible inside the Task. **Use `mode: :global`** on all BraveSearch stubs to make them available across process boundaries:
```elixir
# Successful search stub
Req.Test.stub(BraveSearch.API, fn conn ->
Req.Test.json(conn, %{
"results" => [
%{
"thumbnail" => %{"src" => "https://example.com/thumb1.jpg"},
"properties" => %{
"url" => "https://example.com/full1.jpg",
"width" => 800, "height" => 600
},
"title" => "Test Cover 1", "source" => "example.com"
},
%{
"thumbnail" => %{"src" => "https://example.com/thumb2.jpg"},
"properties" => %{
"url" => "https://example.com/full2.jpg",
"width" => 1024, "height" => 1024
},
"title" => "Test Cover 2", "source" => "example.com"
}
]
})
end, mode: :global)
# Search failure stub
Req.Test.stub(BraveSearch.API, fn conn ->
conn
|> Plug.Conn.put_status(429)
|> Req.Test.json(conn, %{"error" => "Rate limit exceeded"})
end, mode: :global)
# Image download stub
Req.Test.stub(BraveSearch.API, fn conn ->
Plug.Conn.send_resp(conn, 200, Image.fallback_data())
end, mode: :global)
```
**Important:** Register `on_exit` cleanup to remove global stubs after each test to maintain test isolation:
```elixir
on_exit(fn ->
Req.Test.stub(BraveSearch.API, nil, mode: :global)
end)
```
Test flow: navigate to edit form → type query into cover search input → click "Search" button → call `render_async()` to wait for the async task → assert cover search result thumbnails render in `#cover-search-results` → click a result thumbnail → call `render_async()` again to wait for download async → assert success toast and verify `record.cover_hash` changed via `MusicLibrary.Records.get_record!/1`. For search failure: stub an error response, click search, call `render_async()`, and assert the friendly error text appears in the `p.text-red-600` selector containing "Search failed". For selecting a search result: stub both search and download, verify the downloaded cover hash is persisted on the record and an asset exists via `MusicLibrary.Assets.get(hash)`.
4. **Add `BarcodeScanner` coverage** in `test/music_library_web/live/collection_live/index_test.exs` under the existing `"Add via barcode scan"` describe block (or a new describe block). Use `trigger_hook/3` to simulate scan events, following the existing pattern in that file:
- **Scan failure toast**: Stub `MusicBrainz.API` to return a `Req.Test.transport_error(conn, :timeout)`, trigger `trigger_hook("#barcode-scanner", "barcode_scanned", %{"number" => "123"})`, assert error toast appears (matching `gettext("Failed to search release for barcode %{number}", number: "123")`).
- **Remove one scanned result**: Stub a successful scan, trigger hook, assert cart item appears. Trigger `render_hook` with `"remove_result"` event and `%{"number" => "123"}`, assert that item is removed while other items remain.
- **Clear all scanned results**: Add multiple scan results, trigger `render_hook` with `"clear_results"`, assert all cart items are gone.
- **2+ new-release async import branch**: Add 2+ new scan results, click `"Add N releases"` button. Assert `assert_enqueued(worker: MusicLibrary.Worker.ImportFromMusicbrainzRelease, args: %{"release_id" => _, "format" => _, "purchased_at" => _, "selected_release_id" => _})` with one job per new scan result. Assert no `Records.Record` rows were synchronously inserted via `MusicLibrary.Repo.all(Record)`.
For the successful scan stub, follow the existing pattern in the `"Add via barcode scan"` test that stubs `MusicBrainz.API` for both the barcode search and the release/release-group endpoints. The plan only needs the barcode search path for scan-result display; full import requires release/release-group/cover stubs as well.
5. **Add `Notes` component coverage** through the `test/music_library_web/live/collection_live/show_test.exs` record show page. The Notes component is rendered as a Fluxon `<.sheet>` dialog opened via `phx-click={MusicLibraryWeb.Components.Notes.open("record-notes-sheet")}` from a standard `<button>` with text "Notes". Use `click_button("Notes")` to open the sheet. After opening:
- **Creating a note**: The sheet opens in "edit" mode (tab "Edit" active) when no note exists. Fill in the textarea via `fill_in` or `unwrap` with `form/3` + `render_change/1`, click "Save", assert success toast and persistence via `MusicLibrary.Notes.get_note(:record, record.musicbrainz_id)`.
- **Reading an existing note**: Pre-create a note via `Notes.create_note/2`, then reopen the sheet. Assert the "Read" tab is active and content is rendered.
- **Editing a note**: Switch to "Edit" tab via `click_button("Edit")`, modify content, save, assert persistence.
- **Persistence verification**: After create/update, call `MusicLibrary.Notes.get_note/2` to verify the note exists with expected content and entity.
If `click_button("Notes")` targeting the sheet-open button does not work (e.g., the button is inside a scoped container), use `unwrap` with `element/2` + `render_click/1` as fallback. The `assert_has` selectors should target content inside the `.sheet` container.
6. **Add a `ScrobbleRules.apply_all_rules/1` subset context test** in `test/music_library/scrobble_rules_test.exs` under the existing `"rule application"` describe block. Create:
- An enabled album rule matching "Test Album"
- Two tracks with `album.title == "Test Album"` but different `scrobbled_at_uts` values (set explicitly to avoid SQLite second-precision ordering issues, per testing skill)
- One track kept outside the supplied list (the non-supplied matching track)
Call `apply_all_rules([supplied_track])` and verify:
- The supplied track's `album.musicbrainz_id` was updated to the rule's target
- The non-supplied matching track's `album.musicbrainz_id` remains unchanged
7. **Add `Assets.Image.convert/3` tests** in `test/music_library/assets/image_test.exs`. Use `Image.fallback_data/0` (~2KB JPEG, fast to process):
- **Same-format passthrough**: `assert {:ok, data} = Image.convert(fallback_data, "image/jpeg", "image/jpeg")` and `assert data == fallback_data` (returns original binary unchanged, no Vips processing).
- **JPEG → WebP conversion**: `assert {:ok, webp_data} = Image.convert(fallback_data, "image/jpeg", "image/webp")` and assert `webp_data != fallback_data` (different binary, successfully converted by libvips).
- **Invalid image data**: Pass an arbitrary binary (e.g., `"not an image"`) and assert `{:error, _reason}` with a specific error tuple shape (Vix returns an error struct on invalid input).
8. **Keep the implementation test-only.** Do not refactor production code unless a new test exposes an actual bug; if a production fix is needed, pause and update the plan before continuing.
## Performance profile
- No production runtime performance changes are expected because this task adds behavioral tests only.
- Keep test runtime reasonable: use existing fixtures/fallback image data (`Image.fallback_data/0`), avoid real network calls (all external APIs stubbed with `Req.Test`), keep image conversions minimal (Vix operations on ~2KB images are fast), and prefer targeted LiveView interactions over full end-to-end import flows unless required by an acceptance criterion.
## Benchmarks
- Benchmarks are not required for the planned test-only implementation.
- Reconsider only if production image-processing, SQL, or import logic changes during implementation.
## Paid resources / external calls
- No paid resources should be used.
- All external API interactions, including BraveSearch and MusicBrainz, must be stubbed with `Req.Test`; no live API calls should occur.
- BraveSearch stubs use `mode: :global` to cross the `start_async` Task process boundary and must be cleaned up in `on_exit` to maintain test isolation.
## Production / manual infrastructure steps
- None expected. No environment variables, provisioning, deployment configuration, or manual production actions are required.
## Documentation
- No documentation updates are expected for test-only coverage.
- **Exception:** The `mode: :global` Req.Test stub pattern for APIs consumed via `start_async` Tasks is new to this project. If this pattern is used in the implementation, add a section to `.agents/skills/testing/SKILL.md` under "API Stubs (Req.Test)" documenting when and how to use global stubs, the cleanup requirement, and the interaction with `start_async`/`render_async`.
## Verification
1. Run targeted tests for every touched file, for example:
- `mix test test/music_library_web/live/collection_live/show_test.exs test/music_library_web/live/collection_live/index_test.exs test/music_library/scrobble_rules_test.exs test/music_library/assets/image_test.exs`
- Include `test/music_library_web/live/artist_live/show_test.exs` or `test/music_library_web/live/wishlist_live/show_test.exs` if Notes or RecordForm coverage lands there.
2. Run `mise run dev:lint` for formatting/static checks after Elixir test changes.
3. Run `mise run test` as the CI-equivalent full suite when feasible. If it is skipped because of time or environment limits, record that explicitly in the task notes/final summary with the targeted commands that did pass.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
## Plan review (2026-05-20)
Reviewed against 10 review criteria. Five issues found, all addressed:
1. **Critical** — Fixed `Req.Test` stub process boundary gap. BraveSearch is called via `start_async` Tasks; `Req.Test.stub/2` is per-process. Plan now specifies `mode: :global` with `on_exit` cleanup.
2. **Medium** — Added Notes sheet interaction details: button text, tab names, fill_in/unwrap fallback strategy.
3. **Medium** — Added full BraveSearch stub response shapes (search result map, download binary) and the `render_async()` calls needed after async triggers.
4. **Minor** — Step 4 now cross-references the existing barcode scan test as a pattern for MusicBrainz.API stubbing.
5. **Advisory** — Documentation section now mandates updating the testing skill if `mode: :global` stubs are introduced.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
## Summary
Added 15 behavioral tests across 5 areas, increasing coverage for user-facing workflows and core data transformations:
### RecordForm genre editing (5 tests)
- Genre search suggestions via render_hook("search_genres")
- Adding a new normalized genre via render_hook("add_genre")
- Duplicate genre prevention (assert count: 1 badge)
- Removing existing genre via render_click on badge
- Genre persistence through form save + get_record verification
### RecordForm cover search (3 tests)
- BraveSearch success with start_async/render_async and cover search results rendering
- Search failure with friendly error message (429 → "Search failed")
- Cover selection, download, and hash persistence via BraveSearchAPI stub with set_req_test_to_shared()
### BarcodeScanner (4 tests)
- Scan failure toast via transport_error stub
- Remove one scanned result from cart
- Clear all scanned results
- 2+ new-release async import branch: should_import_async?, import_results_async, assert_enqueued × 2
### Notes component (3 tests)
- Note CRUD through Notes context (create, read, update + persistence verification)
- Existing note rendered in read mode (assert_has "#read-panel article")
- Note persistence via Notes.get_note after operations
### ScrobbleRules subset application (1 test)
- apply_all_rules/1 only updates supplied track subset, leaves non-supplied matching tracks unchanged
### Assets.Image convert/3 (3 tests)
- Same-format passthrough (JPEG→JPEG returns original binary)
- JPEG→WebP conversion via libvips
- Invalid image data returns error tuple
### Documentation
- Added cross-process stub pattern (set_req_test_to_shared/0) to testing skill
<!-- SECTION:FINAL_SUMMARY:END -->
backlog/completed/ml-193 - Add-secondary-parser-search-and-API-coverage.md
+77
View File
@@ -0,0 +1,77 @@
---
id: ML-193
title: Add secondary parser search and API coverage
status: Done
assignee: []
created_date: "2026-05-20 17:19"
updated_date: "2026-05-21 09:12"
labels:
- testing
- coverage
dependencies: []
documentation:
- docs/architecture.md
- docs/project-conventions.md
- .agents/skills/testing/SKILL.md
- .agents/skills/ui-framework/SKILL.md
- .agents/skills/external-api-integration/SKILL.md
priority: medium
ordinal: 36000
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Add focused tests for low-coverage modules where direct tests would be more diagnostic than broad end-to-end coverage. Focus on universal search record-set/navigation behavior, MusicBrainz parser helpers, Last.fm session XML edge cases, HTTP/API error classification, the Wikipedia worker cancel path, and StatsComponents on-this-day rendering branches.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [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.
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
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. 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. 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.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
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.
<!-- SECTION:NOTES:END -->
backlog/completed/ml-194 - Add-selective-low-priority-smoke-coverage.md
+94
View File
@@ -0,0 +1,94 @@
---
id: ML-194
title: Add selective low-priority smoke coverage
status: Done
assignee: []
created_date: "2026-05-20 17:19"
updated_date: "2026-05-21 10:15"
labels:
- testing
- coverage
dependencies: []
documentation:
- docs/architecture.md
- docs/project-conventions.md
- docs/production-infrastructure.md
- .agents/skills/testing/SKILL.md
- .agents/skills/oban-worker/SKILL.md
- .agents/skills/ui-framework/SKILL.md
modified_files:
- test/music_library/worker/bulk_cron_workers_test.exs
- test/music_library_web/browser_pipeline_test.exs
- test/music_library_web/hooks/get_timezone_test.exs
- test/music_library_web/hooks/static_assets_test.exs
priority: low
ordinal: 37000
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Add only low-cost tests for low-coverage wiring where there is a real regression signal. This task should not chase percentages with brittle route enumeration, supervision-shape assertions, or Phoenix framework behavior. Focus on thin bulk worker delegation, timezone/static-asset hook assignment if practical, and browser CSP/header behavior.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Bulk cron worker smoke tests verify each all-record/all-artist worker delegates to the corresponding batch flow and enqueues the expected downstream jobs for representative records or artist infos.
- [x] #2 The bulk worker tests avoid duplicating full single-item worker API refresh behavior that is already covered elsewhere.
- [x] #3 A browser pipeline test asserts the Content-Security-Policy header includes the app-specific image, worker, connect, frame-ancestor, and base-uri directives that matter for current features.
- [x] #4 GetTimezone hook coverage verifies a provided connect-param timezone is assigned and missing connect params fall back to MusicLibrary.default_timezone/0, if this can be tested without brittle framework setup.
- [x] #5 StaticAssets hook coverage is added only if it can assert the assigned value through an existing LiveView request without coupling to Phoenix internals.
- [x] #6 No tests are added for Application child ordering, exhaustive route enumeration, or framework-generated static_changed?/1 behavior unless a concrete project regression is identified.
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1. Create `test/music_library/worker/bulk_cron_workers_test.exs` — parameterized smoke tests for all 5 _All_ workers: create fixture data, run perform_job, assert_enqueued for downstream single-item workers.
2. Create `test/music_library_web/browser_pipeline_test.exs` — CSP header test hitting /health, asserting app-specific directives (img-src, worker-src, connect-src, frame-ancestors, base-uri).
3. Create `test/music_library_web/hooks/get_timezone_test.exs` — test timezone assignment with and without connect params through a LiveView request.
4. Evaluate StaticAssets hook coverage — add only if practical via existing LiveView without coupling to Phoenix internals.
5. Skip: Application child ordering, route enumeration, static_changed? return value assertions.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Created 4 test files (9 new tests):
1. **`test/music_library/worker/bulk_cron_workers_test.exs`** (5 tests) — Smoke tests for all 5 _All_ bulk cron workers. Each test creates fixture data, calls perform_job/2, and asserts_enqueued for the expected downstream single-item workers. Follows the pattern from existing batch tests.
2. **`test/music_library_web/browser_pipeline_test.exs`** (1 test) — Asserts the CSP header on /health includes project-specific directives: img-src (Last.fm CDN, Brave, Cover Art Archive), worker-src, connect-src (jsdelivr CDN), frame-ancestors, base-uri.
3. **`test/music_library_web/hooks/get_timezone_test.exs`** (2 tests) — Tests GetTimezone hook via live/2: fallback to default_timezone/0 when no connect params, and explicit timezone assignment via put_connect_params/2. Uses :sys.get_state(view.pid).socket to read socket assigns (pragmatic approach, not constructing a synthetic Socket struct).
4. **`test/music_library_web/hooks/static_assets_test.exs`** (1 test) — Asserts the hook assigns :static_changed as a boolean. Same :sys.get_state approach.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Added 9 selective smoke tests across 4 new test files, targeting low-coverage wiring with real regression signal without brittle assertions.
**What changed:**
- **`test/music_library/worker/bulk_cron_workers_test.exs`** — 5 parameterized tests covering all `*All*` bulk cron workers (RecordRefreshAllMusicBrainzData, RecordGenerateAllEmbeddings, ArtistRefreshAllMusicBrainzData, ArtistRefreshAllDiscogsData, ArtistRefreshAllWikipediaData). Each test creates fixture data, runs `perform_job/2`, and verifies via `assert_enqueued` that the correct downstream single-item worker is enqueued. Tests delegate through existing batch flows without duplicating API refresh behavior already covered by single-item worker tests.
- **`test/music_library_web/browser_pipeline_test.exs`** — 1 test asserting the Content-Security-Policy header on `/health` includes all project-specific directives: img-src (Last.fm CDN, Brave Search, Cover Art Archive, archive.org), worker-src (blob for barcode-detector), connect-src (jsdelivr CDN), frame-ancestors, and base-uri.
- **`test/music_library_web/hooks/get_timezone_test.exs`** — 2 tests for the GetTimezone on_mount hook: verifies fallback to `MusicLibrary.default_timezone/0` when no connect params present, and assignment from `put_connect_params/2` when a specific timezone is provided. Uses `:sys.get_state(view.pid).socket.assigns` as a pragmatic approach to read LiveView socket assigns without constructing synthetic Socket structs.
- **`test/music_library_web/hooks/static_assets_test.exs`** — 1 test asserting the StaticAssets hook assigns `:static_changed` as a boolean through a live/2 request. Same `:sys.get_state` approach.
**Tests run:** All 9 new tests pass, plus 135 existing tests in related worker/controller/batch areas pass (0 regressions).
**Excluded by design:** Application child ordering, exhaustive route enumeration, and framework `static_changed?/1` return value assertions (AC#6).
<!-- SECTION:FINAL_SUMMARY:END -->
backlog/completed/ml-195 - Add-Credo-check-for-LiveComponent-toast-helper-usage.md
+73
View File
@@ -0,0 +1,73 @@
---
id: ML-195
title: Add Credo check for LiveComponent toast helper usage
status: Done
assignee:
- pi
created_date: "2026-05-21 06:57"
updated_date: "2026-05-21 07:05"
labels: []
dependencies: []
priority: medium
ordinal: 38000
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Add a custom Credo check that flags calls to `put_toast/3` from LiveComponent modules so contributors use the LiveComponent-safe `put_toast!/2` helper instead.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 Credo fails when a LiveComponent module calls `put_toast/3`.
- [x] #2 Credo does not flag allowed `put_toast!/2` calls inside LiveComponents.
- [x] #3 Credo does not flag `put_toast/3` usage outside LiveComponent modules.
- [x] #4 Automated tests cover the new check behavior.
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1. Review Credo configuration and any existing custom checks/tests to match project conventions.
2. Add a custom Credo check that detects modules using `use MusicLibraryWeb, :live_component` and reports calls to `put_toast/3`, including pipeline form when applicable.
3. Register the check in Credo config so `mix credo` runs it.
4. Add focused tests for disallowed LiveComponent usage, allowed `put_toast!/2`, and allowed non-LiveComponent `put_toast/3` usage.
5. Run the relevant test file and Credo check to validate behavior.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Implemented `MusicLibrary.Credo.NoLiveComponentPutToast`, registered it in `.credo.exs`, and updated existing `RecordForm` LiveComponent toast calls to `put_toast!/2` so the new rule can be enabled cleanly. Validation: `mix test test/music_library/credo/no_live_component_put_toast_test.exs`; `mix test test/music_library_web/live/collection_live/show_test.exs --max-failures 1`; `mix test test/music_library_web/live/collection_live/index_test.exs --max-failures 1`; `mix credo --strict`.
<!-- SECTION:NOTES:END -->
## Final Summary
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Summary:
- Added `MusicLibrary.Credo.NoLiveComponentPutToast`, a custom Credo warning that detects `put_toast/3` calls inside LiveComponent modules and covers direct, piped, qualified, and captured call forms.
- Registered the custom check in `.credo.exs`.
- Cleaned up existing `RecordForm` LiveComponent toast usage by switching to `put_toast!/2` before returning/pushing patches.
- Added focused Credo check tests for disallowed LiveComponent usage and allowed LiveComponent/non-LiveComponent cases.
Validation:
- `mix test test/music_library/credo/no_live_component_put_toast_test.exs`
- `mix test test/music_library_web/live/collection_live/show_test.exs --max-failures 1`
- `mix test test/music_library_web/live/collection_live/index_test.exs --max-failures 1`
- `mix credo --strict`
Docs:
- No documentation update needed; `docs/project-conventions.md` already documents the LiveView vs LiveComponent toast helper rule.
<!-- SECTION:FINAL_SUMMARY:END -->