Re: [Openlp-core] [Merge] lp:~trb143/openlp/asserts into lp:openlp
Review: Needs Fixing
generally ok, but there's several assert statements where you have change the
meaning, checking for Falsey/Truthy values rather than checking that the the
result is True, is False, is None, etc.
I've highlight a few with inline comments, but there are some others.
Diff comments:
>
> === modified file 'tests/functional/openlp_core/common/test_init.py'
> --- tests/functional/openlp_core/common/test_init.py 2017-11-18 23:14:28
> +
> +++ tests/functional/openlp_core/common/test_init.py 2017-12-09 16:45:47
> +
> @@ -259,7 +258,7 @@
> 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"
wouldn't 'assert result is False' be better?
>
> def test_delete_file_path_success(self):
> """
> @@ -272,7 +271,7 @@
> 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'
Same again here
>
> def test_delete_file_path_no_file_exists(self):
> """
> @@ -364,4 +363,4 @@
>
> # 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
and here 'assert result is None'
--
https://code.launchpad.net/~trb143/openlp/asserts/+merge/335002
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Merge] lp:~trb143/openlp/asserts into lp:openlp
Tim Bentley has proposed merging lp:~trb143/openlp/asserts into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~trb143/openlp/asserts/+merge/335002
Refactor asserts in api and common packages
lp:~trb143/openlp/asserts (revision 2806)
https://ci.openlp.io/job/Branch-01-Pull/2346/ [SUCCESS]
https://ci.openlp.io/job/Branch-02-Functional-Tests/2247/ [SUCCESS]
https://ci.openlp.io/job/Branch-03-Interface-Tests/2113/ [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code_Analysis/1439/[SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test_Coverage/1257/[SUCCESS]
https://ci.openlp.io/job/Branch-04c-Code_Analysis2/387/[SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/216/ [FAILURE]
Stopping after failure
--
Your team OpenLP Core is requested to review the proposed merge of
lp:~trb143/openlp/asserts into lp:openlp.
=== modified file 'tests/functional/openlp_core/api/http/test_error.py'
--- tests/functional/openlp_core/api/http/test_error.py 2017-03-04 19:17:59 +
+++ tests/functional/openlp_core/api/http/test_error.py 2017-12-09 16:45:47 +
@@ -42,8 +42,8 @@
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 @@
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'
=== modified file 'tests/functional/openlp_core/api/http/test_http.py'
--- tests/functional/openlp_core/api/http/test_http.py 2017-12-02 10:52:13 +
+++ tests/functional/openlp_core/api/http/test_http.py 2017-12-09 16:45:47 +
@@ -53,8 +53,8 @@
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 == 1, '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 @@
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 == 0, 'The http thread should not have been called'
=== modified file 'tests/functional/openlp_core/api/http/test_wsgiapp.py'
--- tests/functional/openlp_core/api/http/test_wsgiapp.py 2017-08-12 20:08:12 +
+++ tests/functional/openlp_core/api/http/test_wsgiapp.py 2017-12-09 16:45:47 +
@@ -61,7 +61,7 @@
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 @@
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')
=== modified file 'tests/functional/openlp_core/api/test_deploy.py'
--- tests/functional/openlp_core/api/test_deploy.py 2017-11-18 11:23:15 +
+++ tests/functional/openlp_core/api/test_deploy.py 2017-12-09 16:45:47 +
@@ -58,4 +58,4 @@
deploy_zipfile(self.app_root_path, 'site.zip')
# THEN: test if www director
[Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-m into lp:openlp
The proposal to merge lp:~alisonken1/openlp/pjlink2-m into lp:openlp has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~alisonken1/openlp/pjlink2-m/+merge/335001 -- Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-m into lp:openlp
Review: Approve -- https://code.launchpad.net/~alisonken1/openlp/pjlink2-m/+merge/335001 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-m into lp:openlp
Review: Approve Great. Thanks for those changes :-) -- https://code.launchpad.net/~alisonken1/openlp/pjlink2-m/+merge/335001 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-m into lp:openlp
Ken Roberts has proposed merging lp:~alisonken1/openlp/pjlink2-m into lp:openlp.
Commit message:
PJLink2 update M
Requested reviews:
Tim Bentley (trb143)
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-m/+merge/335001
- Added pjlink.process_pjlink
- Split pjlink.check_login() to use process_pjlink()
- Added QAbstractSocket connect enum to constants
- Minor code cleanups for connection and command processing
- Updated packet queueing
- Fix get_object_filtered()
- Fix tests in test_projector_pjlink_base
- Fix tests in test_projector_pjlink_cmd_routing
- Added tests for process_pjlink method
- Updated test_projector_bugfixes_01
- Some OLP style cleanups
lp:~alisonken1/openlp/pjlink2-m (revision 2796)
https://ci.openlp.io/job/Branch-01-Pull/2339/ [SUCCESS]
https://ci.openlp.io/job/Branch-02-Functional-Tests/2240/ [SUCCESS]
https://ci.openlp.io/job/Branch-03-Interface-Tests/2110/ [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code_Analysis/1436/[SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test_Coverage/1255/[SUCCESS]
https://ci.openlp.io/job/Branch-04c-Code_Analysis2/385/[SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/214/ [FAILURE]
--
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/projectors/__init__.py'
--- openlp/core/projectors/__init__.py 2017-11-16 23:53:53 +
+++ openlp/core/projectors/__init__.py 2017-12-09 11:19:42 +
@@ -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):
"""
=== modified file 'openlp/core/projectors/constants.py'
--- openlp/core/projectors/constants.py 2017-11-10 11:59:38 +
+++ openlp/core/projectors/constants.py 2017-12-09 11:19:42 +
@@ -144,6 +144,24 @@
}
}
+# 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.
=== modified file 'openlp/core/projectors/db.py'
--- openlp/core/projectors/db.py 2017-11-10 11:59:38 +
+++ openlp/core/projectors/db.py 2017-12-09 11:19:42 +
@@ -415,7 +415,7 @@
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:
=== modified file 'openlp/core/projectors/pjlink.py'
--- openlp/core/projectors/pjlink.py 2017-11-24 19:08:23 +
+++ openlp/core/projectors/pjlink.py 2017-12-09 11:19:42 +
@@ -58,8 +58,7 @@
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')
@@ -111,7 +110,7 @@
"""
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,7 +122,9 @@
'INST': self.process_inst,
'LAMP': self.process_lamp,
'NAME': self.process_name,
-'PJLINK': self.check_login,
+'PJLINK': self.process_pjlink,
+# TODO: Part of check_login refactor - remove when done
+# 'PJLINK': se
[Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-m into lp:openlp
The proposal to merge lp:~alisonken1/openlp/pjlink2-m into lp:openlp has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~alisonken1/openlp/pjlink2-m/+merge/334722 -- Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
