From fb6807636d16e5d8557b4d4a3e4285336e8415d4 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 3 Apr 2019 12:22:53 +0200 Subject: [PATCH] Add tests for config upgrade functions --- picard/config_upgrade.py | 152 ++++++++++++------------ picard/tagger.py | 2 +- test/test_config_upgrade.py | 224 ++++++++++++++++++++++++++++++++++++ 3 files changed, 306 insertions(+), 72 deletions(-) create mode 100644 test/test_config_upgrade.py diff --git a/picard/config_upgrade.py b/picard/config_upgrade.py index 1edae333b..1a3c3cc7c 100644 --- a/picard/config_upgrade.py +++ b/picard/config_upgrade.py @@ -22,9 +22,15 @@ import re from PyQt5 import QtWidgets -from picard import ( - config, - log, +from picard import log +from picard.config import ( + BoolOption, + IntOption, + TextOption, +) +from picard.const import ( + DEFAULT_FILE_NAMING_FORMAT, + DEFAULT_NUMBERED_SCRIPT_NAME, ) # TO ADD AN UPGRADE HOOK: @@ -36,7 +42,7 @@ from picard import ( # -def upgrade_to_v1_0_0_final_0(): +def upgrade_to_v1_0_0_final_0(config, interactive=True, merge=True): """In version 1.0, the file naming formats for single and various artist releases were merged. """ _s = config.setting @@ -46,57 +52,59 @@ def upgrade_to_v1_0_0_final_0(): _s["file_naming_format"] = ( "$if($eq(%%compilation%%,1),\n$noop(Various Artist " "albums)\n%s,\n$noop(Single Artist Albums)\n%s)" % ( - _s.value("va_file_naming_format", config.TextOption), + _s.value("va_file_naming_format", TextOption), _s["file_naming_format"] )) _s.remove("va_file_naming_format") _s.remove("use_va_format") if "va_file_naming_format" in _s and "use_va_format" in _s: - msgbox = QtWidgets.QMessageBox() - if _s.value("use_va_format", config.BoolOption): + if _s.value("use_va_format", BoolOption): remove_va_file_naming_format() - msgbox.information(msgbox, - _("Various Artists file naming scheme removal"), - _("The separate file naming scheme for various artists " - "albums has been removed in this version of Picard.\n" - "Your file naming scheme has automatically been " - "merged with that of single artist albums."), - QtWidgets.QMessageBox.Ok) + if interactive: + msgbox = QtWidgets.QMessageBox() + msgbox.information(msgbox, + _("Various Artists file naming scheme removal"), + _("The separate file naming scheme for various artists " + "albums has been removed in this version of Picard.\n" + "Your file naming scheme has automatically been " + "merged with that of single artist albums."), + QtWidgets.QMessageBox.Ok) - elif (_s.value("va_file_naming_format", config.TextOption) + elif (_s.value("va_file_naming_format", TextOption) != r"$if2(%albumartist%,%artist%)/%album%/$if($gt(%totaldis" "cs%,1),%discnumber%-,)$num(%tracknumber%,2) %artist% - " "%title%"): - - msgbox.setWindowTitle(_("Various Artists file naming scheme removal")) - msgbox.setText(_("The separate file naming scheme for various artists " - "albums has been removed in this version of Picard.\n" - "You currently do not use this option, but have a " - "separate file naming scheme defined.\n" - "Do you want to remove it or merge it with your file " - "naming scheme for single artist albums?")) - msgbox.setIcon(QtWidgets.QMessageBox.Question) - merge_button = msgbox.addButton(_('Merge'), QtWidgets.QMessageBox.AcceptRole) - msgbox.addButton(_('Remove'), QtWidgets.QMessageBox.DestructiveRole) - msgbox.exec() - merge = msgbox.clickedButton() == merge_button + if interactive: + msgbox = QtWidgets.QMessageBox() + msgbox.setWindowTitle(_("Various Artists file naming scheme removal")) + msgbox.setText(_("The separate file naming scheme for various artists " + "albums has been removed in this version of Picard.\n" + "You currently do not use this option, but have a " + "separate file naming scheme defined.\n" + "Do you want to remove it or merge it with your file " + "naming scheme for single artist albums?")) + msgbox.setIcon(QtWidgets.QMessageBox.Question) + merge_button = msgbox.addButton(_('Merge'), QtWidgets.QMessageBox.AcceptRole) + msgbox.addButton(_('Remove'), QtWidgets.QMessageBox.DestructiveRole) + msgbox.exec() + merge = msgbox.clickedButton() == merge_button remove_va_file_naming_format(merge=merge) else: # default format, disabled remove_va_file_naming_format(merge=False) -def upgrade_to_v1_3_0_dev_1(): +def upgrade_to_v1_3_0_dev_1(config): """Option "windows_compatible_filenames" was renamed "windows_compatibility" (PICARD-110). """ old_opt = "windows_compatible_filenames" new_opt = "windows_compatibility" - rename_option(old_opt, new_opt, config.BoolOption, True) + rename_option(config, old_opt, new_opt, BoolOption, True) -def upgrade_to_v1_3_0_dev_2(): +def upgrade_to_v1_3_0_dev_2(config): """Option "preserved_tags" is now using comma instead of spaces as tag separator (PICARD-536) """ _s = config.setting @@ -105,7 +113,7 @@ def upgrade_to_v1_3_0_dev_2(): _s[opt] = re.sub(r"\s+", ",", _s[opt].strip()) -def upgrade_to_v1_3_0_dev_3(): +def upgrade_to_v1_3_0_dev_3(config): """Options were made to support lists (solving PICARD-144 and others) """ _s = config.setting @@ -118,10 +126,13 @@ def upgrade_to_v1_3_0_dev_3(): } for (opt, sep) in option_separators.items(): if opt in _s: - _s[opt] = _s.raw_value(opt).split(sep) + try: + _s[opt] = _s.raw_value(opt, qtype='QString').split(sep) + except AttributeError: + pass -def upgrade_to_v1_3_0_dev_4(): +def upgrade_to_v1_3_0_dev_4(config): """Option "release_type_scores" is now a list of tuples """ _s = config.setting @@ -139,10 +150,14 @@ def upgrade_to_v1_3_0_dev_4(): opt = "release_type_scores" if opt in _s: - _s[opt] = load_release_type_scores(_s.raw_value(opt)) + try: + _s[opt] = load_release_type_scores(_s.raw_value(opt, qtype='QString')) + except AttributeError: + pass -def upgrade_to_v1_4_0_dev_2(): + +def upgrade_to_v1_4_0_dev_2(config): """Options "username" and "password" are removed and replaced with OAuth tokens """ @@ -153,7 +168,7 @@ def upgrade_to_v1_4_0_dev_2(): _s.remove(opt) -def upgrade_to_v1_4_0_dev_3(): +def upgrade_to_v1_4_0_dev_3(config): """Cover art providers options were moved to a list of tuples""" _s = config.setting map_ca_provider = [ @@ -166,45 +181,40 @@ def upgrade_to_v1_4_0_dev_3(): newopts = [] for old, new in map_ca_provider: if old in _s: - newopts.append((new, _s.value(old, config.BoolOption, True))) + newopts.append((new, _s.value(old, BoolOption, True))) _s['ca_providers'] = newopts -def upgrade_to_v1_4_0_dev_4(): +OLD_DEFAULT_FILE_NAMING_FORMAT = "$if2(%albumartist%,%artist%)/" \ + "$if($ne(%albumartist%,),%album%/)" \ + "$if($gt(%totaldiscs%,1),%discnumber%-,)" \ + "$if($ne(%albumartist%,),$num(%tracknumber%,2) ,)" \ + "$if(%_multiartist%,%artist% - ,)" \ + "%title%" + + +def upgrade_to_v1_4_0_dev_4(config): """Adds trailing comma to default file names for scripts""" _s = config.setting - _DEFAULT_FILE_NAMING_FORMAT = "$if2(%albumartist%,%artist%)/" \ - "$if($ne(%albumartist%,),%album%/)" \ - "$if($gt(%totaldiscs%,1),%discnumber%-,)" \ - "$if($ne(%albumartist%,),$num(%tracknumber%,2) ,)" \ - "$if(%_multiartist%,%artist% - ,)" \ - "%title%" - if _s["file_naming_format"] == _DEFAULT_FILE_NAMING_FORMAT: - _DEFAULT_FILE_NAMING_FORMAT = "$if2(%albumartist%,%artist%)/" \ - "$if($ne(%albumartist%,),%album%/,)" \ - "$if($gt(%totaldiscs%,1),%discnumber%-,)" \ - "$if($ne(%albumartist%,),$num(%tracknumber%,2) ,)" \ - "$if(%_multiartist%,%artist% - ,)" \ - "%title%" - _s["file_naming_format"] = _DEFAULT_FILE_NAMING_FORMAT + if _s["file_naming_format"] == OLD_DEFAULT_FILE_NAMING_FORMAT: + _s["file_naming_format"] = DEFAULT_FILE_NAMING_FORMAT -def upgrade_to_v1_4_0_dev_5(): +def upgrade_to_v1_4_0_dev_5(config): """Using Picard.ini configuration file on all platforms""" # this is done in Config.__init__() -def upgrade_to_v1_4_0_dev_6(): +def upgrade_to_v1_4_0_dev_6(config): """Adds support for multiple and selective tagger scripts""" _s = config.setting - DEFAULT_NUMBERED_SCRIPT_NAME = N_("My script %d") old_enabled_option = "enable_tagger_script" old_script_text_option = "tagger_script" list_of_scripts = [] if old_enabled_option in _s: - _s["enable_tagger_scripts"] = _s.value(old_enabled_option, config.BoolOption, False) + _s["enable_tagger_scripts"] = _s.value(old_enabled_option, BoolOption, False) if old_script_text_option in _s: - old_script_text = _s.value(old_script_text_option, config.TextOption, "") + old_script_text = _s.value(old_script_text_option, TextOption, "") if old_script_text: old_script = (0, _(DEFAULT_NUMBERED_SCRIPT_NAME) % 1, _s["enable_tagger_scripts"], old_script_text) list_of_scripts.append(old_script) @@ -213,14 +223,14 @@ def upgrade_to_v1_4_0_dev_6(): _s.remove(old_script_text_option) -def upgrade_to_v1_4_0_dev_7(): +def upgrade_to_v1_4_0_dev_7(config): """Option "save_only_front_images_to_tags" was renamed to "embed_only_one_front_image".""" old_opt = "save_only_front_images_to_tags" new_opt = "embed_only_one_front_image" - rename_option(old_opt, new_opt, config.BoolOption, True) + rename_option(config, old_opt, new_opt, BoolOption, True) -def upgrade_to_v2_0_0_dev_3(): +def upgrade_to_v2_0_0_dev_3(config): """Option "caa_image_size" value has different meaning.""" _s = config.setting opt = "caa_image_size" @@ -239,28 +249,28 @@ def upgrade_to_v2_0_0_dev_3(): _s[opt] = _CAA_SIZE_COMPAT[value] -def upgrade_to_v2_1_0_dev_1(): +def upgrade_to_v2_1_0_dev_1(config): """Upgrade genre related options""" _s = config.setting if "folksonomy_tags" in _s and _s["folksonomy_tags"]: _s["use_genres"] = True - rename_option("max_tags", "max_genres", config.IntOption, 5) - rename_option("min_tag_usage", "min_genre_usage", config.IntOption, 90) - rename_option("ignore_tags", "ignore_genres", config.TextOption, "") - rename_option("join_tags", "join_genres", config.TextOption, "") - rename_option("only_my_tags", "only_my_genres", config.BoolOption, False) - rename_option("artists_tags", "artists_genres", config.BoolOption, False) + rename_option(config, "max_tags", "max_genres", IntOption, 5) + rename_option(config, "min_tag_usage", "min_genre_usage", IntOption, 90) + rename_option(config, "ignore_tags", "ignore_genres", TextOption, "") + rename_option(config, "join_tags", "join_genres", TextOption, "") + rename_option(config, "only_my_tags", "only_my_genres", BoolOption, False) + rename_option(config, "artists_tags", "artists_genres", BoolOption, False) -def rename_option(old_opt, new_opt, option_type, default): +def rename_option(config, old_opt, new_opt, option_type, default): _s = config.setting if old_opt in _s: _s[new_opt] = _s.value(old_opt, option_type, default) _s.remove(old_opt) -def upgrade_config(): - cfg = config.config +def upgrade_config(config): + cfg = config cfg.register_upgrade_hook(upgrade_to_v1_0_0_final_0) cfg.register_upgrade_hook(upgrade_to_v1_3_0_dev_1) cfg.register_upgrade_hook(upgrade_to_v1_3_0_dev_2) diff --git a/picard/tagger.py b/picard/tagger.py index 0363606ba..b2ef2ed64 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -223,7 +223,7 @@ class Tagger(QtWidgets.QApplication): # translated setup_gettext(localedir, config.setting["ui_language"], log.debug) - upgrade_config() + upgrade_config(config.config) self.webservice = WebService() self.mb_api = MBAPIHelper(self.webservice) diff --git a/test/test_config_upgrade.py b/test/test_config_upgrade.py new file mode 100644 index 000000000..905cf7d62 --- /dev/null +++ b/test/test_config_upgrade.py @@ -0,0 +1,224 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# Copyright (C) 2019 Laurent Monin +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +from test.test_config import TestPicardConfigCommon + +from picard.config import ( + BoolOption, + IntOption, + ListOption, + TextOption, +) +from picard.config_upgrade import ( + OLD_DEFAULT_FILE_NAMING_FORMAT, + upgrade_to_v1_0_0_final_0, + upgrade_to_v1_3_0_dev_1, + upgrade_to_v1_3_0_dev_2, + upgrade_to_v1_3_0_dev_3, + upgrade_to_v1_3_0_dev_4, + upgrade_to_v1_4_0_dev_2, + upgrade_to_v1_4_0_dev_3, + upgrade_to_v1_4_0_dev_4, + upgrade_to_v1_4_0_dev_6, + upgrade_to_v1_4_0_dev_7, + upgrade_to_v2_0_0_dev_3, + upgrade_to_v2_1_0_dev_1, +) +from picard.const import ( + DEFAULT_FILE_NAMING_FORMAT, + DEFAULT_NUMBERED_SCRIPT_NAME, +) + + +class TestPicardConfigUpgrades(TestPicardConfigCommon): + + def test_upgrade_to_v1_0_0_final_0_A(self): + TextOption('setting', 'file_naming_format', '') + + self.config.setting['va_file_naming_format'] = 'abc' + self.config.setting['use_va_format'] = True + + self.assertIn('va_file_naming_format', self.config.setting) + self.assertIn('use_va_format', self.config.setting) + + upgrade_to_v1_0_0_final_0(self.config, interactive=False, merge=True) + self.assertNotIn('va_file_naming_format', self.config.setting) + self.assertNotIn('use_va_format', self.config.setting) + self.assertIn('file_naming_format', self.config.setting) + + def test_upgrade_to_v1_0_0_final_0_B(self): + TextOption('setting', 'file_naming_format', '') + + self.config.setting['va_file_naming_format'] = 'abc' + self.config.setting['use_va_format'] = "" + + self.assertIn('va_file_naming_format', self.config.setting) + self.assertIn('use_va_format', self.config.setting) + + upgrade_to_v1_0_0_final_0(self.config, interactive=False, merge=False) + self.assertNotIn('va_file_naming_format', self.config.setting) + self.assertNotIn('use_va_format', self.config.setting) + self.assertNotIn('file_naming_format', self.config.setting) + + def test_upgrade_to_v1_3_0_dev_1(self): + BoolOption('setting', 'windows_compatibility', False) + + self.config.setting['windows_compatible_filenames'] = True + upgrade_to_v1_3_0_dev_1(self.config) + self.assertNotIn('windows_compatible_filenames', self.config.setting) + self.assertTrue(self.config.setting['windows_compatibility']) + + def test_upgrade_to_v1_3_0_dev_2(self): + TextOption('setting', 'preserved_tags', '') + + self.config.setting['preserved_tags'] = "a b c " + upgrade_to_v1_3_0_dev_2(self.config) + self.assertEqual("a,b,c", self.config.setting['preserved_tags']) + + def test_upgrade_to_v1_3_0_dev_3(self): + ListOption("setting", "preferred_release_countries", []) + ListOption("setting", "preferred_release_formats", []) + ListOption("setting", "enabled_plugins", []) + ListOption("setting", "caa_image_types", []) + ListOption("setting", "metadata_box_sizes", []) + + self.config.setting['preferred_release_countries'] = "a b c" + self.config.setting['preferred_release_formats'] = "a b c" + self.config.setting['enabled_plugins'] = 'a b c' + self.config.setting['caa_image_types'] = 'a b c' + self.config.setting['metadata_box_sizes'] = 'a b c' + + upgrade_to_v1_3_0_dev_3(self.config) + self.assertEqual(["a", "b", "c"], self.config.setting['preferred_release_countries']) + self.assertEqual(["a", "b", "c"], self.config.setting['preferred_release_formats']) + self.assertEqual(["a", "b", "c"], self.config.setting['enabled_plugins']) + self.assertEqual(["a", "b", "c"], self.config.setting['caa_image_types']) + self.assertEqual(["a", "b", "c"], self.config.setting['metadata_box_sizes']) + + def test_upgrade_to_v1_3_0_dev_4(self): + ListOption("setting", "release_type_scores", []) + + self.config.setting['release_type_scores'] = "a 0.1 b 0.2 c 1" + upgrade_to_v1_3_0_dev_4(self.config) + + self.assertEqual([('a', 0.1), ('b', 0.2), ('c', 1.0)], self.config.setting['release_type_scores']) + + def test_upgrade_to_v1_4_0_dev_2(self): + self.config.setting['username'] = 'abc' + self.config.setting['password'] = 'abc' # nosec + + upgrade_to_v1_4_0_dev_2(self.config) + self.assertNotIn('username', self.config.setting) + self.assertNotIn('password', self.config.setting) + + def test_upgrade_to_v1_4_0_dev_3(self): + ListOption("setting", "ca_providers", []) + + self.config.setting['ca_provider_use_amazon'] = True + self.config.setting['ca_provider_use_caa'] = True + self.config.setting['ca_provider_use_whitelist'] = False + self.config.setting['ca_provider_use_caa_release_group_fallback'] = True + + upgrade_to_v1_4_0_dev_3(self.config) + self.assertIn('ca_providers', self.config.setting) + self.assertIn(('Amazon', True), self.config.setting['ca_providers']) + self.assertIn(('Cover Art Archive', True), self.config.setting['ca_providers']) + self.assertIn(('Whitelist', False), self.config.setting['ca_providers']) + self.assertIn(('CaaReleaseGroup', True), self.config.setting['ca_providers']) + self.assertEqual(len(self.config.setting['ca_providers']), 4) + + def test_upgrade_to_v1_4_0_dev_4(self): + TextOption("setting", "file_naming_format", "") + + self.config.setting['file_naming_format'] = 'xxx' + upgrade_to_v1_4_0_dev_4(self.config) + self.assertEqual('xxx', self.config.setting['file_naming_format']) + + self.config.setting['file_naming_format'] = OLD_DEFAULT_FILE_NAMING_FORMAT + upgrade_to_v1_4_0_dev_4(self.config) + self.assertEqual(DEFAULT_FILE_NAMING_FORMAT, self.config.setting['file_naming_format']) + + def test_upgrade_to_v1_4_0_dev_6(self): + BoolOption('setting', 'enable_tagger_scripts', False) + ListOption('setting', 'list_of_scripts', []) + + self.config.setting['enable_tagger_script'] = True + self.config.setting['tagger_script'] = "abc" + upgrade_to_v1_4_0_dev_6(self.config) + + self.assertNotIn('enable_tagger_script', self.config.setting) + self.assertNotIn('tagger_script', self.config.setting) + + self.assertTrue(self.config.setting['enable_tagger_scripts']) + self.assertEqual([(0, DEFAULT_NUMBERED_SCRIPT_NAME % 1, True, 'abc')], self.config.setting['list_of_scripts']) + + def test_upgrade_to_v1_4_0_dev_7(self): + BoolOption('setting', 'embed_only_one_front_image', False) + + self.config.setting['save_only_front_images_to_tags'] = True + upgrade_to_v1_4_0_dev_7(self.config) + self.assertNotIn('save_only_front_images_to_tags', self.config.setting) + self.assertTrue(self.config.setting['embed_only_one_front_image']) + + def test_upgrade_to_v2_0_0_dev_3(self): + IntOption("setting", "caa_image_size", 500) + + self.config.setting['caa_image_size'] = 0 + upgrade_to_v2_0_0_dev_3(self.config) + self.assertEqual(250, self.config.setting['caa_image_size']) + + self.config.setting['caa_image_size'] = 501 + upgrade_to_v2_0_0_dev_3(self.config) + self.assertEqual(501, self.config.setting['caa_image_size']) + + def test_upgrade_to_v2_1_0_dev_1(self): + BoolOption("setting", "use_genres", False) + IntOption("setting", "max_genres", 5) + IntOption("setting", "min_genre_usage", 90) + TextOption("setting", "ignore_genres", "seen live, favorites, fixme, owned") + TextOption("setting", "join_genres", "") + BoolOption("setting", "only_my_genres", False) + BoolOption("setting", "artists_genres", False) + BoolOption("setting", "folksonomy_tags", False) + + self.config.setting['folksonomy_tags'] = True + self.config.setting['max_tags'] = 6 + self.config.setting['min_tag_usage'] = 85 + self.config.setting['ignore_tags'] = "abc" + self.config.setting['join_tags'] = "abc" + self.config.setting['only_my_tags'] = True + self.config.setting['artists_tags'] = True + + upgrade_to_v2_1_0_dev_1(self.config) + self.assertEqual(self.config.setting['use_genres'], True) + self.assertEqual(self.config.setting['max_genres'], 6) + self.assertEqual(self.config.setting['min_genre_usage'], 85) + self.assertEqual(self.config.setting['ignore_genres'], "abc") + self.assertEqual(self.config.setting['join_genres'], "abc") + self.assertEqual(self.config.setting['only_my_genres'], True) + self.assertEqual(self.config.setting['artists_genres'], True) + + self.assertIn('folksonomy_tags', self.config.setting) + + self.assertNotIn('max_tags', self.config.setting) + self.assertNotIn('min_tag_usage', self.config.setting) + self.assertNotIn('ignore_tags', self.config.setting) + self.assertNotIn('join_tags', self.config.setting) + self.assertNotIn('only_my_tags', self.config.setting) + self.assertNotIn('artists_tags', self.config.setting)