Improve Test Coverage

src/api/remove.ts

@@ -26,7 +26,7 @@ export type RemoveCallback =
  *
  * @public
  */
-export function removeTags(filepath: string): boolean | Error
+export function removeTags(filepath: string): true | Error
 
 /**
  * Removes asynchronously any written ID3-Frames from the specified file.
@@ -51,9 +51,6 @@ function removeTagsSync(filepath: string) {
     }
 
     const newData = removeTagsFromBuffer(data)
-    if(!newData) {
-        return false
-    }
 
     try {
         fs.writeFileSync(filepath, newData, 'binary')
@@ -72,10 +69,6 @@ function removeTagsAsync(filepath: string, callback: RemoveCallback) {
         }
 
         const newData = removeTagsFromBuffer(data)
-        if(!newData) {
-            callback(error)
-            return
-        }
 
         fs.writeFile(filepath, newData, 'binary', (error) => {
             if(error) {

src/api/update.ts

@@ -5,6 +5,17 @@ import { read } from "./read"
 import { updateTags } from '../updateTags'
 import { write, WriteCallback } from "./write"
 
+/**
+ * Updates ID3-Tags asynchronously in the specified file.
+ *
+ * @public
+ */
+export function update(
+    tags: WriteTags,
+    filebuffer: string | Buffer,
+    callback: WriteCallback
+): void
+
 /**
  * Updates ID3-Tags from the given buffer.
  *
@@ -27,17 +38,6 @@ import { write, WriteCallback } from "./write"
     options?: Options
 ): true | Error
 
-/**
- * Updates ID3-Tags asynchronously in the specified file.
- *
- * @public
- */
- export function update(
-    tags: WriteTags,
-    filebuffer: string | Buffer,
-    callback: WriteCallback
-): void
-
 /**
  * Updates ID3-Tags asynchronously from the given buffer or specified file.
  *

src/id3-tag.ts

@@ -92,10 +92,6 @@ export function removeId3Tag(data: Buffer) {
     }
     const encodedSize = subarray(data, tagPosition + Header.offset.size, 4)
 
-    if (!isValidEncodedSize(encodedSize)) {
-        return false
-    }
-
     if (data.length >= tagPosition + Header.size) {
         const size = decodeSize(encodedSize)
         return Buffer.concat([

test/api/remove.ts

@@ -0,0 +1,70 @@
+import * as NodeID3 from '../../index'
+import assert = require('assert')
+import chai = require('chai')
+import * as fs from 'fs'
+
+describe('NodeID3 API', function () {
+    describe('#removeTags()', function() {
+        const nonExistingFilepath = './hopefully-does-not-exist.mp3'
+        it('sync not existing filepath', function() {
+            chai.assert.isFalse(fs.existsSync(nonExistingFilepath))
+            chai.assert.instanceOf(
+                NodeID3.removeTags(nonExistingFilepath), Error
+            )
+        })
+        it('async not existing filepath', function() {
+            chai.assert.isFalse(fs.existsSync(nonExistingFilepath))
+            NodeID3.removeTags(nonExistingFilepath, function(err) {
+                if(!(err instanceof Error)) {
+                    assert.fail("No error thrown on non-existing filepath")
+                }
+            })
+        })
+
+        const titleTag = {
+            title: "abc"
+        } satisfies NodeID3.WriteTags
+        const filepath = './testfile.mp3'
+
+        describe('valid buffer', function() {
+            const prefixBuffer = Buffer.from([0x01, 0x02, 0x03])
+            const postfixBuffer = Buffer.from([0x04, 0x05, 0x06])
+            const buffer = Buffer.concat([
+                prefixBuffer,
+                NodeID3.create(titleTag),
+                postfixBuffer
+            ])
+            const bufferAfterRemove = Buffer.concat([
+                prefixBuffer,
+                postfixBuffer
+            ])
+
+            beforeEach(function() {
+                fs.writeFileSync(filepath, buffer)
+            })
+
+            afterEach(function() {
+                fs.unlinkSync(filepath)
+            })
+
+            it('sync remove tags from file', function() {
+                NodeID3.removeTags(filepath)
+                assert.deepStrictEqual(
+                    fs.readFileSync(filepath),
+                    bufferAfterRemove
+                )
+            })
+
+            it('async remove tags from file', function(done) {
+                NodeID3.removeTags(filepath, (err) => {
+                    assert.equal(err, null)
+                    assert.deepStrictEqual(
+                        fs.readFileSync(filepath),
+                        bufferAfterRemove
+                    )
+                    done()
+                })
+            })
+        })
+    })
+})

test/api/update.ts

@@ -0,0 +1,79 @@
+import * as NodeID3 from '../../index'
+import assert = require('assert')
+import chai = require('chai')
+import * as fs from 'fs'
+import { WriteCallback } from '../../index'
+
+describe('NodeID3 API', function () {
+    describe('#update()', function() {
+        const titleTag = {
+            title: "abc"
+        } satisfies NodeID3.WriteTags
+        const albumTag = {
+            album: "def"
+        }
+        const tags = {...titleTag, ...albumTag}
+        const filepath = './testfile.mp3'
+
+        beforeEach(function() {
+            fs.writeFileSync(filepath, NodeID3.create(titleTag))
+        })
+
+        it('sync add tag to existing', function() {
+            chai.assert.isTrue(NodeID3.update(albumTag, filepath))
+            assert.deepStrictEqual(
+                NodeID3.read(filepath, {noRaw: true}),
+                tags
+            )
+        })
+
+        it('async add tag to existing', function(done) {
+            NodeID3.update(albumTag, filepath, (error, data) => {
+                chai.assert.isNull(error)
+                assert.deepStrictEqual(
+                    NodeID3.read(filepath, {noRaw: true}),
+                    tags
+                )
+                done()
+            })
+        })
+
+        it('compare key', function() {
+            const beforeTags = {
+                userDefinedText: [{
+                    description: 'description',
+                    value: 'some value'
+                }],
+                private: [{
+                    ownerIdentifier: 'ownerIdentifier',
+                    data: Buffer.from('data')
+                }]
+            } satisfies NodeID3.Tags
+            const addTags = {
+                userDefinedText: [{
+                    description: 'description',
+                    value: 'some other value'
+                }],
+                private: {
+                    ownerIdentifier: 'ownerIdentifier',
+                    data: Buffer.from('data2')
+                }
+            } satisfies NodeID3.Tags
+            const afterTags = {
+                userDefinedText: addTags.userDefinedText,
+                private: [...beforeTags.private, addTags.private]
+            } satisfies NodeID3.Tags
+            const beforeBuffer = NodeID3.create(beforeTags)
+            const afterBuffer = NodeID3.update(addTags, beforeBuffer)
+
+            assert.deepStrictEqual(
+                NodeID3.read(afterBuffer, {noRaw: true}),
+                afterTags
+            )
+        })
+
+        afterEach(function() {
+            fs.unlinkSync(filepath)
+        })
+    })
+})

test/frames.ts

@@ -1,14 +1,15 @@
 import * as NodeID3 from '../index'
 import assert = require('assert')
+import { expect } from 'chai'
 
 /**
  * Some characters to test unicode encoding.
  */
 const unicodeTestCharacters = "-äé"
+const TagConstants = NodeID3.TagConstants
 
 describe('NodeID3 frames', function () {
     it('read() matches create()', function () {
-        const TagConstants = NodeID3.TagConstants
         const tags = {
             /**
              * COMM
@@ -162,4 +163,123 @@ describe('NodeID3 frames', function () {
         const readTags = NodeID3.read(createdBuffer, { noRaw: true})
         assert.deepStrictEqual(tags, readTags)
     })
+
+    describe('read() does not match create()', function() {
+        it('create throws', function() {
+            const throwingTags = [
+                { POPM: {} },
+                { POPM: {
+                    email: 'test'
+                }},
+                { POPM: {
+                    email: 'test',
+                    rating: 1
+                }},
+                { CTOC: {} },
+                { USLT: {} },
+                { CHAP: {} },
+                { COMM: {} },
+                { TALB: null },
+                { WCOM: null },
+                { APIC: {
+                    mime: "a",
+                    type: {
+                        id: TagConstants.AttachedPicture.PictureType.FRONT_COVER
+                    },
+                    description: "d",
+                    imageBuffer: ""
+                }},
+                { COMM: {
+                    language: 'asdf',
+                    text: 'text'
+                }},
+                { COMR: {
+                    prices: {
+                        EURO: 13
+                    }
+                }}
+            ]
+
+            for(const throwingTag of throwingTags) {
+                expect(() => NodeID3.create(throwingTag as never)).to.throw()
+            }
+        })
+
+        it('frame builder changes data', function() {
+            const tags = {
+                unsynchronisedLyrics: 'just a string',
+                commercialFrame: {
+                    validUntil: { year: 2023, month: 9, day: 'a'},
+                    receivedAs: TagConstants.CommercialFrame.ReceivedAs.OTHER,
+                },
+                tableOfContents: {
+                    elementID: "1"
+                },
+                synchronisedLyrics: {
+                    language: "eng",
+                    timeStampFormat: TagConstants.TimeStampFormat.MILLISECONDS,
+                    contentType: TagConstants.SynchronisedLyrics.ContentType.LYRICS,
+                    synchronisedText: []
+                },
+                private: {
+                    data: 'string'
+                },
+                uniqueFileIdentifier: {
+                    ownerIdentifier: 'a',
+                    identifier: 'b'
+                },
+                image: Buffer.from([0xff, 0xd8, 0xff, 0x00])
+            }
+            const expectedTags = {
+                unsynchronisedLyrics: {
+                    language: 'eng',
+                    shortText: '',
+                    text: tags.unsynchronisedLyrics
+                },
+                commercialFrame: [{
+                    ...tags.commercialFrame,
+                    validUntil: {
+                        year: 0, month: 0, day: 0
+                    },
+                    prices: {},
+                    contactUrl: '',
+                    nameOfSeller: '',
+                    description: '',
+                }],
+                tableOfContents: [{
+                    ...tags.tableOfContents,
+                    isOrdered: false,
+                    elements: [],
+                    tags: { raw: {} }
+                }],
+                synchronisedLyrics: [{
+                    ...tags.synchronisedLyrics,
+                    shortText: ''
+                }],
+                private: [{
+                    ownerIdentifier: '',
+                    data: Buffer.from(tags.private.data, 'utf8')
+                }],
+                uniqueFileIdentifier: [{
+                    ...tags.uniqueFileIdentifier,
+                    identifier: Buffer.from(
+                        tags.uniqueFileIdentifier.identifier
+                    , 'utf8')
+                }],
+                image: {
+                    mime: "image/jpeg",
+                    type: {
+                        id: TagConstants.AttachedPicture.PictureType.FRONT_COVER,
+                        name: "front cover"
+                    },
+                    description: '',
+                    imageBuffer: tags.image
+                }
+            } satisfies NodeID3.Tags
+            assert.deepStrictEqual(
+                NodeID3.read(NodeID3.create(tags as never), {noRaw: true}),
+                expectedTags
+            )
+        })
+    })
 })