From 14377f332963d6066695e743aa261e0afa98cf4b Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 8 Nov 2014 23:15:25 +0200 Subject: [PATCH] [refactor] Renamed "was_download_cancelled" to "was_cancelled" [refactor] Put some more stuff back into initalise() where it belongs [refactor] Rewrote old tests, wrote new tests --- openlp/core/__init__.py | 2 +- openlp/core/ui/firsttimeform.py | 17 +-- openlp/core/ui/mainwindow.py | 2 +- .../openlp_core_ui/test_firsttimeform.py | 137 +++++++++++++----- 4 files changed, 110 insertions(+), 48 deletions(-) diff --git a/openlp/core/__init__.py b/openlp/core/__init__.py index 9f5c69770..503aadee3 100644 --- a/openlp/core/__init__.py +++ b/openlp/core/__init__.py @@ -122,7 +122,7 @@ class OpenLP(OpenLPMixin, QtGui.QApplication): ftw.initialize(screens) if ftw.exec_() == QtGui.QDialog.Accepted: Settings().setValue('core/has run wizard', True) - elif ftw.was_download_cancelled: + elif ftw.was_cancelled: QtCore.QCoreApplication.exit() sys.exit() # Correct stylesheet bugs diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 06a238fed..75f661c3a 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -31,7 +31,6 @@ This module contains the first time wizard. """ import logging import os -import sys import time import urllib.request import urllib.parse @@ -105,7 +104,6 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): Create and set up the first time wizard. """ super(FirstTimeForm, self).__init__(parent) - self.web_access = True self.setup_ui(self) def nextId(self): @@ -147,8 +145,12 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): :param screens: The screens detected by OpenLP """ - self.was_download_cancelled = False self.screens = screens + self.web_access = True + self.was_cancelled = False + self.theme_screenshot_threads = [] + self.theme_screenshot_workers = [] + self.has_run_wizard = False def _download_index(self): """ @@ -166,9 +168,6 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): self.bibles_url = self.web + self.config.get('bibles', 'directory') + '/' self.themes_url = self.web + self.config.get('themes', 'directory') + '/' self.update_screen_list_combo() - self.theme_screenshot_threads = [] - self.theme_screenshot_workers = [] - self.has_run_wizard = False self.downloading = translate('OpenLP.FirstTimeWizard', 'Downloading %s...') # Sort out internet access for downloads if self.web_access: @@ -291,7 +290,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): """ Process the triggering of the cancel button. """ - self.was_download_cancelled = True + self.was_cancelled = True if self.theme_screenshot_workers: for worker in self.theme_screenshot_workers: worker.set_download_canceled(True) @@ -333,7 +332,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): url_file = urllib.request.urlopen(url) filename = open(f_path, "wb") # Download until finished or canceled. - while not self.was_download_cancelled: + while not self.was_cancelled: data = url_file.read(block_size) if not data: break @@ -342,7 +341,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): self._download_progress(block_count, block_size) filename.close() # Delete file if cancelled, it may be a partial file. - if self.was_download_cancelled: + if self.was_cancelled: os.remove(f_path) def _build_theme_screenshots(self): diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 825a12889..48fc1fcbf 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -690,7 +690,7 @@ class MainWindow(QtGui.QMainWindow, Ui_MainWindow, RegistryProperties): first_run_wizard = FirstTimeForm(self) first_run_wizard.initialize(ScreenList()) first_run_wizard.exec_() - if first_run_wizard.was_download_cancelled: + if first_run_wizard.was_cancelled: return self.application.set_busy_cursor() self.first_time() diff --git a/tests/functional/openlp_core_ui/test_firsttimeform.py b/tests/functional/openlp_core_ui/test_firsttimeform.py index ed7a7a9e8..da5305bda 100644 --- a/tests/functional/openlp_core_ui/test_firsttimeform.py +++ b/tests/functional/openlp_core_ui/test_firsttimeform.py @@ -30,6 +30,7 @@ Package to test the openlp.core.ui.firsttimeform package. """ from configparser import ConfigParser +import os from unittest import TestCase from openlp.core.common import Registry @@ -55,52 +56,114 @@ class TestFirstTimeForm(TestCase, TestMixin): def setUp(self): self.setup_application() self.app.setApplicationVersion('0.0') + # Set up a fake "set_normal_cursor" method since we're not dealing with an actual OpenLP application object + self.app.set_normal_cursor = lambda: None Registry.create() Registry().register('application', self.app) - def basic_initialise_test(self): + def initialise_test(self): """ - Test if we can intialise the FirstTimeForm without a config file + Test if we can intialise the FirstTimeForm """ - # GIVEN: A mocked get_web_page, a First Time Wizard and an expected screen object - with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page: - first_time_form = FirstTimeForm(None) - expected_screens = MagicMock() - expected_web_url = 'http://openlp.org/files/frw/' - expected_user_agent = 'OpenLP/0.0' - mocked_get_web_page.return_value = None + # GIVEN: A First Time Wizard and an expected screen object + frw = FirstTimeForm(None) + expected_screens = MagicMock() - # WHEN: The First Time Wizard is initialised - first_time_form.initialize(expected_screens) + # WHEN: The First Time Wizard is initialised + frw.initialize(expected_screens) - # THEN: The First Time Form web configuration file should be accessible and parseable - self.assertEqual(expected_screens, first_time_form.screens, 'The screens should be correct') - self.assertEqual(expected_web_url, first_time_form.web, 'The base path of the URL should be correct') - self.assertIsInstance(first_time_form.config, ConfigParser, 'The config object should be a ConfigParser') - mocked_get_web_page.assert_called_with(expected_web_url + 'download.cfg', - header=('User-Agent', expected_user_agent)) + # THEN: The screens should be set up, and the default values initialised + self.assertEqual(expected_screens, frw.screens, 'The screens should be correct') + self.assertTrue(frw.web_access, 'The default value of self.web_access should be True') + self.assertFalse(frw.was_cancelled, 'The default value of self.was_cancelled should be False') + self.assertListEqual([], frw.theme_screenshot_threads, 'The list of threads should be empty') + self.assertListEqual([], frw.theme_screenshot_workers, 'The list of workers should be empty') + self.assertFalse(frw.has_run_wizard, 'has_run_wizard should be False') - def config_initialise_test(self): + def set_defaults_test(self): """ - Test if we can intialise the FirstTimeForm with a config file + Test that the default values are set when set_defaults() is run """ - # GIVEN: A mocked get_web_page, a First Time Wizard and an expected screen object - with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page: - first_time_form = FirstTimeForm(None) - expected_web_url = 'http://openlp.org/files/frw/' - expected_songs_url = 'http://example.com/frw/songs/' - expected_bibles_url = 'http://example.com/frw/bibles/' - expected_themes_url = 'http://example.com/frw/themes/' - expected_user_agent = 'OpenLP/0.0' - mocked_get_web_page.return_value.read.return_value = FAKE_CONFIG + # GIVEN: An initialised FRW and a whole lot of stuff mocked out + frw = FirstTimeForm(None) + frw.initialize(MagicMock()) + with patch.object(frw, 'restart') as mocked_restart, \ + patch.object(frw, 'cancel_button') as mocked_cancel_button, \ + patch.object(frw, 'no_internet_finish_button') as mocked_no_internet_finish_btn, \ + patch.object(frw, 'currentIdChanged') as mocked_currentIdChanged, \ + patch.object(Registry, 'register_function') as mocked_register_function, \ + patch('openlp.core.ui.firsttimeform.Settings') as MockedSettings, \ + patch('openlp.core.ui.firsttimeform.gettempdir') as mocked_gettempdir, \ + patch('openlp.core.ui.firsttimeform.check_directory_exists') as mocked_check_directory_exists, \ + patch.object(frw.application, 'set_normal_cursor') as mocked_set_normal_cursor: + mocked_settings = MagicMock() + mocked_settings.value.return_value = True + MockedSettings.return_value = mocked_settings + mocked_gettempdir.return_value = 'temp' + expected_temp_path = os.path.join('temp', 'openlp') - # WHEN: The First Time Wizard is initialised - first_time_form.initialize(MagicMock()) + # WHEN: The set_defaults() method is run + frw.set_defaults() - # THEN: The First Time Form web configuration file should be accessible and parseable - self.assertIsInstance(first_time_form.config, ConfigParser, 'The config object should be a ConfigParser') - mocked_get_web_page.assert_called_with(expected_web_url + 'download.cfg', - header=('User-Agent', expected_user_agent)) - self.assertEqual(expected_songs_url, first_time_form.songs_url, 'The songs URL should be correct') - self.assertEqual(expected_bibles_url, first_time_form.bibles_url, 'The bibles URL should be correct') - self.assertEqual(expected_themes_url, first_time_form.themes_url, 'The themes URL should be correct') + # THEN: The default values should have been set + mocked_restart.assert_called_with() + self.assertEqual('http://openlp.org/files/frw/', frw.web, 'The default URL should be set') + mocked_cancel_button.clicked.connect.assert_called_with(frw.on_cancel_button_clicked) + mocked_no_internet_finish_btn.clicked.connect.assert_called_with(frw.on_no_internet_finish_button_clicked) + mocked_currentIdChanged.connect.assert_called_with(frw.on_current_id_changed) + mocked_register_function.assert_called_with('config_screen_changed', frw.update_screen_list_combo) + mocked_no_internet_finish_btn.setVisible.assert_called_with(False) + mocked_settings.value.assert_called_with('core/has run wizard') + mocked_gettempdir.assert_called_with() + mocked_check_directory_exists.assert_called_with(expected_temp_path) + mocked_set_normal_cursor.assert_called_with() + + def update_screen_list_combo_test(self): + """ + Test that the update_screen_list_combo() method works correctly + """ + # GIVEN: A mocked Screen() object and an initialised First Run Wizard and a mocked display_combo_box + expected_screen_list = ['Screen 1', 'Screen 2'] + mocked_screens = MagicMock() + mocked_screens.get_screen_list.return_value = expected_screen_list + frw = FirstTimeForm(None) + frw.initialize(mocked_screens) + with patch.object(frw, 'display_combo_box') as mocked_display_combo_box: + mocked_display_combo_box.count.return_value = 2 + + # WHEN: update_screen_list_combo() is called + frw.update_screen_list_combo() + + # THEN: The combobox should have been updated + mocked_display_combo_box.clear.assert_called_with() + mocked_screens.get_screen_list.assert_called_with() + mocked_display_combo_box.addItems.assert_called_with(expected_screen_list) + mocked_display_combo_box.count.assert_called_with() + mocked_display_combo_box.setCurrentIndex.assert_called_with(1) + + def on_cancel_button_clicked_test(self): + """ + Test that the cancel button click slot shuts down the threads correctly + """ + # GIVEN: A FRW, some mocked threads and workers (that isn't quite done) and other mocked stuff + frw = FirstTimeForm(None) + frw.initialize(MagicMock()) + mocked_worker = MagicMock() + mocked_thread = MagicMock() + mocked_thread.isRunning.side_effect = [True, False] + frw.theme_screenshot_workers.append(mocked_worker) + frw.theme_screenshot_threads.append(mocked_thread) + with patch('openlp.core.ui.firsttimeform.time') as mocked_time, \ + patch.object(frw.application, 'set_normal_cursor') as mocked_set_normal_cursor: + + # WHEN: on_cancel_button_clicked() is called + frw.on_cancel_button_clicked() + + # THEN: The right things should be called in the right order + self.assertTrue(frw.was_cancelled, 'The was_cancelled property should have been set to True') + mocked_worker.set_download_canceled.assert_called_with(True) + mocked_thread.isRunning.assert_called_with() + self.assertEqual(2, mocked_thread.isRunning.call_count, 'isRunning() should have been called twice') + mocked_time.sleep.assert_called_with(0.1) + self.assertEqual(1, mocked_time.sleep.call_count, 'sleep() should have only been called once') + mocked_set_normal_cursor.assert_called_with()