[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

@@ -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

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):

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()

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
+        # GIVEN: A First Time Wizard and an expected screen object
-        with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
+        frw = FirstTimeForm(None)
-            first_time_form = FirstTimeForm(None)
+        expected_screens = MagicMock()
-            expected_screens = MagicMock()
-            expected_web_url = 'http://openlp.org/files/frw/'
-            expected_user_agent = 'OpenLP/0.0'
-            mocked_get_web_page.return_value = None
 
-            # WHEN: The First Time Wizard is initialised
+        # WHEN: The First Time Wizard is initialised
-            first_time_form.initialize(expected_screens)
+        frw.initialize(expected_screens)
 
-            # THEN: The First Time Form web configuration file should be accessible and parseable
+        # THEN: The screens should be set up, and the default values initialised
-            self.assertEqual(expected_screens, first_time_form.screens, 'The screens should be correct')
+        self.assertEqual(expected_screens, frw.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.assertTrue(frw.web_access, 'The default value of self.web_access should be True')
-            self.assertIsInstance(first_time_form.config, ConfigParser, 'The config object should be a ConfigParser')
+        self.assertFalse(frw.was_cancelled, 'The default value of self.was_cancelled should be False')
-            mocked_get_web_page.assert_called_with(expected_web_url + 'download.cfg',
+        self.assertListEqual([], frw.theme_screenshot_threads, 'The list of threads should be empty')
-                                                   header=('User-Agent', expected_user_agent))
+        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
+        # GIVEN: An initialised FRW and a whole lot of stuff mocked out
-        with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
+        frw = FirstTimeForm(None)
-            first_time_form = FirstTimeForm(None)
+        frw.initialize(MagicMock())
-            expected_web_url = 'http://openlp.org/files/frw/'
+        with patch.object(frw, 'restart') as mocked_restart, \
-            expected_songs_url = 'http://example.com/frw/songs/'
+                patch.object(frw, 'cancel_button') as mocked_cancel_button, \
-            expected_bibles_url = 'http://example.com/frw/bibles/'
+                patch.object(frw, 'no_internet_finish_button') as mocked_no_internet_finish_btn, \
-            expected_themes_url = 'http://example.com/frw/themes/'
+                patch.object(frw, 'currentIdChanged') as mocked_currentIdChanged, \
-            expected_user_agent = 'OpenLP/0.0'
+                patch.object(Registry, 'register_function') as mocked_register_function, \
-            mocked_get_web_page.return_value.read.return_value = FAKE_CONFIG
+                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
+            # WHEN: The set_defaults() method is run
-            first_time_form.initialize(MagicMock())
+            frw.set_defaults()
 
-            # THEN: The First Time Form web configuration file should be accessible and parseable
+            # THEN: The default values should have been set
-            self.assertIsInstance(first_time_form.config, ConfigParser, 'The config object should be a ConfigParser')
+            mocked_restart.assert_called_with()
-            mocked_get_web_page.assert_called_with(expected_web_url + 'download.cfg',
+            self.assertEqual('http://openlp.org/files/frw/', frw.web, 'The default URL should be set')
-                                                   header=('User-Agent', expected_user_agent))
+            mocked_cancel_button.clicked.connect.assert_called_with(frw.on_cancel_button_clicked)
-            self.assertEqual(expected_songs_url, first_time_form.songs_url, 'The songs URL should be correct')
+            mocked_no_internet_finish_btn.clicked.connect.assert_called_with(frw.on_no_internet_finish_button_clicked)
-            self.assertEqual(expected_bibles_url, first_time_form.bibles_url, 'The bibles URL should be correct')
+            mocked_currentIdChanged.connect.assert_called_with(frw.on_current_id_changed)
-            self.assertEqual(expected_themes_url, first_time_form.themes_url, 'The themes URL should be correct')
+            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()