Simon Hanna has proposed merging lp:~thelinuxguy/openlp/fix-version-check into
lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~thelinuxguy/openlp/fix-version-check/+merge/335602
Fixed the version checking to be more robust
* Strip the response so empty responses that contain whitespace are in fact
empty
* Change http to https to result in one less query
* Add test for responses containing white space
* Add .cache to bzrignore (generated by pytest when tests fail)
--
Your team OpenLP Core is requested to review the proposed merge of
lp:~thelinuxguy/openlp/fix-version-check into lp:openlp.
=== modified file '.bzrignore'
--- .bzrignore 2017-10-10 07:08:44 +0000
+++ .bzrignore 2017-12-29 10:42:37 +0000
@@ -45,3 +45,4 @@
output
htmlcov
openlp-test-projectordb.sqlite
+.cache
=== modified file 'openlp/core/version.py'
--- openlp/core/version.py 2017-11-18 11:23:15 +0000
+++ openlp/core/version.py 2017-12-29 10:42:37 +0000
@@ -76,12 +76,12 @@
"""
log.debug('VersionWorker - Start')
# I'm not entirely sure why this was here, I'm commenting it out until I hit the same scenario
- time.sleep(1)
- download_url = 'http://www.openlp.org/files/version.txt'
+ # time.sleep(1)
+ download_url = 'https://www.openlp.org/files/version.txt'
if self.current_version['build']:
- download_url = 'http://www.openlp.org/files/nightly_version.txt'
+ download_url = 'https://www.openlp.org/files/nightly_version.txt'
elif int(self.current_version['version'].split('.')[1]) % 2 != 0:
- download_url = 'http://www.openlp.org/files/dev_version.txt'
+ download_url = 'https://www.openlp.org/files/dev_version.txt'
headers = {
'User-Agent': 'OpenLP/{version} {system}/{release}; '.format(version=self.current_version['full'],
system=platform.system(),
@@ -92,7 +92,7 @@
while retries < 3:
try:
response = requests.get(download_url, headers=headers)
- remote_version = response.text
+ remote_version = response.text.strip()
log.debug('New version found: %s', remote_version)
break
except OSError:
=== modified file 'tests/functional/openlp_core/test_version.py'
--- tests/functional/openlp_core/test_version.py 2017-09-13 06:08:38 +0000
+++ tests/functional/openlp_core/test_version.py 2017-12-29 10:42:37 +0000
@@ -63,7 +63,7 @@
worker.start()
# THEN: The check completes and the signal is emitted
- expected_download_url = 'http://www.openlp.org/files/version.txt'
+ expected_download_url = 'https://www.openlp.org/files/version.txt'
expected_headers = {'User-Agent': 'OpenLP/2.0 Linux/4.12.0-1-amd64; '}
mock_requests.get.assert_called_once_with(expected_download_url, headers=expected_headers)
mock_new_version.emit.assert_called_once_with('2.4.6')
@@ -88,7 +88,7 @@
worker.start()
# THEN: The check completes and the signal is emitted
- expected_download_url = 'http://www.openlp.org/files/dev_version.txt'
+ expected_download_url = 'https://www.openlp.org/files/dev_version.txt'
expected_headers = {'User-Agent': 'OpenLP/2.1.3 Linux/4.12.0-1-amd64; '}
mock_requests.get.assert_called_once_with(expected_download_url, headers=expected_headers)
mock_new_version.emit.assert_called_once_with('2.4.6')
@@ -113,7 +113,7 @@
worker.start()
# THEN: The check completes and the signal is emitted
- expected_download_url = 'http://www.openlp.org/files/nightly_version.txt'
+ expected_download_url = 'https://www.openlp.org/files/nightly_version.txt'
expected_headers = {'User-Agent': 'OpenLP/2.1-bzr2345 Linux/4.12.0-1-amd64; '}
mock_requests.get.assert_called_once_with(expected_download_url, headers=expected_headers)
mock_new_version.emit.assert_called_once_with('2.4.6')
@@ -122,6 +122,31 @@
@patch('openlp.core.version.platform')
@patch('openlp.core.version.requests')
+def test_worker_empty_response(mock_requests, mock_platform):
+ """Test the VersionWorkder.start() method for empty responses"""
+ # GIVEN: A last check date, current version, and an instance of worker
+ last_check_date = '1970-01-01'
+ current_version = {'full': '2.1-bzr2345', 'version': '2.1', 'build': '2345'}
+ mock_platform.system.return_value = 'Linux'
+ mock_platform.release.return_value = '4.12.0-1-amd64'
+ mock_requests.get.return_value = MagicMock(text='\n')
+ worker = VersionWorker(last_check_date, current_version)
+
+ # WHEN: The worker is run
+ with patch.object(worker, 'new_version') as mock_new_version, \
+ patch.object(worker, 'quit') as mock_quit:
+ worker.start()
+
+ # THEN: The check completes and the signal is emitted
+ expected_download_url = 'https://www.openlp.org/files/nightly_version.txt'
+ expected_headers = {'User-Agent': 'OpenLP/2.1-bzr2345 Linux/4.12.0-1-amd64; '}
+ mock_requests.get.assert_called_once_with(expected_download_url, headers=expected_headers)
+ assert mock_new_version.emit.call_count == 0
+ mock_quit.emit.assert_called_once_with()
+
+
+@patch('openlp.core.version.platform')
+@patch('openlp.core.version.requests')
def test_worker_start_connection_error(mock_requests, mock_platform):
"""Test the VersionWorkder.start() method when a ConnectionError happens"""
# GIVEN: A last check date, current version, and an instance of worker
@@ -138,7 +163,7 @@
worker.start()
# THEN: The check completes and the signal is emitted
- expected_download_url = 'http://www.openlp.org/files/version.txt'
+ expected_download_url = 'https://www.openlp.org/files/version.txt'
expected_headers = {'User-Agent': 'OpenLP/2.0 Linux/4.12.0-1-amd64; '}
mock_requests.get.assert_called_with(expected_download_url, headers=expected_headers)
assert mock_requests.get.call_count == 3
_______________________________________________
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
