From 835897674b4aaedf4bea15cda59300c34928ed44 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 30 Aug 2026 20:16:29 -0400 Subject: [PATCH] test(artwork): drop specs the retry removal made redundant The artists-first ordering spec asserted the order of a slice literal through an eleven-line mock subclass built only to observe it, and the headstart it guarded barely exists: artists drain in their own pool, so entering the queue first buys them nothing at drain time. The rest were assertions that can no longer discriminate. ReconcileConfigFingerprint holds no queue reference, so asserting it enqueued nothing could not fail. MarkConfigApplied wraps one Property.Put and the reprocess table already covers it end to end. The absent total in the status report is now derived from the source breakdown, so the Sources spec above it asserts the same number by the same path. And both entries of the view-on-absent table hit one branch now that the one-hour threshold is gone; the older one is the guard. Three fingerprint specs asserting one fact about three inputs collapse into a table. Two comments outliving their reason: a fingerprint change no longer re-resolves anything, it reports a stale config. --- cmd/artwork_test.go | 5 -- core/artwork/artwork_test.go | 20 +++----- core/artwork/housekeeping_test.go | 83 +++++++------------------------ 3 files changed, 27 insertions(+), 81 deletions(-) diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index 6a3ade04a..b24fd5d06 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -895,11 +895,6 @@ var _ = Describe("formatStatus", func() { Expect(sources).To(MatchRegexp(`artist\s+absent\s+2`)) }) - It("prints the absent total", func() { - absent := block(formatStatus(rep), "Absent (resolved, no image found)") - Expect(absent).To(MatchRegexp(`artist\s+2`)) - }) - It("says absent states are never retried on their own, and names the command that does", func() { Expect(formatStatus(rep)).To(ContainSubstring("never retried on their own")) Expect(formatStatus(rep)).To(ContainSubstring("artwork reprocess --source absent")) diff --git a/core/artwork/artwork_test.go b/core/artwork/artwork_test.go index 3750805f2..907b300de 100644 --- a/core/artwork/artwork_test.go +++ b/core/artwork/artwork_test.go @@ -204,19 +204,15 @@ var _ = Describe("Artwork", func() { Expect(err).To(MatchError(ErrUnavailable)) }) - DescribeTable("never re-enqueues an absent state on view, however old", - func(id string, attemptedAt time.Time) { - Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ - ItemKind: "al", ItemID: id, AttemptedAt: attemptedAt, - })).To(Succeed()) + It("never re-enqueues an absent state on view, however old", func() { + Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ + ItemKind: "al", ItemID: "al4", AttemptedAt: time.Now().Add(-365 * 24 * time.Hour), + })).To(Succeed()) - _, err := svc.Get(ctx, model.MustParseArtworkID("al-"+id), 0, false) - Expect(err).To(MatchError(ErrUnavailable)) - Expect(queueRepo.Data).To(BeEmpty()) - }, - Entry("just attempted", "al4", time.Now()), - Entry("attempted a year ago", "al4b", time.Now().Add(-365*24*time.Hour)), - ) + _, err := svc.Get(ctx, model.MustParseArtworkID("al-al4"), 0, false) + Expect(err).To(MatchError(ErrUnavailable)) + Expect(queueRepo.Data).To(BeEmpty()) + }) }) Describe("provisional read-through", func() { diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index 9a108a0a9..2027cba9a 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -13,18 +13,6 @@ import ( . "github.com/onsi/gomega" ) -// orderTrackingQueueRepo records the kind of each bulk insert, so tests can assert -// phase ordering (artists-first) that same-priority timestamps can't guarantee. -type orderTrackingQueueRepo struct { - *tests.MockArtworkQueueRepo - callKinds []string -} - -func (o *orderTrackingQueueRepo) EnqueueAllMissing(kind model.Kind, priority int) (int64, error) { - o.callKinds = append(o.callKinds, kind.Prefix()) - return o.MockArtworkQueueRepo.EnqueueAllMissing(kind, priority) -} - var _ = Describe("RefreshableKinds", func() { // The two are meant to describe the same fact. Nothing but this test stops them from drifting, // and a drift would have `artwork explain` report state for a kind that keeps none. @@ -42,7 +30,7 @@ var _ = Describe("Housekeeping", func() { var ( ctx context.Context ds *tests.MockDataStore - queueRepo *orderTrackingQueueRepo + queueRepo *tests.MockArtworkQueueRepo propRepo *tests.MockedPropertyRepo ) @@ -54,34 +42,24 @@ var _ = Describe("Housekeeping", func() { conf.Server.Agents = "spotify" conf.Server.EnableExternalServices = true - queueRepo = &orderTrackingQueueRepo{MockArtworkQueueRepo: tests.CreateMockArtworkQueueRepo()} + queueRepo = tests.CreateMockArtworkQueueRepo() propRepo = &tests.MockedPropertyRepo{} ds = &tests.MockDataStore{MockedArtworkQueue: queueRepo, MockedProperty: propRepo} }) Describe("Fingerprint", func() { - It("changes when a fingerprint-affecting config value changes", func() { - f1 := ConfigFingerprint() - conf.Server.CoverArtPriority = "folder, embedded" - f2 := ConfigFingerprint() - Expect(f1).NotTo(Equal(f2)) - }) + DescribeTable("changes when a fingerprint-affecting config value changes", + func(change func()) { + before := ConfigFingerprint() + change() + Expect(ConfigFingerprint()).NotTo(Equal(before)) + }, + Entry("CoverArtPriority", func() { conf.Server.CoverArtPriority = "folder, embedded" }), + Entry("ArtistImageFolder", func() { conf.Server.ArtistImageFolder = "/after" }), + Entry("EnableM3UExternalAlbumArt", func() { conf.Server.EnableM3UExternalAlbumArt = true }), + ) - It("changes when ArtistImageFolder changes", func() { - conf.Server.ArtistImageFolder = "/before" - f1 := ConfigFingerprint() - conf.Server.ArtistImageFolder = "/after" - Expect(ConfigFingerprint()).NotTo(Equal(f1)) - }) - - It("changes when EnableM3UExternalAlbumArt is toggled", func() { - conf.Server.EnableM3UExternalAlbumArt = false - f1 := ConfigFingerprint() - conf.Server.EnableM3UExternalAlbumArt = true - Expect(ConfigFingerprint()).NotTo(Equal(f1)) - }) - - // Pinned: a changed formula re-resolves every library on upgrade, flooding external providers. + // Pinned: a changed formula tells every existing install its artwork config went stale. It("hashes a given config to a stable value", func() { conf.Server.CoverArtPriority = "cover.*, embedded" conf.Server.ArtistArtPriority = "artist.*, external" @@ -109,7 +87,7 @@ var _ = Describe("Housekeeping", func() { f1 := ConfigFingerprint() consts.Version = original + "-next" Expect(ConfigFingerprint()).To(Equal(f1), - "the version must not invalidate artwork state: it would re-resolve every entity on every build") + "the version must not invalidate artwork state: every build would report a stale config") }) }) @@ -118,7 +96,6 @@ var _ = Describe("Housekeeping", func() { Expect(ReconcileConfigFingerprint(ctx, ds)).To(Succeed()) Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(ConfigFingerprint())) - Expect(queueRepo.Count()).To(BeZero()) }) It("leaves a stale fingerprint stored, so the warning survives a restart", func() { @@ -130,16 +107,6 @@ var _ = Describe("Housekeeping", func() { }) }) - Describe("MarkConfigApplied", func() { - It("overwrites a stale fingerprint with the current one", func() { - Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed()) - - Expect(MarkConfigApplied(ctx, ds)).To(Succeed()) - - Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(ConfigFingerprint())) - }) - }) - Describe("EnqueueMissingAll", func() { var artRepo *tests.MockArtworkRepo @@ -165,23 +132,11 @@ var _ = Describe("Housekeeping", func() { for _, it := range queueRepo.Data { Expect(it.Priority).To(Equal(model.ArtworkPriorityRecheck)) } - Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al2")).ToNot(BeNil()) - Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "pl1")).ToNot(BeNil()) - Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ra", "ra1")).ToNot(BeNil()) - Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al1")).To(BeNil()) - Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar1")).To(BeNil()) - }) - - It("enqueues artists before every other kind", func() { - Expect(enqueueMissingAll(ctx, ds)).To(Succeed()) - - Expect(queueRepo.callKinds).ToNot(BeEmpty()) - firstOther := slices.IndexFunc(queueRepo.callKinds, func(k string) bool { return k != "ar" }) - Expect(firstOther).ToNot(Equal(0), "artists must be the first enqueue call") - if firstOther >= 0 { - Expect(queueRepo.callKinds[firstOther:]).ToNot(ContainElement("ar"), - "no artist enqueue may follow another kind") - } + Expect(findQueued(queueRepo, "al", "al2")).ToNot(BeNil()) + Expect(findQueued(queueRepo, "pl", "pl1")).ToNot(BeNil()) + Expect(findQueued(queueRepo, "ra", "ra1")).ToNot(BeNil()) + Expect(findQueued(queueRepo, "al", "al1")).To(BeNil()) + Expect(findQueued(queueRepo, "ar", "ar1")).To(BeNil()) }) }) })