From f13aa7086b01ba091918745cdb1b33dfd63f77de Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Thu, 19 Mar 2026 14:02:02 -0400 Subject: [PATCH] test(20-02): add round-trip tests for all OGG requirements - TestWriteOggTags_TextFields: all 9 text fields round-trip via dhowden/tag (OGG-01) - TestWriteOggTags_CoverArt: METADATA_BLOCK_PICTURE embed verified (OGG-04) - TestWriteOggTags_ClearCoverArt: cover art clear via nil (OGG-04) - TestWriteOggTags_PartialUpdate: non-edited fields preserved (OGG-02) - TestWriteOggTags_AudioPreservation: audio page data byte-identical (OGG-03) - TestWriteOggTags_AtomicSafety: corrupt file untouched on failure (OGG-05) - TestWriteOggTags_RejectNonVorbis: Theora OGG rejected (OGG-03 error path) - TestWriteOggTags_RejectMultiStream: multi-serial rejected --- backend/tagwriter/ogg_test.go | 406 ++++++++++++++++++++++++++++++++++ 1 file changed, 406 insertions(+) diff --git a/backend/tagwriter/ogg_test.go b/backend/tagwriter/ogg_test.go index 2472b07..6990083 100644 --- a/backend/tagwriter/ogg_test.go +++ b/backend/tagwriter/ogg_test.go @@ -251,3 +251,409 @@ func TestCreateTestOGG_Valid(t *testing.T) { t.Fatalf("metadata.ExtractTags on empty fixture: %v", err) } } + +// --------------------------------------------------------------------------- +// Round-trip tests for OGG requirements (OGG-01 through OGG-06) +// --------------------------------------------------------------------------- + +func TestWriteOggTags_TextFields(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "text.ogg") + createTestOGG(t, path) + + 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 := writeOggTags(testLogger(), path, changes); err != nil { + t.Fatalf("writeOggTags: %v", err) + } + + // Read back with metadata.ExtractTags (dhowden/tag). + tags, err := metadata.ExtractTags(path) + if err != nil { + t.Fatalf("ExtractTags: %v", err) + } + + assertEqual(t, "Title", "Test Title", tags.Title) + assertEqual(t, "Artist", "Test Artist", tags.Artist) + assertEqual(t, "Album", "Test Album", tags.Album) + assertEqual(t, "AlbumArtist", "Test Album Artist", tags.AlbumArtist) + assertEqual(t, "Genre", "Rock", tags.Genre) + assertEqual(t, "Composer", "Test Composer", tags.Composer) + assertEqual(t, "Year", 2024, tags.Year) + assertEqual(t, "TrackNumber", 3, tags.TrackNumber) + assertEqual(t, "DiscNumber", 1, tags.DiscNumber) +} + +func TestWriteOggTags_CoverArt(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "cover.ogg") + createTestOGG(t, path) + + jpegData := tinyJPEG(t) + + changes := TagChanges{ + FieldCoverArt: jpegData, + } + + if err := writeOggTags(testLogger(), path, changes); err != nil { + t.Fatalf("writeOggTags: %v", err) + } + + tags, err := metadata.ExtractTags(path) + if err != nil { + t.Fatalf("ExtractTags: %v", err) + } + + if tags.Picture == nil { + t.Fatal("expected picture data, got nil") + } + + if !bytes.Equal(tags.Picture.Data, jpegData) { + t.Errorf("picture data mismatch: got %d bytes, want %d bytes", + len(tags.Picture.Data), len(jpegData)) + } + + assertEqual(t, "MIME type", "image/jpeg", tags.Picture.MIMEType) +} + +func TestWriteOggTags_ClearCoverArt(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "clear_art.ogg") + createTestOGG(t, path) + + // First add cover art. + jpegData := tinyJPEG(t) + + err := writeOggTags(testLogger(), path, TagChanges{FieldCoverArt: jpegData}) + if err != nil { + t.Fatalf("add cover art: %v", err) + } + + // Verify art was added. + tags, err := metadata.ExtractTags(path) + if err != nil { + t.Fatalf("ExtractTags after add: %v", err) + } + + if tags.Picture == nil { + t.Fatal("expected picture after add, got nil") + } + + // Now clear cover art (nil value). + if err := writeOggTags(testLogger(), path, TagChanges{FieldCoverArt: nil}); err != nil { + t.Fatalf("clear cover art: %v", err) + } + + tags, err = metadata.ExtractTags(path) + if err != nil { + t.Fatalf("ExtractTags after clear: %v", err) + } + + if tags.Picture != nil { + t.Errorf("expected no picture after clear, got %d bytes", len(tags.Picture.Data)) + } +} + +func TestWriteOggTags_PartialUpdate(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "partial.ogg") + createTestOGG(t, path) + + // Write all fields first. + allFields := TagChanges{ + FieldTitle: "Original Title", + FieldArtist: "Original Artist", + FieldAlbum: "Original Album", + FieldAlbumArtist: "Original AA", + FieldGenre: "Jazz", + FieldYear: 2020, + FieldTrackNumber: 5, + FieldDiscNumber: 2, + FieldComposer: "Original Composer", + } + + if err := writeOggTags(testLogger(), path, allFields); err != nil { + t.Fatalf("write all fields: %v", err) + } + + // Update only title and genre. + partial := TagChanges{ + FieldTitle: "Updated Title", + FieldGenre: "Blues", + } + + if err := writeOggTags(testLogger(), path, partial); err != nil { + t.Fatalf("partial update: %v", err) + } + + tags, err := metadata.ExtractTags(path) + if err != nil { + t.Fatalf("ExtractTags: %v", err) + } + + // Changed fields should have new values. + assertEqual(t, "Title", "Updated Title", tags.Title) + assertEqual(t, "Genre", "Blues", tags.Genre) + + // Unchanged fields should be preserved. + assertEqual(t, "Artist", "Original Artist", tags.Artist) + assertEqual(t, "Album", "Original Album", tags.Album) + assertEqual(t, "AlbumArtist", "Original AA", tags.AlbumArtist) + assertEqual(t, "Year", 2020, tags.Year) + assertEqual(t, "TrackNumber", 5, tags.TrackNumber) + assertEqual(t, "DiscNumber", 2, tags.DiscNumber) + assertEqual(t, "Composer", "Original Composer", tags.Composer) +} + +func TestWriteOggTags_AudioPreservation(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "audio.ogg") + createTestOGG(t, path) + + // Capture original audio page data. + pagesBefore, err := parseOggPages(testLogger(), path) + if err != nil { + t.Fatalf("parseOggPages before: %v", err) + } + + audioStartBefore := findAudioPageStart(pagesBefore) + + var originalAudioData []byte + for _, p := range pagesBefore[audioStartBefore:] { + originalAudioData = append(originalAudioData, p.data...) + } + + if len(originalAudioData) == 0 { + t.Fatal("no audio data found in fixture") + } + + // Write tags. + changes := TagChanges{ + FieldTitle: "Audio Test", + FieldArtist: "Audio Artist", + } + + if err := writeOggTags(testLogger(), path, changes); err != nil { + t.Fatalf("writeOggTags: %v", err) + } + + // Parse again and compare audio data. + pagesAfter, err := parseOggPages(testLogger(), path) + if err != nil { + t.Fatalf("parseOggPages after: %v", err) + } + + audioStartAfter := findAudioPageStart(pagesAfter) + + var newAudioData []byte + for _, p := range pagesAfter[audioStartAfter:] { + newAudioData = append(newAudioData, p.data...) + } + + if !bytes.Equal(originalAudioData, newAudioData) { + t.Errorf("audio data changed after tag write: before=%d bytes, after=%d bytes", + len(originalAudioData), len(newAudioData)) + } +} + +func TestWriteOggTags_AtomicSafety(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + // Test 1: Corrupt (non-OGG) file — should fail and leave file untouched. + corruptPath := filepath.Join(dir, "corrupt.ogg") + corruptContent := []byte("not an ogg file at all") + + if err := os.WriteFile(corruptPath, corruptContent, 0o644); err != nil { + t.Fatalf("write corrupt: %v", err) + } + + err := writeOggTags(testLogger(), corruptPath, TagChanges{FieldTitle: "Fail"}) + if err == nil { + t.Fatal("expected error writing to corrupt file, got nil") + } + + // Verify corrupt file is untouched. + content, readErr := os.ReadFile(corruptPath) + if readErr != nil { + t.Fatalf("read corrupt: %v", readErr) + } + + if !bytes.Equal(content, corruptContent) { + t.Error("corrupt file was modified despite write failure") + } + + // Test 2: Valid OGG → write to non-existent directory path should fail. + validPath := filepath.Join(dir, "valid.ogg") + createTestOGG(t, validPath) + + originalData, err := os.ReadFile(validPath) + if err != nil { + t.Fatalf("read valid before: %v", err) + } + + badPath := filepath.Join(dir, "nonexistent", "subdir", "file.ogg") + + writeErr := writeOggTags(testLogger(), badPath, TagChanges{FieldTitle: "Fail"}) + if writeErr == nil { + t.Fatal("expected error for non-existent path, got nil") + } + + // Verify the original valid file is unchanged. + afterData, err := os.ReadFile(validPath) + if err != nil { + t.Fatalf("read valid after: %v", err) + } + + if !bytes.Equal(originalData, afterData) { + t.Error("valid file was modified despite failure elsewhere") + } +} + +func TestWriteOggTags_RejectNonVorbis(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "theora.ogg") + + // Build a valid OGG page structure with a Theora identification header + // instead of Vorbis. + const serialNo uint32 = 0xDEADBEEF + + theoraIdent := make([]byte, 30) + theoraIdent[0] = 0x80 // Theora ID header type + copy(theoraIdent[1:7], "theora") + + identSegs := splitPacketIntoSegments(theoraIdent) + identPage := oggPage{ + headerType: 0x02, // bos + granulePos: 0, + serialNo: serialNo, + seqNo: 0, + segmentTable: identSegs, + data: theoraIdent, + } + + var buf bytes.Buffer + + if err := writeOggPage(&buf, identPage); err != nil { + t.Fatalf("write theora page: %v", err) + } + + // Add an EOS page. + eosPage := oggPage{ + headerType: 0x04, + granulePos: 0, + serialNo: serialNo, + seqNo: 1, + segmentTable: []byte{0}, + data: nil, + } + + if err := writeOggPage(&buf, eosPage); err != nil { + t.Fatalf("write eos page: %v", err) + } + + if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + t.Fatalf("write theora OGG: %v", err) + } + + err := writeOggTags(testLogger(), path, TagChanges{FieldTitle: "Should Fail"}) + if err == nil { + t.Fatal("expected error for non-Vorbis OGG, got nil") + } + + if !errorContains(err, "not an OGG Vorbis") { + t.Errorf("error should mention non-Vorbis, got: %v", err) + } +} + +func TestWriteOggTags_RejectMultiStream(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "multi.ogg") + + // Build an OGG file with pages from two different serial numbers. + // First page is a valid Vorbis bos page. + identPacket := buildVorbisIdentPacket() + identSegs := splitPacketIntoSegments(identPacket) + + const serial1 uint32 = 0x11111111 + const serial2 uint32 = 0x22222222 + + page1 := oggPage{ + headerType: 0x02, // bos + granulePos: 0, + serialNo: serial1, + seqNo: 0, + segmentTable: identSegs, + data: identPacket, + } + + // Second bos page with a different serial number. + page2 := oggPage{ + headerType: 0x02, // bos + granulePos: 0, + serialNo: serial2, + seqNo: 0, + segmentTable: identSegs, + data: identPacket, + } + + var buf bytes.Buffer + + if err := writeOggPage(&buf, page1); err != nil { + t.Fatalf("write page1: %v", err) + } + + if err := writeOggPage(&buf, page2); err != nil { + t.Fatalf("write page2: %v", err) + } + + if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + t.Fatalf("write multi-stream OGG: %v", err) + } + + err := writeOggTags(testLogger(), path, TagChanges{FieldTitle: "Should Fail"}) + if err == nil { + t.Fatal("expected error for multi-stream OGG, got nil") + } + + if !errorContains(err, "multiple streams") { + t.Errorf("error should mention multiple streams, got: %v", err) + } +} + +// errorContains checks if an error's message contains the given substring. +func errorContains(err error, substr string) bool { + if err == nil { + return false + } + + return bytes.Contains( + []byte(err.Error()), + []byte(substr), + ) +}