openlp-core team mailing list archive

[Simon Hanna <simon.hanna@xxxxxxxxxxxxx>]

Simon Hanna has proposed merging lp:~thelinuxguy/openlp/fix-remote into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1533401 in OpenLP: "Remote plugin stays active until a restart"
  https://bugs.launchpad.net/openlp/+bug/1533401

For more details, see:
https://code.launchpad.net/~thelinuxguy/openlp/fix-remote/+merge/282447

Fix the remote plugin not being able to be toggled on and off (It used to not shut the server down)
Fix error displayed on console about not specifying a default parser for BeautifulSoup
Fix a couple of pep8 errors

-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~thelinuxguy/openlp/fix-remote into lp:openlp.

=== modified file 'openlp/plugins/bibles/lib/http.py'
--- openlp/plugins/bibles/lib/http.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/bibles/lib/http.py	2016-01-13 14:53:34 +0000
@@ -288,7 +288,7 @@
         except UnicodeDecodeError:
             page_source = str(page_source, 'cp1251')
         try:
-            soup = BeautifulSoup(page_source)
+            soup = BeautifulSoup(page_source, 'lxml')
         except Exception:
             log.error('BeautifulSoup could not parse the Bible page.')
             send_error_message('parse')
@@ -759,7 +759,7 @@
         page_source = re.sub(pre_parse_regex, pre_parse_substitute, page_source.decode())
     soup = None
     try:
-        soup = BeautifulSoup(page_source)
+        soup = BeautifulSoup(page_source, 'lxml')
         CLEANER_REGEX.sub('', str(soup))
     except Exception:
         log.exception('BeautifulSoup could not parse the bible page.')

=== modified file 'openlp/plugins/remotes/lib/httpserver.py'
--- openlp/plugins/remotes/lib/httpserver.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/remotes/lib/httpserver.py	2016-01-13 14:53:34 +0000
@@ -87,10 +87,6 @@
         """
         self.http_server.start_server()
 
-    def stop(self):
-        log.debug("stop called")
-        self.http_server.stop = True
-
 
 class OpenLPServer(RegistryProperties):
     def __init__(self):
@@ -138,9 +134,9 @@
                 self.httpd = server_class((address, port), CustomHandler)
                 log.debug("Server started for class %s %s %d" % (server_class, address, port))
                 break
-            except OSError:
-                log.debug("failed to start http server thread state %d %s" %
-                          (loop, self.http_thread.isRunning()))
+            except OSError as e:
+                log.debug("failed to start http server thread state {} {}  with error {}".format(
+                    loop, self.http_thread.isRunning(), e))
                 loop += 1
                 time.sleep(0.1)
             except:
@@ -152,8 +148,9 @@
         """
         Stop the server
         """
-        if self.http_thread.isRunning():
-            self.http_thread.stop()
+        self.httpd.shutdown()
+        self.http_thread.exit()
+        self.httpd.socket.close()
         self.httpd = None
         log.debug('Stopped the server.')
 

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2016-01-10 16:01:43 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2016-01-13 14:53:34 +0000
@@ -515,7 +515,7 @@
             self.topics_list_view.addItem(topic_name)
         self.songbooks_list_view.clear()
         for songbook_entry in self.song.songbook_entries:
-            self.add_songbook_entry_to_list(songbook_entry.songbook.id, songbook_entry.songbook.name, 
+            self.add_songbook_entry_to_list(songbook_entry.songbook.id, songbook_entry.songbook.name,
                                             songbook_entry.entry)
         self.audio_list_widget.clear()
         for media in self.song.media_files:

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2016-01-10 16:01:43 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2016-01-13 14:53:34 +0000
@@ -255,9 +255,9 @@
         search_entry = re.sub(r'[^0-9]', '', search_keywords[2])
 
         songbook_entries = (self.plugin.manager.session.query(SongBookEntry)
-                           .join(Book)
-                           .order_by(Book.name)
-                           .order_by(SongBookEntry.entry))
+                            .join(Book)
+                            .order_by(Book.name)
+                            .order_by(SongBookEntry.entry))
         for songbook_entry in songbook_entries:
             if songbook_entry.song.temporary:
                 continue

=== modified file 'tests/functional/openlp_core_lib/test_htmlbuilder.py'
--- tests/functional/openlp_core_lib/test_htmlbuilder.py	2016-01-03 11:47:07 +0000
+++ tests/functional/openlp_core_lib/test_htmlbuilder.py	2016-01-13 14:53:34 +0000
@@ -363,9 +363,8 @@
         """
         Test the webkit_version() function
         """
-        # GIVEN: Webkit 
+        # GIVEN: Webkit
         webkit_ver = float(QtWebKit.qWebKitVersion())
         # WHEN: Retrieving the webkit version
         # THEN: Webkit versions should match
         self.assertEquals(webkit_version(), webkit_ver, "The returned webkit version doesn't match the installed one")
-

=== modified file 'tests/functional/openlp_core_ui/test_mainwindow.py'
--- tests/functional/openlp_core_ui/test_mainwindow.py	2016-01-09 19:10:56 +0000
+++ tests/functional/openlp_core_ui/test_mainwindow.py	2016-01-13 14:53:34 +0000
@@ -189,5 +189,3 @@
             # THEN: The media manager dock is made visible
             self.assertEqual(0, mocked_media_manager_dock.setVisible.call_count)
             mocked_widget.on_focus.assert_called_with()
-
-

=== added file 'tests/functional/openlp_plugins/remotes/test_server.py'
--- tests/functional/openlp_plugins/remotes/test_server.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_plugins/remotes/test_server.py	2016-01-13 14:53:34 +0000
@@ -0,0 +1,88 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# 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; version 2 of the License.                              #
+#                                                                             #
+# 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., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+This module contains tests for the lib submodule of the Remotes plugin.
+"""
+import os
+import urllib.request
+from unittest import TestCase
+
+from openlp.core.common import Settings, Registry
+from openlp.plugins.remotes.lib.httpserver import HTTPSServer, ThreadingHTTPServer, OpenLPServer
+from tests.functional import MagicMock, patch, mock_open
+from tests.helpers.testmixin import TestMixin
+
+__default_settings__ = {
+    'remotes/twelve hour': True,
+    'remotes/port': 4318,
+    'remotes/https port': 4319,
+    'remotes/https enabled': False,
+    'remotes/user id': 'openlp',
+    'remotes/password': 'password',
+    'remotes/authentication enabled': False,
+    'remotes/ip address': '0.0.0.0'
+}
+
+TEST_PATH = os.path.abspath(os.path.dirname(__file__))
+
+
+class TestRemoteServer(TestCase, TestMixin):
+    """
+    Test the functions in the :mod:`httpserver` module.
+    """
+    def setUp(self):
+        """
+        Create the UI
+        """
+        Registry.create()
+        self.setup_application()
+        self.build_settings()
+        Settings().extend_default_settings(__default_settings__)
+
+    def tearDown(self):
+        """
+        Delete all the C++ objects at the end so that we don't have a segfault
+        """
+        self.destroy_settings()
+
+    @patch('openlp.plugins.remotes.lib.httpserver.HttpThread')
+    def test_start_http_server(self, h):
+        server = OpenLPServer()
+        start_server_instance = MagicMock()
+        httpd = MagicMock()
+        server.httpd = httpd
+        server.start_server_instance = start_server_instance
+        server.start_server()
+        start_server_instance.assert_called_with('0.0.0.0', 4318, ThreadingHTTPServer)
+        httpd.serve_forever.assert_called_with()
+
+    @patch('openlp.plugins.remotes.lib.httpserver.HttpThread')
+    def test_start_https_server(self, h):
+        Settings().setValue('remotes/https enabled', True)
+        server = OpenLPServer()
+        start_server_instance = MagicMock()
+        httpd = MagicMock()
+        server.httpd = httpd
+        server.start_server_instance = start_server_instance
+        server.start_server()
+        start_server_instance.assert_called_with('0.0.0.0', 4319, HTTPSServer)
+        httpd.serve_forever.assert_called_with()

=== modified file 'tests/resources/projector/data.py'
--- tests/resources/projector/data.py	2016-01-09 17:21:20 +0000
+++ tests/resources/projector/data.py	2016-01-13 14:53:34 +0000
@@ -59,4 +59,3 @@
                   name='___TEST_THREE___',
                   location='location three',
                   notes='notes three')
-