Merge lp:~raoul-snyman/openlp/settings-upgrade into lp:openlp
- settings-upgrade
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 2787 |
Proposed branch: | lp:~raoul-snyman/openlp/settings-upgrade |
Merge into: | lp:openlp |
Diff against target: |
402 lines (+214/-49) 4 files modified
nose2.cfg (+10/-5) openlp/core/common/settings.py (+26/-19) tests/functional/openlp_core/common/test_settings.py (+178/-24) tests/interfaces/openlp_core/ui/media/vendor/test_mediainfoWrapper.py (+0/-1) |
To merge this branch: | bzr merge lp:~raoul-snyman/openlp/settings-upgrade |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Bentley | Approve | ||
Phill | Approve | ||
Review via email: mp+333788@code.launchpad.net |
Commit message
Add multi-to-one settings upgrading, and merge all the post-2.4 settings changes into __setting_
Description of the change
Add multi-to-one settings upgrading, and merge all the post-2.4 settings changes into __setting_
Add this to your merge proposal:
-------
lp:~raoul-snyman/openlp/settings-upgrade (revision 2791)
https:/
https:/
https:/
https:/
https:/
https:/
https:/
Stopping after failure, see https:/
Tim Bentley (trb143) : | # |
Preview Diff
1 | === modified file 'nose2.cfg' | |||
2 | --- nose2.cfg 2016-08-10 09:07:49 +0000 | |||
3 | +++ nose2.cfg 2017-11-16 05:12:46 +0000 | |||
4 | @@ -1,5 +1,5 @@ | |||
5 | 1 | [unittest] | 1 | [unittest] |
7 | 2 | verbose = true | 2 | verbose = True |
8 | 3 | plugins = nose2.plugins.mp | 3 | plugins = nose2.plugins.mp |
9 | 4 | 4 | ||
10 | 5 | [log-capture] | 5 | [log-capture] |
11 | @@ -9,14 +9,19 @@ | |||
12 | 9 | log-level = ERROR | 9 | log-level = ERROR |
13 | 10 | 10 | ||
14 | 11 | [test-result] | 11 | [test-result] |
17 | 12 | always-on = true | 12 | always-on = True |
18 | 13 | descriptions = true | 13 | descriptions = True |
19 | 14 | 14 | ||
20 | 15 | [coverage] | 15 | [coverage] |
22 | 16 | always-on = true | 16 | always-on = True |
23 | 17 | coverage = openlp | 17 | coverage = openlp |
24 | 18 | coverage-report = html | 18 | coverage-report = html |
25 | 19 | 19 | ||
26 | 20 | [multiprocess] | 20 | [multiprocess] |
28 | 21 | always-on = false | 21 | always-on = False |
29 | 22 | processes = 4 | 22 | processes = 4 |
30 | 23 | |||
31 | 24 | [output-buffer] | ||
32 | 25 | always-on = True | ||
33 | 26 | stderr = True | ||
34 | 27 | stdout = False | ||
35 | 23 | 28 | ||
36 | === modified file 'openlp/core/common/settings.py' | |||
37 | --- openlp/core/common/settings.py 2017-11-10 22:50:04 +0000 | |||
38 | +++ openlp/core/common/settings.py 2017-11-16 05:12:46 +0000 | |||
39 | @@ -40,7 +40,7 @@ | |||
40 | 40 | 40 | ||
41 | 41 | # Fix for bug #1014422. | 41 | # Fix for bug #1014422. |
42 | 42 | X11_BYPASS_DEFAULT = True | 42 | X11_BYPASS_DEFAULT = True |
44 | 43 | if is_linux(): | 43 | if is_linux(): # pragma: no cover |
45 | 44 | # Default to False on Gnome. | 44 | # Default to False on Gnome. |
46 | 45 | X11_BYPASS_DEFAULT = bool(not os.environ.get('GNOME_DESKTOP_SESSION_ID')) | 45 | X11_BYPASS_DEFAULT = bool(not os.environ.get('GNOME_DESKTOP_SESSION_ID')) |
47 | 46 | # Default to False on Xfce. | 46 | # Default to False on Xfce. |
48 | @@ -206,11 +206,14 @@ | |||
49 | 206 | 'projector/source dialog type': 0 # Source select dialog box type | 206 | 'projector/source dialog type': 0 # Source select dialog box type |
50 | 207 | } | 207 | } |
51 | 208 | __file_path__ = '' | 208 | __file_path__ = '' |
52 | 209 | # Settings upgrades prior to 3.0 | ||
53 | 209 | __setting_upgrade_1__ = [ | 210 | __setting_upgrade_1__ = [ |
54 | 210 | # Changed during 2.2.x development. | ||
55 | 211 | ('songs/search as type', 'advanced/search as type', []), | 211 | ('songs/search as type', 'advanced/search as type', []), |
56 | 212 | ('media/players', 'media/players_temp', [(media_players_conv, None)]), # Convert phonon to system | 212 | ('media/players', 'media/players_temp', [(media_players_conv, None)]), # Convert phonon to system |
57 | 213 | ('media/players_temp', 'media/players', []), # Move temp setting from above to correct setting | 213 | ('media/players_temp', 'media/players', []), # Move temp setting from above to correct setting |
58 | 214 | ] | ||
59 | 215 | # Settings upgrades for 3.0 (aka 2.6) | ||
60 | 216 | __setting_upgrade_2__ = [ | ||
61 | 214 | ('advanced/default color', 'core/logo background color', []), # Default image renamed + moved to general > 2.4. | 217 | ('advanced/default color', 'core/logo background color', []), # Default image renamed + moved to general > 2.4. |
62 | 215 | ('advanced/default image', 'core/logo file', []), # Default image renamed + moved to general after 2.4. | 218 | ('advanced/default image', 'core/logo file', []), # Default image renamed + moved to general after 2.4. |
63 | 216 | ('remotes/https enabled', '', []), | 219 | ('remotes/https enabled', '', []), |
64 | @@ -231,9 +234,7 @@ | |||
65 | 231 | # Last search type was renamed to last used search type in 2.6 since Bible search value type changed in 2.6. | 234 | # Last search type was renamed to last used search type in 2.6 since Bible search value type changed in 2.6. |
66 | 232 | ('songs/last search type', 'songs/last used search type', []), | 235 | ('songs/last search type', 'songs/last used search type', []), |
67 | 233 | ('bibles/last search type', '', []), | 236 | ('bibles/last search type', '', []), |
71 | 234 | ('custom/last search type', 'custom/last used search type', [])] | 237 | ('custom/last search type', 'custom/last used search type', []), |
69 | 235 | |||
70 | 236 | __setting_upgrade_2__ = [ | ||
72 | 237 | # The following changes are being made for the conversion to using Path objects made in 2.6 development | 238 | # The following changes are being made for the conversion to using Path objects made in 2.6 development |
73 | 238 | ('advanced/data path', 'advanced/data path', [(str_to_path, None)]), | 239 | ('advanced/data path', 'advanced/data path', [(str_to_path, None)]), |
74 | 239 | ('crashreport/last directory', 'crashreport/last directory', [(str_to_path, None)]), | 240 | ('crashreport/last directory', 'crashreport/last directory', [(str_to_path, None)]), |
75 | @@ -467,32 +468,38 @@ | |||
76 | 467 | for version in range(current_version, __version__): | 468 | for version in range(current_version, __version__): |
77 | 468 | version += 1 | 469 | version += 1 |
78 | 469 | upgrade_list = getattr(self, '__setting_upgrade_{version}__'.format(version=version)) | 470 | upgrade_list = getattr(self, '__setting_upgrade_{version}__'.format(version=version)) |
80 | 470 | for old_key, new_key, rules in upgrade_list: | 471 | for old_keys, new_key, rules in upgrade_list: |
81 | 471 | # Once removed we don't have to do this again. - Can be removed once fully switched to the versioning | 472 | # Once removed we don't have to do this again. - Can be removed once fully switched to the versioning |
82 | 472 | # system. | 473 | # system. |
84 | 473 | if not self.contains(old_key): | 474 | if not isinstance(old_keys, (tuple, list)): |
85 | 475 | old_keys = [old_keys] | ||
86 | 476 | if any([not self.contains(old_key) for old_key in old_keys]): | ||
87 | 477 | log.warning('One of {} does not exist, skipping upgrade'.format(old_keys)) | ||
88 | 474 | continue | 478 | continue |
89 | 475 | if new_key: | 479 | if new_key: |
90 | 476 | # Get the value of the old_key. | 480 | # Get the value of the old_key. |
92 | 477 | old_value = super(Settings, self).value(old_key) | 481 | old_values = [super(Settings, self).value(old_key) for old_key in old_keys] |
93 | 478 | # When we want to convert the value, we have to figure out the default value (because we cannot get | 482 | # When we want to convert the value, we have to figure out the default value (because we cannot get |
94 | 479 | # the default value from the central settings dict. | 483 | # the default value from the central settings dict. |
95 | 480 | if rules: | 484 | if rules: |
98 | 481 | default_value = rules[0][1] | 485 | default_values = rules[0][1] |
99 | 482 | old_value = self._convert_value(old_value, default_value) | 486 | if not isinstance(default_values, (list, tuple)): |
100 | 487 | default_values = [default_values] | ||
101 | 488 | old_values = [self._convert_value(old_value, default_value) | ||
102 | 489 | for old_value, default_value in zip(old_values, default_values)] | ||
103 | 483 | # Iterate over our rules and check what the old_value should be "converted" to. | 490 | # Iterate over our rules and check what the old_value should be "converted" to. |
105 | 484 | for new, old in rules: | 491 | new_value = None |
106 | 492 | for new_rule, old_rule in rules: | ||
107 | 485 | # If the value matches with the condition (rule), then use the provided value. This is used to | 493 | # If the value matches with the condition (rule), then use the provided value. This is used to |
108 | 486 | # convert values. E. g. an old value 1 results in True, and 0 in False. | 494 | # convert values. E. g. an old value 1 results in True, and 0 in False. |
113 | 487 | if callable(new): | 495 | if callable(new_rule): |
114 | 488 | old_value = new(old_value) | 496 | new_value = new_rule(*old_values) |
115 | 489 | elif old == old_value: | 497 | elif old_rule in old_values: |
116 | 490 | old_value = new | 498 | new_value = new_rule |
117 | 491 | break | 499 | break |
122 | 492 | self.setValue(new_key, old_value) | 500 | self.setValue(new_key, new_value) |
123 | 493 | if new_key != old_key: | 501 | [self.remove(old_key) for old_key in old_keys if old_key != new_key] |
124 | 494 | self.remove(old_key) | 502 | self.setValue('settings/version', version) |
121 | 495 | self.setValue('settings/version', version) | ||
125 | 496 | 503 | ||
126 | 497 | def value(self, key): | 504 | def value(self, key): |
127 | 498 | """ | 505 | """ |
128 | 499 | 506 | ||
129 | === modified file 'tests/functional/openlp_core/common/test_settings.py' | |||
130 | --- tests/functional/openlp_core/common/test_settings.py 2017-10-07 07:05:07 +0000 | |||
131 | +++ tests/functional/openlp_core/common/test_settings.py 2017-11-16 05:12:46 +0000 | |||
132 | @@ -22,10 +22,12 @@ | |||
133 | 22 | """ | 22 | """ |
134 | 23 | Package to test the openlp.core.lib.settings package. | 23 | Package to test the openlp.core.lib.settings package. |
135 | 24 | """ | 24 | """ |
136 | 25 | from pathlib import Path | ||
137 | 25 | from unittest import TestCase | 26 | from unittest import TestCase |
139 | 26 | from unittest.mock import patch | 27 | from unittest.mock import call, patch |
140 | 27 | 28 | ||
142 | 28 | from openlp.core.common.settings import Settings | 29 | from openlp.core.common import settings |
143 | 30 | from openlp.core.common.settings import Settings, media_players_conv | ||
144 | 29 | 31 | ||
145 | 30 | from tests.helpers.testmixin import TestMixin | 32 | from tests.helpers.testmixin import TestMixin |
146 | 31 | 33 | ||
147 | @@ -47,28 +49,58 @@ | |||
148 | 47 | """ | 49 | """ |
149 | 48 | self.destroy_settings() | 50 | self.destroy_settings() |
150 | 49 | 51 | ||
156 | 50 | def test_settings_basic(self): | 52 | def test_media_players_conv(self): |
157 | 51 | """ | 53 | """Test the media players conversion function""" |
158 | 52 | Test the Settings creation and its default usage | 54 | # GIVEN: A list of media players |
159 | 53 | """ | 55 | media_players = 'phonon,webkit,vlc' |
160 | 54 | # GIVEN: A new Settings setup | 56 | |
161 | 57 | # WHEN: The media converter function is called | ||
162 | 58 | result = media_players_conv(media_players) | ||
163 | 59 | |||
164 | 60 | # THEN: The list should have been converted correctly | ||
165 | 61 | assert result == 'system,webkit,vlc' | ||
166 | 62 | |||
167 | 63 | def test_default_value(self): | ||
168 | 64 | """Test reading a setting that doesn't exist yet""" | ||
169 | 65 | # GIVEN: A setting that doesn't exist yet | ||
170 | 55 | 66 | ||
171 | 56 | # WHEN reading a setting for the first time | 67 | # WHEN reading a setting for the first time |
172 | 57 | default_value = Settings().value('core/has run wizard') | 68 | default_value = Settings().value('core/has run wizard') |
173 | 58 | 69 | ||
174 | 59 | # THEN the default value is returned | 70 | # THEN the default value is returned |
176 | 60 | self.assertFalse(default_value, 'The default value should be False') | 71 | assert default_value is False, 'The default value should be False' |
177 | 61 | 72 | ||
178 | 73 | def test_save_new_value(self): | ||
179 | 74 | """Test saving a new setting""" | ||
180 | 75 | # GIVEN: A setting that hasn't been saved yet | ||
181 | 62 | # WHEN a new value is saved into config | 76 | # WHEN a new value is saved into config |
182 | 63 | Settings().setValue('core/has run wizard', True) | 77 | Settings().setValue('core/has run wizard', True) |
183 | 64 | 78 | ||
184 | 65 | # THEN the new value is returned when re-read | 79 | # THEN the new value is returned when re-read |
186 | 66 | self.assertTrue(Settings().value('core/has run wizard'), 'The saved value should have been returned') | 80 | assert Settings().value('core/has run wizard') is True, 'The saved value should have been returned' |
187 | 81 | |||
188 | 82 | def test_set_up_default_values(self): | ||
189 | 83 | """Test that the default values are updated""" | ||
190 | 84 | # GIVEN: A Settings object with defaults | ||
191 | 85 | # WHEN: set_up_default_values() is called | ||
192 | 86 | Settings.set_up_default_values() | ||
193 | 87 | |||
194 | 88 | # THEN: The default values should have been added to the dictionary | ||
195 | 89 | assert 'advanced/default service name' in Settings.__default_settings__ | ||
196 | 90 | |||
197 | 91 | def test_get_default_value(self): | ||
198 | 92 | """Test that the default value for a setting is returned""" | ||
199 | 93 | # GIVEN: A Settings class with a default value | ||
200 | 94 | Settings.__default_settings__['test/moo'] = 'baa' | ||
201 | 95 | |||
202 | 96 | # WHEN: get_default_value() is called | ||
203 | 97 | result = Settings().get_default_value('test/moo') | ||
204 | 98 | |||
205 | 99 | # THEN: The correct default value should be returned | ||
206 | 100 | assert result == 'baa' | ||
207 | 67 | 101 | ||
208 | 68 | def test_settings_override(self): | 102 | def test_settings_override(self): |
212 | 69 | """ | 103 | """Test the Settings creation and its override usage""" |
210 | 70 | Test the Settings creation and its override usage | ||
211 | 71 | """ | ||
213 | 72 | # GIVEN: an override for the settings | 104 | # GIVEN: an override for the settings |
214 | 73 | screen_settings = { | 105 | screen_settings = { |
215 | 74 | 'test/extend': 'very wide', | 106 | 'test/extend': 'very wide', |
216 | @@ -79,18 +111,22 @@ | |||
217 | 79 | extend = Settings().value('test/extend') | 111 | extend = Settings().value('test/extend') |
218 | 80 | 112 | ||
219 | 81 | # THEN the default value is returned | 113 | # THEN the default value is returned |
221 | 82 | self.assertEqual('very wide', extend, 'The default value of "very wide" should be returned') | 114 | assert extend == 'very wide', 'The default value of "very wide" should be returned' |
222 | 115 | |||
223 | 116 | def test_save_existing_setting(self): | ||
224 | 117 | """Test that saving an existing setting returns the new value""" | ||
225 | 118 | # GIVEN: An existing setting | ||
226 | 119 | Settings().extend_default_settings({'test/existing value': None}) | ||
227 | 120 | Settings().setValue('test/existing value', 'old value') | ||
228 | 83 | 121 | ||
229 | 84 | # WHEN a new value is saved into config | 122 | # WHEN a new value is saved into config |
231 | 85 | Settings().setValue('test/extend', 'very short') | 123 | Settings().setValue('test/existing value', 'new value') |
232 | 86 | 124 | ||
233 | 87 | # THEN the new value is returned when re-read | 125 | # THEN the new value is returned when re-read |
235 | 88 | self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned') | 126 | assert Settings().value('test/existing value') == 'new value', 'The saved value should be returned' |
236 | 89 | 127 | ||
237 | 90 | def test_settings_override_with_group(self): | 128 | def test_settings_override_with_group(self): |
241 | 91 | """ | 129 | """Test the Settings creation and its override usage - with groups""" |
239 | 92 | Test the Settings creation and its override usage - with groups | ||
240 | 93 | """ | ||
242 | 94 | # GIVEN: an override for the settings | 130 | # GIVEN: an override for the settings |
243 | 95 | screen_settings = { | 131 | screen_settings = { |
244 | 96 | 'test/extend': 'very wide', | 132 | 'test/extend': 'very wide', |
245 | @@ -112,9 +148,7 @@ | |||
246 | 112 | self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned') | 148 | self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned') |
247 | 113 | 149 | ||
248 | 114 | def test_settings_nonexisting(self): | 150 | def test_settings_nonexisting(self): |
252 | 115 | """ | 151 | """Test the Settings on query for non-existing value""" |
250 | 116 | Test the Settings on query for non-existing value | ||
251 | 117 | """ | ||
253 | 118 | # GIVEN: A new Settings setup | 152 | # GIVEN: A new Settings setup |
254 | 119 | with self.assertRaises(KeyError) as cm: | 153 | with self.assertRaises(KeyError) as cm: |
255 | 120 | # WHEN reading a setting that doesn't exists | 154 | # WHEN reading a setting that doesn't exists |
256 | @@ -124,9 +158,7 @@ | |||
257 | 124 | self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception') | 158 | self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception') |
258 | 125 | 159 | ||
259 | 126 | def test_extend_default_settings(self): | 160 | def test_extend_default_settings(self): |
263 | 127 | """ | 161 | """Test that the extend_default_settings method extends the default settings""" |
261 | 128 | Test that the extend_default_settings method extends the default settings | ||
262 | 129 | """ | ||
264 | 130 | # GIVEN: A patched __default_settings__ dictionary | 162 | # GIVEN: A patched __default_settings__ dictionary |
265 | 131 | with patch.dict(Settings.__default_settings__, | 163 | with patch.dict(Settings.__default_settings__, |
266 | 132 | {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 3}, True): | 164 | {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 3}, True): |
267 | @@ -138,3 +170,125 @@ | |||
268 | 138 | self.assertEqual( | 170 | self.assertEqual( |
269 | 139 | Settings.__default_settings__, {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 4, | 171 | Settings.__default_settings__, {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 4, |
270 | 140 | 'test/extended 1': 1, 'test/extended 2': 2}) | 172 | 'test/extended 1': 1, 'test/extended 2': 2}) |
271 | 173 | |||
272 | 174 | @patch('openlp.core.common.settings.QtCore.QSettings.contains') | ||
273 | 175 | @patch('openlp.core.common.settings.QtCore.QSettings.value') | ||
274 | 176 | @patch('openlp.core.common.settings.QtCore.QSettings.setValue') | ||
275 | 177 | @patch('openlp.core.common.settings.QtCore.QSettings.remove') | ||
276 | 178 | def test_upgrade_single_setting(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains): | ||
277 | 179 | """Test that the upgrade mechanism for settings works correctly for single value upgrades""" | ||
278 | 180 | # GIVEN: A settings object with an upgrade step to take (99, so that we don't interfere with real ones) | ||
279 | 181 | local_settings = Settings() | ||
280 | 182 | local_settings.__setting_upgrade_99__ = [ | ||
281 | 183 | ('single/value', 'single/new value', [(str, '')]) | ||
282 | 184 | ] | ||
283 | 185 | settings.__version__ = 99 | ||
284 | 186 | mocked_value.side_effect = [98, 10] | ||
285 | 187 | mocked_contains.return_value = True | ||
286 | 188 | |||
287 | 189 | # WHEN: upgrade_settings() is called | ||
288 | 190 | local_settings.upgrade_settings() | ||
289 | 191 | |||
290 | 192 | # THEN: The correct calls should have been made with the correct values | ||
291 | 193 | assert mocked_value.call_count == 2, 'Settings().value() should have been called twice' | ||
292 | 194 | assert mocked_value.call_args_list == [call('settings/version', 0), call('single/value')] | ||
293 | 195 | assert mocked_setValue.call_count == 2, 'Settings().setValue() should have been called twice' | ||
294 | 196 | assert mocked_setValue.call_args_list == [call('single/new value', '10'), call('settings/version', 99)] | ||
295 | 197 | mocked_contains.assert_called_once_with('single/value') | ||
296 | 198 | mocked_remove.assert_called_once_with('single/value') | ||
297 | 199 | |||
298 | 200 | @patch('openlp.core.common.settings.QtCore.QSettings.contains') | ||
299 | 201 | @patch('openlp.core.common.settings.QtCore.QSettings.value') | ||
300 | 202 | @patch('openlp.core.common.settings.QtCore.QSettings.setValue') | ||
301 | 203 | @patch('openlp.core.common.settings.QtCore.QSettings.remove') | ||
302 | 204 | def test_upgrade_setting_value(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains): | ||
303 | 205 | """Test that the upgrade mechanism for settings correctly uses the new value when it's not a function""" | ||
304 | 206 | # GIVEN: A settings object with an upgrade step to take (99, so that we don't interfere with real ones) | ||
305 | 207 | local_settings = Settings() | ||
306 | 208 | local_settings.__setting_upgrade_99__ = [ | ||
307 | 209 | ('values/old value', 'values/new value', [(True, 1)]) | ||
308 | 210 | ] | ||
309 | 211 | settings.__version__ = 99 | ||
310 | 212 | mocked_value.side_effect = [98, 1] | ||
311 | 213 | mocked_contains.return_value = True | ||
312 | 214 | |||
313 | 215 | # WHEN: upgrade_settings() is called | ||
314 | 216 | local_settings.upgrade_settings() | ||
315 | 217 | |||
316 | 218 | # THEN: The correct calls should have been made with the correct values | ||
317 | 219 | assert mocked_value.call_count == 2, 'Settings().value() should have been called twice' | ||
318 | 220 | assert mocked_value.call_args_list == [call('settings/version', 0), call('values/old value')] | ||
319 | 221 | assert mocked_setValue.call_count == 2, 'Settings().setValue() should have been called twice' | ||
320 | 222 | assert mocked_setValue.call_args_list == [call('values/new value', True), call('settings/version', 99)] | ||
321 | 223 | mocked_contains.assert_called_once_with('values/old value') | ||
322 | 224 | mocked_remove.assert_called_once_with('values/old value') | ||
323 | 225 | |||
324 | 226 | @patch('openlp.core.common.settings.QtCore.QSettings.contains') | ||
325 | 227 | @patch('openlp.core.common.settings.QtCore.QSettings.value') | ||
326 | 228 | @patch('openlp.core.common.settings.QtCore.QSettings.setValue') | ||
327 | 229 | @patch('openlp.core.common.settings.QtCore.QSettings.remove') | ||
328 | 230 | def test_upgrade_multiple_one_invalid(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains): | ||
329 | 231 | """Test that the upgrade mechanism for settings works correctly for multiple values where one is invalid""" | ||
330 | 232 | # GIVEN: A settings object with an upgrade step to take | ||
331 | 233 | local_settings = Settings() | ||
332 | 234 | local_settings.__setting_upgrade_99__ = [ | ||
333 | 235 | (['multiple/value 1', 'multiple/value 2'], 'single/new value', []) | ||
334 | 236 | ] | ||
335 | 237 | settings.__version__ = 99 | ||
336 | 238 | mocked_value.side_effect = [98, 10] | ||
337 | 239 | mocked_contains.side_effect = [True, False] | ||
338 | 240 | |||
339 | 241 | # WHEN: upgrade_settings() is called | ||
340 | 242 | local_settings.upgrade_settings() | ||
341 | 243 | |||
342 | 244 | # THEN: The correct calls should have been made with the correct values | ||
343 | 245 | mocked_value.assert_called_once_with('settings/version', 0) | ||
344 | 246 | mocked_setValue.assert_called_once_with('settings/version', 99) | ||
345 | 247 | assert mocked_contains.call_args_list == [call('multiple/value 1'), call('multiple/value 2')] | ||
346 | 248 | |||
347 | 249 | def test_can_upgrade(self): | ||
348 | 250 | """Test the Settings.can_upgrade() method""" | ||
349 | 251 | # GIVEN: A Settings object | ||
350 | 252 | local_settings = Settings() | ||
351 | 253 | |||
352 | 254 | # WHEN: can_upgrade() is run | ||
353 | 255 | result = local_settings.can_upgrade() | ||
354 | 256 | |||
355 | 257 | # THEN: The result should be True | ||
356 | 258 | assert result is True, 'The settings should be upgradeable' | ||
357 | 259 | |||
358 | 260 | def test_convert_value_setting_none_str(self): | ||
359 | 261 | """Test the Settings._convert_value() method when a setting is None and the default value is a string""" | ||
360 | 262 | # GIVEN: A settings object | ||
361 | 263 | # WHEN: _convert_value() is run | ||
362 | 264 | result = Settings()._convert_value(None, 'string') | ||
363 | 265 | |||
364 | 266 | # THEN: The result should be an empty string | ||
365 | 267 | assert result == '', 'The result should be an empty string' | ||
366 | 268 | |||
367 | 269 | def test_convert_value_setting_none_list(self): | ||
368 | 270 | """Test the Settings._convert_value() method when a setting is None and the default value is a list""" | ||
369 | 271 | # GIVEN: A settings object | ||
370 | 272 | # WHEN: _convert_value() is run | ||
371 | 273 | result = Settings()._convert_value(None, [None]) | ||
372 | 274 | |||
373 | 275 | # THEN: The result should be an empty list | ||
374 | 276 | assert result == [], 'The result should be an empty list' | ||
375 | 277 | |||
376 | 278 | def test_convert_value_setting_json_Path(self): | ||
377 | 279 | """Test the Settings._convert_value() method when a setting is JSON and represents a Path object""" | ||
378 | 280 | # GIVEN: A settings object | ||
379 | 281 | # WHEN: _convert_value() is run | ||
380 | 282 | result = Settings()._convert_value('{"__Path__": ["openlp", "core"]}', None) | ||
381 | 283 | |||
382 | 284 | # THEN: The result should be a Path object | ||
383 | 285 | assert isinstance(result, Path), 'The result should be a Path object' | ||
384 | 286 | |||
385 | 287 | def test_convert_value_setting_bool_str(self): | ||
386 | 288 | """Test the Settings._convert_value() method when a setting is supposed to be a boolean""" | ||
387 | 289 | # GIVEN: A settings object | ||
388 | 290 | # WHEN: _convert_value() is run | ||
389 | 291 | result = Settings()._convert_value('false', True) | ||
390 | 292 | |||
391 | 293 | # THEN: The result should be False | ||
392 | 294 | assert result is False, 'The result should be False' | ||
393 | 141 | 295 | ||
394 | === modified file 'tests/interfaces/openlp_core/ui/media/vendor/test_mediainfoWrapper.py' | |||
395 | --- tests/interfaces/openlp_core/ui/media/vendor/test_mediainfoWrapper.py 2017-10-23 22:09:57 +0000 | |||
396 | +++ tests/interfaces/openlp_core/ui/media/vendor/test_mediainfoWrapper.py 2017-11-16 05:12:46 +0000 | |||
397 | @@ -29,7 +29,6 @@ | |||
398 | 29 | from openlp.core.ui.media.vendor.mediainfoWrapper import MediaInfoWrapper | 29 | from openlp.core.ui.media.vendor.mediainfoWrapper import MediaInfoWrapper |
399 | 30 | 30 | ||
400 | 31 | TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', '..', '..', 'resources', 'media')) | 31 | TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', '..', '..', 'resources', 'media')) |
401 | 32 | |||
402 | 33 | TEST_MEDIA = [['avi_file.avi', 61495], ['mp3_file.mp3', 134426], ['mpg_file.mpg', 9404], ['mp4_file.mp4', 188336]] | 32 | TEST_MEDIA = [['avi_file.avi', 61495], ['mp3_file.mp3', 134426], ['mpg_file.mpg', 9404], ['mp4_file.mp4', 188336]] |
403 | 34 | 33 | ||
404 | 35 | 34 |
Seems good to me, 1 minor comment, but I'd be happy for this to be merged