From 1b28882af4184ecd14e1489c177bc92e88bb9ab3 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Thu, 19 Mar 2026 08:53:36 -0400 Subject: [PATCH] test(19-02): add round-trip tests for all WAV tag writer requirements - TestWriteWavTags_TextFields: 9 field round-trip (WAV-01) - TestWriteWavTags_CoverArt: JPEG embed/read-back (WAV-04) - TestWriteWavTags_ClearCoverArt: embed then clear (WAV-04) - TestWriteWavTags_PartialUpdate: 2-of-9 change, 7 preserved (WAV-01) - TestWriteWavTags_ChunkPreservation: fmt/data/LIST/bext preserved (WAV-02, WAV-03) - TestWriteWavTags_AtomicSafety: failed write leaves file untouched (WAV-05) - TestWriteWavTags_RejectsRF64: RF64 rejected with clear error (WAV-02) - readWavID3Tags uses bogem/id3v2 ParseReader for reliable read-back --- backend/tagwriter/wav_test.go | 414 ++++++++++++++++++++++++++++++++-- 1 file changed, 389 insertions(+), 25 deletions(-) diff --git a/backend/tagwriter/wav_test.go b/backend/tagwriter/wav_test.go index 9529416..d85b447 100644 --- a/backend/tagwriter/wav_test.go +++ b/backend/tagwriter/wav_test.go @@ -5,10 +5,11 @@ import ( "encoding/binary" "os" "path/filepath" + "strconv" + "strings" "testing" id3v2 "github.com/bogem/id3v2/v2" - "github.com/dhowden/tag" "yellowjacket/backend/metadata" ) @@ -121,10 +122,337 @@ func makePCMFmtData() []byte { return buf.Bytes() } +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +func TestWriteWavTags_TextFields(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := createTestWAV(t, dir, "text.wav", nil) + + changes := TagChanges{ + FieldTitle: "Test Title", + FieldArtist: "Test Artist", + FieldAlbum: "Test Album", + FieldAlbumArtist: "Test Album Artist", + FieldGenre: "Rock", + FieldYear: 2024, + FieldTrackNumber: 3, + FieldDiscNumber: 1, + FieldComposer: "Test Composer", + } + + if err := writeWavTags(testLogger(), path, changes); err != nil { + t.Fatalf("writeWavTags: %v", err) + } + + meta := readWavID3Tags(t, path) + + assertStrField(t, "Title", meta.Title, "Test Title") + assertStrField(t, "Artist", meta.Artist, "Test Artist") + assertStrField(t, "Album", meta.Album, "Test Album") + assertStrField(t, "AlbumArtist", meta.AlbumArtist, "Test Album Artist") + assertStrField(t, "Genre", meta.Genre, "Rock") + assertIntField(t, "Year", meta.Year, 2024) + assertIntField(t, "TrackNumber", meta.TrackNumber, 3) + assertIntField(t, "DiscNumber", meta.DiscNumber, 1) + assertStrField(t, "Composer", meta.Composer, "Test Composer") +} + +func TestWriteWavTags_CoverArt(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := createTestWAV(t, dir, "cover.wav", nil) + + art := tinyJPEG(t) + changes := TagChanges{ + FieldCoverArt: art, + } + + if err := writeWavTags(testLogger(), path, changes); err != nil { + t.Fatalf("writeWavTags: %v", err) + } + + meta := readWavID3Tags(t, path) + + if meta.Picture == nil { + t.Fatal("expected picture data, got nil") + } + + if !bytes.Equal(meta.Picture.Data, art) { + t.Errorf("picture data mismatch: got %d bytes, want %d", + len(meta.Picture.Data), len(art)) + } + + assertStrField(t, "MIME", meta.Picture.MIMEType, "image/jpeg") +} + +func TestWriteWavTags_ClearCoverArt(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + art := tinyJPEG(t) + + // Create WAV with cover art embedded. + path := createTestWAV(t, dir, "clear.wav", TagChanges{ + FieldCoverArt: art, + }) + + // Verify art is present before clearing. + meta := readWavID3Tags(t, path) + if meta.Picture == nil { + t.Fatal("expected picture before clear, got nil") + } + + // Clear cover art by setting to nil. + err := writeWavTags(testLogger(), path, TagChanges{ + FieldCoverArt: nil, + }) + if err != nil { + t.Fatalf("writeWavTags (clear): %v", err) + } + + meta = readWavID3Tags(t, path) + if meta.Picture != nil { + t.Errorf("expected no picture after clear, got %d bytes", + len(meta.Picture.Data)) + } +} + +func TestWriteWavTags_PartialUpdate(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + initial := TagChanges{ + FieldTitle: "Original Title", + FieldArtist: "Original Artist", + FieldAlbum: "Original Album", + FieldAlbumArtist: "Original AA", + FieldGenre: "Jazz", + FieldYear: 2020, + FieldTrackNumber: 5, + FieldDiscNumber: 2, + FieldComposer: "Original Composer", + } + + path := createTestWAV(t, dir, "partial.wav", initial) + + // Update only title and artist. + err := writeWavTags(testLogger(), path, TagChanges{ + FieldTitle: "New Title", + FieldArtist: "New Artist", + }) + if err != nil { + t.Fatalf("writeWavTags: %v", err) + } + + meta := readWavID3Tags(t, path) + + // Changed fields. + assertStrField(t, "Title", meta.Title, "New Title") + assertStrField(t, "Artist", meta.Artist, "New Artist") + + // Unchanged fields. + assertStrField(t, "Album", meta.Album, "Original Album") + assertStrField(t, "AlbumArtist", meta.AlbumArtist, "Original AA") + assertStrField(t, "Genre", meta.Genre, "Jazz") + assertIntField(t, "Year", meta.Year, 2020) + assertIntField(t, "TrackNumber", meta.TrackNumber, 5) + assertIntField(t, "DiscNumber", meta.DiscNumber, 2) + assertStrField(t, "Composer", meta.Composer, "Original Composer") +} + +func TestWriteWavTags_ChunkPreservation(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := createTestWAVWithExtraChunks(t, dir, "chunks.wav") + + // Parse original RIFF to capture chunk data before writing. + origFile, err := os.Open(path) + if err != nil { + t.Fatalf("open original: %v", err) + } + + origChunks, err := parseRIFF(origFile) + _ = origFile.Close() + + if err != nil { + t.Fatalf("parse original RIFF: %v", err) + } + + // Record original chunk data by ID string. + origData := map[string][]byte{} + for _, c := range origChunks { + origData[string(c.id[:])] = c.data + } + + // Write a tag to trigger RIFF rewrite. + err = writeWavTags(testLogger(), path, TagChanges{ + FieldTitle: "Chunk Test", + }) + if err != nil { + t.Fatalf("writeWavTags: %v", err) + } + + // Re-parse and verify all non-ID3 chunks are preserved. + newFile, err := os.Open(path) + if err != nil { + t.Fatalf("open after write: %v", err) + } + + newChunks, err := parseRIFF(newFile) + _ = newFile.Close() + + if err != nil { + t.Fatalf("parse new RIFF: %v", err) + } + + // Count non-id3 chunks in both. + origNonID3 := 0 + for _, c := range origChunks { + if !isID3ChunkID(c.id) { + origNonID3++ + } + } + + newNonID3 := 0 + for _, c := range newChunks { + if !isID3ChunkID(c.id) { + newNonID3++ + } + } + + if origNonID3 != newNonID3 { + t.Errorf("non-ID3 chunk count: got %d, want %d", + newNonID3, origNonID3) + } + + // Verify fmt chunk is byte-identical. + checkChunkPreserved(t, newChunks, "fmt ", origData["fmt "]) + + // Verify data chunk is byte-identical (WAV-03: audio preserved). + checkChunkPreserved(t, newChunks, "data", origData["data"]) + + // Verify LIST chunk is preserved. + checkChunkPreserved(t, newChunks, "LIST", origData["LIST"]) + + // Verify bext chunk is preserved. + checkChunkPreserved(t, newChunks, "bext", origData["bext"]) + + // Verify id3 chunk now exists with the written title. + meta := readWavID3Tags(t, path) + assertStrField(t, "Title", meta.Title, "Chunk Test") +} + +// checkChunkPreserved asserts that a chunk with the given ID exists +// in chunks and its data matches want byte-for-byte. +func checkChunkPreserved( + t *testing.T, + chunks []riffChunk, + idStr string, + want []byte, +) { + t.Helper() + + for _, c := range chunks { + if string(c.id[:]) == idStr { + if !bytes.Equal(c.data, want) { + t.Errorf("chunk %q data changed: got %d bytes, want %d", + idStr, len(c.data), len(want)) + } + + return + } + } + + t.Errorf("chunk %q not found after write", idStr) +} + +func TestWriteWavTags_AtomicSafety(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := createTestWAV(t, dir, "atomic.wav", TagChanges{ + FieldTitle: "Before", + }) + + // Read original file content. + originalData, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read original: %v", err) + } + + // Attempt to write to a non-existent directory to force + // AtomicWrite to fail on temp file creation. + badPath := filepath.Join( + dir, "nonexistent", "subdir", "file.wav", + ) + + writeErr := writeWavTags(testLogger(), badPath, TagChanges{ + FieldTitle: "After", + }) + + if writeErr == nil { + t.Fatal("expected error for non-existent path, got nil") + } + + // Verify the original file is completely unchanged. + afterData, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read after failed write: %v", err) + } + + if !bytes.Equal(originalData, afterData) { + t.Error("original file was modified despite write failure") + } +} + +func TestWriteWavTags_RejectsRF64(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + // Build a minimal RF64 file header. + var buf bytes.Buffer + + buf.WriteString("RF64") + _ = binary.Write(&buf, binary.LittleEndian, uint32(0xFFFFFFFF)) + buf.WriteString("WAVE") + + // Minimal ds64 chunk (required for RF64 but we just need + // enough bytes for parseRIFF to hit the RF64 rejection). + buf.WriteString("ds64") + _ = binary.Write(&buf, binary.LittleEndian, uint32(28)) //nolint:mnd + buf.Write(make([]byte, 28)) //nolint:mnd + + path := filepath.Join(dir, "rf64.wav") + if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + t.Fatalf("write RF64 file: %v", err) + } + + err := writeWavTags(testLogger(), path, TagChanges{ + FieldTitle: "Should Fail", + }) + + if err == nil { + t.Fatal("expected error for RF64 file, got nil") + } + + if !strings.Contains(err.Error(), "RF64") { + t.Errorf("error should mention RF64, got: %v", err) + } +} + // readWavID3Tags extracts ID3v2 metadata from a WAV file by parsing -// the RIFF structure and reading the id3 chunk with dhowden/tag. -// This is necessary because dhowden/tag's ReadFrom does not support -// WAV files directly. +// the RIFF structure and reading the id3 chunk with bogem/id3v2. +// dhowden/tag's ReadFrom does not support WAV files, and its +// ReadID3v2Tags fails on empty tags (after clearing all frames). +// Using bogem/id3v2.ParseReader handles all cases correctly. func readWavID3Tags( t *testing.T, path string, @@ -158,39 +486,75 @@ func readWavID3Tags( t.Fatal("no id3 chunk found in WAV file") } - // dhowden/tag ReadID3v2Tags expects an io.ReadSeeker starting - // with the "ID3" magic bytes. - m, err := tag.ReadID3v2Tags(bytes.NewReader(id3Data)) + parsed, err := id3v2.ParseReader( + bytes.NewReader(id3Data), + id3v2.Options{Parse: true}, + ) if err != nil { - t.Fatalf("ReadID3v2Tags: %v", err) + t.Fatalf("ParseReader: %v", err) } - trackNum, _ := m.Track() - discNum, _ := m.Disc() - meta := &metadata.TrackMetadata{ - Title: m.Title(), - Artist: m.Artist(), - Album: m.Album(), - AlbumArtist: m.AlbumArtist(), - Composer: m.Composer(), - Genre: m.Genre(), - Year: m.Year(), - TrackNumber: trackNum, - DiscNumber: discNum, + Title: parsed.Title(), + Artist: parsed.Artist(), + Album: parsed.Album(), + Genre: parsed.Genre(), + Year: atoiSafe(parsed.Year()), } - if pic := m.Picture(); pic != nil { - meta.Picture = &metadata.PictureData{ - Data: pic.Data, - MIMEType: pic.MIMEType, - Ext: pic.Ext, + // Album artist (TPE2). + if frames := parsed.GetFrames( + parsed.CommonID("Band/Orchestra/Accompaniment"), + ); len(frames) > 0 { + if tf, ok := frames[0].(id3v2.TextFrame); ok { + meta.AlbumArtist = tf.Text + } + } + + // Composer (TCOM). + if frames := parsed.GetFrames("TCOM"); len(frames) > 0 { + if tf, ok := frames[0].(id3v2.TextFrame); ok { + meta.Composer = tf.Text + } + } + + // Track number (TRCK). + trckID := parsed.CommonID("Track number/Position in set") + if frames := parsed.GetFrames(trckID); len(frames) > 0 { + if tf, ok := frames[0].(id3v2.TextFrame); ok { + meta.TrackNumber = atoiSafe(tf.Text) + } + } + + // Disc number (TPOS). + tposID := parsed.CommonID("Part of a set") + if frames := parsed.GetFrames(tposID); len(frames) > 0 { + if tf, ok := frames[0].(id3v2.TextFrame); ok { + meta.DiscNumber = atoiSafe(tf.Text) + } + } + + // Cover art (APIC). + apicID := parsed.CommonID("Attached picture") + if frames := parsed.GetFrames(apicID); len(frames) > 0 { + if pf, ok := frames[0].(id3v2.PictureFrame); ok { + meta.Picture = &metadata.PictureData{ + Data: pf.Picture, + MIMEType: pf.MimeType, + } } } return meta } +// atoiSafe converts a string to int, returning 0 on failure. +func atoiSafe(s string) int { + n, _ := strconv.Atoi(s) + + return n +} + // createTestWAVWithExtraChunks creates a WAV file containing // additional non-standard chunks (LIST INFO, bext) to verify that // writeWavTags preserves chunks it does not understand.