From 412b9e37ebdb51c469116e196dae1d6a91cda50b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 17 May 2026 22:27:01 -0400 Subject: [PATCH] test(coverart): no-MBID enrich settles 'none' (per canonical behavior) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator decision: the enricher is canonical. No MBID still runs the provider chain (name-based providers — Deezer/Last.fm — resolve without an MBID); if every provider returns ErrNotFound the row settles cover/artist source 'none' at the current sources version (re-eligible only when the registered provider set changes). It does NOT skip-and-leave-NULL. The two _NoMBID_LeavesNull tests predated the name-based providers (0020 slice) and asserted the old skip→NULL contract. Updated: - TestEnrichArtist_NoMBID_SettlesNone: stub now returns ErrNotFound (realistic MBID-only-provider-with-empty-MBID), expect source 'none'. - TestEnrichAlbum_NoSidecarNoMBID_SettlesNone: empty registry → allWere404 stays true → expect 'none'. Last failing cluster from the CI-integration initiative; suite should now be fully green. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/coverart/artist_enricher_test.go | 12 ++++++++---- internal/coverart/enricher_test.go | 9 ++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/coverart/artist_enricher_test.go b/internal/coverart/artist_enricher_test.go index 352214ed..2751eb49 100644 --- a/internal/coverart/artist_enricher_test.go +++ b/internal/coverart/artist_enricher_test.go @@ -496,7 +496,7 @@ func TestCleanupArtistArt_IdempotentMissingDir(t *testing.T) { // --- MBID guard --- -func TestEnrichArtist_NoMBID_LeavesNull(t *testing.T) { +func TestEnrichArtist_NoMBID_SettlesNone(t *testing.T) { pool := newPool(t) ctx := context.Background() q := dbq.New(pool) @@ -504,9 +504,13 @@ func TestEnrichArtist_NoMBID_LeavesNull(t *testing.T) { resetRegistryForTests() t.Cleanup(resetRegistryForTests) + // No MBID still runs the provider chain (name-based providers can + // resolve without one). An MBID-only provider returns ErrNotFound + // for an empty MBID; with the whole chain returning ErrNotFound the + // row settles 'none' (version-stamped), NOT NULL. stub := &stubArtistProvider{ fakeProvider: fakeProvider{id: "stub-artist", defaultOn: true}, - thumb: []byte("should_not_write"), + err: ErrNotFound, } Register(stub) @@ -523,7 +527,7 @@ func TestEnrichArtist_NoMBID_LeavesNull(t *testing.T) { if err != nil { t.Fatalf("GetArtistByID: %v", err) } - if row.ArtistArtSource != nil { - t.Errorf("source = %v, want nil (no MBID — skip)", row.ArtistArtSource) + if row.ArtistArtSource == nil || *row.ArtistArtSource != "none" { + t.Errorf("source = %v, want 'none' (settled)", row.ArtistArtSource) } } diff --git a/internal/coverart/enricher_test.go b/internal/coverart/enricher_test.go index 27032762..36e8c38a 100644 --- a/internal/coverart/enricher_test.go +++ b/internal/coverart/enricher_test.go @@ -121,7 +121,7 @@ func TestEnrichAlbum_SidecarFound(t *testing.T) { } } -func TestEnrichAlbum_NoSidecarNoMBID_LeavesNull(t *testing.T) { +func TestEnrichAlbum_NoSidecarNoMBID_SettlesNone(t *testing.T) { pool := newPool(t) id, _ := seedAlbumWithTrack(t, pool, "NoMbid", "Artist", "") resetRegistryForTests() // deterministic empty registry (no provider can supply) @@ -134,8 +134,11 @@ func TestEnrichAlbum_NoSidecarNoMBID_LeavesNull(t *testing.T) { if err != nil { t.Fatalf("get: %v", err) } - if row.CoverArtSource != nil { - t.Errorf("source = %v, want nil (NULL — eligible for retry on next scan)", row.CoverArtSource) + // No sidecar, no MBID, no provider yields → the chain settles + // 'none' (allWere404 stays true), version-stamped so it is only + // re-tried when the registered provider set changes. + if row.CoverArtSource == nil || *row.CoverArtSource != "none" { + t.Errorf("source = %v, want 'none' (settled)", row.CoverArtSource) } }