openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #32398
[Merge] lp:~raoul-snyman/openlp/settings-upgrade into lp:openlp
Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/settings-upgrade into lp:openlp.
Commit message:
Add multi-to-one settings upgrading, and merge all the post-2.4 settings changes into __setting_upgrade_2__
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/settings-upgrade/+merge/333788
Add multi-to-one settings upgrading, and merge all the post-2.4 settings changes into __setting_upgrade_2__
Add this to your merge proposal:
--------------------------------------------------------------------------------
lp:~raoul-snyman/openlp/settings-upgrade (revision 2791)
https://ci.openlp.io/job/Branch-01-Pull/2295/ [SUCCESS]
https://ci.openlp.io/job/Branch-02-Functional-Tests/2196/ [SUCCESS]
https://ci.openlp.io/job/Branch-03-Interface-Tests/2074/ [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code_Analysis/1401/ [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test_Coverage/1225/ [SUCCESS]
https://ci.openlp.io/job/Branch-04c-Code_Analysis2/355/ [SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/187/ [FAILURE]
Stopping after failure, see https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/187/console for more details
--
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/settings-upgrade into lp:openlp.
=== modified file 'nose2.cfg'
--- nose2.cfg 2016-08-10 09:07:49 +0000
+++ nose2.cfg 2017-11-16 05:12:46 +0000
@@ -1,5 +1,5 @@
[unittest]
-verbose = true
+verbose = True
plugins = nose2.plugins.mp
[log-capture]
@@ -9,14 +9,19 @@
log-level = ERROR
[test-result]
-always-on = true
-descriptions = true
+always-on = True
+descriptions = True
[coverage]
-always-on = true
+always-on = True
coverage = openlp
coverage-report = html
[multiprocess]
-always-on = false
+always-on = False
processes = 4
+
+[output-buffer]
+always-on = True
+stderr = True
+stdout = False
=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py 2017-11-10 22:50:04 +0000
+++ openlp/core/common/settings.py 2017-11-16 05:12:46 +0000
@@ -40,7 +40,7 @@
# Fix for bug #1014422.
X11_BYPASS_DEFAULT = True
-if is_linux():
+if is_linux(): # pragma: no cover
# Default to False on Gnome.
X11_BYPASS_DEFAULT = bool(not os.environ.get('GNOME_DESKTOP_SESSION_ID'))
# Default to False on Xfce.
@@ -206,11 +206,14 @@
'projector/source dialog type': 0 # Source select dialog box type
}
__file_path__ = ''
+ # Settings upgrades prior to 3.0
__setting_upgrade_1__ = [
- # Changed during 2.2.x development.
('songs/search as type', 'advanced/search as type', []),
('media/players', 'media/players_temp', [(media_players_conv, None)]), # Convert phonon to system
('media/players_temp', 'media/players', []), # Move temp setting from above to correct setting
+ ]
+ # Settings upgrades for 3.0 (aka 2.6)
+ __setting_upgrade_2__ = [
('advanced/default color', 'core/logo background color', []), # Default image renamed + moved to general > 2.4.
('advanced/default image', 'core/logo file', []), # Default image renamed + moved to general after 2.4.
('remotes/https enabled', '', []),
@@ -231,9 +234,7 @@
# Last search type was renamed to last used search type in 2.6 since Bible search value type changed in 2.6.
('songs/last search type', 'songs/last used search type', []),
('bibles/last search type', '', []),
- ('custom/last search type', 'custom/last used search type', [])]
-
- __setting_upgrade_2__ = [
+ ('custom/last search type', 'custom/last used search type', []),
# The following changes are being made for the conversion to using Path objects made in 2.6 development
('advanced/data path', 'advanced/data path', [(str_to_path, None)]),
('crashreport/last directory', 'crashreport/last directory', [(str_to_path, None)]),
@@ -467,32 +468,38 @@
for version in range(current_version, __version__):
version += 1
upgrade_list = getattr(self, '__setting_upgrade_{version}__'.format(version=version))
- for old_key, new_key, rules in upgrade_list:
+ for old_keys, new_key, rules in upgrade_list:
# Once removed we don't have to do this again. - Can be removed once fully switched to the versioning
# system.
- if not self.contains(old_key):
+ if not isinstance(old_keys, (tuple, list)):
+ old_keys = [old_keys]
+ if any([not self.contains(old_key) for old_key in old_keys]):
+ log.warning('One of {} does not exist, skipping upgrade'.format(old_keys))
continue
if new_key:
# Get the value of the old_key.
- old_value = super(Settings, self).value(old_key)
+ old_values = [super(Settings, self).value(old_key) for old_key in old_keys]
# When we want to convert the value, we have to figure out the default value (because we cannot get
# the default value from the central settings dict.
if rules:
- default_value = rules[0][1]
- old_value = self._convert_value(old_value, default_value)
+ default_values = rules[0][1]
+ if not isinstance(default_values, (list, tuple)):
+ default_values = [default_values]
+ old_values = [self._convert_value(old_value, default_value)
+ for old_value, default_value in zip(old_values, default_values)]
# Iterate over our rules and check what the old_value should be "converted" to.
- for new, old in rules:
+ new_value = None
+ for new_rule, old_rule in rules:
# If the value matches with the condition (rule), then use the provided value. This is used to
# convert values. E. g. an old value 1 results in True, and 0 in False.
- if callable(new):
- old_value = new(old_value)
- elif old == old_value:
- old_value = new
+ if callable(new_rule):
+ new_value = new_rule(*old_values)
+ elif old_rule in old_values:
+ new_value = new_rule
break
- self.setValue(new_key, old_value)
- if new_key != old_key:
- self.remove(old_key)
- self.setValue('settings/version', version)
+ self.setValue(new_key, new_value)
+ [self.remove(old_key) for old_key in old_keys if old_key != new_key]
+ self.setValue('settings/version', version)
def value(self, key):
"""
=== modified file 'tests/functional/openlp_core/common/test_settings.py'
--- tests/functional/openlp_core/common/test_settings.py 2017-10-07 07:05:07 +0000
+++ tests/functional/openlp_core/common/test_settings.py 2017-11-16 05:12:46 +0000
@@ -22,10 +22,12 @@
"""
Package to test the openlp.core.lib.settings package.
"""
+from pathlib import Path
from unittest import TestCase
-from unittest.mock import patch
+from unittest.mock import call, patch
-from openlp.core.common.settings import Settings
+from openlp.core.common import settings
+from openlp.core.common.settings import Settings, media_players_conv
from tests.helpers.testmixin import TestMixin
@@ -47,28 +49,58 @@
"""
self.destroy_settings()
- def test_settings_basic(self):
- """
- Test the Settings creation and its default usage
- """
- # GIVEN: A new Settings setup
+ def test_media_players_conv(self):
+ """Test the media players conversion function"""
+ # GIVEN: A list of media players
+ media_players = 'phonon,webkit,vlc'
+
+ # WHEN: The media converter function is called
+ result = media_players_conv(media_players)
+
+ # THEN: The list should have been converted correctly
+ assert result == 'system,webkit,vlc'
+
+ def test_default_value(self):
+ """Test reading a setting that doesn't exist yet"""
+ # GIVEN: A setting that doesn't exist yet
# WHEN reading a setting for the first time
default_value = Settings().value('core/has run wizard')
# THEN the default value is returned
- self.assertFalse(default_value, 'The default value should be False')
+ assert default_value is False, 'The default value should be False'
+ def test_save_new_value(self):
+ """Test saving a new setting"""
+ # GIVEN: A setting that hasn't been saved yet
# WHEN a new value is saved into config
Settings().setValue('core/has run wizard', True)
# THEN the new value is returned when re-read
- self.assertTrue(Settings().value('core/has run wizard'), 'The saved value should have been returned')
+ assert Settings().value('core/has run wizard') is True, 'The saved value should have been returned'
+
+ def test_set_up_default_values(self):
+ """Test that the default values are updated"""
+ # GIVEN: A Settings object with defaults
+ # WHEN: set_up_default_values() is called
+ Settings.set_up_default_values()
+
+ # THEN: The default values should have been added to the dictionary
+ assert 'advanced/default service name' in Settings.__default_settings__
+
+ def test_get_default_value(self):
+ """Test that the default value for a setting is returned"""
+ # GIVEN: A Settings class with a default value
+ Settings.__default_settings__['test/moo'] = 'baa'
+
+ # WHEN: get_default_value() is called
+ result = Settings().get_default_value('test/moo')
+
+ # THEN: The correct default value should be returned
+ assert result == 'baa'
def test_settings_override(self):
- """
- Test the Settings creation and its override usage
- """
+ """Test the Settings creation and its override usage"""
# GIVEN: an override for the settings
screen_settings = {
'test/extend': 'very wide',
@@ -79,18 +111,22 @@
extend = Settings().value('test/extend')
# THEN the default value is returned
- self.assertEqual('very wide', extend, 'The default value of "very wide" should be returned')
+ assert extend == 'very wide', 'The default value of "very wide" should be returned'
+
+ def test_save_existing_setting(self):
+ """Test that saving an existing setting returns the new value"""
+ # GIVEN: An existing setting
+ Settings().extend_default_settings({'test/existing value': None})
+ Settings().setValue('test/existing value', 'old value')
# WHEN a new value is saved into config
- Settings().setValue('test/extend', 'very short')
+ Settings().setValue('test/existing value', 'new value')
# THEN the new value is returned when re-read
- self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned')
+ assert Settings().value('test/existing value') == 'new value', 'The saved value should be returned'
def test_settings_override_with_group(self):
- """
- Test the Settings creation and its override usage - with groups
- """
+ """Test the Settings creation and its override usage - with groups"""
# GIVEN: an override for the settings
screen_settings = {
'test/extend': 'very wide',
@@ -112,9 +148,7 @@
self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned')
def test_settings_nonexisting(self):
- """
- Test the Settings on query for non-existing value
- """
+ """Test the Settings on query for non-existing value"""
# GIVEN: A new Settings setup
with self.assertRaises(KeyError) as cm:
# WHEN reading a setting that doesn't exists
@@ -124,9 +158,7 @@
self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception')
def test_extend_default_settings(self):
- """
- Test that the extend_default_settings method extends the default settings
- """
+ """Test that the extend_default_settings method extends the default settings"""
# GIVEN: A patched __default_settings__ dictionary
with patch.dict(Settings.__default_settings__,
{'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 3}, True):
@@ -138,3 +170,125 @@
self.assertEqual(
Settings.__default_settings__, {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 4,
'test/extended 1': 1, 'test/extended 2': 2})
+
+ @patch('openlp.core.common.settings.QtCore.QSettings.contains')
+ @patch('openlp.core.common.settings.QtCore.QSettings.value')
+ @patch('openlp.core.common.settings.QtCore.QSettings.setValue')
+ @patch('openlp.core.common.settings.QtCore.QSettings.remove')
+ def test_upgrade_single_setting(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains):
+ """Test that the upgrade mechanism for settings works correctly for single value upgrades"""
+ # GIVEN: A settings object with an upgrade step to take (99, so that we don't interfere with real ones)
+ local_settings = Settings()
+ local_settings.__setting_upgrade_99__ = [
+ ('single/value', 'single/new value', [(str, '')])
+ ]
+ settings.__version__ = 99
+ mocked_value.side_effect = [98, 10]
+ mocked_contains.return_value = True
+
+ # WHEN: upgrade_settings() is called
+ local_settings.upgrade_settings()
+
+ # THEN: The correct calls should have been made with the correct values
+ assert mocked_value.call_count == 2, 'Settings().value() should have been called twice'
+ assert mocked_value.call_args_list == [call('settings/version', 0), call('single/value')]
+ assert mocked_setValue.call_count == 2, 'Settings().setValue() should have been called twice'
+ assert mocked_setValue.call_args_list == [call('single/new value', '10'), call('settings/version', 99)]
+ mocked_contains.assert_called_once_with('single/value')
+ mocked_remove.assert_called_once_with('single/value')
+
+ @patch('openlp.core.common.settings.QtCore.QSettings.contains')
+ @patch('openlp.core.common.settings.QtCore.QSettings.value')
+ @patch('openlp.core.common.settings.QtCore.QSettings.setValue')
+ @patch('openlp.core.common.settings.QtCore.QSettings.remove')
+ def test_upgrade_setting_value(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains):
+ """Test that the upgrade mechanism for settings correctly uses the new value when it's not a function"""
+ # GIVEN: A settings object with an upgrade step to take (99, so that we don't interfere with real ones)
+ local_settings = Settings()
+ local_settings.__setting_upgrade_99__ = [
+ ('values/old value', 'values/new value', [(True, 1)])
+ ]
+ settings.__version__ = 99
+ mocked_value.side_effect = [98, 1]
+ mocked_contains.return_value = True
+
+ # WHEN: upgrade_settings() is called
+ local_settings.upgrade_settings()
+
+ # THEN: The correct calls should have been made with the correct values
+ assert mocked_value.call_count == 2, 'Settings().value() should have been called twice'
+ assert mocked_value.call_args_list == [call('settings/version', 0), call('values/old value')]
+ assert mocked_setValue.call_count == 2, 'Settings().setValue() should have been called twice'
+ assert mocked_setValue.call_args_list == [call('values/new value', True), call('settings/version', 99)]
+ mocked_contains.assert_called_once_with('values/old value')
+ mocked_remove.assert_called_once_with('values/old value')
+
+ @patch('openlp.core.common.settings.QtCore.QSettings.contains')
+ @patch('openlp.core.common.settings.QtCore.QSettings.value')
+ @patch('openlp.core.common.settings.QtCore.QSettings.setValue')
+ @patch('openlp.core.common.settings.QtCore.QSettings.remove')
+ def test_upgrade_multiple_one_invalid(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains):
+ """Test that the upgrade mechanism for settings works correctly for multiple values where one is invalid"""
+ # GIVEN: A settings object with an upgrade step to take
+ local_settings = Settings()
+ local_settings.__setting_upgrade_99__ = [
+ (['multiple/value 1', 'multiple/value 2'], 'single/new value', [])
+ ]
+ settings.__version__ = 99
+ mocked_value.side_effect = [98, 10]
+ mocked_contains.side_effect = [True, False]
+
+ # WHEN: upgrade_settings() is called
+ local_settings.upgrade_settings()
+
+ # THEN: The correct calls should have been made with the correct values
+ mocked_value.assert_called_once_with('settings/version', 0)
+ mocked_setValue.assert_called_once_with('settings/version', 99)
+ assert mocked_contains.call_args_list == [call('multiple/value 1'), call('multiple/value 2')]
+
+ def test_can_upgrade(self):
+ """Test the Settings.can_upgrade() method"""
+ # GIVEN: A Settings object
+ local_settings = Settings()
+
+ # WHEN: can_upgrade() is run
+ result = local_settings.can_upgrade()
+
+ # THEN: The result should be True
+ assert result is True, 'The settings should be upgradeable'
+
+ def test_convert_value_setting_none_str(self):
+ """Test the Settings._convert_value() method when a setting is None and the default value is a string"""
+ # GIVEN: A settings object
+ # WHEN: _convert_value() is run
+ result = Settings()._convert_value(None, 'string')
+
+ # THEN: The result should be an empty string
+ assert result == '', 'The result should be an empty string'
+
+ def test_convert_value_setting_none_list(self):
+ """Test the Settings._convert_value() method when a setting is None and the default value is a list"""
+ # GIVEN: A settings object
+ # WHEN: _convert_value() is run
+ result = Settings()._convert_value(None, [None])
+
+ # THEN: The result should be an empty list
+ assert result == [], 'The result should be an empty list'
+
+ def test_convert_value_setting_json_Path(self):
+ """Test the Settings._convert_value() method when a setting is JSON and represents a Path object"""
+ # GIVEN: A settings object
+ # WHEN: _convert_value() is run
+ result = Settings()._convert_value('{"__Path__": ["openlp", "core"]}', None)
+
+ # THEN: The result should be a Path object
+ assert isinstance(result, Path), 'The result should be a Path object'
+
+ def test_convert_value_setting_bool_str(self):
+ """Test the Settings._convert_value() method when a setting is supposed to be a boolean"""
+ # GIVEN: A settings object
+ # WHEN: _convert_value() is run
+ result = Settings()._convert_value('false', True)
+
+ # THEN: The result should be False
+ assert result is False, 'The result should be False'
=== modified file 'tests/interfaces/openlp_core/ui/media/vendor/test_mediainfoWrapper.py'
--- tests/interfaces/openlp_core/ui/media/vendor/test_mediainfoWrapper.py 2017-10-23 22:09:57 +0000
+++ tests/interfaces/openlp_core/ui/media/vendor/test_mediainfoWrapper.py 2017-11-16 05:12:46 +0000
@@ -29,7 +29,6 @@
from openlp.core.ui.media.vendor.mediainfoWrapper import MediaInfoWrapper
TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', '..', '..', 'resources', 'media'))
-
TEST_MEDIA = [['avi_file.avi', 61495], ['mp3_file.mp3', 134426], ['mpg_file.mpg', 9404], ['mp4_file.mp4', 188336]]
Follow ups
