From ac64ebef7954b72b32c1f3211bab1e01031d147a Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 19 Jun 2026 10:36:40 -0400 Subject: [PATCH] test(lyrics): remove redundant and cross-layer test assertions A spec-by-spec audit of the lyrics tests found assertions duplicated across layers and within files. Trim them so each test owns one concern: - model/lyrics_embedded_test.go: scope the embedded TTML specs to dispatch + language defaulting; the deep cue/agent/byte details are owned by lyrics_ttml_test.go. - model/lyrics_test.go: drop the no-marker Enhanced LRC spec; the plain-line (nil cues) path is already covered by the mixed Enhanced/plain LRC spec. - model/lyricsfile_test.go: merge the two version-marker rejection specs into a single DescribeTable (same production branch). - model/metadata/map_mediafile_test.go: drop the >32KB TTML mapping test; the tag-length cap is covered in metadata_test.go and TTML parsing in the model lyrics tests. - model/metadata/metadata_test.go: drop the 60KB lyrics-cap test, subsumed by the truncation test that pins the exact ~1MB lyrics cap boundary. No production code changed; coverage is preserved at the correct layer. --- model/lyrics_embedded_test.go | 12 ++------- model/lyrics_test.go | 10 -------- model/lyricsfile_test.go | 23 ++++++++--------- model/metadata/map_mediafile_test.go | 38 ---------------------------- model/metadata/metadata_test.go | 11 -------- 5 files changed, 12 insertions(+), 82 deletions(-) diff --git a/model/lyrics_embedded_test.go b/model/lyrics_embedded_test.go index e370cfaa7..77f17973a 100644 --- a/model/lyrics_embedded_test.go +++ b/model/lyrics_embedded_test.go @@ -28,22 +28,14 @@ var _ = Describe("ParseEmbedded", func() { list, err := ParseEmbedded("ENG", content) + // ParseEmbedded's job is to detect TTML and apply the tag language as the + // default; the parser's cue/agent details are covered in lyrics_ttml_test.go. Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) Expect(list[0].Kind).To(Equal("main")) Expect(list[0].Lang).To(Equal("eng")) Expect(list[0].Synced).To(BeTrue()) - Expect(list[0].Agents).To(Equal([]Agent{{ID: "lead", Role: "main", Name: "Lead Vocal"}})) - Expect(list[0].Line).To(HaveLen(1)) - Expect(list[0].Line[0].Start).To(Equal(new(int64(1000)))) - Expect(list[0].Line[0].End).To(Equal(new(int64(3000)))) Expect(list[0].Line[0].Value).To(Equal("Hello world")) - Expect(list[0].Line[0].Cue).To(HaveLen(2)) - Expect(list[0].Line[0].Cue[0].AgentID).To(Equal("lead")) - Expect(list[0].Line[0].Cue[0].ByteStart).To(Equal(0)) - Expect(list[0].Line[0].Cue[0].ByteEnd).To(Equal(5)) - Expect(list[0].Line[0].Cue[1].ByteStart).To(Equal(6)) - Expect(list[0].Line[0].Cue[1].ByteEnd).To(Equal(10)) }) It("should preserve embedded TTML translation and pronunciation tracks", func() { diff --git a/model/lyrics_test.go b/model/lyrics_test.go index cf7923e89..b772e2f5e 100644 --- a/model/lyrics_test.go +++ b/model/lyrics_test.go @@ -148,16 +148,6 @@ var _ = Describe("ToLyrics", func() { })) }) - It("should ignore Enhanced LRC markers and return plain lines when no markers present", func() { - a, b := int64(1000), int64(3000) - lyrics, err := ToLyrics("xxx", "[00:01.00]Plain line\n[00:03.00]Another plain line") - Expect(err).ToNot(HaveOccurred()) - Expect(lyrics.Line).To(Equal([]Line{ - {Start: &a, Value: "Plain line"}, - {Start: &b, Value: "Another plain line"}, - })) - }) - It("should handle mixed Enhanced and plain LRC lines", func() { lyrics, err := ToLyrics("xxx", "[00:01.00]<00:01.00>Some <00:01.50>lyrics\n[00:03.00]Plain line\n[00:05.00]<00:05.00>More <00:05.50>words") Expect(err).ToNot(HaveOccurred()) diff --git a/model/lyricsfile_test.go b/model/lyricsfile_test.go index c883fc99e..a3588a2ea 100644 --- a/model/lyricsfile_test.go +++ b/model/lyricsfile_test.go @@ -7,23 +7,20 @@ import ( ) var _ = Describe("ParseLyricsfile", func() { - It("returns nil,nil for YAML that does not look like a Lyricsfile", func() { - lyrics, err := ParseLyricsfile("hello: world\n") - Expect(err).ToNot(HaveOccurred()) - Expect(lyrics).To(BeNil()) - }) - - It("returns nil,nil for Lyricsfile-shaped YAML without the version marker", func() { - input := `metadata: + DescribeTable("returns nil,nil for YAML without the Lyricsfile version marker", + func(input string) { + lyrics, err := ParseLyricsfile(input) + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics).To(BeNil()) + }, + Entry("arbitrary YAML", "hello: world\n"), + Entry("Lyricsfile-shaped but unversioned", `metadata: title: 'Looks close' lines: - text: "But should not be claimed" start_ms: 1000 -` - lyrics, err := ParseLyricsfile(input) - Expect(err).ToNot(HaveOccurred()) - Expect(lyrics).To(BeNil()) - }) +`), + ) It("returns an error for invalid YAML", func() { _, err := ParseLyricsfile("not: valid: yaml: [") diff --git a/model/metadata/map_mediafile_test.go b/model/metadata/map_mediafile_test.go index 0770a79f2..75a7ed358 100644 --- a/model/metadata/map_mediafile_test.go +++ b/model/metadata/map_mediafile_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "os" "sort" - "strings" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/metadata" @@ -117,43 +116,6 @@ var _ = Describe("ToMediaFile", func() { sort.Slice(expected, func(i, j int) bool { return expected[i].Lang < expected[j].Lang }) Expect(actual).To(Equal(expected)) }) - - It("should parse embedded TTML lyrics longer than the metadata tag max length", func() { - padding := strings.Repeat(`padding`, 1400) - content := ` - - - - - ` + padding + ` - - - - - -
-

Long embedded TTML line

-
- -` - - // Guards that embedded lyrics longer than the old 32KB tag cap are no - // longer truncated before parsing. Parser correctness lives in the - // model lyrics tests; here we only confirm the full tag is mapped. - Expect(len(content)).To(BeNumerically(">", 32768)) - - mf = toMediaFile(model.RawTags{ - "LYRICS:ENG": {content}, - }) - var actual model.LyricList - err := json.Unmarshal([]byte(mf.Lyrics), &actual) - Expect(err).ToNot(HaveOccurred()) - - Expect(actual).To(HaveLen(1)) - Expect(actual[0].Line).To(ContainElement(model.Line{ - Start: new(int64(1000)), End: new(int64(2500)), Value: "Long embedded TTML line", - })) - }) }) Describe("BPM", func() { diff --git a/model/metadata/metadata_test.go b/model/metadata/metadata_test.go index 7573ac34a..7ebe9fa4a 100644 --- a/model/metadata/metadata_test.go +++ b/model/metadata/metadata_test.go @@ -122,17 +122,6 @@ var _ = Describe("Metadata", func() { Expect(pair[0].Value()).To(HaveLen(1048570)) }) - It("keeps embedded lyrics that exceed the old 32KB cap", func() { - props.Tags = model.RawTags{ - "lyrics:xxx": {strings.Repeat("a", 60000)}, - } - md = metadata.New(filePath, props) - - pair := md.Pairs(model.TagLyrics) - Expect(pair).To(HaveLen(1)) - Expect(pair[0].Value()).To(HaveLen(60000)) - }) - It("should split multiple values", func() { props.Tags = model.RawTags{ "Genre": {"Rock/Pop;;Punk"},