Increased test coverage of format tests

picard/file.py

@@ -117,7 +117,7 @@ class File(QtCore.QObject, Item):
         self.item = None
 
     def __repr__(self):
-        return '<File %r>' % self.base_filename
+        return '<%s %r>' % (type(self).__name__, self.base_filename)
 
     @property
     def new_metadata(self):

picard/formats/__init__.py

@@ -116,6 +116,7 @@ from picard.formats.vorbis import (
     FLACFile,
     OggFLACFile,
     OggSpeexFile,
+    OggTheoraFile,
     OggVorbisFile,
     OggAudioFile,
     OggVideoFile,
@@ -124,6 +125,7 @@ from picard.formats.vorbis import (
 register_format(FLACFile)
 register_format(OggFLACFile)
 register_format(OggSpeexFile)
+register_format(OggTheoraFile)
 register_format(OggVorbisFile)
 register_format(OggOpusFile)
 register_format(OggAudioFile)

picard/formats/vorbis.py

@@ -315,10 +315,6 @@ class FLACFile(VCommentFile):
     NAME = "FLAC"
     _File = mutagen.flac.FLAC
 
-    def _info(self, metadata, file):
-        super()._info(metadata, file)
-        metadata['~format'] = self.NAME
-
 
 class OggFLACFile(VCommentFile):
 
@@ -376,6 +372,7 @@ def OggAudioFile(filename):
 
 OggAudioFile.EXTENSIONS = [".oga"]
 OggAudioFile.NAME = "Ogg Audio"
+OggAudioFile.supports_tag = VCommentFile.supports_tag
 
 
 def OggVideoFile(filename):
@@ -386,3 +383,4 @@ def OggVideoFile(filename):
 
 OggVideoFile.EXTENSIONS = [".ogv"]
 OggVideoFile.NAME = "Ogg Video"
+OggVideoFile.supports_tag = VCommentFile.supports_tag

test/formats/common.py

@@ -185,11 +185,16 @@ class CommonTests:
 
         def copy_file_tmp(self, filename, ext):
             fd, copy = mkstemp(suffix=ext)
-            self.addCleanup(os.unlink, copy)
+            self.addCleanup(self.remove_file_tmp, copy)
             os.close(fd)
             shutil.copy(filename, copy)
             return copy
 
+        @staticmethod
+        def remove_file_tmp(tmpfile):
+            if os.path.isfile(tmpfile):
+                os.unlink(tmpfile)
+
     class SimpleFormatsTestCase(BaseFileTestCase):
 
         expected_info = None
@@ -208,6 +213,18 @@ class CommonTests:
                 value = metadata.length if key == 'length' else metadata[key]
                 self.assertEqual(value, expected_value)
 
+        def _test_supported_tags(self, tags):
+            metadata = Metadata(tags)
+            loaded_metadata = save_and_load_metadata(self.filename, metadata)
+            for (key, value) in tags.items():
+                self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value))
+
+        def _test_unsupported_tags(self, tags):
+            metadata = Metadata(tags)
+            loaded_metadata = save_and_load_metadata(self.filename, metadata)
+            for tag in tags:
+                self.assertTrue(tag not in loaded_metadata, '%s: %r != None' % (tag, loaded_metadata[tag]))
+
     class TagFormatsTestCase(SimpleFormatsTestCase):
 
         def setUp(self):
@@ -368,14 +385,27 @@ class CommonTests:
             self.assertEqual(f._fixed_splitext('.test'), os.path.splitext('.test'))
             self.assertNotEqual(f._fixed_splitext(f.EXTENSIONS[0]), os.path.splitext(f.EXTENSIONS[0]))
 
-        def _test_supported_tags(self, tags):
-            metadata = Metadata(tags)
-            loaded_metadata = save_and_load_metadata(self.filename, metadata)
-            for (key, value) in tags.items():
-                self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value))
+        @skipUnlessTestfile
+        def test_clear_existing_tags_off(self):
+            config.setting['clear_existing_tags'] = False
+            existing_metadata = Metadata({'artist': 'The Artist'})
+            save_metadata(self.filename, existing_metadata)
+            new_metadata = Metadata({'title': 'The Title'})
+            loaded_metadata = save_and_load_metadata(self.filename, new_metadata)
+            self.assertEqual(existing_metadata['artist'], loaded_metadata['artist'])
+            self.assertEqual(new_metadata['title'], loaded_metadata['title'])
 
-        def _test_unsupported_tags(self, tags):
-            metadata = Metadata(tags)
+        @skipUnlessTestfile
+        def test_clear_existing_tags_on(self):
+            config.setting['clear_existing_tags'] = True
+            existing_metadata = Metadata({'artist': 'The Artist'})
+            save_metadata(self.filename, existing_metadata)
+            new_metadata = Metadata({'title': 'The Title'})
+            loaded_metadata = save_and_load_metadata(self.filename, new_metadata)
+            self.assertNotIn('artist', loaded_metadata)
+            self.assertEqual(new_metadata['title'], loaded_metadata['title'])
+
+        def test_lyrcis_with_description(self):
+            metadata = Metadata({'lyrics:foo': 'bar'})
             loaded_metadata = save_and_load_metadata(self.filename, metadata)
-            for tag in tags:
-                self.assertTrue(tag not in loaded_metadata, '%s: %r != None' % (tag, loaded_metadata[tag]))
+            self.assertEqual(metadata['lyrics:foo'], loaded_metadata['lyrics'])

test/formats/test_apev2.py

@@ -1,11 +1,25 @@
+import os
+
+from mutagen.apev2 import (
+    BINARY,
+    APEValue,
+)
+
 from test.picardtestcase import PicardTestCase
 
-from picard.formats import apev2
+from picard import config
+from picard.formats import (
+    apev2,
+    open_,
+)
+from picard.metadata import Metadata
 
 from .common import (
     TAGS,
     CommonTests,
     load_metadata,
+    save_raw,
+    skipUnlessTestfile,
 )
 from .coverart import CommonCoverArtTests
 
@@ -48,6 +62,15 @@ class CommonApeTests:
             for key in INVALID_KEYS + apev2.UNSUPPORTED_TAGS:
                 self.assertFalse(supports_tag(key), '%r should be unsupported' % key)
 
+        @skipUnlessTestfile
+        def test_invalid_coverart(self):
+            metadata = {
+                'Cover Art (Front)': APEValue(b'filename.png\0NOTPNGDATA', BINARY)
+            }
+            save_raw(self.filename, metadata)
+            loaded_metadata = load_metadata(self.filename)
+            self.assertEqual(0, len(loaded_metadata.images))
+
 
 class MonkeysAudioTest(CommonApeTests.ApeTestCase):
     testfile = 'test.ape'
@@ -69,6 +92,35 @@ class WavPackTest(CommonApeTests.ApeTestCase):
         '~sample_rate': '44100',
     }
 
+    @skipUnlessTestfile
+    def test_save_wavpack_correction_file(self):
+        config.setting['rename_files'] = True
+        config.setting['move_files'] = False
+        config.setting['ascii_filenames'] = False
+        config.setting['windows_compatibility'] = False
+        config.setting['dont_write_tags'] = True
+        config.setting['preserve_timestamps'] = False
+        config.setting['delete_empty_dirs'] = False
+        config.setting['save_images_to_files'] = False
+        config.setting['file_naming_format'] = '%title%'
+        # Create dummy WavPack correction file
+        source_file_wvc = self.filename + 'c'
+        open(source_file_wvc, 'a').close()
+        # Open file and rename it
+        f = open_(self.filename)
+        metadata = Metadata({'title': 'renamed_' + os.path.basename(self.filename)})
+        self.assertTrue(os.path.isfile(self.filename))
+        target_file_wv = f._save_and_rename(self.filename, metadata)
+        target_file_wvc = target_file_wv + 'c'
+        # Register cleanups
+        self.addCleanup(os.unlink, target_file_wv)
+        self.addCleanup(os.unlink, target_file_wvc)
+        # Check both the WavPack file and the correction file got moved
+        self.assertFalse(os.path.isfile(self.filename))
+        self.assertFalse(os.path.isfile(source_file_wvc))
+        self.assertTrue(os.path.isfile(target_file_wv))
+        self.assertTrue(os.path.isfile(target_file_wvc))
+
 
 class MusepackSV7Test(CommonApeTests.ApeTestCase):
     testfile = 'test-sv7.mpc'

test/formats/test_id3.py

@@ -229,6 +229,11 @@ class CommonId3Tests:
             self.assertEqual(1, len(raw_metadata['TXXX:Replaygain_Album_Peak'].text))
             self.assertNotIn('TXXX:REPLAYGAIN_ALBUM_PEAK', raw_metadata)
 
+        def test_lyrcis_with_description(self):
+            metadata = Metadata({'lyrics:foo': 'bar'})
+            loaded_metadata = save_and_load_metadata(self.filename, metadata)
+            self.assertEqual(metadata['lyrics:foo'], loaded_metadata['lyrics:foo'])
+
 
 class MP3Test(CommonId3Tests.Id3TestCase):
     testfile = 'test.mp3'

test/formats/test_vorbis.py

@@ -1,6 +1,8 @@
 import base64
 import logging
 import os
+import shutil
+from tempfile import mkstemp
 
 from test.picardtestcase import PicardTestCase
 
@@ -124,6 +126,7 @@ class FLACTest(CommonVorbisTests.VorbisTestCase):
         'length': 82,
         '~channels': '2',
         '~sample_rate': '44100',
+        '~format': 'FLAC',
     }
 
     @skipUnlessTestfile
@@ -178,6 +181,24 @@ class OggOpusTest(CommonVorbisTests.VorbisTestCase):
         self._test_supported_tags(tags)
 
 
+class OggTheoraTest(CommonVorbisTests.VorbisTestCase):
+    testfile = 'test.ogv'
+    supports_ratings = True
+    expected_info = {
+        'length': 520,
+        '~bitrate': '200.0',
+    }
+
+
+class OggFlacTest(CommonVorbisTests.VorbisTestCase):
+    testfile = 'test-oggflac.oga'
+    supports_ratings = True
+    expected_info = {
+        'length': 82,
+        '~channels': '2',
+    }
+
+
 class VorbisUtilTest(PicardTestCase):
     def test_sanitize_key(self):
         sanitized = vorbis.sanitize_key(' \x1f=}~')
@@ -208,5 +229,38 @@ class FlacCoverArtTest(CommonCoverArtTests.CoverArtTestCase):
             self.assertEqual(pic.height, test.height)
 
 
+class OggAudioVideoFileTest(PicardTestCase):
+    def test_ogg_audio(self):
+        self._test_file_is_type(
+            vorbis.OggAudioFile,
+            self._copy_file_tmp('test-oggflac.oga', '.oga'),
+            vorbis.OggFLACFile)
+        self._test_file_is_type(
+            vorbis.OggAudioFile,
+            self._copy_file_tmp('test.spx', '.oga'),
+            vorbis.OggSpeexFile)
+        self._test_file_is_type(
+            vorbis.OggAudioFile,
+            self._copy_file_tmp('test.ogg', '.oga'),
+            vorbis.OggVorbisFile)
+
+    def test_ogg_video(self):
+        self._test_file_is_type(
+            vorbis.OggVideoFile,
+            self._copy_file_tmp('test.ogv', '.ogv'),
+            vorbis.OggTheoraFile)
+
+    def _test_file_is_type(self, factory, filename, expected_type):
+        f = factory(filename)
+        self.assertIsInstance(f, expected_type)
+
+    def _copy_file_tmp(self, filename, ext):
+        fd, copy = mkstemp(suffix=ext)
+        self.addCleanup(os.unlink, copy)
+        os.close(fd)
+        shutil.copy(os.path.join('test', 'data', filename), copy)
+        return copy
+
+
 class OggCoverArtTest(CommonCoverArtTests.CoverArtTestCase):
     testfile = 'test.ogg'

test/formats/test_wav.py

@@ -1,4 +1,9 @@
-from .common import CommonTests
+from .common import (
+    REPLAYGAIN_TAGS,
+    TAGS,
+    CommonTests,
+    skipUnlessTestfile,
+)
 
 
 class WAVTest(CommonTests.SimpleFormatsTestCase):
@@ -9,3 +14,11 @@ class WAVTest(CommonTests.SimpleFormatsTestCase):
         '~sample_rate': '44100',
         '~bits_per_sample': '16',
     }
+
+    def setUp(self):
+        super().setUp()
+        self.unsupported_tags = {**TAGS, **REPLAYGAIN_TAGS}
+
+    @skipUnlessTestfile
+    def test_unsupported_tags(self):
+        self._test_unsupported_tags(self.unsupported_tags)

test/picardtestcase.py

@@ -21,6 +21,7 @@ class FakeTagger(QtCore.QObject):
         self.tagger_stats_changed.connect(self.emit)
         self.exit_cleanup = []
         self.files = {}
+        self.stopping = False
 
     def register_cleanup(self, func):
         self.exit_cleanup.append(func)

test/test_bytes2human.py

@@ -45,6 +45,15 @@ class Testbytes2human(PicardTestCase):
         except Exception as e:
             self.fail('Unexpected exception: %s' % e)
 
+    def test_calc_unit_raises_value_error(self):
+        self.assertRaises(ValueError, bytes2human.calc_unit, 1, None)
+        self.assertRaises(ValueError, bytes2human.calc_unit, 1, 100)
+        self.assertRaises(ValueError, bytes2human.calc_unit, 1, 999)
+        self.assertRaises(ValueError, bytes2human.calc_unit, 1, 1023)
+        self.assertRaises(ValueError, bytes2human.calc_unit, 1, 1025)
+        self.assertEquals((1.0, 'B'), bytes2human.calc_unit(1, 1024))
+        self.assertEquals((1.0, 'B'), bytes2human.calc_unit(1, 1000))
+
     def run_test(self, lang='C', create_test_data=False):
         """
         Compare data generated with sample files
@@ -70,12 +79,12 @@ class Testbytes2human(PicardTestCase):
                 p *= n
                 for x in [0.1, 0.5, 0.99, 0.9999, 1, 1.5]:
                     values.append(int(p * x))
-        l = []
+        list = []
         for x in sorted(values):
-            l.append(";".join([str(x), bytes2human.decimal(x),
+            list.append(";".join([str(x), bytes2human.decimal(x),
                                   bytes2human.binary(x),
                                   bytes2human.short_string(x, 1024, 2)]))
-        return l
+        return list
 
     def _save_expected_to(self, path, a_list):
         with open(path, 'wb') as f: