Files
minstrel/internal/library/genre_test.go
T
bvandeusen 8e1d25a772
test-go / test (push) Successful in 53s
test-go / integration (push) Successful in 5m2s
fix(scanner): repair acronym and apostrophe casing on genre tags — #2468
Operator decision: keep the ID3v1 table canonical, fix the casing.

The operator's library carries "Edm", "Idm", "Aor", "Uk Garage", "Uk
Hardcore", "Trap Edm", "Glitch Hop Edm" and "Children'S Music" — an external
tag editor title-cased the whole genre field. The "'S" is the giveaway.

Fixed at SCAN time, not in the display layer: taste_profile.sql reads
tracks.genre directly, so a cosmetic-only fix would leave the taste
vocabulary holding "Edm" while the UI showed "EDM", and any correctly
tagged file would contribute a second, separate tag.

trueUpCasing only ever changes case, never letters, so it cannot silently
turn one genre into a different one — that is what separates it from the
label-remapping idea this task rejected. Two narrow rules:

- A short, evidence-led acronym list, matched case-insensitively so "edm",
  "Edm" and "EDM" all land on "EDM". This is a deliberate exception to the
  project's rule that genre case is exposed as the file says it: "Rock" and
  "rock" still stay separate rows, because folding those is a judgement about
  labels, whereas there is no genre named "Edm".
- Apostrophe suffixes from a FIXED contraction list, so "Children'S" is
  repaired while "O'Brien" and "D'Angelo" keep their capital. A blanket
  "lowercase after an apostrophe" would have broken both.

Matching uses the word's letter core rather than the raw word, so "(Edm)"
and "Edm," are repaired and their punctuation re-attached. Interior
punctuation stays in the core, so "Lo-Fi" and "R&B" are compared whole and
cannot match a fragment by accident. My first version missed this and a test
expecting "(Live EDM)" caught it.

Names resolved from the ID3v1 table are deliberately NOT re-cased, per the
operator's call — entry 40's "AlternRock" stays as the table spells it, with
a test pinning that so a later tidy-up doesn't quietly "fix" it.

tagReadVersion 1 -> 2, so this reaches the existing library on the next scan
rather than new files only. That re-read reuses stored durations, so it costs
tag reads and no ffprobe.
2026-08-07 14:43:36 -04:00

521 lines
17 KiB
Go

package library
import (
"bytes"
"encoding/binary"
"strings"
"testing"
"github.com/dhowden/tag"
)
// rawFrame is a frame with a byte-exact payload, so tests can express encoding
// bytes and embedded nulls that a string-keyed helper can't.
type rawFrame struct {
id string
payload []byte
}
// buildID3v2 assembles a tag for the given major version. Frame size encoding
// differs per version (2.4 is synchsafe, 2.2/2.3 are plain), which is exactly
// the kind of detail a parser gets subtly wrong, so tests build all three.
func buildID3v2(t *testing.T, major byte, frames ...rawFrame) []byte {
t.Helper()
var body bytes.Buffer
for _, f := range frames {
switch major {
case 2:
if len(f.id) != 3 {
t.Fatalf("v2.2 frame id %q must be 3 bytes", f.id)
}
body.WriteString(f.id)
n := len(f.payload)
body.Write([]byte{byte(n >> 16), byte(n >> 8), byte(n)})
case 3:
body.WriteString(f.id)
_ = binary.Write(&body, binary.BigEndian, uint32(len(f.payload)))
body.Write([]byte{0x00, 0x00})
case 4:
body.WriteString(f.id)
body.Write(synchsafeBytes(len(f.payload)))
body.Write([]byte{0x00, 0x00})
}
body.Write(f.payload)
}
var out bytes.Buffer
out.WriteString("ID3")
out.Write([]byte{major, 0x00, 0x00})
out.Write(synchsafeBytes(body.Len()))
out.Write(body.Bytes())
// A few bytes of MPEG sync so dhowden/tag accepts the file shape.
out.Write([]byte{0xFF, 0xFB, 0x90, 0x00})
return out.Bytes()
}
func synchsafeBytes(n int) []byte {
return []byte{
byte((n >> 21) & 0x7F),
byte((n >> 14) & 0x7F),
byte((n >> 7) & 0x7F),
byte(n & 0x7F),
}
}
// utf8Frame builds a text-frame payload: encoding byte 3 (UTF-8) followed by
// values joined with the null separator ID3v2 uses for multiple values.
func utf8Frame(values ...string) []byte {
return append([]byte{0x03}, []byte(strings.Join(values, "\x00"))...)
}
// TestReadID3v2GenreValues_MultiValue is the #2499 regression. dhowden/tag
// rejoins these values with the empty string, producing "Alternative RockRock";
// the whole point of our own reader is that they stay separate.
func TestReadID3v2GenreValues_MultiValue(t *testing.T) {
for _, major := range []byte{2, 3, 4} {
id := "TCON"
if major == 2 {
id = "TCO"
}
data := buildID3v2(t, major, rawFrame{id, utf8Frame("Alternative Rock", "Rock")})
got, err := readID3v2GenreValues(bytes.NewReader(data))
if err != nil {
t.Fatalf("v2.%d: %v", major, err)
}
want := []string{"Alternative Rock", "Rock"}
if !equalStrings(got, want) {
t.Errorf("v2.%d genres = %q, want %q", major, got, want)
}
}
}
// The operator's worst case: eight values welded into one 70-character token.
func TestReadID3v2GenreValues_ManyValues(t *testing.T) {
values := []string{
"Boom Bap", "Downtempo", "Hip Hop", "Instrumental",
"Lo-Fi", "Lo-Fi Hip Hop", "Chillwave", "Instrumental Hip Hop",
}
data := buildID3v2(t, 4, rawFrame{"TCON", utf8Frame(values...)})
got, err := readID3v2GenreValues(bytes.NewReader(data))
if err != nil {
t.Fatal(err)
}
if !equalStrings(got, values) {
t.Errorf("genres = %q, want %q", got, values)
}
}
// A trailing null terminator is legal and must not produce an empty value.
func TestReadID3v2GenreValues_TrailingTerminator(t *testing.T) {
data := buildID3v2(t, 4, rawFrame{"TCON", append(utf8Frame("Jazz"), 0x00)})
got, err := readID3v2GenreValues(bytes.NewReader(data))
if err != nil {
t.Fatal(err)
}
if !equalStrings(got, []string{"Jazz"}) {
t.Errorf("genres = %q, want [Jazz]", got)
}
}
// UTF-16 uses a TWO-byte separator. Splitting it on single nulls would cut
// every ASCII character in half, so this guards the width handling.
func TestReadID3v2GenreValues_UTF16(t *testing.T) {
tests := []struct {
name string
payload []byte
}{
{
// Spec-correct: encoding 1 with a BOM on every value.
name: "utf16le, BOM on each value",
payload: concat([]byte{0x01},
[]byte{0xFF, 0xFE}, utf16LE("Rock"),
[]byte{0x00, 0x00},
[]byte{0xFF, 0xFE}, utf16LE("Pop")),
},
{
// Sloppy but common: BOM only on the first value. Without carrying
// the byte order forward, "Pop" decodes byte-swapped to CJK.
name: "utf16le, BOM only on the first value",
payload: concat([]byte{0x01},
[]byte{0xFF, 0xFE}, utf16LE("Rock"),
[]byte{0x00, 0x00}, utf16LE("Pop")),
},
{
// Encoding 2: big-endian, no BOM anywhere.
name: "utf16be no BOM",
payload: concat([]byte{0x02},
utf16BE("Rock"), []byte{0x00, 0x00}, utf16BE("Pop")),
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
data := buildID3v2(t, 4, rawFrame{"TCON", tc.payload})
got, err := readID3v2GenreValues(bytes.NewReader(data))
if err != nil {
t.Fatal(err)
}
if !equalStrings(got, []string{"Rock", "Pop"}) {
t.Errorf("genres = %q, want [Rock Pop]", got)
}
})
}
}
// ISO-8859-1 must be widened, not reinterpreted as UTF-8 — "Bj\xf6rk" would
// otherwise come back as invalid bytes.
func TestReadID3v2GenreValues_Latin1(t *testing.T) {
payload := append([]byte{0x00}, []byte("Chanson Fran\xe7aise")...)
data := buildID3v2(t, 3, rawFrame{"TCON", payload})
got, err := readID3v2GenreValues(bytes.NewReader(data))
if err != nil {
t.Fatal(err)
}
if !equalStrings(got, []string{"Chanson Française"}) {
t.Errorf("genres = %q, want [Chanson Française]", got)
}
}
// Frames before TCON must be walked over correctly. If the size field were
// decoded with the wrong scheme the walk lands mid-frame and TCON is missed.
func TestReadID3v2GenreValues_SkipsPrecedingFrames(t *testing.T) {
for _, major := range []byte{3, 4} {
data := buildID3v2(t, major,
rawFrame{"TIT2", utf8Frame("Some Title")},
rawFrame{"TPE1", utf8Frame("Some Artist")},
rawFrame{"TCON", utf8Frame("Shoegaze", "Dream Pop")},
)
got, err := readID3v2GenreValues(bytes.NewReader(data))
if err != nil {
t.Fatalf("v2.%d: %v", major, err)
}
if !equalStrings(got, []string{"Shoegaze", "Dream Pop"}) {
t.Errorf("v2.%d genres = %q, want [Shoegaze Dream Pop]", major, got)
}
}
}
func TestReadID3v2GenreValues_NoGenreFrame(t *testing.T) {
data := buildID3v2(t, 4, rawFrame{"TIT2", utf8Frame("Only A Title")})
if _, err := readID3v2GenreValues(bytes.NewReader(data)); err == nil {
t.Fatal("expected an error when no genre frame is present")
}
}
func TestReadID3v2GenreValues_NotAnID3File(t *testing.T) {
if _, err := readID3v2GenreValues(bytes.NewReader([]byte("not a tag at all"))); err == nil {
t.Fatal("expected an error for a file with no ID3v2 tag")
}
}
// Padding after the last frame is zero bytes; the walk must stop rather than
// read a frame id of "\x00\x00\x00\x00".
func TestReadID3v2GenreValues_StopsAtPadding(t *testing.T) {
tagged := buildID3v2(t, 4, rawFrame{"TCON", utf8Frame("Rock")})
// Splice 32 padding bytes in before the MPEG sync trailer, growing the
// declared tag size to match.
body := tagged[10 : len(tagged)-4]
padded := append(append([]byte{}, body...), make([]byte, 32)...)
var out bytes.Buffer
out.WriteString("ID3")
out.Write([]byte{4, 0x00, 0x00})
out.Write(synchsafeBytes(len(padded)))
out.Write(padded)
got, err := readID3v2GenreValues(bytes.NewReader(out.Bytes()))
if err != nil {
t.Fatal(err)
}
if !equalStrings(got, []string{"Rock"}) {
t.Errorf("genres = %q, want [Rock]", got)
}
}
// Unsynchronisation inserts 0xFF 0x00 pairs that must be collapsed before the
// frame list is walked, or every offset past the first pair is wrong.
func TestReadID3v2GenreValues_TagUnsynchronisation(t *testing.T) {
// Latin-1 so a genre can legitimately contain the byte 0xFF ("ÿ"). Once
// unsynchronised that becomes 0xFF 0x00 — which is indistinguishable from a
// value separator until the collapse runs, so this fails loudly if
// undoUnsynchronisation is skipped.
payload := concat([]byte{0x00}, []byte("Ro\xffck"), []byte{0x00}, []byte("Pop"))
inner := buildID3v2(t, 3, rawFrame{"TCON", payload})
body := inner[10 : len(inner)-4]
encoded := bytes.ReplaceAll(body, []byte{0xFF}, []byte{0xFF, 0x00})
if bytes.Equal(encoded, body) {
t.Fatal("test is vacuous: nothing was unsynchronised")
}
var out bytes.Buffer
out.WriteString("ID3")
out.Write([]byte{3, 0x00, 0x80}) // 0x80 = unsynchronisation
out.Write(synchsafeBytes(len(encoded)))
out.Write(encoded)
got, err := readID3v2GenreValues(bytes.NewReader(out.Bytes()))
if err != nil {
t.Fatal(err)
}
if !equalStrings(got, []string{"Roÿck", "Pop"}) {
t.Errorf("genres = %q, want [Roÿck Pop]", got)
}
}
func TestNormaliseGenreValue(t *testing.T) {
tests := []struct {
name string
in string
want []string
}{
{"plain text", "Alternative Rock", []string{"Alternative Rock"}},
{"trims whitespace", " Jazz ", []string{"Jazz"}},
{"empty", "", nil},
{"whitespace only", " ", nil},
// The operator's digit soup, one value at a time.
{"bare numeric", "17", []string{"Rock"}},
{"bare numeric pop", "13", []string{"Pop"}},
{"bare numeric electronic", "52", []string{"Electronic"}},
{"winamp extension range", "187", []string{"Indie Rock"}},
{"numeric out of range", "9999", nil},
{"negative", "-1", nil},
// ID3v2.3 refinement syntax.
{"parenthesised", "(17)", []string{"Rock"}},
{"parenthesised repeated", "(51)(39)", []string{"Techno-Industrial", "Noise"}},
{"parenthesised with refinement", "(17)Hard Rock", []string{"Rock", "Hard Rock"}},
{"remix", "(RX)", []string{"Remix"}},
{"cover", "(CR)", []string{"Cover"}},
{"escaped open paren", "((Weird", []string{"(Weird"}},
{"parenthesised non-numeric", "(Live)", []string{"(Live)"}},
// A label that merely starts with digits is text, not a reference.
{"digits in a name", "1980s", []string{"1980s"}},
{"hyphenated", "Lo-Fi Hip Hop", []string{"Lo-Fi Hip Hop"}},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := normaliseGenreValue(tc.in)
if !equalStrings(got, tc.want) {
t.Errorf("normaliseGenreValue(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}
func TestNormaliseGenres_DedupesCaseInsensitively(t *testing.T) {
got := normaliseGenres([]string{"Rock", "rock", "ROCK", "Pop"})
// First spelling wins — we are not imposing a canonical case here, only
// removing values that repeat within a single file.
if !equalStrings(got, []string{"Rock", "Pop"}) {
t.Errorf("genres = %q, want [Rock Pop]", got)
}
}
// "(40)AlternRock" declares the same genre twice — numerically and in text.
func TestNormaliseGenres_DedupesResolvedNumeric(t *testing.T) {
got := normaliseGenres([]string{"(40)AlternRock"})
if !equalStrings(got, []string{"AlternRock"}) {
t.Errorf("genres = %q, want [AlternRock]", got)
}
}
func TestNormaliseGenres_AllJunkYieldsNil(t *testing.T) {
if got := normaliseGenres([]string{"", " ", "9999"}); got != nil {
t.Errorf("genres = %q, want nil", got)
}
}
// End-to-end through dhowden/tag, which is what the scanner actually calls.
// Proves the welded value never reaches the caller.
func TestExtractGenres_EndToEnd(t *testing.T) {
data := buildID3v2(t, 4,
rawFrame{"TIT2", utf8Frame("A Song")},
rawFrame{"TCON", utf8Frame("Alternative Rock", "Rock")},
)
rs := bytes.NewReader(data)
meta, err := tag.ReadFrom(rs)
if err != nil {
t.Fatalf("tag.ReadFrom: %v", err)
}
// Confirm the upstream behaviour this fix exists for is still present —
// if dhowden ever fixes it, this test tells us the workaround can go.
if welded := meta.Genre(); welded != "Alternative RockRock" {
t.Logf("note: dhowden/tag no longer welds multi-values (got %q)", welded)
}
genres, fellBack := extractGenres(meta, rs)
if fellBack {
t.Error("fellBack = true, want false — the TCON frame is parseable")
}
if !equalStrings(genres, []string{"Alternative Rock", "Rock"}) {
t.Errorf("genres = %q, want [Alternative Rock Rock]", genres)
}
if joined := strings.Join(genres, genreDelimiter); joined != "Alternative Rock;Rock" {
t.Errorf("stored value = %q, want %q", joined, "Alternative Rock;Rock")
}
}
// The digit-soup case, end to end: numeric references resolve to names.
func TestExtractGenres_ResolvesNumericReferences(t *testing.T) {
data := buildID3v2(t, 4, rawFrame{"TCON", utf8Frame("40", "17")})
rs := bytes.NewReader(data)
meta, err := tag.ReadFrom(rs)
if err != nil {
t.Fatalf("tag.ReadFrom: %v", err)
}
genres, _ := extractGenres(meta, rs)
if !equalStrings(genres, []string{"AlternRock", "Rock"}) {
t.Errorf("genres = %q, want [AlternRock Rock]", genres)
}
}
// A file with no genre at all must yield nothing and must NOT be reported as a
// fallback — that would log a warning for every untagged file in the library.
func TestExtractGenres_NoGenreIsNotAFallback(t *testing.T) {
data := buildID3v2(t, 4, rawFrame{"TIT2", utf8Frame("A Song")})
rs := bytes.NewReader(data)
meta, err := tag.ReadFrom(rs)
if err != nil {
t.Fatalf("tag.ReadFrom: %v", err)
}
genres, fellBack := extractGenres(meta, rs)
if len(genres) != 0 {
t.Errorf("genres = %q, want none", genres)
}
if fellBack {
t.Error("fellBack = true for an untagged file; would log on every such file")
}
}
func concat(parts ...[]byte) []byte {
var out []byte
for _, p := range parts {
out = append(out, p...)
}
return out
}
func utf16LE(s string) []byte {
out := make([]byte, 0, len(s)*2)
for _, r := range s {
out = append(out, byte(r), byte(r>>8))
}
return out
}
func utf16BE(s string) []byte {
out := make([]byte, 0, len(s)*2)
for _, r := range s {
out = append(out, byte(r>>8), byte(r))
}
return out
}
func equalStrings(a, b []string) bool {
if len(a) != len(b) {
return false
}
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}
func TestTrueUpCasing(t *testing.T) {
tests := []struct {
name string
in string
want string
}{
// The damage actually present in the operator's library (#2468).
{"edm acronym", "Edm", "EDM"},
{"idm acronym", "Idm", "IDM"},
{"aor acronym", "Aor", "AOR"},
{"uk prefix", "Uk Garage", "UK Garage"},
{"uk hardcore", "Uk Hardcore", "UK Hardcore"},
{"acronym mid-phrase", "Trap Edm", "Trap EDM"},
{"acronym at the end", "Glitch Hop Edm", "Glitch Hop EDM"},
{"possessive", "Children'S Music", "Children's Music"},
// Already correct input must be left exactly alone.
{"correct acronym", "EDM", "EDM"},
{"correct possessive", "Children's Music", "Children's Music"},
// Case-insensitive, so a lower-cased tag also lands on the canonical
// form rather than becoming a third variant.
{"lowercase acronym", "edm", "EDM"},
// Names with an apostrophe followed by a real word are NOT contractions
// and must keep their capital — this is why the suffix list is fixed
// rather than "lowercase anything after an apostrophe".
{"irish surname", "O'Brien Core", "O'Brien Core"},
{"french elision", "D'Angelo Soul", "D'Angelo Soul"},
// Ordinary genres pass through untouched. Case is otherwise exposed as
// the file says it — "Rock" vs "rock" stays a real distinction.
{"plain", "Alternative Rock", "Alternative Rock"},
{"lowercase plain", "rock", "rock"},
{"hyphenated", "Lo-Fi Hip Hop", "Lo-Fi Hip Hop"},
{"ampersand", "R&B", "R&B"},
{"empty", "", ""},
// Punctuation around a word must not hide the acronym inside it.
{"parenthesised acronym", "Hip Hop (Edm)", "Hip Hop (EDM)"},
{"acronym with comma", "Edm, Trap", "EDM, Trap"},
// Interior punctuation stays in the core, so these are compared whole
// and cannot match a fragment by accident.
{"hyphenated stays whole", "Lo-Fi", "Lo-Fi"},
{"ampersand stays whole", "Drum & Bass", "Drum & Bass"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := trueUpCasing(tc.in); got != tc.want {
t.Errorf("trueUpCasing(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}
// The casing repair must never touch a name resolved from the ID3v1 table. The
// operator chose to keep that table canonical, so entry 40's 1990s spelling
// "AlternRock" stays as-is even though it reads like damage.
func TestNormaliseGenreValue_CanonicalTableNamesNotRecased(t *testing.T) {
if got := normaliseGenreValue("40"); !equalStrings(got, []string{"AlternRock"}) {
t.Errorf("bare 40 = %q, want [AlternRock]", got)
}
if got := normaliseGenreValue("(40)"); !equalStrings(got, []string{"AlternRock"}) {
t.Errorf("(40) = %q, want [AlternRock]", got)
}
}
// Casing runs on values that came from the file, including the parenthesised
// and refinement paths.
func TestNormaliseGenreValue_CasingAppliedToFileText(t *testing.T) {
tests := []struct {
in string
want []string
}{
{"Edm", []string{"EDM"}},
{"(17)Uk Garage", []string{"Rock", "UK Garage"}},
// Punctuation around the acronym must not hide it — the letter core is
// what gets matched, and the trimmed edges are re-attached.
{"(Live Edm)", []string{"(Live EDM)"}},
{"((Children'S Music", []string{"(Children's Music"}},
}
for _, tc := range tests {
t.Run(tc.in, func(t *testing.T) {
if got := normaliseGenreValue(tc.in); !equalStrings(got, tc.want) {
t.Errorf("normaliseGenreValue(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}
// Two spellings of one acronym in the same file collapse to a single tag rather
// than surviving as near-duplicates.
func TestNormaliseGenres_AcronymVariantsDedupe(t *testing.T) {
if got := normaliseGenres([]string{"Edm", "EDM", "edm"}); !equalStrings(got, []string{"EDM"}) {
t.Errorf("genres = %q, want [EDM]", got)
}
}