From 159056f06fbb49fc4aa2ecf8ecdc5884d2cb72a0 Mon Sep 17 00:00:00 2001
From: Ken Roberts <alisonken1@gmail.com>
Date: Sun, 3 Dec 2017 16:24:47 -0800
Subject: [PATCH 01/20] PJLink2-M updates

---
 openlp/core/projectors/__init__.py            |   2 -
 openlp/core/projectors/constants.py           |  18 ++
 openlp/core/projectors/db.py                  |   2 +-
 openlp/core/projectors/pjlink.py              | 241 ++++++++++++------
 .../projectors/test_projector_bugfixes_01.py  |  43 +---
 .../projectors/test_projector_pjlink_base.py  |  80 +++---
 .../test_projector_pjlink_cmd_routing.py      |  56 ++--
 ...y => test_projector_pjlink_commands_01.py} |   2 +-
 .../test_projector_pjlink_commands_02.py      | 198 ++++++++++++++
 9 files changed, 449 insertions(+), 193 deletions(-)
 rename tests/functional/openlp_core/projectors/{test_projector_pjlink_commands.py => test_projector_pjlink_commands_01.py} (99%)
 create mode 100644 tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py

diff --git a/openlp/core/projectors/__init__.py b/openlp/core/projectors/__init__.py
index 396422902..66cfd6080 100644
--- a/openlp/core/projectors/__init__.py
+++ b/openlp/core/projectors/__init__.py
@@ -25,8 +25,6 @@
     Initialization for the openlp.core.projectors modules.
 """
 
-from openlp.core.projectors.constants import PJLINK_PORT, ERROR_MSG, ERROR_STRING
-
 
 class DialogSourceStyle(object):
     """
diff --git a/openlp/core/projectors/constants.py b/openlp/core/projectors/constants.py
index 715896133..c700141b8 100644
--- a/openlp/core/projectors/constants.py
+++ b/openlp/core/projectors/constants.py
@@ -144,6 +144,24 @@ PJLINK_VALID_CMD = {
              }
 }
 
+# QAbstractSocketState enums converted to string
+S_QSOCKET_STATE = {
+    0: 'QSocketState - UnconnectedState',
+    1: 'QSocketState - HostLookupState',
+    2: 'QSocketState - ConnectingState',
+    3: 'QSocketState - ConnectedState',
+    4: 'QSocketState - BoundState',
+    5: 'QSocketState - ListeningState (internal use only)',
+    6: 'QSocketState - ClosingState',
+    'UnconnectedState': 0,
+    'HostLookupState': 1,
+    'ConnectingState': 2,
+    'ConnectedState': 3,
+    'BoundState': 4,
+    'ListeningState': 5,
+    'ClosingState': 6,
+}
+
 # Error and status codes
 S_OK = E_OK = 0  # E_OK included since I sometimes forget
 # Error codes. Start at 200 so we don't duplicate system error codes.
diff --git a/openlp/core/projectors/db.py b/openlp/core/projectors/db.py
index 99fe9515b..fe8785861 100644
--- a/openlp/core/projectors/db.py
+++ b/openlp/core/projectors/db.py
@@ -415,7 +415,7 @@ class ProjectorDB(Manager):
         for key in projector.source_available:
             item = self.get_object_filtered(ProjectorSource,
                                             and_(ProjectorSource.code == key,
-                                                 ProjectorSource.projector_id == projector.dbid))
+                                                 ProjectorSource.projector_id == projector.id))
             if item is None:
                 source_dict[key] = PJLINK_DEFAULT_CODES[key]
             else:
diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py
index 16a65bd11..99fb6956c 100644
--- a/openlp/core/projectors/pjlink.py
+++ b/openlp/core/projectors/pjlink.py
@@ -58,8 +58,7 @@ from openlp.core.projectors.constants import CONNECTION_ERRORS, CR, ERROR_MSG, E
     E_AUTHENTICATION, E_CONNECTION_REFUSED, E_GENERAL, E_INVALID_DATA, E_NETWORK, E_NOT_CONNECTED, E_OK, \
     E_PARAMETER, E_PROJECTOR, E_SOCKET_TIMEOUT, E_UNAVAILABLE, E_UNDEFINED, PJLINK_ERRORS, PJLINK_ERST_DATA, \
     PJLINK_ERST_STATUS, PJLINK_MAX_PACKET, PJLINK_PORT, PJLINK_POWR_STATUS, PJLINK_VALID_CMD, \
-    STATUS_STRING, S_CONNECTED, S_CONNECTING, S_INFO, S_NETWORK_RECEIVED, S_NETWORK_SENDING, \
-    S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_STATUS
+    STATUS_STRING, S_CONNECTED, S_CONNECTING, S_INFO, S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_QSOCKET_STATE, S_STATUS
 
 log = logging.getLogger(__name__)
 log.debug('pjlink loaded')
@@ -123,7 +122,8 @@ class PJLinkCommands(object):
             'INST': self.process_inst,
             'LAMP': self.process_lamp,
             'NAME': self.process_name,
-            'PJLINK': self.check_login,
+            'PJLINK': self.process_pjlink,
+            # 'PJLINK': self.check_login,
             'POWR': self.process_powr,
             'SNUM': self.process_snum,
             'SVER': self.process_sver,
@@ -135,7 +135,8 @@ class PJLinkCommands(object):
         """
         Initialize instance variables. Also used to reset projector-specific information to default.
         """
-        log.debug('({ip}) reset_information() connect status is {state}'.format(ip=self.ip, state=self.state()))
+        log.debug('({ip}) reset_information() connect status is {state}'.format(ip=self.ip,
+                                                                                state=S_QSOCKET_STATE[self.state()]))
         self.fan = None  # ERST
         self.filter_time = None  # FILT
         self.lamp = None  # LAMP
@@ -165,6 +166,7 @@ class PJLinkCommands(object):
             self.socket_timer.stop()
         self.send_busy = False
         self.send_queue = []
+        self.priority_queue = []
 
     def process_command(self, cmd, data):
         """
@@ -176,18 +178,19 @@ class PJLinkCommands(object):
         log.debug('({ip}) Processing command "{cmd}" with data "{data}"'.format(ip=self.ip,
                                                                                 cmd=cmd,
                                                                                 data=data))
-        # Check if we have a future command not available yet
-        _cmd = cmd.upper()
+        # cmd should already be in uppercase, but data may be in mixed-case.
+        # Due to some replies should stay as mixed-case, validate using separate uppercase check
         _data = data.upper()
-        if _cmd not in PJLINK_VALID_CMD:
+        # Check if we have a future command not available yet
+        if cmd not in PJLINK_VALID_CMD:
             log.error("({ip}) Ignoring command='{cmd}' (Invalid/Unknown)".format(ip=self.ip, cmd=cmd))
             return
         elif _data == 'OK':
-            log.debug('({ip}) Command "{cmd}" returned OK'.format(ip=self.ip, cmd=cmd))
-            # A command returned successfully, no further processing needed
-            return
-        elif _cmd not in self.pjlink_functions:
-            log.warning("({ip}) Unable to process command='{cmd}' (Future option)".format(ip=self.ip, cmd=cmd))
+            log.debug("({ip}) Command '{cmd}' returned OK".format(ip=self.ip, cmd=cmd))
+            # A command returned successfully, so do a query on command to verify status
+            return self.send_command(cmd=cmd)
+        elif cmd not in self.pjlink_functions:
+            log.warning("({ip}) Unable to process command='{cmd}' (Future option?)".format(ip=self.ip, cmd=cmd))
             return
         elif _data in PJLINK_ERRORS:
             # Oops - projector error
@@ -211,12 +214,10 @@ class PJLinkCommands(object):
             elif _data == PJLINK_ERRORS[E_PROJECTOR]:
                 # Projector/display error
                 self.change_status(E_PROJECTOR)
-            self.receive_data_signal()
             return
         # Command checks already passed
         log.debug('({ip}) Calling function for {cmd}'.format(ip=self.ip, cmd=cmd))
-        self.receive_data_signal()
-        self.pjlink_functions[_cmd](data)
+        self.pjlink_functions[cmd](data)
 
     def process_avmt(self, data):
         """
@@ -430,6 +431,51 @@ class PJLinkCommands(object):
         log.debug('({ip}) Setting projector PJLink name to "{data}"'.format(ip=self.ip, data=self.pjlink_name))
         return
 
+    def process_pjlink(self, data):
+        """
+        Process initial socket connection to terminal.
+
+        :param data: Initial packet with authentication scheme
+        """
+        log.debug("({ip}) Processing PJLINK command".format(ip=self.ip))
+        chk = data.split(" ")
+        if len(chk[0]) != 1:
+            # Invalid - after splitting, first field should be 1 character, either '0' or '1' only
+            log.error("({ip}) Invalid initial authentication scheme - aborting".format(ip=self.ip))
+            return self.disconnect_from_host()
+        elif chk[0] == '0':
+            # Normal connection no authentication
+            if len(chk) > 1:
+                # Invalid data - there should be nothing after a normal authentication scheme
+                log.error("({ip}) Normal connection with extra information - aborting".format(ip=self.ip))
+                return self.disconnect_from_host()
+            elif self.pin:
+                log.error("({ip}) Normal connection but PIN set - aborting".format(ip=self.ip))
+                return self.disconnect_from_host()
+            else:
+                data_hash = None
+        elif chk[0] == '1':
+            if len(chk) < 2:
+                # Not enough information for authenticated connection
+                log.error("({ip}) Authenticated connection but not enough info - aborting".format(ip=self.ip))
+                return self.disconnect_from_host()
+            elif not self.pin:
+                log.error("({ip}) Authenticate connection but no PIN - aborting".format(ip=self.ip))
+                return self.disconnect_from_host()
+            else:
+                data_hash = str(qmd5_hash(salt=chk[1].encode('utf-8'), data=self.pin.encode('utf-8')),
+                                encoding='ascii')
+        # Passed basic checks, so start connection
+        self.readyRead.connect(self.get_socket)
+        if not self.no_poll:
+            log.debug('({ip}) process_pjlink(): Starting timer'.format(ip=self.ip))
+            self.timer.setInterval(2000)  # Set 2 seconds for initial information
+            self.timer.start()
+        self.change_status(S_CONNECTED)
+        log.debug("({ip}) process_pjlink(): Sending 'CLSS' initial command'".format(ip=self.ip))
+        # Since this is an initial connection, make it a priority just in case
+        return self.send_command(cmd="CLSS", salt=data_hash, priority=True)
+
     def process_powr(self, data):
         """
         Power status. See PJLink specification for format.
@@ -573,6 +619,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         self.widget = None  # QListBox entry
         self.timer = None  # Timer that calls the poll_loop
         self.send_queue = []
+        self.priority_queue = []
         self.send_busy = False
         # Socket timer for some possible brain-dead projectors or network cable pulled
         self.socket_timer = None
@@ -586,6 +633,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         self.connected.connect(self.check_login)
         self.disconnected.connect(self.disconnect_from_host)
         self.error.connect(self.get_error)
+        self.projectorReceivedData.connect(self._send_command)
 
     def thread_stopped(self):
         """
@@ -608,6 +656,11 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             self.projectorReceivedData.disconnect(self._send_command)
         except TypeError:
             pass
+        try:
+            self.readyRead.connect(self.get_socket)  # Set in process_pjlink
+        except TypeError:
+            pass
+
         self.disconnect_from_host()
         self.deleteLater()
         self.i_am_running = False
@@ -625,10 +678,10 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         Retrieve information from projector that changes.
         Normally called by timer().
         """
-        if self.state() != self.ConnectedState:
+        if self.state() != S_QSOCKET_STATE['ConnectedState']:
             log.warning("({ip}) poll_loop(): Not connected - returning".format(ip=self.ip))
             return
-        log.debug('({ip}) Updating projector status'.format(ip=self.ip))
+        log.debug('({ip}) poll_loop(): Updating projector status'.format(ip=self.ip))
         # Reset timer in case we were called from a set command
         if self.timer.interval() < self.poll_time:
             # Reset timer to 5 seconds
@@ -640,28 +693,28 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         if self.pjlink_class == '2':
             check_list.extend(['FILT', 'FREZ'])
         for command in check_list:
-            self.send_command(command, queue=True)
+            self.send_command(command)
         # The following commands do not change, so only check them once
         if self.power == S_ON and self.source_available is None:
-            self.send_command('INST', queue=True)
+            self.send_command('INST')
         if self.other_info is None:
-            self.send_command('INFO', queue=True)
+            self.send_command('INFO')
         if self.manufacturer is None:
-            self.send_command('INF1', queue=True)
+            self.send_command('INF1')
         if self.model is None:
-            self.send_command('INF2', queue=True)
+            self.send_command('INF2')
         if self.pjlink_name is None:
-            self.send_command('NAME', queue=True)
+            self.send_command('NAME')
         if self.pjlink_class == '2':
             # Class 2 specific checks
             if self.serial_no is None:
-                self.send_command('SNUM', queue=True)
+                self.send_command('SNUM')
             if self.sw_version is None:
-                self.send_command('SVER', queue=True)
+                self.send_command('SVER')
             if self.model_filter is None:
-                self.send_command('RFIL', queue=True)
+                self.send_command('RFIL')
             if self.model_lamp is None:
-                self.send_command('RLMP', queue=True)
+                self.send_command('RLMP')
 
     def _get_status(self, status):
         """
@@ -713,14 +766,12 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
                                                                     code=status_code,
                                                                     message=status_message if msg is None else msg))
         self.changeStatus.emit(self.ip, status, message)
+        self.projectorUpdateIcons.emit()
 
     @QtCore.pyqtSlot()
     def check_login(self, data=None):
         """
-        Processes the initial connection and authentication (if needed).
-        Starts poll timer if connection is established.
-
-        NOTE: Qt md5 hash function doesn't work with projector authentication. Use the python md5 hash function.
+        Processes the initial connection and convert to a PJLink packet if valid initial connection
 
         :param data: Optional data if called from another routine
         """
@@ -733,12 +784,12 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
                 self.change_status(E_SOCKET_TIMEOUT)
                 return
             read = self.readLine(self.max_size)
-            self.readLine(self.max_size)  # Clean out the trailing \r\n
+            self.readLine(self.max_size)  # Clean out any trailing whitespace
             if read is None:
                 log.warning('({ip}) read is None - socket error?'.format(ip=self.ip))
                 return
             elif len(read) < 8:
-                log.warning('({ip}) Not enough data read)'.format(ip=self.ip))
+                log.warning('({ip}) Not enough data read - skipping'.format(ip=self.ip))
                 return
             data = decode(read, 'utf-8')
             # Possibility of extraneous data on input when reading.
@@ -750,9 +801,16 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         # PJLink initial login will be:
         # 'PJLink 0' - Unauthenticated login - no extra steps required.
         # 'PJLink 1 XXXXXX' Authenticated login - extra processing required.
-        if not data.upper().startswith('PJLINK'):
-            # Invalid response
+        if not data.startswith('PJLINK'):
+            # Invalid initial packet - close socket
+            log.error("({ip}) Invalid initial packet received - closing socket".format(ip=self.ip))
             return self.disconnect_from_host()
+        log.debug("({ip}) check_login(): Formatting initial connection prompt to PJLink packet".format(ip=self.ip))
+        return self.get_data("{start}{clss}{data}".format(start=PJLINK_PREFIX,
+                                                          clss="1",
+                                                          data=data.replace(" ", "=", 1)).encode('utf-8'))
+        # TODO: The below is replaced by process_pjlink() - remove when  working properly
+        """
         if '=' in data:
             # Processing a login reply
             data_check = data.strip().split('=')
@@ -801,6 +859,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             log.debug('({ip}) Starting timer'.format(ip=self.ip))
             self.timer.setInterval(2000)  # Set 2 seconds for initial information
             self.timer.start()
+        """
 
     def _trash_buffer(self, msg=None):
         """
@@ -848,32 +907,43 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             log.debug('({ip}) get_socket(): No data available (-1)'.format(ip=self.ip))
             return self.receive_data_signal()
         self.socket_timer.stop()
-        return self.get_data(buff=read, ip=self.ip)
+        self.get_data(buff=read, ip=self.ip)
+        return self.receive_data_signal()
 
-    def get_data(self, buff, ip):
+    def get_data(self, buff, ip=None):
         """
         Process received data
 
         :param buff:    Data to process.
         :param ip:      (optional) Destination IP.
         """
+        ip = self.ip if ip is None else ip
         log.debug("({ip}) get_data(ip='{ip_in}' buffer='{buff}'".format(ip=self.ip, ip_in=ip, buff=buff))
         # NOTE: Class2 has changed to some values being UTF-8
         data_in = decode(buff, 'utf-8')
         data = data_in.strip()
-        if (len(data) < 7) or (not data.startswith(PJLINK_PREFIX)):
-            return self._trash_buffer(msg='get_data(): Invalid packet - length or prefix')
+        # Initial packet checks
+        if (len(data) < 7):
+            return self._trash_buffer(msg="get_data(): Invalid packet - length")
         elif len(data) > self.max_size:
-            return self._trash_buffer(msg='get_data(): Invalid packet - too long')
+            return self._trash_buffer(msg="get_data(): Invalid packet - too long")
+        elif not data.startswith(PJLINK_PREFIX):
+            return self._trash_buffer(msg="get_data(): Invalid packet - PJLink prefix missing")
         elif '=' not in data:
-            return self._trash_buffer(msg='get_data(): Invalid packet does not have equal')
-        log.debug('({ip}) get_data(): Checking new data "{data}"'.format(ip=self.ip, data=data))
+            return self._trash_buffer(msg="get_data(): Invalid packet - Does not have '='")
+        log.debug("({ip}) get_data(): Checking new data '{data}'".format(ip=self.ip, data=data))
         header, data = data.split('=')
+        # At this point, the header should contain:
+        #   "PVCCCC"
+        #   Where:
+        #       P = PJLINK_PREFIX
+        #       V = PJLink class or version
+        #       C = PJLink command
         try:
-            version, cmd = header[1], header[2:]
+            version, cmd = header[1], header[2:].upper()
         except ValueError as e:
             self.change_status(E_INVALID_DATA)
-            log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip()))
+            log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in))
             return self._trash_buffer('get_data(): Expected header + command + data')
         if cmd not in PJLINK_VALID_CMD:
             log.warning('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd))
@@ -881,6 +951,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         if int(self.pjlink_class) < int(version):
             log.warning('({ip}) get_data(): Projector returned class reply higher '
                         'than projector stated class'.format(ip=self.ip))
+        self.send_busy = False
         return self.process_command(cmd, data)
 
     @QtCore.pyqtSlot(QtNetwork.QAbstractSocket.SocketError)
@@ -910,19 +981,18 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             self.reset_information()
         return
 
-    def send_command(self, cmd, opts='?', salt=None, queue=False):
+    def send_command(self, cmd, opts='?', salt=None, priority=False):
         """
         Add command to output queue if not already in queue.
 
         :param cmd: Command to send
         :param opts: Command option (if any) - defaults to '?' (get information)
         :param salt: Optional  salt for md5 hash initial authentication
-        :param queue: Option to force add to queue rather than sending directly
+        :param priority: Option to send packet now rather than queue it up
         """
         if self.state() != self.ConnectedState:
             log.warning('({ip}) send_command(): Not connected - returning'.format(ip=self.ip))
-            self.send_queue = []
-            return
+            return self.reset_information()
         if cmd not in PJLINK_VALID_CMD:
             log.error('({ip}) send_command(): Invalid command requested - ignoring.'.format(ip=self.ip))
             return
@@ -939,28 +1009,26 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             header = PJLINK_HEADER.format(linkclass=cmd_ver[0])
         else:
             # NOTE: Once we get to version 3 then think about looping
-            log.error('({ip}): send_command(): PJLink class check issue? aborting'.format(ip=self.ip))
+            log.error('({ip}): send_command(): PJLink class check issue? Aborting'.format(ip=self.ip))
             return
         out = '{salt}{header}{command} {options}{suffix}'.format(salt="" if salt is None else salt,
                                                                  header=header,
                                                                  command=cmd,
                                                                  options=opts,
                                                                  suffix=CR)
-        if out in self.send_queue:
-            # Already there, so don't add
-            log.debug('({ip}) send_command(out="{data}") Already in queue - skipping'.format(ip=self.ip,
-                                                                                             data=out.strip()))
-        elif not queue and len(self.send_queue) == 0:
-            # Nothing waiting to send, so just send it
-            log.debug('({ip}) send_command(out="{data}") Sending data'.format(ip=self.ip, data=out.strip()))
-            return self._send_command(data=out)
+        if out in self.priority_queue:
+            log.debug("({ip}) send_command(): Already in priority queue - skipping".format(ip=self.ip))
+        elif out in self.send_queue:
+            log.debug("({ip}) send_command(): Already in normal queue - skipping".format(ip=self.ip))
         else:
-            log.debug('({ip}) send_command(out="{data}") adding to queue'.format(ip=self.ip, data=out.strip()))
-            self.send_queue.append(out)
-            self.projectorReceivedData.emit()
-        log.debug('({ip}) send_command(): send_busy is {data}'.format(ip=self.ip, data=self.send_busy))
-        if not self.send_busy:
-            log.debug('({ip}) send_command() calling _send_string()'.format(ip=self.ip))
+            if priority:
+                log.debug("({ip}) send_command(): Adding to priority queue".format(ip=self.ip))
+                self.priority_queue.append(out)
+            else:
+                log.debug("({ip}) send_command(): Adding to normal queue".format(ip=self.ip))
+                self.send_queue.append(out)
+        if self.priority_queue or self.send_queue:
+            # May be some initial connection setup so make sure we send data
             self._send_command()
 
     @QtCore.pyqtSlot()
@@ -971,43 +1039,53 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         :param data: Immediate data to send
         :param utf8: Send as UTF-8 string otherwise send as ASCII string
         """
-        log.debug('({ip}) _send_string()'.format(ip=self.ip))
-        log.debug('({ip}) _send_string(): Connection status: {data}'.format(ip=self.ip, data=self.state()))
+        # Funny looking data check, but it's a quick check for data=None
+        log.debug("({ip}) _send_command(data='{data}')".format(ip=self.ip, data=data.strip() if data else data))
+        log.debug('({ip}) _send_command(): Connection status: {data}'.format(ip=self.ip,
+                                                                             data=S_QSOCKET_STATE[self.state()]))
         if self.state() != self.ConnectedState:
-            log.debug('({ip}) _send_string() Not connected - abort'.format(ip=self.ip))
-            self.send_queue = []
+            log.debug('({ip}) _send_command() Not connected - abort'.format(ip=self.ip))
             self.send_busy = False
-            return
+            return self.disconnect_from_host()
+        if data and data not in self.priority_queue:
+            log.debug("({ip}) _send_command(): Priority packet - adding to priority queue".format(ip=self.ip))
+            self.priority_queue.append(data)
+
         if self.send_busy:
             # Still waiting for response from last command sent
+            log.debug("({ip}) _send_command(): Still busy, returning".format(ip=self.ip))
+            log.debug('({ip}) _send_command(): Priority queue = {data}'.format(ip=self.ip, data=self.priority_queue))
+            log.debug('({ip}) _send_command(): Normal queue = {data}'.format(ip=self.ip, data=self.send_queue))
             return
-        if data is not None:
-            out = data
-            log.debug('({ip}) _send_string(data="{data}")'.format(ip=self.ip, data=out.strip()))
+
+        if len(self.priority_queue) != 0:
+            out = self.priority_queue.pop(0)
+            log.debug("({ip}) _send_command(): Getting priority queued packet".format(ip=self.ip))
         elif len(self.send_queue) != 0:
             out = self.send_queue.pop(0)
-            log.debug('({ip}) _send_string(queued data="{data}"%s)'.format(ip=self.ip, data=out.strip()))
+            log.debug('({ip}) _send_command(): Getting normal queued packet'.format(ip=self.ip))
         else:
             # No data to send
-            log.debug('({ip}) _send_string(): No data to send'.format(ip=self.ip))
+            log.debug('({ip}) _send_command(): No data to send'.format(ip=self.ip))
             self.send_busy = False
             return
         self.send_busy = True
-        log.debug('({ip}) _send_string(): Sending "{data}"'.format(ip=self.ip, data=out.strip()))
-        log.debug('({ip}) _send_string(): Queue = {data}'.format(ip=self.ip, data=self.send_queue))
+        log.debug('({ip}) _send_command(): Sending "{data}"'.format(ip=self.ip, data=out.strip()))
         self.socket_timer.start()
         sent = self.write(out.encode('{string_encoding}'.format(string_encoding='utf-8' if utf8 else 'ascii')))
         self.waitForBytesWritten(2000)  # 2 seconds should be enough
         if sent == -1:
             # Network error?
-            log.warning("({ip}) _send_command(): -1 received".format(ip=self.ip))
+            log.warning("({ip}) _send_command(): -1 received - disconnecting from host".format(ip=self.ip))
             self.change_status(E_NETWORK,
                                translate('OpenLP.PJLink', 'Error while sending data to projector'))
+            self.disconnect_from_host()
 
     def connect_to_host(self):
         """
         Initiate connection to projector.
         """
+        log.debug("{ip}) connect_to_host(): Starting connection".format(ip=self.ip))
         if self.state() == self.ConnectedState:
             log.warning('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip))
             return
@@ -1023,22 +1101,19 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             if abort:
                 log.warning('({ip}) disconnect_from_host(): Aborting connection'.format(ip=self.ip))
             else:
-                log.warning('({ip}) disconnect_from_host(): Not connected - returning'.format(ip=self.ip))
-            self.reset_information()
+                log.warning('({ip}) disconnect_from_host(): Not connected'.format(ip=self.ip))
         self.disconnectFromHost()
         try:
             self.readyRead.disconnect(self.get_socket)
         except TypeError:
             pass
+        log.debug('({ip}) disconnect_from_host() '
+                  'Current status {data}'.format(ip=self.ip, data=self._get_status(self.status_connect)[0]))
         if abort:
             self.change_status(E_NOT_CONNECTED)
         else:
-            log.debug('({ip}) disconnect_from_host() '
-                      'Current status {data}'.format(ip=self.ip, data=self._get_status(self.status_connect)[0]))
-            if self.status_connect != E_NOT_CONNECTED:
-                self.change_status(S_NOT_CONNECTED)
+            self.change_status(S_NOT_CONNECTED)
         self.reset_information()
-        self.projectorUpdateIcons.emit()
 
     def get_av_mute_status(self):
         """
diff --git a/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py b/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py
index c33220d4a..da0aae47f 100644
--- a/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py
+++ b/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py
@@ -23,12 +23,11 @@
 Package to test the openlp.core.projectors.pjlink base package.
 """
 from unittest import TestCase
-from unittest.mock import patch
 
 from openlp.core.projectors.db import Projector
 from openlp.core.projectors.pjlink import PJLink
 
-from tests.resources.projector.data import TEST_PIN, TEST_CONNECT_AUTHENTICATE, TEST_HASH, TEST1_DATA
+from tests.resources.projector.data import TEST1_DATA
 
 
 class TestPJLinkBugs(TestCase):
@@ -80,43 +79,17 @@ class TestPJLinkBugs(TestCase):
         """
         Test bug 1593882 no pin and authenticated request exception
         """
-        # GIVEN: Test object and mocks
-        mock_socket_timer = patch.object(self.pjlink_test, 'socket_timer').start()
-        mock_timer = patch.object(self.pjlink_test, 'timer').start()
-        mock_authentication = patch.object(self.pjlink_test, 'projectorAuthentication').start()
-        mock_ready_read = patch.object(self.pjlink_test, 'waitForReadyRead').start()
-        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
-        pjlink = self.pjlink_test
-        pjlink.pin = None
-        mock_ready_read.return_value = True
-
-        # WHEN: call with authentication request and pin not set
-        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
-
-        # THEN: 'No Authentication' signal should have been sent
-        mock_authentication.emit.assert_called_with(pjlink.ip)
+        # Test now part of test_projector_pjlink_commands_02
+        # Keeping here for bug reference
+        pass
 
     def test_bug_1593883_pjlink_authentication(self):
         """
-        Test bugfix 1593883 pjlink authentication
+        Test bugfix 1593883 pjlink authentication and ticket 92187
         """
-        # GIVEN: Test object and data
-        mock_socket_timer = patch.object(self.pjlink_test, 'socket_timer').start()
-        mock_timer = patch.object(self.pjlink_test, 'timer').start()
-        mock_send_command = patch.object(self.pjlink_test, 'write').start()
-        mock_state = patch.object(self.pjlink_test, 'state').start()
-        mock_waitForReadyRead = patch.object(self.pjlink_test, 'waitForReadyRead').start()
-        pjlink = self.pjlink_test
-        pjlink.pin = TEST_PIN
-        mock_state.return_value = pjlink.ConnectedState
-        mock_waitForReadyRead.return_value = True
-
-        # WHEN: Athenticated connection is called
-        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
-
-        # THEN: send_command should have the proper authentication
-        self.assertEqual("{test}".format(test=mock_send_command.call_args),
-                         "call(b'{hash}%1CLSS ?\\r')".format(hash=TEST_HASH))
+        # Test now part of test_projector_pjlink_commands_02
+        # Keeping here for bug reference
+        pass
 
     def test_bug_1734275_process_lamp_nonstandard_reply(self):
         """
diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py
index 7253df032..c86904bad 100644
--- a/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py
+++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py
@@ -25,11 +25,11 @@ Package to test the openlp.core.projectors.pjlink base package.
 from unittest import TestCase
 from unittest.mock import call, patch, MagicMock
 
-from openlp.core.projectors.constants import E_PARAMETER, ERROR_STRING, S_ON, S_CONNECTED
+from openlp.core.projectors.constants import E_PARAMETER, ERROR_STRING, S_ON, S_CONNECTED, S_QSOCKET_STATE
 from openlp.core.projectors.db import Projector
 from openlp.core.projectors.pjlink import PJLink
 
-from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE, TEST1_DATA
+from tests.resources.projector.data import TEST1_DATA
 
 pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True)
 
@@ -38,29 +38,17 @@ class TestPJLinkBase(TestCase):
     """
     Tests for the PJLink module
     """
-    @patch.object(pjlink_test, 'readyRead')
-    @patch.object(pjlink_test, 'send_command')
-    @patch.object(pjlink_test, 'waitForReadyRead')
-    @patch('openlp.core.common.qmd5_hash')
-    def test_authenticated_connection_call(self,
-                                           mock_qmd5_hash,
-                                           mock_waitForReadyRead,
-                                           mock_send_command,
-                                           mock_readyRead):
-        """
-        Ticket 92187: Fix for projector connect with PJLink authentication exception.
-        """
-        # GIVEN: Test object
-        pjlink = pjlink_test
+    def setUp(self):
+        '''
+        TestPJLinkCommands part 2 initialization
+        '''
+        self.pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True)
 
-        # WHEN: Calling check_login with authentication request:
-        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
-
-        # THEN: Should have called qmd5_hash
-        self.assertTrue(mock_qmd5_hash.called_with(TEST_SALT,
-                                                   "Connection request should have been called with TEST_SALT"))
-        self.assertTrue(mock_qmd5_hash.called_with(TEST_PIN,
-                                                   "Connection request should have been called with TEST_PIN"))
+    def tearDown(self):
+        '''
+        TestPJLinkCommands part 2 cleanups
+        '''
+        self.pjlink_test = None
 
     @patch.object(pjlink_test, 'change_status')
     def test_status_change(self, mock_change_status):
@@ -110,18 +98,18 @@ class TestPJLinkBase(TestCase):
         # THEN: poll_loop should exit without calling any other method
         self.assertFalse(pjlink.timer.called, 'Should have returned without calling any other method')
 
-    @patch.object(pjlink_test, 'send_command')
-    def test_poll_loop_start(self, mock_send_command):
+    def test_poll_loop_start(self):
         """
         Test PJLink.poll_loop makes correct calls
         """
-        # GIVEN: test object and test data
-        pjlink = pjlink_test
-        pjlink.state = MagicMock()
-        pjlink.timer = MagicMock()
-        pjlink.timer.interval = MagicMock()
-        pjlink.timer.setInterval = MagicMock()
-        pjlink.timer.start = MagicMock()
+        # GIVEN: Mocks and test data
+        mock_state = patch.object(self.pjlink_test, 'state').start()
+        mock_state.return_value = S_QSOCKET_STATE['ConnectedState']
+        mock_timer = patch.object(self.pjlink_test, 'timer').start()
+        mock_timer.interval.return_value = 10
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+
+        pjlink = self.pjlink_test
         pjlink.poll_time = 20
         pjlink.power = S_ON
         pjlink.source_available = None
@@ -130,19 +118,17 @@ class TestPJLinkBase(TestCase):
         pjlink.model = None
         pjlink.pjlink_name = None
         pjlink.ConnectedState = S_CONNECTED
-        pjlink.timer.interval.return_value = 10
-        pjlink.state.return_value = S_CONNECTED
         call_list = [
-            call('POWR', queue=True),
-            call('ERST', queue=True),
-            call('LAMP', queue=True),
-            call('AVMT', queue=True),
-            call('INPT', queue=True),
-            call('INST', queue=True),
-            call('INFO', queue=True),
-            call('INF1', queue=True),
-            call('INF2', queue=True),
-            call('NAME', queue=True),
+            call('POWR'),
+            call('ERST'),
+            call('LAMP'),
+            call('AVMT'),
+            call('INPT'),
+            call('INST'),
+            call('INFO'),
+            call('INF1'),
+            call('INF2'),
+            call('NAME'),
         ]
 
         # WHEN: PJLink.poll_loop is called
@@ -150,8 +136,8 @@ class TestPJLinkBase(TestCase):
 
         # THEN: proper calls were made to retrieve projector data
         # First, call to update the timer with the next interval
-        self.assertTrue(pjlink.timer.setInterval.called, 'Should have updated the timer')
+        self.assertTrue(mock_timer.setInterval.called)
         # Next, should have called the timer to start
-        self.assertTrue(pjlink.timer.start.called, 'Should have started the timer')
+        self.assertTrue(mock_timer.start.called, 'Should have started the timer')
         # Finally, should have called send_command with a list of projetctor status checks
         mock_send_command.assert_has_calls(call_list, 'Should have queued projector queries')
diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py
index 431da0606..a5a6eca40 100644
--- a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py
+++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py
@@ -46,6 +46,18 @@ class TestPJLinkRouting(TestCase):
     """
     Tests for the PJLink module command routing
     """
+    def setUp(self):
+        '''
+        TestPJLinkCommands part 2 initialization
+        '''
+        self.pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True)
+
+    def tearDown(self):
+        '''
+        TestPJLinkCommands part 2 cleanups
+        '''
+        self.pjlink_test = None
+
     @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_call_clss(self, mock_log):
         """
@@ -163,21 +175,20 @@ class TestPJLinkRouting(TestCase):
         mock_change_status.assert_called_once_with(E_AUTHENTICATION)
         mock_log.error.assert_called_with(log_text)
 
-    @patch.object(openlp.core.projectors.pjlink, 'log')
-    def test_process_command_future(self, mock_log):
+    def test_process_command_future(self):
         """
         Test command valid but no method to process yet
         """
-        # GIVEN: Test object
-        pjlink = pjlink_test
-        log_text = "(127.0.0.1) Unable to process command='CLSS' (Future option)"
-        mock_log.reset_mock()
-        # Remove a valid command so we can test valid command but not available yet
-        pjlink.pjlink_functions.pop('CLSS')
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start()
+        mock_functions = patch.object(self.pjlink_test, 'pjlink_functions').start()
+        mock_functions.return_value = []
+
+        pjlink = self.pjlink_test
+        log_text = "(111.111.111.111) Unable to process command='CLSS' (Future option?)"
 
         # WHEN: process_command called with an unknown command
-        with patch.object(pjlink, 'pjlink_functions') as mock_functions:
-            pjlink.process_command(cmd='CLSS', data='DONT CARE')
+        pjlink.process_command(cmd='CLSS', data='DONT CARE')
 
         # THEN: Error should be logged and no command called
         self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method')
@@ -202,23 +213,20 @@ class TestPJLinkRouting(TestCase):
         self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method')
         mock_log.error.assert_called_once_with(log_text)
 
-    @patch.object(pjlink_test, 'pjlink_functions')
-    @patch.object(openlp.core.projectors.pjlink, 'log')
-    def test_process_command_ok(self, mock_log, mock_functions):
+    def test_process_command_ok(self):
         """
         Test command returned success
         """
-        # GIVEN: Test object
-        pjlink = pjlink_test
-        mock_functions.reset_mock()
-        mock_log.reset_mock()
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
 
-        # WHEN: process_command called with an unknown command
-        pjlink.process_command(cmd='CLSS', data='OK')
-        log_text = '(127.0.0.1) Command "CLSS" returned OK'
+        pjlink = self.pjlink_test
+        log_text = "(111.111.111.111) Command 'POWR' returned OK"
 
-        # THEN: Error should be logged and no command called
-        self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method')
-        self.assertEqual(mock_log.debug.call_count, 2, 'log.debug() should have been called twice')
-        # Although we called it twice, only the last log entry is saved
+        # WHEN: process_command called with a command that returns OK
+        pjlink.process_command(cmd='POWR', data='OK')
+
+        # THEN: Appropriate calls should have been made
         mock_log.debug.assert_called_with(log_text)
+        mock_send_command.assert_called_once_with(cmd='POWR')
diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py
similarity index 99%
rename from tests/functional/openlp_core/projectors/test_projector_pjlink_commands.py
rename to tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py
index 32544dd09..0917a9357 100644
--- a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands.py
+++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py
@@ -47,7 +47,7 @@ for pos in range(0, len(PJLINK_ERST_DATA)):
 
 class TestPJLinkCommands(TestCase):
     """
-    Tests for the PJLink module
+    Tests for the PJLinkCommands class part 1
     """
     @patch.object(pjlink_test, 'changeStatus')
     @patch.object(openlp.core.projectors.pjlink, 'log')
diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py
new file mode 100644
index 000000000..08682ccb7
--- /dev/null
+++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py
@@ -0,0 +1,198 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2015 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                          #
+###############################################################################
+"""
+Package to test the openlp.core.projectors.pjlink commands package.
+"""
+from unittest import TestCase
+from unittest.mock import patch, call
+
+import openlp.core.projectors.pjlink
+from openlp.core.projectors.constants import S_CONNECTED
+from openlp.core.projectors.db import Projector
+from openlp.core.projectors.pjlink import PJLink
+
+from tests.resources.projector.data import TEST_HASH, TEST_PIN, TEST_SALT, TEST1_DATA
+
+
+class TestPJLinkCommands(TestCase):
+    """
+    Tests for the PJLinkCommands class part 2
+    """
+    def setUp(self):
+        '''
+        TestPJLinkCommands part 2 initialization
+        '''
+        self.pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True)
+
+    def tearDown(self):
+        '''
+        TestPJLinkCommands part 2 cleanups
+        '''
+        self.pjlink_test = None
+
+    def test_process_pjlink_normal(self):
+        """
+        Test initial connection prompt with no authentication
+        """
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, "log").start()
+        mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        mock_readyRead = patch.object(self.pjlink_test, 'readyRead').start()
+        mock_change_status = patch.object(self.pjlink_test, 'change_status').start()
+        pjlink = self.pjlink_test
+        pjlink.pin = None
+        log_check = [call("({111.111.111.111}) process_pjlink(): Sending 'CLSS' initial command'"), ]
+
+        # WHEN: process_pjlink called with no authentication required
+        pjlink.process_pjlink(data="0")
+
+        # THEN: proper processing should have occured
+        mock_log.debug.has_calls(log_check)
+        mock_disconnect_from_host.assert_not_called()
+        mock_readyRead.connect.assert_called_once()
+        mock_change_status.assert_called_once_with(S_CONNECTED)
+        mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=None)
+
+    def test_process_pjlink_authenticate(self):
+        """
+        Test initial connection prompt with authentication
+        """
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, "log").start()
+        mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        mock_readyRead = patch.object(self.pjlink_test, 'readyRead').start()
+        mock_change_status = patch.object(self.pjlink_test, 'change_status').start()
+        pjlink = self.pjlink_test
+        pjlink.pin = TEST_PIN
+        log_check = [call("({111.111.111.111}) process_pjlink(): Sending 'CLSS' initial command'"), ]
+
+        # WHEN: process_pjlink called with no authentication required
+        pjlink.process_pjlink(data='1 {salt}'.format(salt=TEST_SALT))
+
+        # THEN: proper processing should have occured
+        mock_log.debug.has_calls(log_check)
+        mock_disconnect_from_host.assert_not_called()
+        mock_readyRead.connect.assert_called_once()
+        mock_change_status.assert_called_once_with(S_CONNECTED)
+        mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=TEST_HASH)
+
+    def test_process_pjlink_normal_pin_set_error(self):
+        """
+        Test process_pjlinnk called with no authentication but pin is set
+        """
+        # GIVEN: Initial mocks and data
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start()
+        mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        pjlink = self.pjlink_test
+        pjlink.pin = TEST_PIN
+        log_check = [call('(111.111.111.111) Normal connection but PIN set - aborting'), ]
+
+        # WHEN: process_pjlink called with invalid authentication scheme
+        pjlink.process_pjlink(data='0')
+
+        # THEN: Proper calls should be made
+        mock_log.error.assert_has_calls(log_check)
+        mock_disconnect_from_host.assert_called_once()
+        mock_send_command.assert_not_called()
+
+    def test_process_pjlink_normal_with_salt_error(self):
+        """
+        Test process_pjlinnk called with no authentication but pin is set
+        """
+        # GIVEN: Initial mocks and data
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start()
+        mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        pjlink = self.pjlink_test
+        pjlink.pin = TEST_PIN
+        log_check = [call('(111.111.111.111) Normal connection with extra information - aborting'), ]
+
+        # WHEN: process_pjlink called with invalid authentication scheme
+        pjlink.process_pjlink(data='0 {salt}'.format(salt=TEST_SALT))
+
+        # THEN: Proper calls should be made
+        mock_log.error.assert_has_calls(log_check)
+        mock_disconnect_from_host.assert_called_once()
+        mock_send_command.assert_not_called()
+
+    def test_process_pjlink_invalid_authentication_scheme_length_error(self):
+        """
+        Test initial connection prompt with authentication scheme longer than 1 character
+        """
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start()
+        mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        pjlink = self.pjlink_test
+        log_check = [call('(111.111.111.111) Invalid initial authentication scheme - aborting'), ]
+
+        # WHEN: process_pjlink called with invalid authentication scheme
+        pjlink.process_pjlink(data='01')
+
+        # THEN: socket should be closed and invalid data logged
+        mock_log.error.assert_has_calls(log_check)
+        mock_disconnect_from_host.assert_called_once()
+        mock_send_command.assert_not_called()
+
+    def test_process_pjlink_invalid_authentication_data_length_error(self):
+        """
+        Test initial connection prompt with authentication no salt
+        """
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start()
+        mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        log_check = [call('(111.111.111.111) Authenticated connection but not enough info - aborting'), ]
+        pjlink = self.pjlink_test
+
+        # WHEN: process_pjlink called with no salt
+        pjlink.process_pjlink(data='1')
+
+        # THEN: socket should be closed and invalid data logged
+        mock_log.error.assert_has_calls(log_check)
+        mock_disconnect_from_host.assert_called_once()
+        mock_send_command.assert_not_called()
+
+    def test_process_pjlink_authenticate_pin_not_set_error(self):
+        """
+        Test process_pjlink authentication but pin not set
+        """
+        # GIVEN: Initial mocks and data
+        mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start()
+        mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        log_check = [call('(111.111.111.111) Authenticate connection but no PIN - aborting'), ]
+        pjlink = self.pjlink_test
+        pjlink.pin = None
+
+        # WHEN: process_pjlink called with no salt
+        pjlink.process_pjlink(data='1 {salt}'.format(salt=TEST_SALT))
+
+        # THEN: socket should be closed and invalid data logged
+        mock_log.error.assert_has_calls(log_check)
+        mock_disconnect_from_host.assert_called_once()
+        mock_send_command.assert_not_called()

From 44b82d8ca292c7eab4f440e1860ffec99917ed5e Mon Sep 17 00:00:00 2001
From: Ken Roberts <alisonken1@gmail.com>
Date: Mon, 4 Dec 2017 16:54:15 -0800
Subject: [PATCH 02/20] Fix mocks to use correct python version tests

---
 openlp/core/projectors/pjlink.py                   |  2 +-
 .../test_projector_pjlink_commands_02.py           | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py
index 99fb6956c..0a18aef33 100644
--- a/openlp/core/projectors/pjlink.py
+++ b/openlp/core/projectors/pjlink.py
@@ -657,7 +657,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         except TypeError:
             pass
         try:
-            self.readyRead.connect(self.get_socket)  # Set in process_pjlink
+            self.readyRead.disconnect(self.get_socket)  # Set in process_pjlink
         except TypeError:
             pass
 
diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py
index 08682ccb7..62ba4ec76 100644
--- a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py
+++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py
@@ -69,7 +69,7 @@ class TestPJLinkCommands(TestCase):
         # THEN: proper processing should have occured
         mock_log.debug.has_calls(log_check)
         mock_disconnect_from_host.assert_not_called()
-        mock_readyRead.connect.assert_called_once()
+        self.assertEqual(mock_readyRead.connect.call_count, 1, 'Should have only been called once')
         mock_change_status.assert_called_once_with(S_CONNECTED)
         mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=None)
 
@@ -93,7 +93,7 @@ class TestPJLinkCommands(TestCase):
         # THEN: proper processing should have occured
         mock_log.debug.has_calls(log_check)
         mock_disconnect_from_host.assert_not_called()
-        mock_readyRead.connect.assert_called_once()
+        self.assertEqual(mock_readyRead.connect.call_count, 1, 'Should have only been called once')
         mock_change_status.assert_called_once_with(S_CONNECTED)
         mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=TEST_HASH)
 
@@ -115,7 +115,7 @@ class TestPJLinkCommands(TestCase):
 
         # THEN: Proper calls should be made
         mock_log.error.assert_has_calls(log_check)
-        mock_disconnect_from_host.assert_called_once()
+        self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once')
         mock_send_command.assert_not_called()
 
     def test_process_pjlink_normal_with_salt_error(self):
@@ -136,7 +136,7 @@ class TestPJLinkCommands(TestCase):
 
         # THEN: Proper calls should be made
         mock_log.error.assert_has_calls(log_check)
-        mock_disconnect_from_host.assert_called_once()
+        self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once')
         mock_send_command.assert_not_called()
 
     def test_process_pjlink_invalid_authentication_scheme_length_error(self):
@@ -155,7 +155,7 @@ class TestPJLinkCommands(TestCase):
 
         # THEN: socket should be closed and invalid data logged
         mock_log.error.assert_has_calls(log_check)
-        mock_disconnect_from_host.assert_called_once()
+        self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once')
         mock_send_command.assert_not_called()
 
     def test_process_pjlink_invalid_authentication_data_length_error(self):
@@ -174,7 +174,7 @@ class TestPJLinkCommands(TestCase):
 
         # THEN: socket should be closed and invalid data logged
         mock_log.error.assert_has_calls(log_check)
-        mock_disconnect_from_host.assert_called_once()
+        self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once')
         mock_send_command.assert_not_called()
 
     def test_process_pjlink_authenticate_pin_not_set_error(self):
@@ -194,5 +194,5 @@ class TestPJLinkCommands(TestCase):
 
         # THEN: socket should be closed and invalid data logged
         mock_log.error.assert_has_calls(log_check)
-        mock_disconnect_from_host.assert_called_once()
+        self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once')
         mock_send_command.assert_not_called()

From eec0e325dfc1d61157706019d88493a8a5c2e253 Mon Sep 17 00:00:00 2001
From: Ken Roberts <alisonken1@gmail.com>
Date: Sat, 9 Dec 2017 03:17:05 -0800
Subject: [PATCH 03/20] OLP Style cleanups

---
 openlp/core/projectors/constants.py           |   2 +-
 openlp/core/projectors/pjlink.py              | 141 +++++++++---------
 .../test_projector_pjlink_cmd_routing.py      |   6 +-
 .../test_projector_pjlink_commands_01.py      |  24 +--
 4 files changed, 88 insertions(+), 85 deletions(-)

diff --git a/openlp/core/projectors/constants.py b/openlp/core/projectors/constants.py
index c700141b8..a9410d109 100644
--- a/openlp/core/projectors/constants.py
+++ b/openlp/core/projectors/constants.py
@@ -159,7 +159,7 @@ S_QSOCKET_STATE = {
     'ConnectedState': 3,
     'BoundState': 4,
     'ListeningState': 5,
-    'ClosingState': 6,
+    'ClosingState': 6
 }
 
 # Error and status codes
diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py
index 0a18aef33..12ae7e1d0 100644
--- a/openlp/core/projectors/pjlink.py
+++ b/openlp/core/projectors/pjlink.py
@@ -110,7 +110,7 @@ class PJLinkCommands(object):
         """
         log.debug('PJlinkCommands(args={args} kwargs={kwargs})'.format(args=args, kwargs=kwargs))
         super().__init__()
-        # Map command to function
+        # Map PJLink command to method
         self.pjlink_functions = {
             'AVMT': self.process_avmt,
             'CLSS': self.process_clss,
@@ -123,6 +123,7 @@ class PJLinkCommands(object):
             'LAMP': self.process_lamp,
             'NAME': self.process_name,
             'PJLINK': self.process_pjlink,
+            # TODO: Part of check_login refactor - remove when done
             # 'PJLINK': self.check_login,
             'POWR': self.process_powr,
             'SNUM': self.process_snum,
@@ -183,14 +184,14 @@ class PJLinkCommands(object):
         _data = data.upper()
         # Check if we have a future command not available yet
         if cmd not in PJLINK_VALID_CMD:
-            log.error("({ip}) Ignoring command='{cmd}' (Invalid/Unknown)".format(ip=self.ip, cmd=cmd))
+            log.error('({ip}) Ignoring command="{cmd}" (Invalid/Unknown)'.format(ip=self.ip, cmd=cmd))
             return
         elif _data == 'OK':
-            log.debug("({ip}) Command '{cmd}' returned OK".format(ip=self.ip, cmd=cmd))
+            log.debug('({ip}) Command "{cmd}" returned OK'.format(ip=self.ip, cmd=cmd))
             # A command returned successfully, so do a query on command to verify status
             return self.send_command(cmd=cmd)
         elif cmd not in self.pjlink_functions:
-            log.warning("({ip}) Unable to process command='{cmd}' (Future option?)".format(ip=self.ip, cmd=cmd))
+            log.warning('({ip}) Unable to process command="{cmd}" (Future option?)'.format(ip=self.ip, cmd=cmd))
             return
         elif _data in PJLINK_ERRORS:
             # Oops - projector error
@@ -260,19 +261,19 @@ class PJLinkCommands(object):
         #            : Received: '%1CLSS=Class 1'  (Optoma)
         #            : Received: '%1CLSS=Version1'  (BenQ)
         if len(data) > 1:
-            log.warning("({ip}) Non-standard CLSS reply: '{data}'".format(ip=self.ip, data=data))
+            log.warning('({ip}) Non-standard CLSS reply: "{data}"'.format(ip=self.ip, data=data))
             # Due to stupid projectors not following standards (Optoma, BenQ comes to mind),
             # AND the different responses that can be received, the semi-permanent way to
             # fix the class reply is to just remove all non-digit characters.
             try:
                 clss = re.findall('\d', data)[0]  # Should only be the first match
             except IndexError:
-                log.error("({ip}) No numbers found in class version reply '{data}' - "
-                          "defaulting to class '1'".format(ip=self.ip, data=data))
+                log.error('({ip}) No numbers found in class version reply "{data}" - '
+                          'defaulting to class "1"'.format(ip=self.ip, data=data))
                 clss = '1'
         elif not data.isdigit():
-            log.error("({ip}) NAN clss version reply '{data}' - "
-                      "defaulting to class '1'".format(ip=self.ip, data=data))
+            log.error('({ip}) NAN CLSS version reply "{data}" - '
+                      'defaulting to class "1"'.format(ip=self.ip, data=data))
             clss = '1'
         else:
             clss = data
@@ -290,7 +291,7 @@ class PJLinkCommands(object):
         """
         if len(data) != PJLINK_ERST_DATA['DATA_LENGTH']:
             count = PJLINK_ERST_DATA['DATA_LENGTH']
-            log.warning("{ip}) Invalid error status response '{data}': length != {count}".format(ip=self.ip,
+            log.warning('{ip}) Invalid error status response "{data}": length != {count}'.format(ip=self.ip,
                                                                                                  data=data,
                                                                                                  count=count))
             return
@@ -298,7 +299,7 @@ class PJLinkCommands(object):
             datacheck = int(data)
         except ValueError:
             # Bad data - ignore
-            log.warning("({ip}) Invalid error status response '{data}'".format(ip=self.ip, data=data))
+            log.warning('({ip}) Invalid error status response "{data}"'.format(ip=self.ip, data=data))
             return
         if datacheck == 0:
             self.projector_errors = None
@@ -437,30 +438,30 @@ class PJLinkCommands(object):
 
         :param data: Initial packet with authentication scheme
         """
-        log.debug("({ip}) Processing PJLINK command".format(ip=self.ip))
-        chk = data.split(" ")
+        log.debug('({ip}) Processing PJLINK command'.format(ip=self.ip))
+        chk = data.split(' ')
         if len(chk[0]) != 1:
             # Invalid - after splitting, first field should be 1 character, either '0' or '1' only
-            log.error("({ip}) Invalid initial authentication scheme - aborting".format(ip=self.ip))
+            log.error('({ip}) Invalid initial authentication scheme - aborting'.format(ip=self.ip))
             return self.disconnect_from_host()
         elif chk[0] == '0':
             # Normal connection no authentication
             if len(chk) > 1:
                 # Invalid data - there should be nothing after a normal authentication scheme
-                log.error("({ip}) Normal connection with extra information - aborting".format(ip=self.ip))
+                log.error('({ip}) Normal connection with extra information - aborting'.format(ip=self.ip))
                 return self.disconnect_from_host()
             elif self.pin:
-                log.error("({ip}) Normal connection but PIN set - aborting".format(ip=self.ip))
+                log.error('({ip}) Normal connection but PIN set - aborting'.format(ip=self.ip))
                 return self.disconnect_from_host()
             else:
                 data_hash = None
         elif chk[0] == '1':
             if len(chk) < 2:
                 # Not enough information for authenticated connection
-                log.error("({ip}) Authenticated connection but not enough info - aborting".format(ip=self.ip))
+                log.error('({ip}) Authenticated connection but not enough info - aborting'.format(ip=self.ip))
                 return self.disconnect_from_host()
             elif not self.pin:
-                log.error("({ip}) Authenticate connection but no PIN - aborting".format(ip=self.ip))
+                log.error('({ip}) Authenticate connection but no PIN - aborting'.format(ip=self.ip))
                 return self.disconnect_from_host()
             else:
                 data_hash = str(qmd5_hash(salt=chk[1].encode('utf-8'), data=self.pin.encode('utf-8')),
@@ -472,7 +473,7 @@ class PJLinkCommands(object):
             self.timer.setInterval(2000)  # Set 2 seconds for initial information
             self.timer.start()
         self.change_status(S_CONNECTED)
-        log.debug("({ip}) process_pjlink(): Sending 'CLSS' initial command'".format(ip=self.ip))
+        log.debug('({ip}) process_pjlink(): Sending "CLSS" initial command'.format(ip=self.ip))
         # Since this is an initial connection, make it a priority just in case
         return self.send_command(cmd="CLSS", salt=data_hash, priority=True)
 
@@ -496,7 +497,7 @@ class PJLinkCommands(object):
                     self.send_command('INST')
         else:
             # Log unknown status response
-            log.warning('({ip}) Unknown power response: {data}'.format(ip=self.ip, data=data))
+            log.warning('({ip}) Unknown power response: "{data}"'.format(ip=self.ip, data=data))
         return
 
     def process_rfil(self, data):
@@ -506,9 +507,9 @@ class PJLinkCommands(object):
         if self.model_filter is None:
             self.model_filter = data
         else:
-            log.warning("({ip}) Filter model already set".format(ip=self.ip))
-            log.warning("({ip}) Saved model: '{old}'".format(ip=self.ip, old=self.model_filter))
-            log.warning("({ip}) New model: '{new}'".format(ip=self.ip, new=data))
+            log.warning('({ip}) Filter model already set'.format(ip=self.ip))
+            log.warning('({ip}) Saved model: "{old}"'.format(ip=self.ip, old=self.model_filter))
+            log.warning('({ip}) New model: "{new}"'.format(ip=self.ip, new=data))
 
     def process_rlmp(self, data):
         """
@@ -517,9 +518,9 @@ class PJLinkCommands(object):
         if self.model_lamp is None:
             self.model_lamp = data
         else:
-            log.warning("({ip}) Lamp model already set".format(ip=self.ip))
-            log.warning("({ip}) Saved lamp: '{old}'".format(ip=self.ip, old=self.model_lamp))
-            log.warning("({ip}) New lamp: '{new}'".format(ip=self.ip, new=data))
+            log.warning('({ip}) Lamp model already set'.format(ip=self.ip))
+            log.warning('({ip}) Saved lamp: "{old}"'.format(ip=self.ip, old=self.model_lamp))
+            log.warning('({ip}) New lamp: "{new}"'.format(ip=self.ip, new=data))
 
     def process_snum(self, data):
         """
@@ -528,16 +529,16 @@ class PJLinkCommands(object):
         :param data: Serial number from projector.
         """
         if self.serial_no is None:
-            log.debug("({ip}) Setting projector serial number to '{data}'".format(ip=self.ip, data=data))
+            log.debug('({ip}) Setting projector serial number to "{data}"'.format(ip=self.ip, data=data))
             self.serial_no = data
             self.db_update = False
         else:
             # Compare serial numbers and see if we got the same projector
             if self.serial_no != data:
-                log.warning("({ip}) Projector serial number does not match saved serial number".format(ip=self.ip))
-                log.warning("({ip}) Saved:    '{old}'".format(ip=self.ip, old=self.serial_no))
-                log.warning("({ip}) Received: '{new}'".format(ip=self.ip, new=data))
-                log.warning("({ip}) NOT saving serial number".format(ip=self.ip))
+                log.warning('({ip}) Projector serial number does not match saved serial number'.format(ip=self.ip))
+                log.warning('({ip}) Saved:    "{old}"'.format(ip=self.ip, old=self.serial_no))
+                log.warning('({ip}) Received: "{new}"'.format(ip=self.ip, new=data))
+                log.warning('({ip}) NOT saving serial number'.format(ip=self.ip))
                 self.serial_no_received = data
 
     def process_sver(self, data):
@@ -546,20 +547,20 @@ class PJLinkCommands(object):
         """
         if len(data) > 32:
             # Defined in specs max version is 32 characters
-            log.warning("Invalid software version - too long")
+            log.warning('Invalid software version - too long')
             return
         elif self.sw_version is None:
-            log.debug("({ip}) Setting projector software version to '{data}'".format(ip=self.ip, data=data))
+            log.debug('({ip}) Setting projector software version to "{data}"'.format(ip=self.ip, data=data))
             self.sw_version = data
             self.db_update = True
         else:
             # Compare software version and see if we got the same projector
             if self.serial_no != data:
-                log.warning("({ip}) Projector software version does not match saved "
-                            "software version".format(ip=self.ip))
-                log.warning("({ip}) Saved:    '{old}'".format(ip=self.ip, old=self.sw_version))
-                log.warning("({ip}) Received: '{new}'".format(ip=self.ip, new=data))
-                log.warning("({ip}) Saving new serial number as sw_version_received".format(ip=self.ip))
+                log.warning('({ip}) Projector software version does not match saved '
+                            'software version'.format(ip=self.ip))
+                log.warning('({ip}) Saved:    "{old}"'.format(ip=self.ip, old=self.sw_version))
+                log.warning('({ip}) Received: "{new}"'.format(ip=self.ip, new=data))
+                log.warning('({ip}) Saving new serial number as sw_version_received'.format(ip=self.ip))
                 self.sw_version_received = data
 
 
@@ -586,9 +587,9 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         :param poll_time: Time (in seconds) to poll connected projector
         :param socket_timeout: Time (in seconds) to abort the connection if no response
         """
-        log.debug('PJlink(projector={projector}, args={args} kwargs={kwargs})'.format(projector=projector,
-                                                                                      args=args,
-                                                                                      kwargs=kwargs))
+        log.debug('PJlink(projector="{projector}", args="{args}" kwargs="{kwargs}")'.format(projector=projector,
+                                                                                            args=args,
+                                                                                            kwargs=kwargs))
         super().__init__()
         self.entry = projector
         self.ip = self.entry.ip
@@ -660,7 +661,6 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             self.readyRead.disconnect(self.get_socket)  # Set in process_pjlink
         except TypeError:
             pass
-
         self.disconnect_from_host()
         self.deleteLater()
         self.i_am_running = False
@@ -679,7 +679,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         Normally called by timer().
         """
         if self.state() != S_QSOCKET_STATE['ConnectedState']:
-            log.warning("({ip}) poll_loop(): Not connected - returning".format(ip=self.ip))
+            log.warning('({ip}) poll_loop(): Not connected - returning'.format(ip=self.ip))
             return
         log.debug('({ip}) poll_loop(): Updating projector status'.format(ip=self.ip))
         # Reset timer in case we were called from a set command
@@ -803,12 +803,12 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         # 'PJLink 1 XXXXXX' Authenticated login - extra processing required.
         if not data.startswith('PJLINK'):
             # Invalid initial packet - close socket
-            log.error("({ip}) Invalid initial packet received - closing socket".format(ip=self.ip))
+            log.error('({ip}) Invalid initial packet received - closing socket'.format(ip=self.ip))
             return self.disconnect_from_host()
-        log.debug("({ip}) check_login(): Formatting initial connection prompt to PJLink packet".format(ip=self.ip))
-        return self.get_data("{start}{clss}{data}".format(start=PJLINK_PREFIX,
-                                                          clss="1",
-                                                          data=data.replace(" ", "=", 1)).encode('utf-8'))
+        log.debug('({ip}) check_login(): Formatting initial connection prompt to PJLink packet'.format(ip=self.ip))
+        return self.get_data('{start}{clss}{data}'.format(start=PJLINK_PREFIX,
+                                                          clss='1',
+                                                          data=data.replace(' ', '=', 1)).encode('utf-8'))
         # TODO: The below is replaced by process_pjlink() - remove when  working properly
         """
         if '=' in data:
@@ -865,13 +865,13 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         """
         Clean out extraneous stuff in the buffer.
         """
-        log.warning("({ip}) {message}".format(ip=self.ip, message='Invalid packet' if msg is None else msg))
+        log.warning('({ip}) {message}'.format(ip=self.ip, message='Invalid packet' if msg is None else msg))
         self.send_busy = False
         trash_count = 0
         while self.bytesAvailable() > 0:
             trash = self.read(self.max_size)
             trash_count += len(trash)
-        log.debug("({ip}) Finished cleaning buffer - {count} bytes dropped".format(ip=self.ip,
+        log.debug('({ip}) Finished cleaning buffer - {count} bytes dropped'.format(ip=self.ip,
                                                                                    count=trash_count))
         return
 
@@ -883,7 +883,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         :param data:  Data to process. buffer must be formatted as a proper PJLink packet.
         :param ip:      Destination IP for buffer.
         """
-        log.debug("({ip}) get_buffer(data='{buff}' ip='{ip_in}'".format(ip=self.ip, buff=data, ip_in=ip))
+        log.debug('({ip}) get_buffer(data="{buff}" ip="{ip_in}"'.format(ip=self.ip, buff=data, ip_in=ip))
         if ip is None:
             log.debug("({ip}) get_buffer() Don't know who data is for - exiting".format(ip=self.ip))
             return
@@ -901,7 +901,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             return
         # Although we have a packet length limit, go ahead and use a larger buffer
         read = self.readLine(1024)
-        log.debug("({ip}) get_socket(): '{buff}'".format(ip=self.ip, buff=read))
+        log.debug('({ip}) get_socket(): "{buff}"'.format(ip=self.ip, buff=read))
         if read == -1:
             # No data available
             log.debug('({ip}) get_socket(): No data available (-1)'.format(ip=self.ip))
@@ -917,21 +917,24 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         :param buff:    Data to process.
         :param ip:      (optional) Destination IP.
         """
-        ip = self.ip if ip is None else ip
-        log.debug("({ip}) get_data(ip='{ip_in}' buffer='{buff}'".format(ip=self.ip, ip_in=ip, buff=buff))
+        # Since "self" is not available to options and the "ip" keyword is a "maybe I'll use in the future",
+        # set to default here
+        if ip is None:
+            ip = self.ip
+        log.debug('({ip}) get_data(ip="{ip_in}" buffer="{buff}"'.format(ip=self.ip, ip_in=ip, buff=buff))
         # NOTE: Class2 has changed to some values being UTF-8
         data_in = decode(buff, 'utf-8')
         data = data_in.strip()
         # Initial packet checks
         if (len(data) < 7):
-            return self._trash_buffer(msg="get_data(): Invalid packet - length")
+            return self._trash_buffer(msg='get_data(): Invalid packet - length')
         elif len(data) > self.max_size:
-            return self._trash_buffer(msg="get_data(): Invalid packet - too long")
+            return self._trash_buffer(msg='get_data(): Invalid packet - too long')
         elif not data.startswith(PJLINK_PREFIX):
-            return self._trash_buffer(msg="get_data(): Invalid packet - PJLink prefix missing")
+            return self._trash_buffer(msg='get_data(): Invalid packet - PJLink prefix missing')
         elif '=' not in data:
-            return self._trash_buffer(msg="get_data(): Invalid packet - Does not have '='")
-        log.debug("({ip}) get_data(): Checking new data '{data}'".format(ip=self.ip, data=data))
+            return self._trash_buffer(msg='get_data(): Invalid reply - Does not have "="')
+        log.debug('({ip}) get_data(): Checking new data "{data}"'.format(ip=self.ip, data=data))
         header, data = data.split('=')
         # At this point, the header should contain:
         #   "PVCCCC"
@@ -1017,15 +1020,15 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
                                                                  options=opts,
                                                                  suffix=CR)
         if out in self.priority_queue:
-            log.debug("({ip}) send_command(): Already in priority queue - skipping".format(ip=self.ip))
+            log.debug('({ip}) send_command(): Already in priority queue - skipping'.format(ip=self.ip))
         elif out in self.send_queue:
-            log.debug("({ip}) send_command(): Already in normal queue - skipping".format(ip=self.ip))
+            log.debug('({ip}) send_command(): Already in normal queue - skipping'.format(ip=self.ip))
         else:
             if priority:
-                log.debug("({ip}) send_command(): Adding to priority queue".format(ip=self.ip))
+                log.debug('({ip}) send_command(): Adding to priority queue'.format(ip=self.ip))
                 self.priority_queue.append(out)
             else:
-                log.debug("({ip}) send_command(): Adding to normal queue".format(ip=self.ip))
+                log.debug('({ip}) send_command(): Adding to normal queue'.format(ip=self.ip))
                 self.send_queue.append(out)
         if self.priority_queue or self.send_queue:
             # May be some initial connection setup so make sure we send data
@@ -1040,7 +1043,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         :param utf8: Send as UTF-8 string otherwise send as ASCII string
         """
         # Funny looking data check, but it's a quick check for data=None
-        log.debug("({ip}) _send_command(data='{data}')".format(ip=self.ip, data=data.strip() if data else data))
+        log.debug('({ip}) _send_command(data="{data}")'.format(ip=self.ip, data=data.strip() if data else data))
         log.debug('({ip}) _send_command(): Connection status: {data}'.format(ip=self.ip,
                                                                              data=S_QSOCKET_STATE[self.state()]))
         if self.state() != self.ConnectedState:
@@ -1048,19 +1051,19 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             self.send_busy = False
             return self.disconnect_from_host()
         if data and data not in self.priority_queue:
-            log.debug("({ip}) _send_command(): Priority packet - adding to priority queue".format(ip=self.ip))
+            log.debug('({ip}) _send_command(): Priority packet - adding to priority queue'.format(ip=self.ip))
             self.priority_queue.append(data)
 
         if self.send_busy:
             # Still waiting for response from last command sent
-            log.debug("({ip}) _send_command(): Still busy, returning".format(ip=self.ip))
+            log.debug('({ip}) _send_command(): Still busy, returning'.format(ip=self.ip))
             log.debug('({ip}) _send_command(): Priority queue = {data}'.format(ip=self.ip, data=self.priority_queue))
             log.debug('({ip}) _send_command(): Normal queue = {data}'.format(ip=self.ip, data=self.send_queue))
             return
 
         if len(self.priority_queue) != 0:
             out = self.priority_queue.pop(0)
-            log.debug("({ip}) _send_command(): Getting priority queued packet".format(ip=self.ip))
+            log.debug('({ip}) _send_command(): Getting priority queued packet'.format(ip=self.ip))
         elif len(self.send_queue) != 0:
             out = self.send_queue.pop(0)
             log.debug('({ip}) _send_command(): Getting normal queued packet'.format(ip=self.ip))
@@ -1076,7 +1079,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         self.waitForBytesWritten(2000)  # 2 seconds should be enough
         if sent == -1:
             # Network error?
-            log.warning("({ip}) _send_command(): -1 received - disconnecting from host".format(ip=self.ip))
+            log.warning('({ip}) _send_command(): -1 received - disconnecting from host'.format(ip=self.ip))
             self.change_status(E_NETWORK,
                                translate('OpenLP.PJLink', 'Error while sending data to projector'))
             self.disconnect_from_host()
@@ -1085,7 +1088,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         """
         Initiate connection to projector.
         """
-        log.debug("{ip}) connect_to_host(): Starting connection".format(ip=self.ip))
+        log.debug('{ip}) connect_to_host(): Starting connection'.format(ip=self.ip))
         if self.state() == self.ConnectedState:
             log.warning('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip))
             return
diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py
index a5a6eca40..4ff133061 100644
--- a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py
+++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py
@@ -185,7 +185,7 @@ class TestPJLinkRouting(TestCase):
         mock_functions.return_value = []
 
         pjlink = self.pjlink_test
-        log_text = "(111.111.111.111) Unable to process command='CLSS' (Future option?)"
+        log_text = '(111.111.111.111) Unable to process command="CLSS" (Future option?)'
 
         # WHEN: process_command called with an unknown command
         pjlink.process_command(cmd='CLSS', data='DONT CARE')
@@ -207,7 +207,7 @@ class TestPJLinkRouting(TestCase):
 
         # WHEN: process_command called with an unknown command
         pjlink.process_command(cmd='Unknown', data='Dont Care')
-        log_text = "(127.0.0.1) Ignoring command='Unknown' (Invalid/Unknown)"
+        log_text = '(127.0.0.1) Ignoring command="Unknown" (Invalid/Unknown)'
 
         # THEN: Error should be logged and no command called
         self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method')
@@ -222,7 +222,7 @@ class TestPJLinkRouting(TestCase):
         mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
 
         pjlink = self.pjlink_test
-        log_text = "(111.111.111.111) Command 'POWR' returned OK"
+        log_text = '(111.111.111.111) Command "POWR" returned OK'
 
         # WHEN: process_command called with a command that returns OK
         pjlink.process_command(cmd='POWR', data='OK')
diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py
index 0917a9357..8340a0fd0 100644
--- a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py
+++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py
@@ -580,7 +580,7 @@ class TestPJLinkCommands(TestCase):
 
         # WHEN: Process invalid reply
         pjlink.process_clss('Z')
-        log_text = "(127.0.0.1) NAN clss version reply 'Z' - defaulting to class '1'"
+        log_text = '(127.0.0.1) NAN CLSS version reply "Z" - defaulting to class "1"'
 
         # THEN: Projector class should be set with default value
         self.assertEqual(pjlink.pjlink_class, '1',
@@ -597,7 +597,7 @@ class TestPJLinkCommands(TestCase):
 
         # WHEN: Process invalid reply
         pjlink.process_clss('Invalid')
-        log_text = "(127.0.0.1) No numbers found in class version reply 'Invalid' - defaulting to class '1'"
+        log_text = '(127.0.0.1) No numbers found in class version reply "Invalid" - defaulting to class "1"'
 
         # THEN: Projector class should be set with default value
         self.assertEqual(pjlink.pjlink_class, '1',
@@ -627,7 +627,7 @@ class TestPJLinkCommands(TestCase):
         # GIVEN: Test object
         pjlink = pjlink_test
         pjlink.projector_errors = None
-        log_text = "127.0.0.1) Invalid error status response '11111111': length != 6"
+        log_text = '127.0.0.1) Invalid error status response "11111111": length != 6'
 
         # WHEN: process_erst called with invalid data (too many values
         pjlink.process_erst('11111111')
@@ -645,7 +645,7 @@ class TestPJLinkCommands(TestCase):
         # GIVEN: Test object
         pjlink = pjlink_test
         pjlink.projector_errors = None
-        log_text = "(127.0.0.1) Invalid error status response '1111Z1'"
+        log_text = '(127.0.0.1) Invalid error status response "1111Z1"'
 
         # WHEN: process_erst called with invalid data (too many values
         pjlink.process_erst('1111Z1')
@@ -671,8 +671,8 @@ class TestPJLinkCommands(TestCase):
         # THEN: PJLink instance errors should match chk_value
         for chk in pjlink.projector_errors:
             self.assertEqual(pjlink.projector_errors[chk], chk_string,
-                             "projector_errors['{chk}'] should have been set to {err}".format(chk=chk,
-                                                                                              err=chk_string))
+                             'projector_errors["{chk}"] should have been set to "{err}"'.format(chk=chk,
+                                                                                                err=chk_string))
 
     def test_projector_process_erst_all_error(self):
         """
@@ -690,8 +690,8 @@ class TestPJLinkCommands(TestCase):
         # THEN: PJLink instance errors should match chk_value
         for chk in pjlink.projector_errors:
             self.assertEqual(pjlink.projector_errors[chk], chk_string,
-                             "projector_errors['{chk}'] should have been set to {err}".format(chk=chk,
-                                                                                              err=chk_string))
+                             'projector_errors["{chk}"] should have been set to "{err}"'.format(chk=chk,
+                                                                                                err=chk_string))
 
     def test_projector_process_erst_warn_cover_only(self):
         """
@@ -744,9 +744,9 @@ class TestPJLinkCommands(TestCase):
         pjlink = pjlink_test
         pjlink.source_available = []
         test_data = '21 10 30 31 11 20'
-        test_saved = ['10', '11', '20', '21', '30', '31']
-        log_data = '(127.0.0.1) Setting projector sources_available to ' \
-            '"[\'10\', \'11\', \'20\', \'21\', \'30\', \'31\']"'
+        test_saved = ["10", "11", "20", "21", "30", "31"]
+        log_data = "(127.0.0.1) Setting projector sources_available to " \
+                   "\"['10', '11', '20', '21', '30', '31']\""
         mock_UpdateIcons.reset_mock()
         mock_log.reset_mock()
 
@@ -1021,7 +1021,7 @@ class TestPJLinkCommands(TestCase):
         pjlink.sw_version = None
         pjlink.sw_version_received = None
         test_data = 'Test 1 Subtest 1'
-        test_log = "(127.0.0.1) Setting projector software version to 'Test 1 Subtest 1'"
+        test_log = '(127.0.0.1) Setting projector software version to "Test 1 Subtest 1"'
         mock_log.reset_mock()
 
         # WHEN: process_sver called with invalid data

From d2ba2ad599a1e47b6021ed9f1e1ec3b8828a7e8a Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 15:00:39 +0000
Subject: [PATCH 04/20] First attempt

---
 tests/functional/openlp_core/api/http/test_error.py | 8 ++++----
 tests/functional/openlp_core/api/http/test_http.py  | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/functional/openlp_core/api/http/test_error.py b/tests/functional/openlp_core/api/http/test_error.py
index 1a694705c..599ecfc03 100644
--- a/tests/functional/openlp_core/api/http/test_error.py
+++ b/tests/functional/openlp_core/api/http/test_error.py
@@ -42,8 +42,8 @@ class TestApiError(TestCase):
             raise NotFound()
 
         # THEN: we get an error and a status
-        self.assertEquals('Not Found', context.exception.message, 'A Not Found exception should be thrown')
-        self.assertEquals(404, context.exception.status, 'A 404 status should be thrown')
+        assert 'Not Found' == context.exception.message, 'A Not Found exception should be thrown'
+        assert 404 == context.exception.status, 'A 404 status should be thrown'
 
     def test_server_error(self):
         """
@@ -55,5 +55,5 @@ class TestApiError(TestCase):
             raise ServerError()
 
         # THEN: we get an error and a status
-        self.assertEquals('Server Error', context.exception.message, 'A Not Found exception should be thrown')
-        self.assertEquals(500, context.exception.status, 'A 500 status should be thrown')
+        assert'Server Error' == context.exception.message, 'A Not Found exception should be thrown'
+        assert 500 == context.exception.status, 'A 500 status should be thrown'
diff --git a/tests/functional/openlp_core/api/http/test_http.py b/tests/functional/openlp_core/api/http/test_http.py
index ed584c1b9..c3cd1d1f4 100644
--- a/tests/functional/openlp_core/api/http/test_http.py
+++ b/tests/functional/openlp_core/api/http/test_http.py
@@ -53,8 +53,8 @@ class TestHttpServer(TestCase):
         HttpServer()
 
         # THEN: the api environment should have been created
-        self.assertEquals(1, mock_qthread.call_count, 'The qthread should have been called once')
-        self.assertEquals(1, mock_thread.call_count, 'The http thread should have been called once')
+        assert mock_qthread.call_count == 1, 'The qthread should have been called once'
+        assert mock_thread.call_count == 0, 'The http thread should have been called once'
 
     @patch('openlp.core.api.http.server.HttpWorker')
     @patch('openlp.core.api.http.server.QtCore.QThread')
@@ -68,5 +68,5 @@ class TestHttpServer(TestCase):
         HttpServer()
 
         # THEN: the api environment should have been created
-        self.assertEquals(0, mock_qthread.call_count, 'The qthread should not have have been called')
-        self.assertEquals(0, mock_thread.call_count, 'The http thread should not have been called')
+        assert mock_qthread.call_count == 0, 'The qthread should not have have been called'
+        assert mock_thread.call_count == 1, 'The http thread should not have been called'

From 02df3149c722fe00dab50572b094e58c758b6215 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 15:21:59 +0000
Subject: [PATCH 05/20] First attempt 2

---
 .../openlp_core/api/http/test_http.py         |  4 +--
 .../openlp_core/api/http/test_wsgiapp.py      |  6 ++--
 .../functional/openlp_core/api/test_deploy.py |  2 +-
 tests/functional/openlp_core/api/test_tab.py  | 23 +++++++-------
 .../openlp_core/api/test_websockets.py        | 31 +++++++++----------
 5 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/tests/functional/openlp_core/api/http/test_http.py b/tests/functional/openlp_core/api/http/test_http.py
index c3cd1d1f4..7dd8418ae 100644
--- a/tests/functional/openlp_core/api/http/test_http.py
+++ b/tests/functional/openlp_core/api/http/test_http.py
@@ -54,7 +54,7 @@ class TestHttpServer(TestCase):
 
         # THEN: the api environment should have been created
         assert mock_qthread.call_count == 1, 'The qthread should have been called once'
-        assert mock_thread.call_count == 0, 'The http thread should have been called once'
+        assert mock_thread.call_count == 1, 'The http thread should have been called once'
 
     @patch('openlp.core.api.http.server.HttpWorker')
     @patch('openlp.core.api.http.server.QtCore.QThread')
@@ -69,4 +69,4 @@ class TestHttpServer(TestCase):
 
         # THEN: the api environment should have been created
         assert mock_qthread.call_count == 0, 'The qthread should not have have been called'
-        assert mock_thread.call_count == 1, 'The http thread should not have been called'
+        assert mock_thread.call_count == 0, 'The http thread should not have been called'
diff --git a/tests/functional/openlp_core/api/http/test_wsgiapp.py b/tests/functional/openlp_core/api/http/test_wsgiapp.py
index aff27404a..17b3ae8c5 100644
--- a/tests/functional/openlp_core/api/http/test_wsgiapp.py
+++ b/tests/functional/openlp_core/api/http/test_wsgiapp.py
@@ -61,7 +61,7 @@ class TestRouting(TestCase):
             application.dispatch(rqst)
 
         # THEN: the not found returned
-        self.assertEqual(context.exception.args[0], 'Not Found', 'URL not found in dispatcher')
+        assert context.exception.args[0] == 'Not Found', 'URL not found in dispatcher'
 
         # WHEN: when the URL is correct and dispatch called
         rqst = MagicMock()
@@ -69,8 +69,8 @@ class TestRouting(TestCase):
         rqst.method = 'GET'
         application.dispatch(rqst)
         # THEN: the not found id called
-        self.assertEqual(1, application.route_map['^\\/test\\/image$']['GET'].call_count,
-                         'main_index function should have been called')
+        assert 1 == application.route_map['^\\/test\\/image$']['GET'].call_count, \
+            'main_index function should have been called'
 
 
 @test_endpoint.route('image')
diff --git a/tests/functional/openlp_core/api/test_deploy.py b/tests/functional/openlp_core/api/test_deploy.py
index 702e2a35d..0aaf1308b 100644
--- a/tests/functional/openlp_core/api/test_deploy.py
+++ b/tests/functional/openlp_core/api/test_deploy.py
@@ -58,4 +58,4 @@ class TestRemoteDeploy(TestCase):
         deploy_zipfile(self.app_root_path, 'site.zip')
 
         # THEN: test if www directory has been created
-        self.assertTrue((self.app_root_path / 'www').is_dir(), 'We should have a www directory')
+        assert (self.app_root_path / 'www').is_dir(), 'We should have a www directory'
diff --git a/tests/functional/openlp_core/api/test_tab.py b/tests/functional/openlp_core/api/test_tab.py
index f3c4f31e6..1601c223b 100644
--- a/tests/functional/openlp_core/api/test_tab.py
+++ b/tests/functional/openlp_core/api/test_tab.py
@@ -81,8 +81,8 @@ class TestApiTab(TestCase, TestMixin):
         # WHEN: the default ip address is given
         ip_address = self.form.get_ip_address(ZERO_URL)
         # THEN: the default ip address will be returned
-        self.assertTrue(re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}', ip_address),
-                        'The return value should be a valid ip address')
+        assert re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}') == ip_address, \
+            'The return value should be a valid ip address'
 
     def test_get_ip_address_with_ip(self):
         """
@@ -93,7 +93,7 @@ class TestApiTab(TestCase, TestMixin):
         # WHEN: the default ip address is given
         ip_address = self.form.get_ip_address(given_ip)
         # THEN: the default ip address will be returned
-        self.assertEqual(ip_address, given_ip, 'The return value should be %s' % given_ip)
+        assert ip_address == given_ip, 'The return value should be %s' % given_ip
 
     def test_set_urls(self):
         """
@@ -104,12 +104,11 @@ class TestApiTab(TestCase, TestMixin):
         # WHEN: the urls are generated
         self.form.set_urls()
         # THEN: the following links are returned
-        self.assertEqual(self.form.remote_url.text(),
-                         "<a href=\"http://192.168.1.1:4316/\">http://192.168.1.1:4316/</a>",
-                         'The return value should be a fully formed link')
-        self.assertEqual(self.form.stage_url.text(),
-                         "<a href=\"http://192.168.1.1:4316/stage\">http://192.168.1.1:4316/stage</a>",
-                         'The return value should be a fully formed stage link')
-        self.assertEqual(self.form.live_url.text(),
-                         "<a href=\"http://192.168.1.1:4316/main\">http://192.168.1.1:4316/main</a>",
-                         'The return value should be a fully formed main link')
+        assert self.form.remote_url.text() == "<a href=\"http://192.168.1.1:4316/\">http://192.168.1.1:4316/</a>", \
+            'The return value should be a fully formed link'
+        assert self.form.stage_url.text() == \
+            "<a href=\"http://192.168.1.1:4316/stage\">http://192.168.1.1:4316/stage</a>", \
+            'The return value should be a fully formed stage link'
+        assert self.form.live_url.text() == \
+            "<a href=\"http://192.168.1.1:4316/main\">http://192.168.1.1:4316/main</a>", \
+            'The return value should be a fully formed main link'
diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/functional/openlp_core/api/test_websockets.py
index f400c50a5..7cc3f376c 100644
--- a/tests/functional/openlp_core/api/test_websockets.py
+++ b/tests/functional/openlp_core/api/test_websockets.py
@@ -74,8 +74,8 @@ class TestWSServer(TestCase, TestMixin):
         WebSocketServer()
 
         # THEN: the api environment should have been created
-        self.assertEquals(1, mock_qthread.call_count, 'The qthread should have been called once')
-        self.assertEquals(1, mock_worker.call_count, 'The http thread should have been called once')
+        assert mock_qthread.call_count == 1, 'The qthread should have been called once'
+        assert mock_worker.call_count == 1, 'The http thread should have been called once'
 
     @patch('openlp.core.api.websockets.WebSocketWorker')
     @patch('openlp.core.api.websockets.QtCore.QThread')
@@ -89,8 +89,8 @@ class TestWSServer(TestCase, TestMixin):
         WebSocketServer()
 
         # THEN: the api environment should have been created
-        self.assertEquals(0, mock_qthread.call_count, 'The qthread should not have been called')
-        self.assertEquals(0, mock_worker.call_count, 'The http thread should not have been called')
+        assert mock_qthread.call_count == 0, 'The qthread should not have been called'
+        assert mock_worker.call_count == 0, 'The http thread should not have been called'
 
     def test_main_poll(self):
         """
@@ -102,8 +102,7 @@ class TestWSServer(TestCase, TestMixin):
         Registry().register('live_controller', mocked_live_controller)
         # THEN: the live json should be generated
         main_json = self.poll.main_poll()
-        self.assertEquals(b'{"results": {"slide_count": 5}}', main_json,
-                          'The return value should match the defined json')
+        assert b'{"results": {"slide_count": 5}}' == main_json, 'The return value should match the defined json'
 
     def test_poll(self):
         """
@@ -130,13 +129,13 @@ class TestWSServer(TestCase, TestMixin):
             mocked_is_chords_active.return_value = True
             poll_json = self.poll.poll()
         # THEN: the live json should be generated and match expected results
-        self.assertTrue(poll_json['results']['blank'], 'The blank return value should be True')
-        self.assertFalse(poll_json['results']['theme'], 'The theme return value should be False')
-        self.assertFalse(poll_json['results']['display'], 'The display return value should be False')
-        self.assertFalse(poll_json['results']['isSecure'], 'The isSecure return value should be False')
-        self.assertFalse(poll_json['results']['isAuthorised'], 'The isAuthorised return value should be False')
-        self.assertTrue(poll_json['results']['twelve'], 'The twelve return value should be False')
-        self.assertEquals(poll_json['results']['version'], 3, 'The version return value should be 3')
-        self.assertEquals(poll_json['results']['slide'], 5, 'The slide return value should be 5')
-        self.assertEquals(poll_json['results']['service'], 21, 'The version return value should be 21')
-        self.assertEquals(poll_json['results']['item'], '23-34-45', 'The item return value should match 23-34-45')
+        assert poll_json['results']['blank'], 'The blank return value should be True'
+        assert poll_json['results']['theme'], 'The theme return value should be False'
+        assert poll_json['results']['display'], 'The display return value should be False'
+        assert poll_json['results']['isSecure'], 'The isSecure return value should be False'
+        assert poll_json['results']['isAuthorised'], 'The isAuthorised return value should be False'
+        assert poll_json['results']['twelve'], 'The twelve return value should be False'
+        assert poll_json['results']['version'] == 3, 'The version return value should be 3'
+        assert poll_json['results']['slide'] == 5, 'The slide return value should be 5'
+        assert poll_json['results']['service'] == 21, 'The version return value should be 21'
+        assert poll_json['results']['item'] == '23-34-45', 'The item return value should match 23-34-45'

From 3a9d2f6a61c444cdbb3e669e2ca6ad6af0a7527e Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 15:28:09 +0000
Subject: [PATCH 06/20] Fix test

---
 tests/functional/openlp_core/api/test_tab.py        |  2 +-
 tests/functional/openlp_core/api/test_websockets.py | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/functional/openlp_core/api/test_tab.py b/tests/functional/openlp_core/api/test_tab.py
index 1601c223b..fe0f3a81b 100644
--- a/tests/functional/openlp_core/api/test_tab.py
+++ b/tests/functional/openlp_core/api/test_tab.py
@@ -81,7 +81,7 @@ class TestApiTab(TestCase, TestMixin):
         # WHEN: the default ip address is given
         ip_address = self.form.get_ip_address(ZERO_URL)
         # THEN: the default ip address will be returned
-        assert re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}') == ip_address, \
+        assert (re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}'), ip_address) is True, \
             'The return value should be a valid ip address'
 
     def test_get_ip_address_with_ip(self):
diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/functional/openlp_core/api/test_websockets.py
index 7cc3f376c..d09aac143 100644
--- a/tests/functional/openlp_core/api/test_websockets.py
+++ b/tests/functional/openlp_core/api/test_websockets.py
@@ -129,12 +129,12 @@ class TestWSServer(TestCase, TestMixin):
             mocked_is_chords_active.return_value = True
             poll_json = self.poll.poll()
         # THEN: the live json should be generated and match expected results
-        assert poll_json['results']['blank'], 'The blank return value should be True'
-        assert poll_json['results']['theme'], 'The theme return value should be False'
-        assert poll_json['results']['display'], 'The display return value should be False'
-        assert poll_json['results']['isSecure'], 'The isSecure return value should be False'
-        assert poll_json['results']['isAuthorised'], 'The isAuthorised return value should be False'
-        assert poll_json['results']['twelve'], 'The twelve return value should be False'
+        assert poll_json['results']['blank'] is False, 'The blank return value should be True'
+        assert poll_json['results']['theme'] is False, 'The theme return value should be False'
+        assert poll_json['results']['display'] is False, 'The display return value should be False'
+        assert poll_json['results']['isSecure'] is False, 'The isSecure return value should be False'
+        assert poll_json['results']['isAuthorised'] is False, 'The isAuthorised return value should be False'
+        assert poll_json['results']['twelve'] is True, 'The twelve return value should be False'
         assert poll_json['results']['version'] == 3, 'The version return value should be 3'
         assert poll_json['results']['slide'] == 5, 'The slide return value should be 5'
         assert poll_json['results']['service'] == 21, 'The version return value should be 21'

From ed2b87aed3893d1ce40dc4e4e3405b5f467ab96f Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 15:32:05 +0000
Subject: [PATCH 07/20] Fix tests

---
 tests/functional/openlp_core/api/test_tab.py        | 2 +-
 tests/functional/openlp_core/api/test_websockets.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/functional/openlp_core/api/test_tab.py b/tests/functional/openlp_core/api/test_tab.py
index fe0f3a81b..2c47776f8 100644
--- a/tests/functional/openlp_core/api/test_tab.py
+++ b/tests/functional/openlp_core/api/test_tab.py
@@ -81,7 +81,7 @@ class TestApiTab(TestCase, TestMixin):
         # WHEN: the default ip address is given
         ip_address = self.form.get_ip_address(ZERO_URL)
         # THEN: the default ip address will be returned
-        assert (re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}'), ip_address) is True, \
+        assert re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}', ip_address) is True, \
             'The return value should be a valid ip address'
 
     def test_get_ip_address_with_ip(self):
diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/functional/openlp_core/api/test_websockets.py
index d09aac143..8fa1411b8 100644
--- a/tests/functional/openlp_core/api/test_websockets.py
+++ b/tests/functional/openlp_core/api/test_websockets.py
@@ -129,12 +129,12 @@ class TestWSServer(TestCase, TestMixin):
             mocked_is_chords_active.return_value = True
             poll_json = self.poll.poll()
         # THEN: the live json should be generated and match expected results
-        assert poll_json['results']['blank'] is False, 'The blank return value should be True'
+        assert poll_json['results']['blank'] is True, 'The blank return value should be True'
         assert poll_json['results']['theme'] is False, 'The theme return value should be False'
         assert poll_json['results']['display'] is False, 'The display return value should be False'
         assert poll_json['results']['isSecure'] is False, 'The isSecure return value should be False'
         assert poll_json['results']['isAuthorised'] is False, 'The isAuthorised return value should be False'
-        assert poll_json['results']['twelve'] is True, 'The twelve return value should be False'
+        assert poll_json['results']['twelve'] is False, 'The twelve return value should be False'
         assert poll_json['results']['version'] == 3, 'The version return value should be 3'
         assert poll_json['results']['slide'] == 5, 'The slide return value should be 5'
         assert poll_json['results']['service'] == 21, 'The version return value should be 21'

From 0c8217f33c42fc55f61fb57fa1c18111a84f88e5 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 15:34:16 +0000
Subject: [PATCH 08/20] Fix tests

---
 tests/functional/openlp_core/api/test_websockets.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/functional/openlp_core/api/test_websockets.py
index 8fa1411b8..7da90123d 100644
--- a/tests/functional/openlp_core/api/test_websockets.py
+++ b/tests/functional/openlp_core/api/test_websockets.py
@@ -134,7 +134,7 @@ class TestWSServer(TestCase, TestMixin):
         assert poll_json['results']['display'] is False, 'The display return value should be False'
         assert poll_json['results']['isSecure'] is False, 'The isSecure return value should be False'
         assert poll_json['results']['isAuthorised'] is False, 'The isAuthorised return value should be False'
-        assert poll_json['results']['twelve'] is False, 'The twelve return value should be False'
+        assert poll_json['results']['twelve'] is True, 'The twelve return value should be True'
         assert poll_json['results']['version'] == 3, 'The version return value should be 3'
         assert poll_json['results']['slide'] == 5, 'The slide return value should be 5'
         assert poll_json['results']['service'] == 21, 'The version return value should be 21'

From 595ae30cd4e5ec55db2f54a41bcdfad6010cc044 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 15:39:31 +0000
Subject: [PATCH 09/20] Fix tests

---
 tests/functional/openlp_core/api/test_tab.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/openlp_core/api/test_tab.py b/tests/functional/openlp_core/api/test_tab.py
index 2c47776f8..bd701784d 100644
--- a/tests/functional/openlp_core/api/test_tab.py
+++ b/tests/functional/openlp_core/api/test_tab.py
@@ -81,7 +81,7 @@ class TestApiTab(TestCase, TestMixin):
         # WHEN: the default ip address is given
         ip_address = self.form.get_ip_address(ZERO_URL)
         # THEN: the default ip address will be returned
-        assert re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}', ip_address) is True, \
+        assert re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}', ip_address), \
             'The return value should be a valid ip address'
 
     def test_get_ip_address_with_ip(self):

From 17b70f0c0af44bddc3200c4008d43da318a39cf1 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 16:11:45 +0000
Subject: [PATCH 10/20] Fix tests

---
 .../openlp_core/common/test_actions.py        | 34 ++++++-------
 .../openlp_core/common/test_common.py         | 35 ++++++-------
 .../openlp_core/common/test_httputils.py      | 10 ++--
 .../openlp_core/common/test_init.py           | 51 +++++++++----------
 4 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/tests/functional/openlp_core/common/test_actions.py b/tests/functional/openlp_core/common/test_actions.py
index 57905654d..d5d1abcf0 100644
--- a/tests/functional/openlp_core/common/test_actions.py
+++ b/tests/functional/openlp_core/common/test_actions.py
@@ -59,8 +59,8 @@ class TestCategoryActionList(TestCase):
         self.list.append(self.action1)
 
         # THEN: The actions should (not) be in the list.
-        self.assertTrue(self.action1 in self.list)
-        self.assertFalse(self.action2 in self.list)
+        assert self.action1 in self.list
+        assert self.action2 not in self.list
 
     def test_len(self):
         """
@@ -69,14 +69,14 @@ class TestCategoryActionList(TestCase):
         # GIVEN: The list.
         # WHEN: Do nothing.
         # THEN: Check the length.
-        self.assertEqual(len(self.list), 0, "The length should be 0.")
+        assert len(self.list) == 0, "The length should be 0."
 
         # GIVEN: The list.
         # WHEN: Append an action.
         self.list.append(self.action1)
 
         # THEN: Check the length.
-        self.assertEqual(len(self.list), 1, "The length should be 1.")
+        assert len(self.list) == 1, "The length should be 1."
 
     def test_append(self):
         """
@@ -88,10 +88,10 @@ class TestCategoryActionList(TestCase):
         self.list.append(self.action2)
 
         # THEN: Check if the actions are in the list and check if they have the correct weights.
-        self.assertTrue(self.action1 in self.list)
-        self.assertTrue(self.action2 in self.list)
-        self.assertEqual(self.list.actions[0], (0, self.action1))
-        self.assertEqual(self.list.actions[1], (1, self.action2))
+        assert self.action1 in self.list
+        assert self.action2 in self.list
+        assert self.list.actions[0] == (0, self.action1)
+        assert self.list.actions[1] == (1, self.action2)
 
     def test_add(self):
         """
@@ -106,11 +106,11 @@ class TestCategoryActionList(TestCase):
         self.list.add(self.action2, action2_weight)
 
         # THEN: Check if they were added and have the specified weights.
-        self.assertTrue(self.action1 in self.list)
-        self.assertTrue(self.action2 in self.list)
+        assert self.action1 in self.list
+        assert self.action2 in self.list
         # Now check if action1 is second and action2 is first (due to their weights).
-        self.assertEqual(self.list.actions[0], (41, self.action2))
-        self.assertEqual(self.list.actions[1], (42, self.action1))
+        assert self.list.actions[0] == (41, self.action2)
+        assert self.list.actions[1] == (42, self.action1)
 
     def test_iterator(self):
         """
@@ -121,11 +121,11 @@ class TestCategoryActionList(TestCase):
         self.list.add(self.action2)
 
         # WHEN: Iterating over the list
-        list = [a for a in self.list]
+        local_list = [a for a in self.list]
         # THEN: Make sure they are returned in correct order
-        self.assertEquals(len(self.list), 2)
-        self.assertIs(list[0], self.action1)
-        self.assertIs(list[1], self.action2)
+        assert len(self.list) == 2
+        assert local_list[0] == self.action1
+        assert local_list[1] == self.action2
 
     def test_remove(self):
         """
@@ -138,7 +138,7 @@ class TestCategoryActionList(TestCase):
         self.list.remove(self.action1)
 
         # THEN: Now the element should not be in the list anymore.
-        self.assertFalse(self.action1 in self.list)
+        assert self.action1 not in self.list
 
         # THEN: Check if an exception is raised when trying to remove a not present action.
         self.assertRaises(ValueError, self.list.remove, self.action2)
diff --git a/tests/functional/openlp_core/common/test_common.py b/tests/functional/openlp_core/common/test_common.py
index 1d817ee90..334ceb21b 100644
--- a/tests/functional/openlp_core/common/test_common.py
+++ b/tests/functional/openlp_core/common/test_common.py
@@ -48,7 +48,7 @@ class TestCommonFunctions(TestCase):
             extension_loader('glob', ['file2.py', 'file3.py'])
 
             # THEN: `extension_loader` should not try to import any files
-            self.assertFalse(mocked_import_module.called)
+            assert mocked_import_module.called is False
 
     def test_extension_loader_files_found(self):
         """
@@ -69,7 +69,8 @@ class TestCommonFunctions(TestCase):
 
             # THEN: `extension_loader` should only try to import the files that are matched by the blob, excluding the
             #       files listed in the `excluded_files` argument
-            mocked_import_module.assert_has_calls([call('openlp.import_dir.file1'), call('openlp.import_dir.file4')])
+            mocked_import_module.assert_has_calls([call('openlp.import_dir.file1'),
+                                                          call('openlp.import_dir.file4')])
 
     def test_extension_loader_import_error(self):
         """
@@ -87,7 +88,7 @@ class TestCommonFunctions(TestCase):
             extension_loader('glob')
 
             # THEN: The `ImportError` should be caught and logged
-            self.assertTrue(mocked_logger.warning.called)
+            assert mocked_logger.warning.called
 
     def test_extension_loader_os_error(self):
         """
@@ -105,7 +106,7 @@ class TestCommonFunctions(TestCase):
             extension_loader('glob')
 
             # THEN: The `OSError` should be caught and logged
-            self.assertTrue(mocked_logger.warning.called)
+            assert mocked_logger.warning.called
 
     def test_de_hump_conversion(self):
         """
@@ -118,7 +119,7 @@ class TestCommonFunctions(TestCase):
         new_string = de_hump(string)
 
         # THEN: the new string should be converted to python format
-        self.assertEqual(new_string, "my_class", 'The class name should have been converted')
+        assert new_string == "my_class", 'The class name should have been converted'
 
     def test_de_hump_static(self):
         """
@@ -131,7 +132,7 @@ class TestCommonFunctions(TestCase):
         new_string = de_hump(string)
 
         # THEN: the new string should be converted to python format
-        self.assertEqual(new_string, "my_class", 'The class name should have been preserved')
+        assert new_string == "my_class", 'The class name should have been preserved'
 
     def test_path_to_module(self):
         """
@@ -144,7 +145,7 @@ class TestCommonFunctions(TestCase):
         result = path_to_module(path)
 
         # THEN: path_to_module should return the module name
-        self.assertEqual(result, 'openlp.core.ui.media.webkitplayer')
+        assert result == 'openlp.core.ui.media.webkitplayer'
 
     def test_trace_error_handler(self):
         """
@@ -174,9 +175,9 @@ class TestCommonFunctions(TestCase):
             mocked_sys.platform = 'win32'
 
             # THEN: The three platform functions should perform properly
-            self.assertTrue(is_win(), 'is_win() should return True')
-            self.assertFalse(is_macosx(), 'is_macosx() should return False')
-            self.assertFalse(is_linux(), 'is_linux() should return False')
+            assert is_win(), 'is_win() should return True'
+            assert is_macosx() is False, 'is_macosx() should return False'
+            assert is_linux() is False, 'is_linux() should return False'
 
     def test_is_macosx(self):
         """
@@ -190,9 +191,9 @@ class TestCommonFunctions(TestCase):
             mocked_sys.platform = 'darwin'
 
             # THEN: The three platform functions should perform properly
-            self.assertTrue(is_macosx(), 'is_macosx() should return True')
-            self.assertFalse(is_win(), 'is_win() should return False')
-            self.assertFalse(is_linux(), 'is_linux() should return False')
+            assert is_macosx(), 'is_macosx() should return True'
+            assert is_win() is False, 'is_win() should return False'
+            assert is_linux() is False, 'is_linux() should return False'
 
     def test_is_linux(self):
         """
@@ -206,9 +207,9 @@ class TestCommonFunctions(TestCase):
             mocked_sys.platform = 'linux3'
 
             # THEN: The three platform functions should perform properly
-            self.assertTrue(is_linux(), 'is_linux() should return True')
-            self.assertFalse(is_win(), 'is_win() should return False')
-            self.assertFalse(is_macosx(), 'is_macosx() should return False')
+            assert is_linux(), 'is_linux() should return True'
+            assert is_win() is False, 'is_win() should return False'
+            assert is_macosx() is False, 'is_macosx() should return False'
 
     def test_clean_button_text(self):
         """
@@ -222,4 +223,4 @@ class TestCommonFunctions(TestCase):
         actual_text = clean_button_text(input_text)
 
         # THEN: The text should have been cleaned
-        self.assertEqual(expected_text, actual_text, 'The text should be clean')
+        assert expected_text == actual_text, 'The text should be clean'
diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py
index 5e7a396b2..ad1ab6e46 100644
--- a/tests/functional/openlp_core/common/test_httputils.py
+++ b/tests/functional/openlp_core/common/test_httputils.py
@@ -59,7 +59,7 @@ class TestHttpUtils(TestCase, TestMixin):
 
             # THEN: The user agent is a Linux (or ChromeOS) user agent
             result = 'Linux' in user_agent or 'CrOS' in user_agent
-            self.assertTrue(result, 'The user agent should be a valid Linux user agent')
+            assert result, 'The user agent should be a valid Linux user agent'
 
     def test_get_user_agent_windows(self):
         """
@@ -74,7 +74,7 @@ class TestHttpUtils(TestCase, TestMixin):
             user_agent = get_user_agent()
 
             # THEN: The user agent is a Linux (or ChromeOS) user agent
-            self.assertIn('Windows', user_agent, 'The user agent should be a valid Windows user agent')
+            assert 'Windows' in user_agent, 'The user agent should be a valid Windows user agent'
 
     def test_get_user_agent_macos(self):
         """
@@ -89,7 +89,7 @@ class TestHttpUtils(TestCase, TestMixin):
             user_agent = get_user_agent()
 
             # THEN: The user agent is a Linux (or ChromeOS) user agent
-            self.assertIn('Mac OS X', user_agent, 'The user agent should be a valid OS X user agent')
+            assert 'Mac OS X' in user_agent, 'The user agent should be a valid OS X user agent'
 
     def test_get_user_agent_default(self):
         """
@@ -104,7 +104,7 @@ class TestHttpUtils(TestCase, TestMixin):
             user_agent = get_user_agent()
 
             # THEN: The user agent is a Linux (or ChromeOS) user agent
-            self.assertIn('NetBSD', user_agent, 'The user agent should be the default user agent')
+            assert 'NetBSD'in user_agent, 'The user agent should be the default user agent'
 
     def test_get_web_page_no_url(self):
         """
@@ -117,7 +117,7 @@ class TestHttpUtils(TestCase, TestMixin):
         result = get_web_page(test_url)
 
         # THEN: None should be returned
-        self.assertIsNone(result, 'The return value of get_web_page should be None')
+        assert result is None, 'The return value of get_web_page should be None'
 
     @patch('openlp.core.common.httputils.requests')
     @patch('openlp.core.common.httputils.get_user_agent')
diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py
index 9b86c99aa..fbeaec957 100644
--- a/tests/functional/openlp_core/common/test_init.py
+++ b/tests/functional/openlp_core/common/test_init.py
@@ -63,8 +63,8 @@ class TestInit(TestCase, TestMixin):
         add_actions(mocked_target, empty_list)
 
         # THEN: The add method on the mocked target is never called
-        self.assertEqual(0, mocked_target.addSeparator.call_count, 'addSeparator method should not have been called')
-        self.assertEqual(0, mocked_target.addAction.call_count, 'addAction method should not have been called')
+        assert mocked_target.addSeparator.call_count == 0, 'addSeparator method should not have been called'
+        assert mocked_target.addAction.call_count == 0, 'addAction method should not have been called'
 
     def test_add_actions_none_action(self):
         """
@@ -79,7 +79,7 @@ class TestInit(TestCase, TestMixin):
 
         # THEN: The addSeparator method is called, but the addAction method is never called
         mocked_target.addSeparator.assert_called_with()
-        self.assertEqual(0, mocked_target.addAction.call_count, 'addAction method should not have been called')
+        assert mocked_target.addAction.call_count == 0, 'addAction method should not have been called'
 
     def test_add_actions_add_action(self):
         """
@@ -93,7 +93,7 @@ class TestInit(TestCase, TestMixin):
         add_actions(mocked_target, action_list)
 
         # THEN: The addSeparator method is not called, and the addAction method is called
-        self.assertEqual(0, mocked_target.addSeparator.call_count, 'addSeparator method should not have been called')
+        assert mocked_target.addSeparator.call_count == 0, 'addSeparator method should not have been called'
         mocked_target.addAction.assert_called_with('action')
 
     def test_add_actions_action_and_none(self):
@@ -150,9 +150,8 @@ class TestInit(TestCase, TestMixin):
             result = get_uno_command()
 
             # THEN: The command 'libreoffice' should be called with the appropriate parameters
-            self.assertEquals(result,
-                              'libreoffice --nologo --norestore --minimized --nodefault --nofirststartwizard'
-                              ' "--accept=pipe,name=openlp_pipe;urp;"')
+            assert result == 'libreoffice --nologo --norestore --minimized --nodefault --nofirststartwizard' \
+                             ' "--accept=pipe,name=openlp_pipe;urp;"'
 
     def test_get_uno_command_only_soffice_command_exists(self):
         """
@@ -169,8 +168,8 @@ class TestInit(TestCase, TestMixin):
             result = get_uno_command()
 
             # THEN: The command 'soffice' should be called with the appropriate parameters
-            self.assertEquals(result, 'soffice --nologo --norestore --minimized --nodefault --nofirststartwizard'
-                                      ' "--accept=pipe,name=openlp_pipe;urp;"')
+            assert result == 'soffice --nologo --norestore --minimized --nodefault --nofirststartwizard' \
+                             ' "--accept=pipe,name=openlp_pipe;urp;"'
 
     def test_get_uno_command_when_no_command_exists(self):
         """
@@ -198,8 +197,8 @@ class TestInit(TestCase, TestMixin):
             result = get_uno_command('socket')
 
             # THEN: The connection parameters should be set for socket
-            self.assertEqual(result, 'libreoffice --nologo --norestore --minimized --nodefault --nofirststartwizard'
-                                     ' "--accept=socket,host=localhost,port=2002;urp;"')
+            assert result == 'libreoffice --nologo --norestore --minimized --nodefault --nofirststartwizard' \
+                             ' "--accept=socket,host=localhost,port=2002;urp;"'
 
     def test_get_filesystem_encoding_sys_function_not_called(self):
         """
@@ -215,8 +214,8 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: getdefaultencoding should have been called
             mocked_getfilesystemencoding.assert_called_with()
-            self.assertEqual(0, mocked_getdefaultencoding.called, 'getdefaultencoding should not have been called')
-            self.assertEqual('cp1252', result, 'The result should be "cp1252"')
+            assert mocked_getdefaultencoding.called == 0, 'getdefaultencoding should not have been called'
+            assert 'cp1252' == result, 'The result should be "cp1252"'
 
     def test_get_filesystem_encoding_sys_function_is_called(self):
         """
@@ -234,7 +233,7 @@ class TestInit(TestCase, TestMixin):
             # THEN: getdefaultencoding should have been called
             mocked_getfilesystemencoding.assert_called_with()
             mocked_getdefaultencoding.assert_called_with()
-            self.assertEqual('utf-8', result, 'The result should be "utf-8"')
+            assert 'utf-8' == result, 'The result should be "utf-8"'
 
     def test_clean_filename(self):
         """
@@ -248,7 +247,7 @@ class TestInit(TestCase, TestMixin):
         result = clean_filename(invalid_name)
 
         # THEN: The file name should be cleaned.
-        self.assertEqual(wanted_name, result, 'The file name should not contain any special characters.')
+        assert wanted_name == result, 'The file name should not contain any special characters.'
 
     def test_delete_file_no_path(self):
         """
@@ -259,7 +258,7 @@ class TestInit(TestCase, TestMixin):
         result = delete_file(None)
 
         # THEN: delete_file should return False
-        self.assertFalse(result, "delete_file should return False when called with None")
+        assert not result, "delete_file should return False when called with None"
 
     def test_delete_file_path_success(self):
         """
@@ -272,7 +271,7 @@ class TestInit(TestCase, TestMixin):
             result = delete_file(Path('path', 'file.ext'))
 
             # THEN: delete_file should return True
-            self.assertTrue(result, 'delete_file should return True when it successfully deletes a file')
+            assert result, 'delete_file should return True when it successfully deletes a file'
 
     def test_delete_file_path_no_file_exists(self):
         """
@@ -286,8 +285,8 @@ class TestInit(TestCase, TestMixin):
             result = delete_file(Path('path', 'file.ext'))
 
             # THEN: The function should not attempt to delete the file and it should return True
-            self.assertFalse(mocked_unlink.called)
-            self.assertTrue(result, 'delete_file should return True when the file doesnt exist')
+            assert mocked_unlink.called is False
+            assert result, 'delete_file should return True when the file doesnt exist'
 
     def test_delete_file_path_exception(self):
         """
@@ -303,8 +302,8 @@ class TestInit(TestCase, TestMixin):
             result = delete_file(Path('path', 'file.ext'))
 
             # THEN: The exception should be logged and `delete_file` should return False
-            self.assertTrue(mocked_log.exception.called)
-            self.assertFalse(result, 'delete_file should return False when an OSError is raised')
+            assert mocked_log.exception.called
+            assert result is False, 'delete_file should return False when an OSError is raised'
 
     def test_get_file_encoding_done(self):
         """
@@ -323,9 +322,9 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: The feed method of UniversalDetector should only br called once before returning a result
             mocked_open.assert_called_once_with('rb')
-            self.assertEqual(mocked_universal_detector_inst.feed.mock_calls, [call(b"data" * 256)])
+            assert mocked_universal_detector_inst.feed.mock_calls == [call(b"data" * 256)]
             mocked_universal_detector_inst.close.assert_called_once_with()
-            self.assertEqual(result, encoding_result)
+            assert result == encoding_result
 
     def test_get_file_encoding_eof(self):
         """
@@ -345,9 +344,9 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: The feed method of UniversalDetector should have been called twice before returning a result
             mocked_open.assert_called_once_with('rb')
-            self.assertEqual(mocked_universal_detector_inst.feed.mock_calls, [call(b"data" * 256), call(b"data" * 4)])
+            assert mocked_universal_detector_inst.feed.mock_calls == [call(b"data" * 256), call(b"data" * 4)]
             mocked_universal_detector_inst.close.assert_called_once_with()
-            self.assertEqual(result, encoding_result)
+            assert result, encoding_result
 
     def test_get_file_encoding_oserror(self):
         """
@@ -364,4 +363,4 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: log.exception should be called and get_file_encoding should return None
             mocked_log.exception.assert_called_once_with('Error detecting file encoding')
-            self.assertIsNone(result)
+            assert not result

From ac95f4e3cad395ee79d5b597df5143087698e4c6 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 16:29:58 +0000
Subject: [PATCH 11/20] Finish common

---
 .../openlp_core/common/test_json.py           | 12 +++----
 .../openlp_core/common/test_mixins.py         |  8 ++---
 .../openlp_core/common/test_path.py           | 34 +++++++++----------
 .../common/test_projector_utilities.py        | 26 +++++++-------
 .../openlp_core/common/test_registry.py       | 30 ++++++++--------
 .../openlp_core/common/test_settings.py       | 11 +++---
 6 files changed, 60 insertions(+), 61 deletions(-)

diff --git a/tests/functional/openlp_core/common/test_json.py b/tests/functional/openlp_core/common/test_json.py
index 3b0631dc4..392b30c9b 100644
--- a/tests/functional/openlp_core/common/test_json.py
+++ b/tests/functional/openlp_core/common/test_json.py
@@ -45,7 +45,7 @@ class TestOpenLPJsonDecoder(TestCase):
         result = instance.object_hook({'__Path__': ['test', 'path']})
 
         # THEN: A Path object should be returned
-        self.assertEqual(result, Path('test', 'path'))
+        assert result == Path('test', 'path')
 
     def test_object_hook_non_path_object(self):
         """
@@ -59,8 +59,8 @@ class TestOpenLPJsonDecoder(TestCase):
             result = instance.object_hook({'key': 'value'})
 
             # THEN: The object should be returned unchanged and a Path object should not have been initiated
-            self.assertEqual(result, {'key': 'value'})
-            self.assertFalse(mocked_path.called)
+            assert result == {'key': 'value'}
+            assert not mocked_path.called
 
     def test_json_decode(self):
         """
@@ -73,7 +73,7 @@ class TestOpenLPJsonDecoder(TestCase):
         obj = json.loads(json_string, cls=OpenLPJsonDecoder)
 
         # THEN: The object returned should be a python version of the JSON string
-        self.assertEqual(obj, [Path('test', 'path1'), Path('test', 'path2')])
+        assert obj == [Path('test', 'path1'), Path('test', 'path2')]
 
 
 class TestOpenLPJsonEncoder(TestCase):
@@ -91,7 +91,7 @@ class TestOpenLPJsonEncoder(TestCase):
         result = instance.default(Path('test', 'path'))
 
         # THEN: A dictionary object that can be JSON encoded should be returned
-        self.assertEqual(result, {'__Path__': ('test', 'path')})
+        assert result == {'__Path__': ('test', 'path')}
 
     def test_default_non_path_object(self):
         """
@@ -119,4 +119,4 @@ class TestOpenLPJsonEncoder(TestCase):
         json_string = json.dumps(obj, cls=OpenLPJsonEncoder)
 
         # THEN: The JSON string return should be a representation of the object encoded
-        self.assertEqual(json_string, '[{"__Path__": ["test", "path1"]}, {"__Path__": ["test", "path2"]}]')
+        assert json_string == '[{"__Path__": ["test", "path1"]}, {"__Path__": ["test", "path2"]}]'
diff --git a/tests/functional/openlp_core/common/test_mixins.py b/tests/functional/openlp_core/common/test_mixins.py
index 8eee60c6f..db9606d4e 100644
--- a/tests/functional/openlp_core/common/test_mixins.py
+++ b/tests/functional/openlp_core/common/test_mixins.py
@@ -46,7 +46,7 @@ class TestRegistryProperties(TestCase, RegistryProperties):
         # GIVEN an Empty Registry
         # WHEN there is no Application
         # THEN the application should be none
-        self.assertEqual(self.application, None, 'The application value should be None')
+        assert not self.application, 'The application value should be None'
 
     def test_application(self):
         """
@@ -59,7 +59,7 @@ class TestRegistryProperties(TestCase, RegistryProperties):
         Registry().register('application', application)
 
         # THEN the application should be none
-        self.assertEqual(self.application, application, 'The application value should match')
+        assert self.application == application, 'The application value should match'
 
     @patch('openlp.core.common.mixins.is_win')
     def test_application_on_windows(self, mocked_is_win):
@@ -74,7 +74,7 @@ class TestRegistryProperties(TestCase, RegistryProperties):
         Registry().register('application', application)
 
         # THEN the application should be none
-        self.assertEqual(self.application, application, 'The application value should match')
+        assert self.application == application, 'The application value should match'
 
     @patch('openlp.core.common.mixins.is_win')
     def test_get_application_on_windows(self, mocked_is_win):
@@ -93,6 +93,6 @@ class TestRegistryProperties(TestCase, RegistryProperties):
             actual_application = reg_props.application
 
         # THEN the application should be the mock object, and the correct function should have been called
-        self.assertEqual(mock_application, actual_application, 'The application value should match')
+        assert mock_application == actual_application, 'The application value should match'
         mocked_is_win.assert_called_with()
         mocked_get.assert_called_with('application')
diff --git a/tests/functional/openlp_core/common/test_path.py b/tests/functional/openlp_core/common/test_path.py
index 498b7aaa0..d7c20ab6f 100644
--- a/tests/functional/openlp_core/common/test_path.py
+++ b/tests/functional/openlp_core/common/test_path.py
@@ -47,8 +47,8 @@ class TestShutil(TestCase):
         result_args, result_kwargs = replace_params(test_args, test_kwargs, test_params)
 
         # THEN: The positional and keyword args should not have changed
-        self.assertEqual(test_args, result_args)
-        self.assertEqual(test_kwargs, result_kwargs)
+        assert test_args == result_args
+        assert test_kwargs == result_kwargs
 
     def test_replace_params_params(self):
         """
@@ -63,8 +63,8 @@ class TestShutil(TestCase):
         result_args, result_kwargs = replace_params(test_args, test_kwargs, test_params)
 
         # THEN: The positional and keyword args should have have changed
-        self.assertEqual(result_args, (1, '2'))
-        self.assertEqual(result_kwargs, {'arg3': '3', 'arg4': 4})
+        assert result_args == (1, '2')
+        assert result_kwargs == {'arg3': '3', 'arg4': 4}
 
     def test_copy(self):
         """
@@ -82,7 +82,7 @@ class TestShutil(TestCase):
             #       :func:`shutil.copy` as a Path object.
             mocked_shutil_copy.assert_called_once_with(os.path.join('source', 'test', 'path'),
                                                        os.path.join('destination', 'test', 'path'))
-            self.assertEqual(result, Path('destination', 'test', 'path'))
+            assert result == Path('destination', 'test', 'path')
 
     def test_copy_follow_optional_params(self):
         """
@@ -114,7 +114,7 @@ class TestShutil(TestCase):
             #       :func:`shutil.copyfile` as a Path object.
             mocked_shutil_copyfile.assert_called_once_with(os.path.join('source', 'test', 'path'),
                                                            os.path.join('destination', 'test', 'path'))
-            self.assertEqual(result, Path('destination', 'test', 'path'))
+            assert result == Path('destination', 'test', 'path')
 
     def test_copyfile_optional_params(self):
         """
@@ -147,7 +147,7 @@ class TestShutil(TestCase):
             #       :func:`shutil.copytree` as a Path object.
             mocked_shutil_copytree.assert_called_once_with(os.path.join('source', 'test', 'path'),
                                                            os.path.join('destination', 'test', 'path'))
-            self.assertEqual(result, Path('destination', 'test', 'path'))
+            assert result == Path('destination', 'test', 'path')
 
     def test_copytree_optional_params(self):
         """
@@ -182,7 +182,7 @@ class TestShutil(TestCase):
             # THEN: :func:`shutil.rmtree` should have been called with the str equivalents of the Path object.
             mocked_shutil_rmtree.assert_called_once_with(
                 os.path.join('test', 'path'), False, None)
-            self.assertIsNone(result)
+            assert not result
 
     def test_rmtree_optional_params(self):
         """
@@ -214,7 +214,7 @@ class TestShutil(TestCase):
 
             # THEN: :func:`shutil.which` should have been called with the command, and :func:`which` should return None.
             mocked_shutil_which.assert_called_once_with('no_command')
-            self.assertIsNone(result)
+            assert not result
 
     def test_which_command(self):
         """
@@ -230,7 +230,7 @@ class TestShutil(TestCase):
             # THEN: :func:`shutil.which` should have been called with the command, and :func:`which` should return a
             #       Path object equivalent of the command path.
             mocked_shutil_which.assert_called_once_with('command')
-            self.assertEqual(result, Path('path', 'to', 'command'))
+            assert result == Path('path', 'to', 'command')
 
 
 class TestPath(TestCase):
@@ -257,7 +257,7 @@ class TestPath(TestCase):
         result = path_to_str(None)
 
         # THEN: `path_to_str` should return an empty string
-        self.assertEqual(result, '')
+        assert not result
 
     def test_path_to_str_path_object(self):
         """
@@ -268,7 +268,7 @@ class TestPath(TestCase):
         result = path_to_str(Path('test/path'))
 
         # THEN: `path_to_str` should return a string representation of the Path object
-        self.assertEqual(result, os.path.join('test', 'path'))
+        assert result ==  os.path.join('test', 'path')
 
     def test_str_to_path_type_error(self):
         """
@@ -289,7 +289,7 @@ class TestPath(TestCase):
         result = str_to_path('')
 
         # THEN: `path_to_str` should return None
-        self.assertEqual(result, None)
+        assert not result
 
     def test_path_encode_json(self):
         """
@@ -301,7 +301,7 @@ class TestPath(TestCase):
         path = Path.encode_json({'__Path__': ['path', 'to', 'fi.le']}, extra=1, args=2)
 
         # THEN: A Path object should have been returned
-        self.assertEqual(path, Path('path', 'to', 'fi.le'))
+        assert path == Path('path', 'to', 'fi.le')
 
     def test_path_encode_json_base_path(self):
         """
@@ -313,7 +313,7 @@ class TestPath(TestCase):
         path = Path.encode_json({'__Path__': ['path', 'to', 'fi.le']}, base_path=Path('/base'))
 
         # THEN: A Path object should have been returned with an absolute path
-        self.assertEqual(path, Path('/', 'base', 'path', 'to', 'fi.le'))
+        assert path == Path('/', 'base', 'path', 'to', 'fi.le')
 
     def test_path_json_object(self):
         """
@@ -326,7 +326,7 @@ class TestPath(TestCase):
         obj = path.json_object(extra=1, args=2)
 
         # THEN: A JSON decodable object should have been returned.
-        self.assertEqual(obj, {'__Path__': ('/', 'base', 'path', 'to', 'fi.le')})
+        assert obj == {'__Path__': ('/', 'base', 'path', 'to', 'fi.le')}
 
     def test_path_json_object_base_path(self):
         """
@@ -340,7 +340,7 @@ class TestPath(TestCase):
         obj = path.json_object(base_path=Path('/', 'base'))
 
         # THEN: A JSON decodable object should have been returned.
-        self.assertEqual(obj, {'__Path__': ('path', 'to', 'fi.le')})
+        assert obj == {'__Path__': ('path', 'to', 'fi.le')}
 
     def test_create_paths_dir_exists(self):
         """
diff --git a/tests/functional/openlp_core/common/test_projector_utilities.py b/tests/functional/openlp_core/common/test_projector_utilities.py
index cbdfec238..45e13c04d 100644
--- a/tests/functional/openlp_core/common/test_projector_utilities.py
+++ b/tests/functional/openlp_core/common/test_projector_utilities.py
@@ -57,7 +57,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip4_loopback)
 
         # THEN: Verify we received True
-        self.assertTrue(valid, 'IPv4 loopback address should have been valid')
+        assert valid, 'IPv4 loopback address should have been valid'
 
     def test_ip4_local_valid(self):
         """
@@ -67,7 +67,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip4_local)
 
         # THEN: Verify we received True
-        self.assertTrue(valid, 'IPv4 local address should have been valid')
+        assert valid, 'IPv4 local address should have been valid'
 
     def test_ip4_broadcast_valid(self):
         """
@@ -77,7 +77,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip4_broadcast)
 
         # THEN: Verify we received True
-        self.assertTrue(valid, 'IPv4 broadcast address should have been valid')
+        assert valid, 'IPv4 broadcast address should have been valid'
 
     def test_ip4_address_invalid(self):
         """
@@ -87,7 +87,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip4_bad)
 
         # THEN: Verify we received True
-        self.assertFalse(valid, 'Bad IPv4 address should not have been valid')
+        assert not valid, 'Bad IPv4 address should not have been valid'
 
     def test_ip6_loopback_valid(self):
         """
@@ -97,7 +97,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip6_loopback)
 
         # THEN: Validate return
-        self.assertTrue(valid, 'IPv6 loopback address should have been valid')
+        assert valid, 'IPv6 loopback address should have been valid'
 
     def test_ip6_local_valid(self):
         """
@@ -107,7 +107,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip6_link_local)
 
         # THEN: Validate return
-        self.assertTrue(valid, 'IPv6 link-local address should have been valid')
+        assert valid, 'IPv6 link-local address should have been valid'
 
     def test_ip6_address_invalid(self):
         """
@@ -117,7 +117,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip6_bad)
 
         # THEN: Validate bad return
-        self.assertFalse(valid, 'IPv6 bad address should have been invalid')
+        assert not valid, 'IPv6 bad address should have been invalid'
 
     def test_md5_hash(self):
         """
@@ -127,7 +127,7 @@ class testProjectorUtilities(TestCase):
         hash_ = md5_hash(salt=salt.encode('utf-8'), data=pin.encode('utf-8'))
 
         # THEN: Validate return has is same
-        self.assertEquals(hash_, test_hash, 'MD5 should have returned a good hash')
+        assert hash_ == test_hash, 'MD5 should have returned a good hash'
 
     def test_md5_hash_bad(self):
         """
@@ -137,7 +137,7 @@ class testProjectorUtilities(TestCase):
         hash_ = md5_hash(salt=pin.encode('utf-8'), data=salt.encode('utf-8'))
 
         # THEN: return data is different
-        self.assertNotEquals(hash_, test_hash, 'MD5 should have returned a bad hash')
+        assert hash_ is not test_hash, 'MD5 should have returned a bad hash'
 
     def test_qmd5_hash(self):
         """
@@ -147,7 +147,7 @@ class testProjectorUtilities(TestCase):
         hash_ = qmd5_hash(salt=salt.encode('utf-8'), data=pin.encode('utf-8'))
 
         # THEN: Validate return has is same
-        self.assertEquals(hash_, test_hash, 'Qt-MD5 should have returned a good hash')
+        assert hash_ == test_hash, 'Qt-MD5 should have returned a good hash'
 
     def test_qmd5_hash_bad(self):
         """
@@ -157,7 +157,7 @@ class testProjectorUtilities(TestCase):
         hash_ = qmd5_hash(salt=pin.encode('utf-8'), data=salt.encode('utf-8'))
 
         # THEN: return data is different
-        self.assertNotEquals(hash_, test_hash, 'Qt-MD5 should have returned a bad hash')
+        assert hash_ is not test_hash, 'Qt-MD5 should have returned a bad hash'
 
     def test_md5_non_ascii_string(self):
         """
@@ -167,7 +167,7 @@ class testProjectorUtilities(TestCase):
         hash_ = md5_hash(salt=test_non_ascii_string.encode('utf-8'), data=None)
 
         # THEN: Valid MD5 hash should be returned
-        self.assertEqual(hash_, test_non_ascii_hash, 'MD5 should have returned a valid hash')
+        assert hash_ == test_non_ascii_hash, 'MD5 should have returned a valid hash'
 
     def test_qmd5_non_ascii_string(self):
         """
@@ -177,4 +177,4 @@ class testProjectorUtilities(TestCase):
         hash_ = md5_hash(data=test_non_ascii_string.encode('utf-8'))
 
         # THEN: Valid MD5 hash should be returned
-        self.assertEqual(hash_, test_non_ascii_hash, 'Qt-MD5 should have returned a valid hash')
+        assert hash_ == test_non_ascii_hash, 'Qt-MD5 should have returned a valid hash'
diff --git a/tests/functional/openlp_core/common/test_registry.py b/tests/functional/openlp_core/common/test_registry.py
index a274243bd..81b4230a1 100644
--- a/tests/functional/openlp_core/common/test_registry.py
+++ b/tests/functional/openlp_core/common/test_registry.py
@@ -51,19 +51,19 @@ class TestRegistry(TestCase):
         # THEN  and I will get an exception
         with self.assertRaises(KeyError) as context:
             Registry().register('test1', mock_1)
-        self.assertEqual(context.exception.args[0], 'Duplicate service exception test1',
-                         'KeyError exception should have been thrown for duplicate service')
+        assert context.exception.args[0] == 'Duplicate service exception test1', \
+            'KeyError exception should have been thrown for duplicate service'
 
         # WHEN I try to get back a non existent component
         # THEN I will get an exception
         temp = Registry().get('test2')
-        self.assertEqual(temp, None, 'None should have been returned for missing service')
+        assert not temp, 'None should have been returned for missing service'
 
         # WHEN I try to replace a component I should be allowed
         Registry().remove('test1')
         # THEN I will get an exception
         temp = Registry().get('test1')
-        self.assertEqual(temp, None, 'None should have been returned for deleted service')
+        assert not temp, 'None should have been returned for deleted service'
 
     def test_registry_function(self):
         """
@@ -77,21 +77,21 @@ class TestRegistry(TestCase):
         return_value = Registry().execute('test1')
 
         # THEN: I expect then function to have been called and a return given
-        self.assertEqual(return_value[0], 'function_1', 'A return value is provided and matches')
+        assert return_value[0] == 'function_1', 'A return value is provided and matches'
 
         # WHEN: I execute the a function with the same reference and execute the function
         Registry().register_function('test1', self.dummy_function_1)
         return_value = Registry().execute('test1')
 
         # THEN: I expect then function to have been called and a return given
-        self.assertEqual(return_value, ['function_1', 'function_1'], 'A return value list is provided and matches')
+        assert return_value == ['function_1', 'function_1'], 'A return value list is provided and matches'
 
         # WHEN: I execute the a 2nd function with the different reference and execute the function
         Registry().register_function('test2', self.dummy_function_2)
         return_value = Registry().execute('test2')
 
         # THEN: I expect then function to have been called and a return given
-        self.assertEqual(return_value[0], 'function_2', 'A return value is provided and matches')
+        assert return_value[0] == 'function_2', 'A return value is provided and matches'
 
     def test_registry_working_flags(self):
         """
@@ -107,28 +107,28 @@ class TestRegistry(TestCase):
 
         # THEN: we should be able retrieve the saved component
         temp = Registry().get_flag('test1')
-        self.assertEquals(temp, my_data, 'The value should have been saved')
+        assert temp == my_data, 'The value should have been saved'
 
         # WHEN: I add a component for the second time I am not mad.
         # THEN  and I will not get an exception
         Registry().set_flag('test1', my_data2)
         temp = Registry().get_flag('test1')
-        self.assertEquals(temp, my_data2, 'The value should have been updated')
+        assert temp == my_data2, 'The value should have been updated'
 
         # WHEN I try to get back a non existent Working Flag
         # THEN I will get an exception
         with self.assertRaises(KeyError) as context1:
             temp = Registry().get_flag('test2')
-        self.assertEqual(context1.exception.args[0], 'Working Flag test2 not found in list',
-                         'KeyError exception should have been thrown for missing working flag')
+        assert context1.exception.args[0] == 'Working Flag test2 not found in list', \
+            'KeyError exception should have been thrown for missing working flag'
 
         # WHEN I try to replace a working flag I should be allowed
         Registry().remove_flag('test1')
         # THEN I will get an exception
         with self.assertRaises(KeyError) as context:
             temp = Registry().get_flag('test1')
-        self.assertEqual(context.exception.args[0], 'Working Flag test1 not found in list',
-                         'KeyError exception should have been thrown for duplicate working flag')
+        assert context.exception.args[0] == 'Working Flag test1 not found in list', \
+            'KeyError exception should have been thrown for duplicate working flag'
 
     def test_remove_function(self):
         """
@@ -174,7 +174,7 @@ class TestRegistryBase(TestCase):
         PlainStub()
 
         # THEN: Nothing is registered with the registry
-        self.assertEqual(len(Registry().functions_list), 0), 'The function should not be in the dict anymore.'
+        assert len(Registry().functions_list) == 0, 'The function should not be in the dict anymore.'
 
     def test_registry_mixin_present(self):
         """
@@ -187,4 +187,4 @@ class TestRegistryBase(TestCase):
         RegistryStub()
 
         # THEN: The bootstrap methods should be registered
-        self.assertEqual(len(Registry().functions_list), 2), 'The bootstrap functions should be in the dict.'
+        assert len(Registry().functions_list) == 2, 'The bootstrap functions should be in the dict.'
diff --git a/tests/functional/openlp_core/common/test_settings.py b/tests/functional/openlp_core/common/test_settings.py
index ff6ee8765..4cad58af0 100644
--- a/tests/functional/openlp_core/common/test_settings.py
+++ b/tests/functional/openlp_core/common/test_settings.py
@@ -139,13 +139,13 @@ class TestSettings(TestCase, TestMixin):
         extend = settings.value('extend')
 
         # THEN the default value is returned
-        self.assertEqual('very wide', extend, 'The default value defined should be returned')
+        assert 'very wide' == extend, 'The default value defined should be returned'
 
         # WHEN a new value is saved into config
         Settings().setValue('test/extend', 'very short')
 
         # THEN the new value is returned when re-read
-        self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned')
+        assert '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"""
@@ -155,7 +155,7 @@ class TestSettings(TestCase, TestMixin):
             Settings().value('core/does not exists')
 
         # THEN: An exception with the non-existing key should be thrown
-        self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception')
+        assert 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"""
@@ -167,9 +167,8 @@ class TestSettings(TestCase, TestMixin):
             Settings.extend_default_settings({'test/setting 3': 4, 'test/extended 1': 1, 'test/extended 2': 2})
 
             # THEN: The _default_settings__ dictionary_ should have the new keys
-            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})
+            assert 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')

From c25a446839a075f434905c9b3186bc990160f7f4 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 9 Dec 2017 16:37:26 +0000
Subject: [PATCH 12/20] Finish common again

---
 tests/functional/openlp_core/common/test_common.py | 2 +-
 tests/functional/openlp_core/common/test_path.py   | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/tests/functional/openlp_core/common/test_common.py b/tests/functional/openlp_core/common/test_common.py
index 334ceb21b..5ffc664cd 100644
--- a/tests/functional/openlp_core/common/test_common.py
+++ b/tests/functional/openlp_core/common/test_common.py
@@ -70,7 +70,7 @@ class TestCommonFunctions(TestCase):
             # THEN: `extension_loader` should only try to import the files that are matched by the blob, excluding the
             #       files listed in the `excluded_files` argument
             mocked_import_module.assert_has_calls([call('openlp.import_dir.file1'),
-                                                          call('openlp.import_dir.file4')])
+                                                   call('openlp.import_dir.file4')])
 
     def test_extension_loader_import_error(self):
         """
diff --git a/tests/functional/openlp_core/common/test_path.py b/tests/functional/openlp_core/common/test_path.py
index d7c20ab6f..42d78cc7e 100644
--- a/tests/functional/openlp_core/common/test_path.py
+++ b/tests/functional/openlp_core/common/test_path.py
@@ -177,12 +177,11 @@ class TestShutil(TestCase):
             path = Path('test', 'path')
 
             # WHEN: Calling :func:`openlp.core.common.path.rmtree` with the path parameter as Path object type
-            result = path.rmtree()
+            path.rmtree()
 
             # THEN: :func:`shutil.rmtree` should have been called with the str equivalents of the Path object.
             mocked_shutil_rmtree.assert_called_once_with(
                 os.path.join('test', 'path'), False, None)
-            assert not result
 
     def test_rmtree_optional_params(self):
         """
@@ -268,7 +267,7 @@ class TestPath(TestCase):
         result = path_to_str(Path('test/path'))
 
         # THEN: `path_to_str` should return a string representation of the Path object
-        assert result ==  os.path.join('test', 'path')
+        assert result == os.path.join('test', 'path')
 
     def test_str_to_path_type_error(self):
         """

From ca936f5e1a3bf8fd1e877964a249a4c941fd1b4c Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Fri, 15 Dec 2017 16:19:42 +0000
Subject: [PATCH 13/20] fixes

---
 tests/functional/openlp_core/common/test_actions.py         | 4 ++--
 tests/functional/openlp_core/common/test_httputils.py       | 2 +-
 tests/functional/openlp_core/common/test_init.py            | 6 +++---
 tests/functional/openlp_core/common/test_json.py            | 2 +-
 tests/functional/openlp_core/common/test_mixins.py          | 2 +-
 tests/functional/openlp_core/common/test_path.py            | 6 +++---
 .../openlp_core/common/test_projector_utilities.py          | 6 +++---
 tests/functional/openlp_core/common/test_registry.py        | 6 +++---
 tests/functional/openlp_core/ui/test_mainwindow.py          | 2 +-
 9 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/tests/functional/openlp_core/common/test_actions.py b/tests/functional/openlp_core/common/test_actions.py
index d5d1abcf0..7d5ac6fca 100644
--- a/tests/functional/openlp_core/common/test_actions.py
+++ b/tests/functional/openlp_core/common/test_actions.py
@@ -124,8 +124,8 @@ class TestCategoryActionList(TestCase):
         local_list = [a for a in self.list]
         # THEN: Make sure they are returned in correct order
         assert len(self.list) == 2
-        assert local_list[0] == self.action1
-        assert local_list[1] == self.action2
+        assert local_list[0] is self.action1
+        assert local_list[1] is self.action2
 
     def test_remove(self):
         """
diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py
index ad1ab6e46..0b39033d9 100644
--- a/tests/functional/openlp_core/common/test_httputils.py
+++ b/tests/functional/openlp_core/common/test_httputils.py
@@ -240,4 +240,4 @@ class TestHttpUtils(TestCase, TestMixin):
 
         # THEN: socket.timeout should have been caught
         # NOTE: Test is if $tmpdir/tempfile is still there, then test fails since ftw deletes bad downloaded files
-        assert not os.path.exists(self.tempfile), 'tempfile should have been deleted'
+        assert os.path.exists(self.tempfile) is False, 'tempfile should have been deleted'
diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py
index fbeaec957..5292f414e 100644
--- a/tests/functional/openlp_core/common/test_init.py
+++ b/tests/functional/openlp_core/common/test_init.py
@@ -258,7 +258,7 @@ class TestInit(TestCase, TestMixin):
         result = delete_file(None)
 
         # THEN: delete_file should return False
-        assert not result, "delete_file should return False when called with None"
+        assert result is False, "delete_file should return False when called with None"
 
     def test_delete_file_path_success(self):
         """
@@ -271,7 +271,7 @@ class TestInit(TestCase, TestMixin):
             result = delete_file(Path('path', 'file.ext'))
 
             # THEN: delete_file should return True
-            assert result, 'delete_file should return True when it successfully deletes a file'
+            assert result is True, 'delete_file should return True when it successfully deletes a file'
 
     def test_delete_file_path_no_file_exists(self):
         """
@@ -363,4 +363,4 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: log.exception should be called and get_file_encoding should return None
             mocked_log.exception.assert_called_once_with('Error detecting file encoding')
-            assert not result
+            assert result is None
diff --git a/tests/functional/openlp_core/common/test_json.py b/tests/functional/openlp_core/common/test_json.py
index 392b30c9b..18cba3b4e 100644
--- a/tests/functional/openlp_core/common/test_json.py
+++ b/tests/functional/openlp_core/common/test_json.py
@@ -60,7 +60,7 @@ class TestOpenLPJsonDecoder(TestCase):
 
             # THEN: The object should be returned unchanged and a Path object should not have been initiated
             assert result == {'key': 'value'}
-            assert not mocked_path.called
+            assert mocked_path.called is False
 
     def test_json_decode(self):
         """
diff --git a/tests/functional/openlp_core/common/test_mixins.py b/tests/functional/openlp_core/common/test_mixins.py
index db9606d4e..f49c2b797 100644
--- a/tests/functional/openlp_core/common/test_mixins.py
+++ b/tests/functional/openlp_core/common/test_mixins.py
@@ -46,7 +46,7 @@ class TestRegistryProperties(TestCase, RegistryProperties):
         # GIVEN an Empty Registry
         # WHEN there is no Application
         # THEN the application should be none
-        assert not self.application, 'The application value should be None'
+        assert self.application is None, 'The application value should be None'
 
     def test_application(self):
         """
diff --git a/tests/functional/openlp_core/common/test_path.py b/tests/functional/openlp_core/common/test_path.py
index 42d78cc7e..b069c251d 100644
--- a/tests/functional/openlp_core/common/test_path.py
+++ b/tests/functional/openlp_core/common/test_path.py
@@ -213,7 +213,7 @@ class TestShutil(TestCase):
 
             # THEN: :func:`shutil.which` should have been called with the command, and :func:`which` should return None.
             mocked_shutil_which.assert_called_once_with('no_command')
-            assert not result
+        assert result is None
 
     def test_which_command(self):
         """
@@ -256,7 +256,7 @@ class TestPath(TestCase):
         result = path_to_str(None)
 
         # THEN: `path_to_str` should return an empty string
-        assert not result
+        assert result == ''
 
     def test_path_to_str_path_object(self):
         """
@@ -288,7 +288,7 @@ class TestPath(TestCase):
         result = str_to_path('')
 
         # THEN: `path_to_str` should return None
-        assert not result
+        assert result is None
 
     def test_path_encode_json(self):
         """
diff --git a/tests/functional/openlp_core/common/test_projector_utilities.py b/tests/functional/openlp_core/common/test_projector_utilities.py
index 45e13c04d..db163c152 100644
--- a/tests/functional/openlp_core/common/test_projector_utilities.py
+++ b/tests/functional/openlp_core/common/test_projector_utilities.py
@@ -45,7 +45,7 @@ ip6_link_local = 'fe80::223:14ff:fe99:d315'
 ip6_bad = 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff'
 
 
-class testProjectorUtilities(TestCase):
+class TestProjectorUtilities(TestCase):
     """
     Validate functions in the projector utilities module
     """
@@ -87,7 +87,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip4_bad)
 
         # THEN: Verify we received True
-        assert not valid, 'Bad IPv4 address should not have been valid'
+        assert valid is False, 'Bad IPv4 address should not have been valid'
 
     def test_ip6_loopback_valid(self):
         """
@@ -117,7 +117,7 @@ class testProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip6_bad)
 
         # THEN: Validate bad return
-        assert not valid, 'IPv6 bad address should have been invalid'
+        assert valid is False, 'IPv6 bad address should have been invalid'
 
     def test_md5_hash(self):
         """
diff --git a/tests/functional/openlp_core/common/test_registry.py b/tests/functional/openlp_core/common/test_registry.py
index 81b4230a1..7e8a80e14 100644
--- a/tests/functional/openlp_core/common/test_registry.py
+++ b/tests/functional/openlp_core/common/test_registry.py
@@ -57,13 +57,13 @@ class TestRegistry(TestCase):
         # WHEN I try to get back a non existent component
         # THEN I will get an exception
         temp = Registry().get('test2')
-        assert not temp, 'None should have been returned for missing service'
+        assert temp is False, 'None should have been returned for missing service'
 
         # WHEN I try to replace a component I should be allowed
         Registry().remove('test1')
         # THEN I will get an exception
         temp = Registry().get('test1')
-        assert not temp, 'None should have been returned for deleted service'
+        assert temp is False, 'None should have been returned for deleted service'
 
     def test_registry_function(self):
         """
@@ -142,7 +142,7 @@ class TestRegistry(TestCase):
         Registry().remove_function('test1', self.dummy_function_1)
 
         # THEN: The method should not be available.
-        assert not Registry().functions_list['test1'], 'The function should not be in the dict anymore.'
+        assert Registry().functions_list['test1'] is None, 'The function should not be in the dict anymore.'
 
     def dummy_function_1(self):
         return "function_1"
diff --git a/tests/functional/openlp_core/ui/test_mainwindow.py b/tests/functional/openlp_core/ui/test_mainwindow.py
index 4a0404eab..9dee80c0d 100644
--- a/tests/functional/openlp_core/ui/test_mainwindow.py
+++ b/tests/functional/openlp_core/ui/test_mainwindow.py
@@ -106,7 +106,7 @@ class TestMainWindow(TestCase, TestMixin):
             self.main_window.open_cmd_line_files("")
 
             # THEN the file should not be opened
-            assert not mocked_load_file.called, 'load_file should not have been called'
+            assert mocked_load_file.called is False, 'load_file should not have been called'
 
     def test_main_window_title(self):
         """

From eb208380469ad0cc4ebff7bd19b96a2ba6add226 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Fri, 15 Dec 2017 16:30:10 +0000
Subject: [PATCH 14/20] fixes

---
 tests/functional/openlp_core/common/test_registry.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/functional/openlp_core/common/test_registry.py b/tests/functional/openlp_core/common/test_registry.py
index 7e8a80e14..6f3cc5752 100644
--- a/tests/functional/openlp_core/common/test_registry.py
+++ b/tests/functional/openlp_core/common/test_registry.py
@@ -57,13 +57,13 @@ class TestRegistry(TestCase):
         # WHEN I try to get back a non existent component
         # THEN I will get an exception
         temp = Registry().get('test2')
-        assert temp is False, 'None should have been returned for missing service'
+        assert temp is None, 'None should have been returned for missing service'
 
         # WHEN I try to replace a component I should be allowed
         Registry().remove('test1')
         # THEN I will get an exception
         temp = Registry().get('test1')
-        assert temp is False, 'None should have been returned for deleted service'
+        assert temp is None, 'None should have been returned for deleted service'
 
     def test_registry_function(self):
         """
@@ -142,7 +142,7 @@ class TestRegistry(TestCase):
         Registry().remove_function('test1', self.dummy_function_1)
 
         # THEN: The method should not be available.
-        assert Registry().functions_list['test1'] is None, 'The function should not be in the dict anymore.'
+        assert Registry().functions_list['test1'] == [], 'The function should not be in the dict anymore.'
 
     def dummy_function_1(self):
         return "function_1"

From efe7129f214ce37b5c4875e6ad3bdb6fa88c5e7f Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sat, 16 Dec 2017 08:43:33 +0000
Subject: [PATCH 15/20] fixes

---
 tests/functional/openlp_core/common/test_common.py        | 6 +++---
 tests/functional/openlp_core/common/test_httputils.py     | 2 +-
 tests/functional/openlp_core/common/test_init.py          | 4 ++--
 .../openlp_core/common/test_projector_utilities.py        | 8 ++++----
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/functional/openlp_core/common/test_common.py b/tests/functional/openlp_core/common/test_common.py
index 5ffc664cd..368705821 100644
--- a/tests/functional/openlp_core/common/test_common.py
+++ b/tests/functional/openlp_core/common/test_common.py
@@ -175,7 +175,7 @@ class TestCommonFunctions(TestCase):
             mocked_sys.platform = 'win32'
 
             # THEN: The three platform functions should perform properly
-            assert is_win(), 'is_win() should return True'
+            assert is_win() is True, 'is_win() should return True'
             assert is_macosx() is False, 'is_macosx() should return False'
             assert is_linux() is False, 'is_linux() should return False'
 
@@ -191,7 +191,7 @@ class TestCommonFunctions(TestCase):
             mocked_sys.platform = 'darwin'
 
             # THEN: The three platform functions should perform properly
-            assert is_macosx(), 'is_macosx() should return True'
+            assert is_macosx() is True, 'is_macosx() should return True'
             assert is_win() is False, 'is_win() should return False'
             assert is_linux() is False, 'is_linux() should return False'
 
@@ -207,7 +207,7 @@ class TestCommonFunctions(TestCase):
             mocked_sys.platform = 'linux3'
 
             # THEN: The three platform functions should perform properly
-            assert is_linux(), 'is_linux() should return True'
+            assert is_linux() is True, 'is_linux() should return True'
             assert is_win() is False, 'is_win() should return False'
             assert is_macosx() is False, 'is_macosx() should return False'
 
diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py
index 0b39033d9..b7c08993f 100644
--- a/tests/functional/openlp_core/common/test_httputils.py
+++ b/tests/functional/openlp_core/common/test_httputils.py
@@ -59,7 +59,7 @@ class TestHttpUtils(TestCase, TestMixin):
 
             # THEN: The user agent is a Linux (or ChromeOS) user agent
             result = 'Linux' in user_agent or 'CrOS' in user_agent
-            assert result, 'The user agent should be a valid Linux user agent'
+            assert result is True, 'The user agent should be a valid Linux user agent'
 
     def test_get_user_agent_windows(self):
         """
diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py
index 5292f414e..9965a07ee 100644
--- a/tests/functional/openlp_core/common/test_init.py
+++ b/tests/functional/openlp_core/common/test_init.py
@@ -286,7 +286,7 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: The function should not attempt to delete the file and it should return True
             assert mocked_unlink.called is False
-            assert result, 'delete_file should return True when the file doesnt exist'
+            assert result is True, 'delete_file should return True when the file doesnt exist'
 
     def test_delete_file_path_exception(self):
         """
@@ -346,7 +346,7 @@ class TestInit(TestCase, TestMixin):
             mocked_open.assert_called_once_with('rb')
             assert mocked_universal_detector_inst.feed.mock_calls == [call(b"data" * 256), call(b"data" * 4)]
             mocked_universal_detector_inst.close.assert_called_once_with()
-            assert result, encoding_result
+            assert result == encoding_result
 
     def test_get_file_encoding_oserror(self):
         """
diff --git a/tests/functional/openlp_core/common/test_projector_utilities.py b/tests/functional/openlp_core/common/test_projector_utilities.py
index db163c152..5a3f886cf 100644
--- a/tests/functional/openlp_core/common/test_projector_utilities.py
+++ b/tests/functional/openlp_core/common/test_projector_utilities.py
@@ -67,7 +67,7 @@ class TestProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip4_local)
 
         # THEN: Verify we received True
-        assert valid, 'IPv4 local address should have been valid'
+        assert valid is True, 'IPv4 local address should have been valid'
 
     def test_ip4_broadcast_valid(self):
         """
@@ -77,7 +77,7 @@ class TestProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip4_broadcast)
 
         # THEN: Verify we received True
-        assert valid, 'IPv4 broadcast address should have been valid'
+        assert valid is True, 'IPv4 broadcast address should have been valid'
 
     def test_ip4_address_invalid(self):
         """
@@ -97,7 +97,7 @@ class TestProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip6_loopback)
 
         # THEN: Validate return
-        assert valid, 'IPv6 loopback address should have been valid'
+        assert valid is True, 'IPv6 loopback address should have been valid'
 
     def test_ip6_local_valid(self):
         """
@@ -107,7 +107,7 @@ class TestProjectorUtilities(TestCase):
         valid = verify_ip_address(addr=ip6_link_local)
 
         # THEN: Validate return
-        assert valid, 'IPv6 link-local address should have been valid'
+        assert valid is True, 'IPv6 link-local address should have been valid'
 
     def test_ip6_address_invalid(self):
         """

From fbaeca26f251634727fc495dc16fc121d947048c Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sun, 17 Dec 2017 14:12:27 +0000
Subject: [PATCH 16/20] 2 more done

---
 .../openlp_core/display/test_renderer.py         | 10 +++++-----
 .../openlp_core/display/test_screens.py          |  6 +++---
 tests/functional/openlp_core/lib/test_db.py      | 16 ++++++++--------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tests/functional/openlp_core/display/test_renderer.py b/tests/functional/openlp_core/display/test_renderer.py
index c30fe083b..b1478c7b9 100644
--- a/tests/functional/openlp_core/display/test_renderer.py
+++ b/tests/functional/openlp_core/display/test_renderer.py
@@ -113,7 +113,7 @@ class TestRenderer(TestCase):
         result = get_start_tags(given_raw_text)
 
         # THEN: Check if the correct tuple is returned.
-        self.assertEqual(result, expected_tuple), 'A tuple should be returned containing the text with correct ' \
+        assert result == expected_tuple, 'A tuple should be returned containing the text with correct ' \
             'tags, the opening tags, and the opening html tags.'
 
     def test_word_split(self):
@@ -128,7 +128,7 @@ class TestRenderer(TestCase):
         result_words = words_split(given_line)
 
         # THEN: The word lists should be the same.
-        self.assertListEqual(result_words, expected_words)
+        assert result_words == expected_words
 
     def test_format_slide_logical_split(self):
         """
@@ -145,7 +145,7 @@ class TestRenderer(TestCase):
         result_words = renderer.format_slide(given_line, service_item)
 
         # THEN: The word lists should be the same.
-        self.assertListEqual(result_words, expected_words)
+        assert result_words == expected_words
 
     def test_format_slide_blank_before_split(self):
         """
@@ -162,7 +162,7 @@ class TestRenderer(TestCase):
         result_words = renderer.format_slide(given_line, service_item)
 
         # THEN: The blanks have been removed.
-        self.assertListEqual(result_words, expected_words)
+        assert result_words == expected_words
 
     def test_format_slide_blank_after_split(self):
         """
@@ -179,7 +179,7 @@ class TestRenderer(TestCase):
         result_words = renderer.format_slide(given_line, service_item)
 
         # THEN: The blanks have been removed.
-        self.assertListEqual(result_words, expected_words)
+        assert result_words == expected_words
 
     @patch('openlp.core.display.renderer.QtWebKitWidgets.QWebView')
     @patch('openlp.core.display.renderer.build_lyrics_format_css')
diff --git a/tests/functional/openlp_core/display/test_screens.py b/tests/functional/openlp_core/display/test_screens.py
index 258f3b69a..960f9d0a8 100644
--- a/tests/functional/openlp_core/display/test_screens.py
+++ b/tests/functional/openlp_core/display/test_screens.py
@@ -75,6 +75,6 @@ class TestScreenList(TestCase):
 
         # THEN: The screen should have been added and the screens should be identical
         new_screen_count = len(self.screens.screen_list)
-        self.assertEqual(old_screen_count + 1, new_screen_count, 'The new_screens list should be bigger')
-        self.assertEqual(SCREEN, self.screens.screen_list.pop(),
-                         'The 2nd screen should be identical to the first screen')
+        assert old_screen_count + 1 == new_screen_count, 'The new_screens list should be bigger'
+        assert SCREEN == self.screens.screen_list.pop(), \
+            'The 2nd screen should be identical to the first screen'
diff --git a/tests/functional/openlp_core/lib/test_db.py b/tests/functional/openlp_core/lib/test_db.py
index 871980498..033508965 100644
--- a/tests/functional/openlp_core/lib/test_db.py
+++ b/tests/functional/openlp_core/lib/test_db.py
@@ -80,8 +80,8 @@ class TestDB(TestCase):
             MockedMetaData.assert_called_with(bind=mocked_engine)
             mocked_sessionmaker.assert_called_with(autoflush=True, autocommit=False, bind=mocked_engine)
             mocked_scoped_session.assert_called_with(mocked_sessionmaker_object)
-            self.assertIs(session, mocked_scoped_session_object, 'The ``session`` object should be the mock')
-            self.assertIs(metadata, mocked_metadata, 'The ``metadata`` object should be the mock')
+            assert session is mocked_scoped_session_object, 'The ``session`` object should be the mock'
+            assert metadata is mocked_metadata, 'The ``metadata`` object should be the mock'
 
     def test_init_db_defaults(self):
         """
@@ -94,8 +94,8 @@ class TestDB(TestCase):
         session, metadata = init_db(db_url)
 
         # THEN: Valid session and metadata objects should be returned
-        self.assertIsInstance(session, ScopedSession, 'The ``session`` object should be a ``ScopedSession`` instance')
-        self.assertIsInstance(metadata, MetaData, 'The ``metadata`` object should be a ``MetaData`` instance')
+        assert isinstance(session, ScopedSession), 'The ``session`` object should be a ``ScopedSession`` instance'
+        assert isinstance(metadata, MetaData), 'The ``metadata`` object should be a ``MetaData`` instance'
 
     def test_get_upgrade_op(self):
         """
@@ -116,7 +116,7 @@ class TestDB(TestCase):
             op = get_upgrade_op(mocked_session)
 
             # THEN: The op object should be mocked_op, and the correction function calls should have been made
-            self.assertIs(op, mocked_op, 'The return value should be the mocked object')
+            assert op is mocked_op, 'The return value should be the mocked object'
             mocked_session.bind.connect.assert_called_with()
             MockedMigrationContext.configure.assert_called_with(mocked_connection)
             MockedOperations.assert_called_with(mocked_context)
@@ -139,7 +139,7 @@ class TestDB(TestCase):
             # THEN: The AppLocation.get_section_data_path and delete_file methods should have been called
             MockedAppLocation.get_section_data_path.assert_called_with(test_plugin)
             mocked_delete_file.assert_called_with(test_location)
-            self.assertTrue(result, 'The result of delete_file should be True (was rigged that way)')
+            assert result is True, 'The result of delete_file should be True (was rigged that way)'
 
     def test_delete_database_with_db_file_name(self):
         """
@@ -160,7 +160,7 @@ class TestDB(TestCase):
             # THEN: The AppLocation.get_section_data_path and delete_file methods should have been called
             MockedAppLocation.get_section_data_path.assert_called_with(test_plugin)
             mocked_delete_file.assert_called_with(test_location)
-            self.assertFalse(result, 'The result of delete_file should be False (was rigged that way)')
+            assert result is False, 'The result of delete_file should be False (was rigged that way)'
 
     def test_skip_db_upgrade_with_no_database(self):
         """
@@ -174,4 +174,4 @@ class TestDB(TestCase):
         upgrade_db(url, mocked_upgrade)
 
         # THEN: upgrade should NOT have been called
-        self.assertFalse(mocked_upgrade.called, 'Database upgrade function should NOT have been called')
+        assert mocked_upgrade.called is False, 'Database upgrade function should NOT have been called'

From 9f735e0cc5524b559d4e08f92bc0960fe6e085a3 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sun, 17 Dec 2017 15:35:35 +0000
Subject: [PATCH 17/20] 1 more done

---
 .../openlp_core/lib/test_exceptions.py        |   2 +-
 .../openlp_core/lib/test_htmlbuilder.py       |  22 +--
 .../openlp_core/lib/test_image_manager.py     |  30 ++--
 tests/functional/openlp_core/lib/test_lib.py  | 149 +++++++++---------
 4 files changed, 97 insertions(+), 106 deletions(-)

diff --git a/tests/functional/openlp_core/lib/test_exceptions.py b/tests/functional/openlp_core/lib/test_exceptions.py
index c0de323b7..7ffc122c1 100644
--- a/tests/functional/openlp_core/lib/test_exceptions.py
+++ b/tests/functional/openlp_core/lib/test_exceptions.py
@@ -42,4 +42,4 @@ class TestValidationError(TestCase):
 
         # THEN: Then calling str on the error should return the correct text and it should be an instance of `Exception`
         assert str(error) == 'Test ValidationError'
-        assert isinstance(error, Exception)
+        assert isinstance(error, Exception) is True
diff --git a/tests/functional/openlp_core/lib/test_htmlbuilder.py b/tests/functional/openlp_core/lib/test_htmlbuilder.py
index 7c8d7809f..ffd796ecb 100644
--- a/tests/functional/openlp_core/lib/test_htmlbuilder.py
+++ b/tests/functional/openlp_core/lib/test_htmlbuilder.py
@@ -309,7 +309,7 @@ class Htmbuilder(TestCase, TestMixin):
             html = build_html(item, screen, is_live, background, plugins=plugins)
 
             # THEN: The returned html should match.
-            self.assertEqual(html, HTML, 'The returned html should match')
+            assert html == HTML, 'The returned html should match'
 
     def test_build_background_css_radial(self):
         """
@@ -325,7 +325,7 @@ class Htmbuilder(TestCase, TestMixin):
         css = build_background_css(item, width)
 
         # THEN: The returned css should match.
-        self.assertEqual(BACKGROUND_CSS_RADIAL, css, 'The background css should be equal.')
+        assert BACKGROUND_CSS_RADIAL == css, 'The background css should be equal.'
 
     def test_build_lyrics_css(self):
         """
@@ -346,7 +346,7 @@ class Htmbuilder(TestCase, TestMixin):
             css = build_lyrics_css(item)
 
             # THEN: The css should be equal.
-            self.assertEqual(LYRICS_CSS, css, 'The lyrics css should be equal.')
+            assert LYRICS_CSS == css, 'The lyrics css should be equal.'
 
     def test_build_lyrics_outline_css(self):
         """
@@ -363,7 +363,7 @@ class Htmbuilder(TestCase, TestMixin):
         css = build_lyrics_outline_css(theme_data)
 
         # THEN: The css should be equal.
-        self.assertEqual(LYRICS_OUTLINE_CSS, css, 'The outline css should be equal.')
+        assert LYRICS_OUTLINE_CSS == css, 'The outline css should be equal.'
 
     def test_build_lyrics_format_css(self):
         """
@@ -386,7 +386,7 @@ class Htmbuilder(TestCase, TestMixin):
         css = build_lyrics_format_css(theme_data, width, height)
 
         # THEN: They should be equal.
-        self.assertEqual(LYRICS_FORMAT_CSS, css, 'The lyrics format css should be equal.')
+        assert LYRICS_FORMAT_CSS == css, 'The lyrics format css should be equal.'
 
     def test_build_footer_css(self):
         """
@@ -404,7 +404,7 @@ class Htmbuilder(TestCase, TestMixin):
         css = build_footer_css(item, height)
 
         # THEN: THE css should be the same.
-        self.assertEqual(FOOTER_CSS, css, 'The footer strings should be equal.')
+        assert FOOTER_CSS == css, 'The footer strings should be equal.'
 
     def test_build_footer_css_wrap(self):
         """
@@ -423,7 +423,7 @@ class Htmbuilder(TestCase, TestMixin):
         css = build_footer_css(item, height)
 
         # THEN: Footer should wrap
-        self.assertEqual(FOOTER_CSS_WRAP, css, 'The footer strings should be equal.')
+        assert FOOTER_CSS_WRAP == css, 'The footer strings should be equal.'
 
     def test_build_footer_invalid(self):
         """
@@ -443,8 +443,8 @@ class Htmbuilder(TestCase, TestMixin):
         css.append(build_footer_css(item, height))
 
         # THEN: Footer should wrap
-        self.assertEqual(FOOTER_CSS_INVALID, css[0], 'The footer strings should be blank.')
-        self.assertEqual(FOOTER_CSS_INVALID, css[1], 'The footer strings should be blank.')
+        assert FOOTER_CSS_INVALID == css[0], 'The footer strings should be blank.'
+        assert FOOTER_CSS_INVALID == css[1], 'The footer strings should be blank.'
 
     def test_webkit_version(self):
         """
@@ -454,7 +454,7 @@ class Htmbuilder(TestCase, TestMixin):
         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")
+        assert webkit_version() == webkit_ver, "The returned webkit version doesn't match the installed one"
 
     def test_build_chords_css(self):
         """
@@ -468,4 +468,4 @@ class Htmbuilder(TestCase, TestMixin):
         chord_css = build_chords_css()
 
         # THEN: The build css should look as expected
-        self.assertEqual(CHORD_CSS_ENABLED, chord_css, 'The chord CSS should look as expected')
+        assert CHORD_CSS_ENABLED == chord_css, 'The chord CSS should look as expected'
diff --git a/tests/functional/openlp_core/lib/test_image_manager.py b/tests/functional/openlp_core/lib/test_image_manager.py
index c39f6e8f4..0b37b412d 100644
--- a/tests/functional/openlp_core/lib/test_image_manager.py
+++ b/tests/functional/openlp_core/lib/test_image_manager.py
@@ -84,7 +84,7 @@ class TestImageManager(TestCase, TestMixin):
         # THEN a KeyError is thrown
         with self.assertRaises(KeyError) as context:
             self.image_manager.get_image(TEST_PATH, 'church1.jpg')
-        self.assertNotEquals(context.exception, '', 'KeyError exception should have been thrown for missing image')
+        assert context.exception is not '', 'KeyError exception should have been thrown for missing image'
 
     def test_different_dimension_image(self):
         """
@@ -98,7 +98,7 @@ class TestImageManager(TestCase, TestMixin):
         image = self.image_manager.get_image(full_path, 'church.jpg', 80, 80)
 
         # THEN: The return should be of type image
-        self.assertEqual(isinstance(image, QtGui.QImage), True, 'The returned object should be a QImage')
+        assert isinstance(image, QtGui.QImage) is True, 'The returned object should be a QImage'
 
         # WHEN: adding the same image with different dimensions
         self.image_manager.add_image(full_path, 'church.jpg', None, 100, 100)
@@ -116,7 +116,7 @@ class TestImageManager(TestCase, TestMixin):
         # WHEN: calling with correct image, but wrong dimensions
         with self.assertRaises(KeyError) as context:
             self.image_manager.get_image(full_path, 'church.jpg', 120, 120)
-        self.assertNotEquals(context.exception, '', 'KeyError exception should have been thrown for missing dimension')
+        assert context.exception is not '', 'KeyError exception should have been thrown for missing dimension'
 
     def test_process_cache(self):
         """
@@ -141,10 +141,8 @@ class TestImageManager(TestCase, TestMixin):
             #  is being processed (see mocked methods/functions).
             # Note: Priority.Normal means, that the resize_image() was not completed yet (because afterwards the #
             # priority is adjusted to Priority.Lowest).
-            self.assertEqual(self.get_image_priority(image1), Priority.Normal,
-                             "image1's priority should be 'Priority.Normal'")
-            self.assertEqual(self.get_image_priority(image2), Priority.Normal,
-                             "image2's priority should be 'Priority.Normal'")
+            assert self.get_image_priority(image1) == Priority.Normal, "image1's priority should be 'Priority.Normal'"
+            assert self.get_image_priority(image2) == Priority.Normal, "image2's priority should be 'Priority.Normal'"
 
             # WHEN: Add more images.
             self.image_manager.add_image(TEST_PATH, image3, None)
@@ -162,15 +160,15 @@ class TestImageManager(TestCase, TestMixin):
             # Because empty() is not reliable, wait a litte; just to make sure.
             time.sleep(0.1)
             # THEN: The images' priority reflect how they were processed.
-            self.assertEqual(self.image_manager._conversion_queue.qsize(), 0, "The queue should be empty.")
-            self.assertEqual(self.get_image_priority(image1), Priority.Lowest,
-                             "The image should have not been requested (=Lowest)")
-            self.assertEqual(self.get_image_priority(image2), Priority.Lowest,
-                             "The image should have not been requested (=Lowest)")
-            self.assertEqual(self.get_image_priority(image3), Priority.Low,
-                             "Only the QImage should have been requested (=Low).")
-            self.assertEqual(self.get_image_priority(image4), Priority.Urgent,
-                             "The image bytes should have been requested (=Urgent).")
+            assert self.image_manager._conversion_queue.qsize() == 0, "The queue should be empty."
+            assert self.get_image_priority(image1) == Priority.Lowest, \
+                "The image should have not been requested (=Lowest)"
+            assert self.get_image_priority(image2) == Priority.Lowest, \
+                "The image should have not been requested (=Lowest)"
+            assert self.get_image_priority(image3) == Priority.Low, \
+                "Only the QImage should have been requested (=Low)."
+            assert self.get_image_priority(image4) == Priority.Urgent, \
+                "The image bytes should have been requested (=Urgent)."
 
     def get_image_priority(self, image):
         """
diff --git a/tests/functional/openlp_core/lib/test_lib.py b/tests/functional/openlp_core/lib/test_lib.py
index e9788060e..9230ca5b0 100644
--- a/tests/functional/openlp_core/lib/test_lib.py
+++ b/tests/functional/openlp_core/lib/test_lib.py
@@ -49,8 +49,8 @@ class TestLib(TestCase):
         true_result = str_to_bool(true_boolean)
 
         # THEN: We should get back a True bool
-        self.assertIsInstance(true_result, bool, 'The result should be a boolean')
-        self.assertTrue(true_result, 'The result should be True')
+        assert isinstance(true_result, bool), 'The result should be a boolean'
+        assert true_result is True, 'The result should be True'
 
     def test_str_to_bool_with_bool_false(self):
         """
@@ -63,8 +63,8 @@ class TestLib(TestCase):
         false_result = str_to_bool(false_boolean)
 
         # THEN: We should get back a True bool
-        self.assertIsInstance(false_result, bool, 'The result should be a boolean')
-        self.assertFalse(false_result, 'The result should be True')
+        assert isinstance(false_result, bool), 'The result should be a boolean'
+        assert false_result is False, 'The result should be True'
 
     def test_str_to_bool_with_integer(self):
         """
@@ -77,7 +77,7 @@ class TestLib(TestCase):
         int_result = str_to_bool(int_string)
 
         # THEN: we should get back a false
-        self.assertFalse(int_result, 'The result should be False')
+        assert int_result is False, 'The result should be False'
 
     def test_str_to_bool_with_invalid_string(self):
         """
@@ -90,7 +90,7 @@ class TestLib(TestCase):
         str_result = str_to_bool(invalid_string)
 
         # THEN: we should get back a false
-        self.assertFalse(str_result, 'The result should be False')
+        assert str_result is False, 'The result should be False'
 
     def test_str_to_bool_with_string_false(self):
         """
@@ -103,7 +103,7 @@ class TestLib(TestCase):
         false_result = str_to_bool(false_string)
 
         # THEN: we should get back a false
-        self.assertFalse(false_result, 'The result should be False')
+        assert false_result is False, 'The result should be False'
 
     def test_str_to_bool_with_string_no(self):
         """
@@ -116,7 +116,7 @@ class TestLib(TestCase):
         str_result = str_to_bool(no_string)
 
         # THEN: we should get back a false
-        self.assertFalse(str_result, 'The result should be False')
+        assert str_result is False, 'The result should be False'
 
     def test_str_to_bool_with_true_string_value(self):
         """
@@ -129,7 +129,7 @@ class TestLib(TestCase):
         true_result = str_to_bool(true_string)
 
         # THEN: we should get back a true
-        self.assertTrue(true_result, 'The result should be True')
+        assert true_result is True, 'The result should be True'
 
     def test_str_to_bool_with_yes_string_value(self):
         """
@@ -142,7 +142,7 @@ class TestLib(TestCase):
         str_result = str_to_bool(yes_string)
 
         # THEN: we should get back a true
-        self.assertTrue(str_result, 'The result should be True')
+        assert str_result is True, 'The result should be True'
 
     def test_get_text_file_string_no_file(self):
         """
@@ -157,7 +157,7 @@ class TestLib(TestCase):
 
             # THEN: The result should be False
             file_path.is_file.assert_called_with()
-            self.assertFalse(result, 'False should be returned if no file exists')
+            assert result is False, 'False should be returned if no file exists'
 
     def test_get_text_file_string_read_error(self):
         """
@@ -176,7 +176,7 @@ class TestLib(TestCase):
             # THEN: None should be returned
             file_path.is_file.assert_called_once_with()
             file_path.open.assert_called_once_with('r', encoding='utf-8')
-            self.assertIsNone(result, 'None should be returned if the file cannot be opened')
+            assert result is None, 'None should be returned if the file cannot be opened'
 
     def test_get_text_file_string_decode_error(self):
         """
@@ -195,7 +195,7 @@ class TestLib(TestCase):
         result = build_icon(icon)
 
         # THEN: The result should be the same icon as we passed in
-        self.assertIs(icon, result, 'The result should be the same icon as we passed in')
+        assert icon is result, 'The result should be the same icon as we passed in'
 
     def test_build_icon_with_resource(self):
         """
@@ -217,7 +217,7 @@ class TestLib(TestCase):
             MockedQPixmap.assert_called_with(resource_uri)
             # There really should be more assert statements here but due to type checking and things they all break. The
             # best we can do is to assert that we get back a MagicMock object.
-            self.assertIsInstance(result, MagicMock, 'The result should be a MagicMock, because we mocked it out')
+            assert isinstance(result, MagicMock), 'The result should be a MagicMock, because we mocked it out'
 
     def test_image_to_byte(self):
         """
@@ -240,8 +240,8 @@ class TestLib(TestCase):
             MockedQtCore.QBuffer.assert_called_with(mocked_byte_array)
             mocked_buffer.open.assert_called_with('writeonly')
             mocked_image.save.assert_called_with(mocked_buffer, "PNG")
-            self.assertFalse(mocked_byte_array.toBase64.called)
-            self.assertEqual(mocked_byte_array, result, 'The mocked out byte array should be returned')
+            assert mocked_byte_array.toBase64.called is False
+            assert mocked_byte_array == result, 'The mocked out byte array should be returned'
 
     def test_image_to_byte_base_64(self):
         """
@@ -266,8 +266,7 @@ class TestLib(TestCase):
             mocked_buffer.open.assert_called_with('writeonly')
             mocked_image.save.assert_called_with(mocked_buffer, "PNG")
             mocked_byte_array.toBase64.assert_called_with()
-            self.assertEqual('base64mock', result, 'The result should be the return value of the mocked out '
-                                                   'base64 method')
+            assert 'base64mock' == result, 'The result should be the return value of the mocked out base64 method'
 
     def test_create_thumb_with_size(self):
         """
@@ -286,16 +285,16 @@ class TestLib(TestCase):
             pass
 
         # Only continue when the thumb does not exist.
-        self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.')
+        assert thumb_path.exists() is False, 'Test was not run, because the thumb already exists.'
 
         # WHEN: Create the thumb.
         icon = create_thumb(image_path, thumb_path, size=thumb_size)
 
         # THEN: Check if the thumb was created and scaled to the given size.
         self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists')
-        self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon')
-        self.assertFalse(icon.isNull(), 'The icon should not be null')
-        self.assertEqual(thumb_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size')
+        assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
+        assert icon.isNull()is False, 'The icon should not be null'
+        assert thumb_size == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # Remove the thumb so that the test actually tests if the thumb will be created.
         try:
@@ -320,17 +319,16 @@ class TestLib(TestCase):
             pass
 
         # Only continue when the thumb does not exist.
-        self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.')
+        assert thumb_path.exists() is False, 'Test was not run, because the thumb already exists.'
 
         # WHEN: Create the thumb.
         icon = create_thumb(image_path, thumb_path)
 
         # THEN: Check if the thumb was created, retaining its aspect ratio.
         self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists')
-        self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon')
-        self.assertFalse(icon.isNull(), 'The icon should not be null')
-        self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(),
-                         'The thumb should have the given size')
+        assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
+        assert icon.isNull() is False, 'The icon should not be null'
+        assert expected_size == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # Remove the thumb so that the test actually tests if the thumb will be created.
         try:
@@ -356,17 +354,16 @@ class TestLib(TestCase):
             pass
 
         # Only continue when the thumb does not exist.
-        self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.')
+        assert thumb_path.exists() is False, 'Test was not run, because the thumb already exists.'
 
         # WHEN: Create the thumb.
         icon = create_thumb(image_path, thumb_path, size=thumb_size)
 
         # THEN: Check if the thumb was created, retaining its aspect ratio.
-        self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists')
-        self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon')
-        self.assertFalse(icon.isNull(), 'The icon should not be null')
-        self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(),
-                         'The thumb should have the given size')
+        assert thumb_path.exists() is True, 'Test was not ran, because the thumb already exists'
+        assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
+        assert icon.isNull() is False, 'The icon should not be null'
+        assert expected_size == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # Remove the thumb so that the test actually tests if the thumb will be created.
         try:
@@ -392,17 +389,16 @@ class TestLib(TestCase):
             pass
 
         # Only continue when the thumb does not exist.
-        self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.')
+        assert thumb_path.exists() is False, 'Test was not run, because the thumb already exists.'
 
         # WHEN: Create the thumb.
         icon = create_thumb(image_path, thumb_path, size=thumb_size)
 
         # THEN: Check if the thumb was created, retaining its aspect ratio.
-        self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists')
-        self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon')
-        self.assertFalse(icon.isNull(), 'The icon should not be null')
-        self.assertEqual(
-            expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size')
+        assert thumb_path.exists() is True, 'Test was not ran, because the thumb already exists'
+        assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
+        assert icon.isNull() is False, 'The icon should not be null'
+        assert expected_size == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # Remove the thumb so that the test actually tests if the thumb will be created.
         try:
@@ -428,17 +424,16 @@ class TestLib(TestCase):
             pass
 
         # Only continue when the thumb does not exist.
-        self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.')
+        assert thumb_path.exists() is False, 'Test was not run, because the thumb already exists.'
 
         # WHEN: Create the thumb.
         icon = create_thumb(image_path, thumb_path, size=thumb_size)
 
         # THEN: Check if the thumb was created, retaining its aspect ratio.
         self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists')
-        self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon')
-        self.assertFalse(icon.isNull(), 'The icon should not be null')
-        self.assertEqual(
-            expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size')
+        assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
+        assert icon.isNull() is False, 'The icon should not be null'
+        assert expected_size == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # Remove the thumb so that the test actually tests if the thumb will be created.
         try:
@@ -465,7 +460,7 @@ class TestLib(TestCase):
             pass
 
         # Only continue when the thumb does not exist.
-        self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.')
+        assert thumb_path.exists() is False, 'Test was not run, because the thumb already exists.'
 
         # WHEN: Create the thumb.
         with patch('openlp.core.lib.QtGui.QImageReader.size') as mocked_size:
@@ -474,10 +469,9 @@ class TestLib(TestCase):
 
         # THEN: Check if the thumb was created with aspect ratio of 1.
         self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists')
-        self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon')
-        self.assertFalse(icon.isNull(), 'The icon should not be null')
-        self.assertEqual(
-            expected_size_1, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size')
+        assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
+        assert icon.isNull() is False, 'The icon should not be null'
+        assert expected_size_1 == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # WHEN: Create the thumb.
         with patch('openlp.core.lib.QtGui.QImageReader.size') as mocked_size:
@@ -485,10 +479,9 @@ class TestLib(TestCase):
             icon = create_thumb(image_path, thumb_path, size=thumb_size)
 
         # THEN: Check if the thumb was created with aspect ratio of 1.
-        self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon')
-        self.assertFalse(icon.isNull(), 'The icon should not be null')
-        self.assertEqual(
-            expected_size_2, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size')
+        assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
+        assert icon.isNull() is False, 'The icon should not be null'
+        assert expected_size_2 == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # Remove the thumb so that the test actually tests if the thumb will be created.
         try:
@@ -511,7 +504,7 @@ class TestLib(TestCase):
 
         # THEN: The selectedIndexes function should have been called and the result should be true
         mocked_list_widget.selectedIndexes.assert_called_with()
-        self.assertTrue(result, 'The result should be True')
+        assert result is True, 'The result should be True'
 
     def test_check_item_selected_false(self):
         """
@@ -532,7 +525,7 @@ class TestLib(TestCase):
             # THEN: The selectedIndexes function should have been called and the result should be true
             mocked_list_widget.selectedIndexes.assert_called_with()
             MockedQtWidgets.QMessageBox.information.assert_called_with('parent', 'mocked translate', 'message')
-            self.assertFalse(result, 'The result should be False')
+            assert result is False, 'The result should be False'
 
     def test_clean_tags(self):
         """
@@ -554,7 +547,7 @@ class TestLib(TestCase):
             result_string = clean_tags(string_to_pass)
 
             # THEN: The strings should be identical.
-            self.assertEqual(wanted_string, result_string, 'The strings should be identical')
+            assert wanted_string == result_string, 'The strings should be identical'
 
     def test_expand_tags(self):
         """
@@ -593,7 +586,7 @@ class TestLib(TestCase):
             result_string = expand_tags(string_to_pass)
 
             # THEN: The strings should be identical.
-            self.assertEqual(wanted_string, result_string, 'The strings should be identical.')
+            assert wanted_string == result_string, 'The strings should be identical.'
 
     def test_validate_thumb_file_does_not_exist(self):
         """
@@ -609,7 +602,7 @@ class TestLib(TestCase):
 
             # THEN: we should have called a few functions, and the result should be False
             thumb_path.exists.assert_called_once_with()
-            self.assertFalse(result, 'The result should be False')
+            assert result is False, 'The result should be False'
 
     def test_validate_thumb_file_exists_and_newer(self):
         """
@@ -624,7 +617,7 @@ class TestLib(TestCase):
             result = validate_thumb(file_path, thumb_path)
 
             # THEN: `validate_thumb` should return True
-            self.assertTrue(result)
+            assert result is True
 
     def test_validate_thumb_file_exists_and_older(self):
         """
@@ -639,7 +632,7 @@ class TestLib(TestCase):
 
         # THEN: `validate_thumb` should return False
         thumb_path.stat.assert_called_once_with()
-        self.assertFalse(result, 'The result should be False')
+        assert result is False, 'The result should be False'
 
     def test_resize_thumb(self):
         """
@@ -658,9 +651,9 @@ class TestLib(TestCase):
 
         # THEN: Check if the size is correct and the background was set.
         result_size = image.size()
-        self.assertEqual(wanted_height, result_size.height(), 'The image should have the requested height.')
-        self.assertEqual(wanted_width, result_size.width(), 'The image should have the requested width.')
-        self.assertEqual(image.pixel(0, 0), wanted_background_rgb, 'The background should be white.')
+        assert wanted_height == result_size.height(), 'The image should have the requested height.'
+        assert wanted_width == result_size.width(), 'The image should have the requested width.'
+        assert image.pixel(0, 0) == wanted_background_rgb, 'The background should be white.'
 
     def test_resize_thumb_ignoring_aspect_ratio(self):
         """
@@ -679,9 +672,9 @@ class TestLib(TestCase):
 
         # THEN: Check if the size is correct and the background was set.
         result_size = image.size()
-        self.assertEqual(wanted_height, result_size.height(), 'The image should have the requested height.')
-        self.assertEqual(wanted_width, result_size.width(), 'The image should have the requested width.')
-        self.assertEqual(image.pixel(0, 0), wanted_background_rgb, 'The background should be white.')
+        assert wanted_height == result_size.height(), 'The image should have the requested height.'
+        assert wanted_width == result_size.width(), 'The image should have the requested width.'
+        assert image.pixel(0, 0) == wanted_background_rgb, 'The background should be white.'
 
     @patch('openlp.core.lib.QtCore.QLocale.createSeparatedList')
     def test_create_separated_list_qlocate(self, mocked_createSeparatedList):
@@ -696,8 +689,8 @@ class TestLib(TestCase):
         string_result = create_separated_list(string_list)
 
         # THEN: We should have "Author 1, Author 2, and Author 3"
-        self.assertEqual(string_result, 'Author 1, Author 2 and Author 3', 'The string should be "Author 1, '
-                         'Author 2, and Author 3".')
+        assert string_result == 'Author 1, Author 2 and Author 3', \
+            'The string should be "Author 1, Author 2, and Author 3".'
 
     def test_create_separated_list_empty_list(self):
         """
@@ -710,7 +703,7 @@ class TestLib(TestCase):
         string_result = create_separated_list(string_list)
 
         # THEN: We shoud have an emptry string.
-        self.assertEqual(string_result, '', 'The string sould be empty.')
+        assert string_result == '', 'The string sould be empty.'
 
     def test_create_separated_list_with_one_item(self):
         """
@@ -723,7 +716,7 @@ class TestLib(TestCase):
         string_result = create_separated_list(string_list)
 
         # THEN: We should have "Author 1"
-        self.assertEqual(string_result, 'Author 1', 'The string should be "Author 1".')
+        assert string_result == 'Author 1', 'The string should be "Author 1".'
 
     def test_create_separated_list_with_two_items(self):
         """
@@ -736,7 +729,7 @@ class TestLib(TestCase):
         string_result = create_separated_list(string_list)
 
         # THEN: We should have "Author 1 and Author 2"
-        self.assertEqual(string_result, 'Author 1 and Author 2', 'The string should be "Author 1 and Author 2".')
+        assert string_result == 'Author 1 and Author 2', 'The string should be "Author 1 and Author 2".'
 
     def test_create_separated_list_with_three_items(self):
         """
@@ -749,8 +742,8 @@ class TestLib(TestCase):
         string_result = create_separated_list(string_list)
 
         # THEN: We should have "Author 1, Author 2 and Author 3"
-        self.assertEqual(string_result, 'Author 1, Author 2 and Author 3', 'The string should be "Author 1, '
-                         'Author 2, and Author 3".')
+        assert string_result == 'Author 1, Author 2 and Author 3', \
+            'The string should be "Author 1, Author 2, and Author 3".'
 
     def test_expand_chords(self):
         """
@@ -766,7 +759,7 @@ class TestLib(TestCase):
         expected_html = '<span class="chordline firstchordline">H<span class="chord"><span><strong>C</strong></span>' \
                         '</span>alleluya.<span class="chord"><span><strong>F</strong></span></span><span class="ws">' \
                         '&nbsp;&nbsp;</span> <span class="chord"><span><strong>G</strong></span></span></span>'
-        self.assertEqual(expected_html, text_with_expanded_chords, 'The expanded chords should look as expected!')
+        assert expected_html == text_with_expanded_chords, 'The expanded chords should look as expected!'
 
     def test_expand_chords2(self):
         """
@@ -782,7 +775,7 @@ class TestLib(TestCase):
         expected_html = '<span class="chordline firstchordline">I<span class="chord"><span><strong>D</strong></span>' \
                         '</span>&#x27;M NOT MOVED BY WHAT I SEE HALLE<span class="chord"><span><strong>F</strong>' \
                         '</span></span>LUJA<span class="chord"><span><strong>C</strong></span></span>H</span>'
-        self.assertEqual(expected_html, text_with_expanded_chords, 'The expanded chords should look as expected!')
+        assert expected_html == text_with_expanded_chords, 'The expanded chords should look as expected!'
 
     def test_compare_chord_lyric_short_chord(self):
         """
@@ -810,7 +803,7 @@ class TestLib(TestCase):
         ret = compare_chord_lyric(chord, lyrics)
 
         # THEN: The returned value should 4 because the chord is longer than the lyric
-        self.assertEquals(4, ret, 'The returned value should 4 because the chord is longer than the lyric')
+        assert 4 == ret, 'The returned value should 4 because the chord is longer than the lyric'
 
     def test_find_formatting_tags(self):
         """
@@ -825,7 +818,7 @@ class TestLib(TestCase):
         active_tags = find_formatting_tags(lyrics, tags)
 
         # THEN: The list of active tags should contain only 'st'
-        self.assertListEqual(['st'], active_tags, 'The list of active tags should contain only "st"')
+        assert ['st'] == active_tags, 'The list of active tags should contain only "st"'
 
     def test_expand_chords_for_printing(self):
         """
@@ -862,4 +855,4 @@ class TestLib(TestCase):
                         '<table class="segment" cellpadding="0" cellspacing="0" border="0" align="left"><tr ' \
                         'class="chordrow"><td class="chord">F</td></tr><tr><td class="lyrics">{st}{/st}&nbsp;</td>' \
                         '</tr></table></td></tr></table>'
-        self.assertEqual(expected_html, text_with_expanded_chords, 'The expanded chords should look as expected!')
+        assert expected_html == text_with_expanded_chords, 'The expanded chords should look as expected!'

From f76f04994d229ea5584cacc9bb87cf1fe81fd7ea Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sun, 17 Dec 2017 17:52:17 +0000
Subject: [PATCH 18/20] lib done

---
 .../openlp_core/lib/test_mediamanageritem.py  |  14 +-
 .../openlp_core/lib/test_pluginmanager.py     |  52 ++---
 .../openlp_core/lib/test_serviceitem.py       | 178 +++++++++---------
 .../functional/openlp_core/lib/test_theme.py  |  16 +-
 tests/functional/openlp_core/lib/test_ui.py   |  92 ++++-----
 5 files changed, 180 insertions(+), 172 deletions(-)

diff --git a/tests/functional/openlp_core/lib/test_mediamanageritem.py b/tests/functional/openlp_core/lib/test_mediamanageritem.py
index a6152cbdf..6c659a5bc 100644
--- a/tests/functional/openlp_core/lib/test_mediamanageritem.py
+++ b/tests/functional/openlp_core/lib/test_mediamanageritem.py
@@ -69,11 +69,11 @@ class TestMediaManagerItem(TestCase, TestMixin):
         # WHEN: Object is created
         mmi.required_icons()
         # THEN: Default icons should be populated
-        self.assertFalse(mmi.has_import_icon, 'There should be no import icon by default')
-        self.assertTrue(mmi.has_new_icon, 'By default a new icon should be present')
-        self.assertFalse(mmi.has_file_icon, 'There should be no file icon by default')
-        self.assertTrue(mmi.has_delete_icon, 'By default a delete icon should be present')
-        self.assertFalse(mmi.add_to_service_item, 'There should be no add_to_service icon by default')
+        assert mmi.has_import_icon is False, 'There should be no import icon by default'
+        assert mmi.has_new_icon is True, 'By default a new icon should be present'
+        assert mmi.has_file_icon is False, 'There should be no file icon by default'
+        assert mmi.has_delete_icon is True, 'By default a delete icon should be present'
+        assert mmi.add_to_service_item is False, 'There should be no add_to_service icon by default'
 
     @patch('openlp.core.lib.mediamanageritem.Settings')
     @patch('openlp.core.lib.mediamanageritem.MediaManagerItem.on_live_click')
@@ -111,5 +111,5 @@ class TestMediaManagerItem(TestCase, TestMixin):
         mmi.on_double_clicked()
 
         # THEN: on_live_click() should have been called
-        self.assertEqual(0, mocked_on_live_click.call_count, 'on_live_click() should not have been called')
-        self.assertEqual(0, mocked_on_preview_click.call_count, 'on_preview_click() should not have been called')
+        assert 0 == mocked_on_live_click.call_count, 'on_live_click() should not have been called'
+        assert 0 == mocked_on_preview_click.call_count, 'on_preview_click() should not have been called'
diff --git a/tests/functional/openlp_core/lib/test_pluginmanager.py b/tests/functional/openlp_core/lib/test_pluginmanager.py
index 52e9d91bf..163f52877 100644
--- a/tests/functional/openlp_core/lib/test_pluginmanager.py
+++ b/tests/functional/openlp_core/lib/test_pluginmanager.py
@@ -64,8 +64,8 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_media_manager()
 
         # THEN: The create_media_manager_item() method should have been called
-        self.assertEqual(0, mocked_plugin.create_media_manager_item.call_count,
-                         'The create_media_manager_item() method should not have been called.')
+        assert 0 == mocked_plugin.create_media_manager_item.call_count, \
+            'The create_media_manager_item() method should not have been called.'
 
     def test_hook_media_manager_with_active_plugin(self):
         """
@@ -97,8 +97,8 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_settings_tabs()
 
         # THEN: The hook_settings_tabs() method should have been called
-        self.assertEqual(0, mocked_plugin.create_media_manager_item.call_count,
-                         'The create_media_manager_item() method should not have been called.')
+        assert 0 == mocked_plugin.create_media_manager_item.call_count, \
+            'The create_media_manager_item() method should not have been called.'
 
     def test_hook_settings_tabs_with_disabled_plugin_and_mocked_form(self):
         """
@@ -117,10 +117,10 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_settings_tabs()
 
         # THEN: The create_settings_tab() method should not have been called, but the plugins lists should be the same
-        self.assertEqual(0, mocked_plugin.create_settings_tab.call_count,
-                         'The create_media_manager_item() method should not have been called.')
-        self.assertEqual(mocked_settings_form.plugin_manager.plugins, plugin_manager.plugins,
-                         'The plugins on the settings form should be the same as the plugins in the plugin manager')
+        assert 0 == mocked_plugin.create_settings_tab.call_count, \
+            'The create_media_manager_item() method should not have been called.'
+        assert mocked_settings_form.plugin_manager.plugins == plugin_manager.plugins, \
+            'The plugins on the settings form should be the same as the plugins in the plugin manager'
 
     def test_hook_settings_tabs_with_active_plugin_and_mocked_form(self):
         """
@@ -139,10 +139,10 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_settings_tabs()
 
         # THEN: The create_media_manager_item() method should have been called with the mocked settings form
-        self.assertEqual(1, mocked_plugin.create_settings_tab.call_count,
-                         'The create_media_manager_item() method should have been called once.')
-        self.assertEqual(plugin_manager.plugins, mocked_settings_form.plugin_manager.plugins,
-                         'The plugins on the settings form should be the same as the plugins in the plugin manager')
+        assert 1 == mocked_plugin.create_settings_tab.call_count, \
+            'The create_media_manager_item() method should have been called once.'
+        assert plugin_manager.plugins == mocked_settings_form.plugin_manager.plugins, \
+            'The plugins on the settings form should be the same as the plugins in the plugin manager'
 
     def test_hook_settings_tabs_with_active_plugin_and_no_form(self):
         """
@@ -174,8 +174,8 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_import_menu()
 
         # THEN: The create_media_manager_item() method should have been called
-        self.assertEqual(0, mocked_plugin.add_import_menu_item.call_count,
-                         'The add_import_menu_item() method should not have been called.')
+        assert 0 == mocked_plugin.add_import_menu_item.call_count, \
+            'The add_import_menu_item() method should not have been called.'
 
     def test_hook_import_menu_with_active_plugin(self):
         """
@@ -207,8 +207,8 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_export_menu()
 
         # THEN: The add_export_menu_item() method should not have been called
-        self.assertEqual(0, mocked_plugin.add_export_menu_item.call_count,
-                         'The add_export_menu_item() method should not have been called.')
+        assert 0 == mocked_plugin.add_export_menu_item.call_count, \
+            'The add_export_menu_item() method should not have been called.'
 
     def test_hook_export_menu_with_active_plugin(self):
         """
@@ -241,8 +241,8 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_upgrade_plugin_settings(settings)
 
         # THEN: The upgrade_settings() method should not have been called
-        self.assertEqual(0, mocked_plugin.upgrade_settings.call_count,
-                         'The upgrade_settings() method should not have been called.')
+        assert 0 == mocked_plugin.upgrade_settings.call_count, \
+            'The upgrade_settings() method should not have been called.'
 
     def test_hook_upgrade_plugin_settings_with_active_plugin(self):
         """
@@ -275,8 +275,8 @@ class TestPluginManager(TestCase):
         plugin_manager.hook_tools_menu()
 
         # THEN: The add_tools_menu_item() method should have been called
-        self.assertEqual(0, mocked_plugin.add_tools_menu_item.call_count,
-                         'The add_tools_menu_item() method should not have been called.')
+        assert 0 == mocked_plugin.add_tools_menu_item.call_count, \
+            'The add_tools_menu_item() method should not have been called.'
 
     def test_hook_tools_menu_with_active_plugin(self):
         """
@@ -310,7 +310,7 @@ class TestPluginManager(TestCase):
 
         # THEN: The is_active() method should have been called, and initialise() method should NOT have been called
         mocked_plugin.is_active.assert_called_with()
-        self.assertEqual(0, mocked_plugin.initialise.call_count, 'The initialise() method should not have been called.')
+        assert 0 == mocked_plugin.initialise.call_count, 'The initialise() method should not have been called.'
 
     def test_initialise_plugins_with_active_plugin(self):
         """
@@ -346,7 +346,7 @@ class TestPluginManager(TestCase):
 
         # THEN: The is_active() method should have been called, and initialise() method should NOT have been called
         mocked_plugin.is_active.assert_called_with()
-        self.assertEqual(0, mocked_plugin.finalise.call_count, 'The finalise() method should not have been called.')
+        assert 0 == mocked_plugin.finalise.call_count, 'The finalise() method should not have been called.'
 
     def test_finalise_plugins_with_active_plugin(self):
         """
@@ -380,7 +380,7 @@ class TestPluginManager(TestCase):
         result = plugin_manager.get_plugin_by_name('Missing Plugin')
 
         # THEN: The is_active() and finalise() methods should have been called
-        self.assertIsNone(result, 'The result for get_plugin_by_name should be None')
+        assert result is None, 'The result for get_plugin_by_name should be None'
 
     def test_get_plugin_by_name_exists(self):
         """
@@ -396,7 +396,7 @@ class TestPluginManager(TestCase):
         result = plugin_manager.get_plugin_by_name('Mocked Plugin')
 
         # THEN: The is_active() and finalise() methods should have been called
-        self.assertEqual(result, mocked_plugin, 'The result for get_plugin_by_name should be the mocked plugin')
+        assert result == mocked_plugin, 'The result for get_plugin_by_name should be the mocked plugin'
 
     def test_new_service_created_with_disabled_plugin(self):
         """
@@ -414,8 +414,8 @@ class TestPluginManager(TestCase):
 
         # THEN: The isActive() method should have been called, and initialise() method should NOT have been called
         mocked_plugin.is_active.assert_called_with()
-        self.assertEqual(0, mocked_plugin.new_service_created.call_count,
-                         'The new_service_created() method should not have been called.')
+        assert 0 == mocked_plugin.new_service_created.call_count, \
+            'The new_service_created() method should not have been called.'
 
     def test_new_service_created_with_active_plugin(self):
         """
diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py
index 78ad018fb..2ee142be4 100644
--- a/tests/functional/openlp_core/lib/test_serviceitem.py
+++ b/tests/functional/openlp_core/lib/test_serviceitem.py
@@ -28,7 +28,9 @@ from unittest.mock import MagicMock, patch
 
 from openlp.core.common import md5_hash
 from openlp.core.common.registry import Registry
+from openlp.core.common.settings import Settings
 from openlp.core.lib import ItemCapabilities, ServiceItem, ServiceItemType, FormattingTags
+from tests.helpers.testmixin import TestMixin
 
 from tests.utils import assert_length, convert_file_service_item
 
@@ -59,19 +61,31 @@ RENDERED_VERSE = 'The Lord said to <span style="-webkit-text-fill-color:red">Noa
 FOOTER = ['Arky Arky (Unknown)', 'Public Domain', 'CCLI 123456']
 TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources', 'service'))
 
+__default_settings__ = {
+    'songs/enable chords': True,
+}
 
-class TestServiceItem(TestCase):
+
+class TestServiceItem(TestCase, TestMixin):
 
     def setUp(self):
         """
         Set up the Registry
         """
+        self.build_settings()
+        Settings().extend_default_settings(__default_settings__)
         Registry.create()
         mocked_renderer = MagicMock()
         mocked_renderer.format_slide.return_value = [VERSE]
         Registry().register('renderer', mocked_renderer)
         Registry().register('image_manager', MagicMock())
 
+    def tearDown(self):
+        """
+        Clean up
+        """
+        self.destroy_settings()
+
     def test_service_item_basic(self):
         """
         Test the Service Item - basic test
@@ -82,8 +96,8 @@ class TestServiceItem(TestCase):
         service_item = ServiceItem(None)
 
         # THEN: We should get back a valid service item
-        self.assertTrue(service_item.is_valid, 'The new service item should be valid')
-        self.assertTrue(service_item.missing_frames(), 'There should not be any frames in the service item')
+        assert service_item.is_valid is True, 'The new service item should be valid'
+        assert service_item.missing_frames() is True, 'There should not be any frames in the service item'
 
     def test_service_item_load_custom_from_service(self):
         """
@@ -99,7 +113,7 @@ class TestServiceItem(TestCase):
         service_item.set_from_service(line)
 
         # THEN: We should get back a valid service item
-        self.assertTrue(service_item.is_valid, 'The new service item should be valid')
+        assert service_item.is_valid, 'The new service item should be valid'
         assert_length(0, service_item._display_frames, 'The service item should have no display frames')
         assert_length(5, service_item.capabilities, 'There should be 5 default custom item capabilities')
 
@@ -107,14 +121,14 @@ class TestServiceItem(TestCase):
         service_item.render(True)
 
         # THEN: The frames should also be valid
-        self.assertEqual('Test Custom', service_item.get_display_title(), 'The title should be "Test Custom"')
-        self.assertEqual(CLEANED_VERSE[:-1], service_item.get_frames()[0]['text'],
-                         'The returned text matches the input, except the last line feed')
-        self.assertEqual(RENDERED_VERSE.split('\n', 1)[0], service_item.get_rendered_frame(1),
-                         'The first line has been returned')
-        self.assertEqual('Slide 1', service_item.get_frame_title(0), '"Slide 1" has been returned as the title')
-        self.assertEqual('Slide 2', service_item.get_frame_title(1), '"Slide 2" has been returned as the title')
-        self.assertEqual('', service_item.get_frame_title(2), 'Blank has been returned as the title of slide 3')
+        assert 'Test Custom' == service_item.get_display_title(), 'The title should be "Test Custom"'
+        assert CLEANED_VERSE[:-1] == service_item.get_frames()[0]['text'], \
+            'The returned text matches the input, except the last line feed'
+        assert RENDERED_VERSE.split('\n', 1)[0] == service_item.get_rendered_frame(1), \
+            'The first line has been returned'
+        assert 'Slide 1' == service_item.get_frame_title(0), '"Slide 1" has been returned as the title'
+        assert 'Slide 2' == service_item.get_frame_title(1), '"Slide 2" has been returned as the title'
+        assert '' == service_item.get_frame_title(2), 'Blank has been returned as the title of slide 3'
 
     def test_service_item_load_image_from_service(self):
         """
@@ -138,26 +152,22 @@ class TestServiceItem(TestCase):
             service_item.set_from_service(line, TEST_PATH)
 
         # THEN: We should get back a valid service item
-        self.assertTrue(service_item.is_valid, 'The new service item should be valid')
-        self.assertEqual(os.path.normpath(test_file), os.path.normpath(service_item.get_rendered_frame(0)),
-                         'The first frame should match the path to the image')
-        self.assertEqual(frame_array, service_item.get_frames()[0],
-                         'The return should match frame array1')
-        self.assertEqual(test_file, service_item.get_frame_path(0),
-                         'The frame path should match the full path to the image')
-        self.assertEqual(image_name, service_item.get_frame_title(0),
-                         'The frame title should match the image name')
-        self.assertEqual(image_name, service_item.get_display_title(),
-                         'The display title should match the first image name')
-        self.assertTrue(service_item.is_image(), 'This service item should be of an "image" type')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanMaintain),
-                        'This service item should be able to be Maintained')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanPreview),
-                        'This service item should be able to be be Previewed')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanLoop),
-                        'This service item should be able to be run in a can be made to Loop')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanAppend),
-                        'This service item should be able to have new items added to it')
+        assert service_item.is_valid is True, 'The new service item should be valid'
+        assert os.path.normpath(test_file) == os.path.normpath(service_item.get_rendered_frame(0)), \
+            'The first frame should match the path to the image'
+        assert frame_array == service_item.get_frames()[0], 'The return should match frame array1'
+        assert test_file == service_item.get_frame_path(0), 'The frame path should match the full path to the image'
+        assert image_name == service_item.get_frame_title(0), 'The frame title should match the image name'
+        assert image_name == service_item.get_display_title(), 'The display title should match the first image name'
+        assert service_item.is_image() is True, 'This service item should be of an "image" type'
+        assert service_item.is_capable(ItemCapabilities.CanMaintain) is True, \
+            'This service item should be able to be Maintained'
+        assert service_item.is_capable(ItemCapabilities.CanPreview) is True, \
+            'This service item should be able to be be Previewed'
+        assert service_item.is_capable(ItemCapabilities.CanLoop) is True, \
+            'This service item should be able to be run in a can be made to Loop'
+        assert service_item.is_capable(ItemCapabilities.CanAppend) is True, \
+            'This service item should be able to have new items added to it'
 
     def test_service_item_load_image_from_local_service(self):
         """
@@ -193,35 +203,33 @@ class TestServiceItem(TestCase):
 
         # This test is copied from service_item.py, but is changed since to conform to
         # new layout of service item. The layout use in serviceitem_image_2.osd is actually invalid now.
-        self.assertTrue(service_item.is_valid, 'The first service item should be valid')
-        self.assertTrue(service_item2.is_valid, 'The second service item should be valid')
+        assert service_item.is_valid is True, 'The first service item should be valid'
+        assert service_item2.is_valid is True, 'The second service item should be valid'
         # These test will fail on windows due to the difference in folder seperators
         if os.name != 'nt':
-            self.assertEqual(test_file1, service_item.get_rendered_frame(0),
-                             'The first frame should match the path to the image')
-            self.assertEqual(test_file2, service_item2.get_rendered_frame(0),
-                             'The Second frame should match the path to the image')
-            self.assertEqual(frame_array1, service_item.get_frames()[0], 'The return should match the frame array1')
-            self.assertEqual(frame_array2, service_item2.get_frames()[0], 'The return should match the frame array2')
-            self.assertEqual(test_file1, service_item.get_frame_path(0),
-                             'The frame path should match the full path to the image')
-            self.assertEqual(test_file2, service_item2.get_frame_path(0),
-                             'The frame path should match the full path to the image')
-        self.assertEqual(image_name1, service_item.get_frame_title(0),
-                         'The 1st frame title should match the image name')
-        self.assertEqual(image_name2, service_item2.get_frame_title(0),
-                         'The 2nd frame title should match the image name')
-        self.assertEqual(service_item.name, service_item.title.lower(),
-                         'The plugin name should match the display title, as there are > 1 Images')
-        self.assertTrue(service_item.is_image(), 'This service item should be of an "image" type')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanMaintain),
-                        'This service item should be able to be Maintained')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanPreview),
-                        'This service item should be able to be be Previewed')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanLoop),
-                        'This service item should be able to be run in a can be made to Loop')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.CanAppend),
-                        'This service item should be able to have new items added to it')
+            assert test_file1 == service_item.get_rendered_frame(0), \
+                'The first frame should match the path to the image'
+            assert test_file2 == service_item2.get_rendered_frame(0), \
+                'The Second frame should match the path to the image'
+            assert frame_array1 == service_item.get_frames()[0], 'The return should match the frame array1'
+            assert frame_array2 == service_item2.get_frames()[0], 'The return should match the frame array2'
+            assert test_file1 == service_item.get_frame_path(0), \
+                'The frame path should match the full path to the image'
+            assert test_file2 == service_item2.get_frame_path(0), \
+                'The frame path should match the full path to the image'
+        assert image_name1 == service_item.get_frame_title(0), 'The 1st frame title should match the image name'
+        assert image_name2 == service_item2.get_frame_title(0), 'The 2nd frame title should match the image name'
+        assert service_item.name == service_item.title.lower(), \
+            'The plugin name should match the display title, as there are > 1 Images'
+        assert service_item.is_image() is True, 'This service item should be of an "image" type'
+        assert service_item.is_capable(ItemCapabilities.CanMaintain) is True, \
+            'This service item should be able to be Maintained'
+        assert service_item.is_capable(ItemCapabilities.CanPreview) is True, \
+            'This service item should be able to be be Previewed'
+        assert service_item.is_capable(ItemCapabilities.CanLoop) is True, \
+            'This service item should be able to be run in a can be made to Loop'
+        assert service_item.is_capable(ItemCapabilities.CanAppend) is True, \
+            'This service item should be able to have new items added to it'
 
     def test_add_from_command_for_a_presentation(self):
         """
@@ -240,8 +248,8 @@ class TestServiceItem(TestCase):
         service_item.add_from_command(TEST_PATH, presentation_name, image, display_title, notes)
 
         # THEN: verify that it is setup as a Command and that the frame data matches
-        self.assertEqual(service_item.service_item_type, ServiceItemType.Command, 'It should be a Command')
-        self.assertEqual(service_item.get_frames()[0], frame, 'Frames should match')
+        assert service_item.service_item_type == ServiceItemType.Command, 'It should be a Command'
+        assert service_item.get_frames()[0] == frame, 'Frames should match'
 
     def test_add_from_comamnd_without_display_title_and_notes(self):
         """
@@ -258,8 +266,8 @@ class TestServiceItem(TestCase):
         service_item.add_from_command(TEST_PATH, image_name, image)
 
         # THEN: verify that it is setup as a Command and that the frame data matches
-        self.assertEqual(service_item.service_item_type, ServiceItemType.Command, 'It should be a Command')
-        self.assertEqual(service_item.get_frames()[0], frame, 'Frames should match')
+        assert service_item.service_item_type == ServiceItemType.Command, 'It should be a Command'
+        assert service_item.get_frames()[0] == frame, 'Frames should match'
 
     @patch(u'openlp.core.lib.serviceitem.ServiceItem.image_manager')
     @patch('openlp.core.lib.serviceitem.AppLocation.get_section_data_path')
@@ -287,9 +295,9 @@ class TestServiceItem(TestCase):
         service_item.add_from_command(TEST_PATH, presentation_name, thumb, display_title, notes)
 
         # THEN: verify that it is setup as a Command and that the frame data matches
-        self.assertEqual(service_item.service_item_type, ServiceItemType.Command, 'It should be a Command')
-        self.assertEqual(service_item.get_frames()[0], frame, 'Frames should match')
-        self.assertEqual(1, mocked_image_manager.add_image.call_count, 'image_manager should be used')
+        assert service_item.service_item_type == ServiceItemType.Command, 'It should be a Command'
+        assert service_item.get_frames()[0] == frame, 'Frames should match'
+        assert 1 == mocked_image_manager.add_image.call_count, 'image_manager should be used'
 
     def test_service_item_load_optical_media_from_service(self):
         """
@@ -306,11 +314,11 @@ class TestServiceItem(TestCase):
             service_item.set_from_service(line)
 
         # THEN: We should get back a valid service item with optical media info
-        self.assertTrue(service_item.is_valid, 'The service item should be valid')
-        self.assertTrue(service_item.is_capable(ItemCapabilities.IsOptical), 'The item should be Optical')
-        self.assertEqual(service_item.start_time, 654.375, 'Start time should be 654.375')
-        self.assertEqual(service_item.end_time, 672.069, 'End time should be 672.069')
-        self.assertEqual(service_item.media_length, 17.694, 'Media length should be 17.694')
+        assert service_item.is_valid is True, 'The service item should be valid'
+        assert service_item.is_capable(ItemCapabilities.IsOptical) is True, 'The item should be Optical' 
+        assert service_item.start_time == 654.375, 'Start time should be 654.375' 
+        assert service_item.end_time == 672.069, 'End time should be 672.069' 
+        assert service_item.media_length == 17.694, 'Media length should be 17.694'
 
     def test_service_item_load_song_and_audio_from_service(self):
         """
@@ -326,22 +334,22 @@ class TestServiceItem(TestCase):
         service_item.set_from_service(line, '/test/')
 
         # THEN: We should get back a valid service item
-        self.assertTrue(service_item.is_valid, 'The new service item should be valid')
-        assert_length(0, service_item._display_frames, 'The service item should have no display frames')
-        assert_length(7, service_item.capabilities, 'There should be 7 default custom item capabilities')
+        assert service_item.is_valid, 'The new service item should be valid'
+        assert 0 == len(service_item._display_frames), 'The service item should have no display frames'
+        assert 7 == len(service_item.capabilities), 'There should be 7 default custom item capabilities'
 
         # WHEN: We render the frames of the service item
         service_item.render(True)
 
         # THEN: The frames should also be valid
-        self.assertEqual('Amazing Grace', service_item.get_display_title(), 'The title should be "Amazing Grace"')
-        self.assertEqual(CLEANED_VERSE[:-1], service_item.get_frames()[0]['text'],
-                         'The returned text matches the input, except the last line feed')
-        self.assertEqual(RENDERED_VERSE.split('\n', 1)[0], service_item.get_rendered_frame(1),
-                         'The first line has been returned')
-        self.assertEqual('Amazing Grace! how sweet the s', service_item.get_frame_title(0),
-                         '"Amazing Grace! how sweet the s" has been returned as the title')
-        self.assertEqual('’Twas grace that taught my hea', service_item.get_frame_title(1),
-                         '"’Twas grace that taught my hea" has been returned as the title')
-        self.assertEqual('/test/amazing_grace.mp3', service_item.background_audio[0],
-                         '"/test/amazing_grace.mp3" should be in the background_audio list')
+        assert 'Amazing Grace' == service_item.get_display_title(), 'The title should be "Amazing Grace"'
+        assert CLEANED_VERSE[:-1] == service_item.get_frames()[0]['text'], \
+            'The returned text matches the input, except the last line feed'
+        assert RENDERED_VERSE.split('\n', 1)[0] == service_item.get_rendered_frame(1), \
+            'The first line has been returned'
+        assert 'Amazing Grace! how sweet the s' == service_item.get_frame_title(0), \
+            '"Amazing Grace! how sweet the s" has been returned as the title'
+        assert '’Twas grace that taught my hea' == service_item.get_frame_title(1), \
+            '"’Twas grace that taught my hea" has been returned as the title'
+        assert '/test/amazing_grace.mp3' == service_item.background_audio[0], \
+            '"/test/amazing_grace.mp3" should be in the background_audio list'
diff --git a/tests/functional/openlp_core/lib/test_theme.py b/tests/functional/openlp_core/lib/test_theme.py
index 93bc06f24..4e6407d1c 100644
--- a/tests/functional/openlp_core/lib/test_theme.py
+++ b/tests/functional/openlp_core/lib/test_theme.py
@@ -90,8 +90,8 @@ class TestTheme(TestCase):
 
         # THEN: The filename of the background should be correct
         expected_filename = path / 'MyBeautifulTheme' / 'video.mp4'
-        self.assertEqual(expected_filename, theme.background_filename)
-        self.assertEqual('MyBeautifulTheme', theme.theme_name)
+        assert expected_filename == theme.background_filename
+        assert 'MyBeautifulTheme' == theme.theme_name
 
     def test_save_retrieve(self):
         """
@@ -107,9 +107,9 @@ class TestTheme(TestCase):
         self.check_theme(lt)
 
     def check_theme(self, theme):
-        self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"')
-        self.assertEqual('solid', theme.background_type, 'background_type should be "solid"')
-        self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0')
-        self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False')
-        self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"')
-        self.assertEqual(47, len(theme.__dict__), 'The theme should have 47 attributes')
+        assert '#000000' == theme.background_border_color, 'background_border_color should be "#000000"'
+        assert 'solid' == theme.background_type, 'background_type should be "solid"'
+        assert 0 == theme.display_vertical_align, 'display_vertical_align should be 0'
+        assert theme.font_footer_bold is False, 'font_footer_bold should be False'
+        assert 'Arial' == theme.font_main_name, 'font_main_name should be "Arial"'
+        assert 47 == len(theme.__dict__), 'The theme should have 47 attributes'
diff --git a/tests/functional/openlp_core/lib/test_ui.py b/tests/functional/openlp_core/lib/test_ui.py
index 8aec310ba..b17597306 100644
--- a/tests/functional/openlp_core/lib/test_ui.py
+++ b/tests/functional/openlp_core/lib/test_ui.py
@@ -49,8 +49,8 @@ class TestUi(TestCase):
         add_welcome_page(wizard, ':/wizards/wizard_firsttime.bmp')
 
         # THEN: The wizard should have one page with a pixmap.
-        self.assertEqual(1, len(wizard.pageIds()), 'The wizard should have one page.')
-        self.assertIsInstance(wizard.page(0).pixmap(QtWidgets.QWizard.WatermarkPixmap), QtGui.QPixmap)
+        assert 1 == len(wizard.pageIds()), 'The wizard should have one page.'
+        assert isinstance(wizard.page(0).pixmap(QtWidgets.QWizard.WatermarkPixmap), QtGui.QPixmap)
 
     def test_create_button_box(self):
         """
@@ -63,22 +63,22 @@ class TestUi(TestCase):
         btnbox = create_button_box(dialog, 'my_btns', ['ok', 'save', 'cancel', 'close', 'defaults'])
 
         # THEN: We should get a QDialogButtonBox with five buttons
-        self.assertIsInstance(btnbox, QtWidgets.QDialogButtonBox)
-        self.assertEqual(5, len(btnbox.buttons()))
+        assert isinstance(btnbox, QtWidgets.QDialogButtonBox)
+        assert 5 == len(btnbox.buttons())
 
         # WHEN: We create the button box with a custom button
         btnbox = create_button_box(dialog, 'my_btns', None, [QtWidgets.QPushButton('Custom')])
         # THEN: We should get a QDialogButtonBox with one button
-        self.assertIsInstance(btnbox, QtWidgets.QDialogButtonBox)
-        self.assertEqual(1, len(btnbox.buttons()))
+        assert isinstance(btnbox, QtWidgets.QDialogButtonBox)
+        assert 1 == len(btnbox.buttons())
 
         # WHEN: We create the button box with a custom button and a custom role
         btnbox = create_button_box(dialog, 'my_btns', None,
                                    [(QtWidgets.QPushButton('Help'), QtWidgets.QDialogButtonBox.HelpRole)])
         # THEN: We should get a QDialogButtonBox with one button with a certain role
-        self.assertIsInstance(btnbox, QtWidgets.QDialogButtonBox)
-        self.assertEqual(1, len(btnbox.buttons()))
-        self.assertEqual(QtWidgets.QDialogButtonBox.HelpRole, btnbox.buttonRole(btnbox.buttons()[0]))
+        assert isinstance(btnbox, QtWidgets.QDialogButtonBox)
+        assert 1 == len(btnbox.buttons())
+        assert QtWidgets.QDialogButtonBox.HelpRole, btnbox.buttonRole(btnbox.buttons()[0])
 
     def test_create_horizontal_adjusting_combo_box(self):
         """
@@ -91,9 +91,9 @@ class TestUi(TestCase):
         combo = create_horizontal_adjusting_combo_box(dialog, 'combo1')
 
         # THEN: We should get a ComboBox
-        self.assertIsInstance(combo, QtWidgets.QComboBox)
-        self.assertEqual('combo1', combo.objectName())
-        self.assertEqual(QtWidgets.QComboBox.AdjustToMinimumContentsLength, combo.sizeAdjustPolicy())
+        assert isinstance(combo, QtWidgets.QComboBox)
+        assert 'combo1' == combo.objectName()
+        assert QtWidgets.QComboBox.AdjustToMinimumContentsLength == combo.sizeAdjustPolicy()
 
     def test_create_button(self):
         """
@@ -106,26 +106,26 @@ class TestUi(TestCase):
         btn = create_button(dialog, 'my_btn')
 
         # THEN: We should get a button with a name
-        self.assertIsInstance(btn, QtWidgets.QPushButton)
-        self.assertEqual('my_btn', btn.objectName())
-        self.assertTrue(btn.isEnabled())
+        assert isinstance(btn, QtWidgets.QPushButton)
+        assert 'my_btn' == btn.objectName()
+        assert btn.isEnabled() is True
 
         # WHEN: We create a button with some attributes
         btn = create_button(dialog, 'my_btn', text='Hello', tooltip='How are you?', enabled=False)
 
         # THEN: We should get a button with those attributes
-        self.assertIsInstance(btn, QtWidgets.QPushButton)
-        self.assertEqual('Hello', btn.text())
-        self.assertEqual('How are you?', btn.toolTip())
-        self.assertFalse(btn.isEnabled())
+        assert isinstance(btn, QtWidgets.QPushButton)
+        assert 'Hello' == btn.text()
+        assert 'How are you?' == btn.toolTip()
+        assert btn.isEnabled() is False
 
         # WHEN: We create a toolbutton
         btn = create_button(dialog, 'my_btn', btn_class='toolbutton')
 
         # THEN: We should get a toolbutton
-        self.assertIsInstance(btn, QtWidgets.QToolButton)
-        self.assertEqual('my_btn', btn.objectName())
-        self.assertTrue(btn.isEnabled())
+        assert isinstance(btn, QtWidgets.QToolButton)
+        assert 'my_btn' == btn.objectName()
+        assert btn.isEnabled() is True
 
     def test_create_action(self):
         """
@@ -138,19 +138,19 @@ class TestUi(TestCase):
         action = create_action(dialog, 'my_action')
 
         # THEN: We should get a QAction
-        self.assertIsInstance(action, QtWidgets.QAction)
-        self.assertEqual('my_action', action.objectName())
+        assert isinstance(action, QtWidgets.QAction)
+        assert 'my_action' == action.objectName()
 
         # WHEN: We create an action with some properties
         action = create_action(dialog, 'my_action', text='my text', icon=':/wizards/wizard_firsttime.bmp',
                                tooltip='my tooltip', statustip='my statustip')
 
         # THEN: These properties should be set
-        self.assertIsInstance(action, QtWidgets.QAction)
-        self.assertEqual('my text', action.text())
-        self.assertIsInstance(action.icon(), QtGui.QIcon)
-        self.assertEqual('my tooltip', action.toolTip())
-        self.assertEqual('my statustip', action.statusTip())
+        assert isinstance(action, QtWidgets.QAction)
+        assert 'my text' == action.text()
+        assert isinstance(action.icon(), QtGui.QIcon)
+        assert 'my tooltip' == action.toolTip() 
+        assert 'my statustip' == action.statusTip()
 
     def test_create_action_on_mac_osx(self):
         """
@@ -186,8 +186,8 @@ class TestUi(TestCase):
             create_action(dialog, 'my_action')
 
             # THEN: setIconVisibleInMenu should not be called
-            self.assertEqual(0, mocked_action.setIconVisibleInMenu.call_count,
-                             'setIconVisibleInMenu should not have been called')
+            assert 0 == mocked_action.setIconVisibleInMenu.call_count, \
+                             'setIconVisibleInMenu should not have been called'
 
     def test_create_checked_disabled_invisible_action(self):
         """
@@ -200,9 +200,9 @@ class TestUi(TestCase):
         action = create_action(dialog, 'my_action', checked=True, enabled=False, visible=False)
 
         # THEN: These properties should be set
-        self.assertTrue(action.isChecked(), 'The action should be checked')
-        self.assertFalse(action.isEnabled(), 'The action should be disabled')
-        self.assertFalse(action.isVisible(), 'The action should be invisble')
+        assert action.isChecked() is True, 'The action should be checked'
+        assert action.isEnabled() is False, 'The action should be disabled'
+        assert action.isVisible() is False, 'The action should be invisble'
 
     def test_create_action_separator(self):
         """
@@ -215,7 +215,7 @@ class TestUi(TestCase):
         action = create_action(dialog, 'my_action', separator=True)
 
         # THEN: The action should be a separator
-        self.assertTrue(action.isSeparator(), 'The action should be a separator')
+        assert action.isSeparator() is True, 'The action should be a separator'
 
     def test_create_valign_selection_widgets(self):
         """
@@ -228,11 +228,11 @@ class TestUi(TestCase):
         label, combo = create_valign_selection_widgets(dialog)
 
         # THEN: We should get a label and a combobox.
-        self.assertEqual(translate('OpenLP.Ui', '&Vertical Align:'), label.text())
-        self.assertIsInstance(combo, QtWidgets.QComboBox)
-        self.assertEqual(combo, label.buddy())
+        assert translate('OpenLP.Ui', '&Vertical Align:') == label.text()
+        assert isinstance(combo, QtWidgets.QComboBox)
+        assert combo == label.buddy()
         for text in [UiStrings().Top, UiStrings().Middle, UiStrings().Bottom]:
-            self.assertTrue(combo.findText(text) >= 0)
+            assert combo.findText(text) >= 0
 
     def test_find_and_set_in_combo_box(self):
         """
@@ -247,19 +247,19 @@ class TestUi(TestCase):
         find_and_set_in_combo_box(combo, 'Four', set_missing=False)
 
         # THEN: The index should not have changed
-        self.assertEqual(1, combo.currentIndex())
+        assert 1 == combo.currentIndex() 
 
         # WHEN: We call the method with a non-existing value
         find_and_set_in_combo_box(combo, 'Four')
 
         # THEN: The index should have been reset
-        self.assertEqual(0, combo.currentIndex())
+        assert 0 == combo.currentIndex()
 
         # WHEN: We call the method with the default behavior
         find_and_set_in_combo_box(combo, 'Three')
 
         # THEN: The index should have changed
-        self.assertEqual(2, combo.currentIndex())
+        assert 2 == combo.currentIndex()
 
     def test_create_widget_action(self):
         """
@@ -272,8 +272,8 @@ class TestUi(TestCase):
         action = create_widget_action(button, 'some action')
 
         # THEN: The action should be returned
-        self.assertIsInstance(action, QtWidgets.QAction)
-        self.assertEqual(action.objectName(), 'some action')
+        assert isinstance(action, QtWidgets.QAction)
+        assert action.objectName() == 'some action'
 
     def test_set_case_insensitive_completer(self):
         """
@@ -288,5 +288,5 @@ class TestUi(TestCase):
 
         # THEN: The Combobox should have a completer which is case insensitive
         completer = line_edit.completer()
-        self.assertIsInstance(completer, QtWidgets.QCompleter)
-        self.assertEqual(completer.caseSensitivity(), QtCore.Qt.CaseInsensitive)
+        assert isinstance(completer, QtWidgets.QCompleter)
+        assert completer.caseSensitivity() == QtCore.Qt.CaseInsensitive

From c2215d1a1e3ec4ec33288e7f8516778a1bef0da9 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Sun, 17 Dec 2017 20:19:19 +0000
Subject: [PATCH 19/20] lib done pep 8

---
 tests/functional/openlp_core/lib/test_serviceitem.py | 6 +++---
 tests/functional/openlp_core/lib/test_ui.py          | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py
index 2ee142be4..f07efd2b4 100644
--- a/tests/functional/openlp_core/lib/test_serviceitem.py
+++ b/tests/functional/openlp_core/lib/test_serviceitem.py
@@ -315,9 +315,9 @@ class TestServiceItem(TestCase, TestMixin):
 
         # THEN: We should get back a valid service item with optical media info
         assert service_item.is_valid is True, 'The service item should be valid'
-        assert service_item.is_capable(ItemCapabilities.IsOptical) is True, 'The item should be Optical' 
-        assert service_item.start_time == 654.375, 'Start time should be 654.375' 
-        assert service_item.end_time == 672.069, 'End time should be 672.069' 
+        assert service_item.is_capable(ItemCapabilities.IsOptical) is True, 'The item should be Optical'
+        assert service_item.start_time == 654.375, 'Start time should be 654.375'
+        assert service_item.end_time == 672.069, 'End time should be 672.069'
         assert service_item.media_length == 17.694, 'Media length should be 17.694'
 
     def test_service_item_load_song_and_audio_from_service(self):
diff --git a/tests/functional/openlp_core/lib/test_ui.py b/tests/functional/openlp_core/lib/test_ui.py
index b17597306..7c1e94f36 100644
--- a/tests/functional/openlp_core/lib/test_ui.py
+++ b/tests/functional/openlp_core/lib/test_ui.py
@@ -149,7 +149,7 @@ class TestUi(TestCase):
         assert isinstance(action, QtWidgets.QAction)
         assert 'my text' == action.text()
         assert isinstance(action.icon(), QtGui.QIcon)
-        assert 'my tooltip' == action.toolTip() 
+        assert 'my tooltip' == action.toolTip()
         assert 'my statustip' == action.statusTip()
 
     def test_create_action_on_mac_osx(self):
@@ -187,7 +187,7 @@ class TestUi(TestCase):
 
             # THEN: setIconVisibleInMenu should not be called
             assert 0 == mocked_action.setIconVisibleInMenu.call_count, \
-                             'setIconVisibleInMenu should not have been called'
+                'setIconVisibleInMenu should not have been called'
 
     def test_create_checked_disabled_invisible_action(self):
         """
@@ -247,7 +247,7 @@ class TestUi(TestCase):
         find_and_set_in_combo_box(combo, 'Four', set_missing=False)
 
         # THEN: The index should not have changed
-        assert 1 == combo.currentIndex() 
+        assert 1 == combo.currentIndex()
 
         # WHEN: We call the method with a non-existing value
         find_and_set_in_combo_box(combo, 'Four')

From 054e6e08b0a79d5a0997f25e64d9554e56f23152 Mon Sep 17 00:00:00 2001
From: Tim Bentley <tim.bentley@gmail.com>
Date: Mon, 18 Dec 2017 17:10:04 +0000
Subject: [PATCH 20/20] Fixes

---
 tests/functional/openlp_core/lib/test_exceptions.py    | 2 +-
 tests/functional/openlp_core/lib/test_image_manager.py | 2 +-
 tests/functional/openlp_core/lib/test_lib.py           | 2 +-
 tests/functional/openlp_core/lib/test_serviceitem.py   | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/functional/openlp_core/lib/test_exceptions.py b/tests/functional/openlp_core/lib/test_exceptions.py
index 7ffc122c1..c0de323b7 100644
--- a/tests/functional/openlp_core/lib/test_exceptions.py
+++ b/tests/functional/openlp_core/lib/test_exceptions.py
@@ -42,4 +42,4 @@ class TestValidationError(TestCase):
 
         # THEN: Then calling str on the error should return the correct text and it should be an instance of `Exception`
         assert str(error) == 'Test ValidationError'
-        assert isinstance(error, Exception) is True
+        assert isinstance(error, Exception)
diff --git a/tests/functional/openlp_core/lib/test_image_manager.py b/tests/functional/openlp_core/lib/test_image_manager.py
index 0b37b412d..3c47e81a1 100644
--- a/tests/functional/openlp_core/lib/test_image_manager.py
+++ b/tests/functional/openlp_core/lib/test_image_manager.py
@@ -98,7 +98,7 @@ class TestImageManager(TestCase, TestMixin):
         image = self.image_manager.get_image(full_path, 'church.jpg', 80, 80)
 
         # THEN: The return should be of type image
-        assert isinstance(image, QtGui.QImage) is True, 'The returned object should be a QImage'
+        assert isinstance(image, QtGui.QImage), 'The returned object should be a QImage'
 
         # WHEN: adding the same image with different dimensions
         self.image_manager.add_image(full_path, 'church.jpg', None, 100, 100)
diff --git a/tests/functional/openlp_core/lib/test_lib.py b/tests/functional/openlp_core/lib/test_lib.py
index 9230ca5b0..7cc44ec4e 100644
--- a/tests/functional/openlp_core/lib/test_lib.py
+++ b/tests/functional/openlp_core/lib/test_lib.py
@@ -293,7 +293,7 @@ class TestLib(TestCase):
         # THEN: Check if the thumb was created and scaled to the given size.
         self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists')
         assert isinstance(icon, QtGui.QIcon), 'The icon should be a QIcon'
-        assert icon.isNull()is False, 'The icon should not be null'
+        assert icon.isNull() is False, 'The icon should not be null'
         assert thumb_size == QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size'
 
         # Remove the thumb so that the test actually tests if the thumb will be created.
diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py
index f07efd2b4..d069f18bf 100644
--- a/tests/functional/openlp_core/lib/test_serviceitem.py
+++ b/tests/functional/openlp_core/lib/test_serviceitem.py
@@ -113,7 +113,7 @@ class TestServiceItem(TestCase, TestMixin):
         service_item.set_from_service(line)
 
         # THEN: We should get back a valid service item
-        assert service_item.is_valid, 'The new service item should be valid'
+        assert service_item.is_valid is True, 'The new service item should be valid'
         assert_length(0, service_item._display_frames, 'The service item should have no display frames')
         assert_length(5, service_item.capabilities, 'There should be 5 default custom item capabilities')
 
@@ -334,7 +334,7 @@ class TestServiceItem(TestCase, TestMixin):
         service_item.set_from_service(line, '/test/')
 
         # THEN: We should get back a valid service item
-        assert service_item.is_valid, 'The new service item should be valid'
+        assert service_item.is_valid is True, 'The new service item should be valid'
         assert 0 == len(service_item._display_frames), 'The service item should have no display frames'
         assert 7 == len(service_item.capabilities), 'There should be 7 default custom item capabilities'