diff --git a/tests/api_jobs/test_base.py b/tests/api_jobs/test_base.py index be9f29d2f..26a9b1761 100644 --- a/tests/api_jobs/test_base.py +++ b/tests/api_jobs/test_base.py @@ -1,6 +1,6 @@ import pytest -from sdclientapi import AuthError, RequestTimeoutError +from sdclientapi import AuthError, RequestTimeoutError, ServerConnectionError from securedrop_client.api_jobs.base import ApiInaccessibleError, ApiJob from tests.factory import dummy_job_factory @@ -68,15 +68,18 @@ def test_ApiJob_auth_error(mocker): assert not api_job.failure_signal.emit.called -def test_ApiJob_timeout_error(mocker): - return_value = RequestTimeoutError() +@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError]) +def test_ApiJob_timeout_error(mocker, exception): + """If the server times out or is unreachable, the corresponding + exception should be raised""" + return_value = exception() api_job_cls = dummy_job_factory(mocker, return_value) api_job = api_job_cls() mock_api_client = mocker.MagicMock() mock_session = mocker.MagicMock() - with pytest.raises(RequestTimeoutError): + with pytest.raises(exception): api_job._do_call_api(mock_api_client, mock_session) assert not api_job.success_signal.emit.called @@ -98,12 +101,13 @@ def test_ApiJob_other_error(mocker): api_job.failure_signal.emit.assert_called_once_with(return_value) -def test_ApiJob_retry_suceeds_after_failed_attempt(mocker): +@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError]) +def test_ApiJob_retry_suceeds_after_failed_attempt(mocker, exception): """Retry logic: after failed attempt should succeed""" number_of_attempts = 5 success_return_value = 'now works' - return_values = [RequestTimeoutError(), success_return_value] + return_values = [exception(), success_return_value] api_job_cls = dummy_job_factory(mocker, return_values, remaining_attempts=number_of_attempts) api_job = api_job_cls() @@ -119,12 +123,13 @@ def test_ApiJob_retry_suceeds_after_failed_attempt(mocker): api_job.success_signal.emit.assert_called_once_with(success_return_value) -def test_ApiJob_retry_exactly_n_attempts_times(mocker): +@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError]) +def test_ApiJob_retry_exactly_n_attempts_times(mocker, exception): """Retry logic: boundary value case - 5th attempt should succeed""" number_of_attempts = 5 success_return_value = 'now works' - return_values = [RequestTimeoutError()] * (number_of_attempts - 1) + [success_return_value] + return_values = [exception()] * (number_of_attempts - 1) + [success_return_value] api_job_cls = dummy_job_factory(mocker, return_values, remaining_attempts=number_of_attempts) api_job = api_job_cls() @@ -140,11 +145,12 @@ def test_ApiJob_retry_exactly_n_attempts_times(mocker): api_job.success_signal.emit.assert_called_once_with(success_return_value) -def test_ApiJob_retry_timeout(mocker): +@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError]) +def test_ApiJob_retry_timeout(mocker, exception): """Retry logic: If we exceed the number of attempts, the job will still fail""" number_of_attempts = 5 - return_values = [RequestTimeoutError()] * (number_of_attempts + 1) + return_values = [exception()] * (number_of_attempts + 1) api_job_cls = dummy_job_factory(mocker, return_values, remaining_attempts=number_of_attempts) api_job = api_job_cls() @@ -152,7 +158,7 @@ def test_ApiJob_retry_timeout(mocker): mock_api_client = mocker.MagicMock() mock_session = mocker.MagicMock() - with pytest.raises(RequestTimeoutError): + with pytest.raises(exception): api_job._do_call_api(mock_api_client, mock_session) assert api_job.remaining_attempts == 0 diff --git a/tests/api_jobs/test_uploads.py b/tests/api_jobs/test_uploads.py index 4e2ba3c25..19f8c2b38 100644 --- a/tests/api_jobs/test_uploads.py +++ b/tests/api_jobs/test_uploads.py @@ -218,11 +218,14 @@ def test_send_reply_failure_unknown_error(homedir, mocker, session, session_make assert len(drafts) == 1 +@pytest.mark.parametrize("exception", + [sdclientapi.RequestTimeoutError, + sdclientapi.ServerConnectionError]) def test_send_reply_failure_timeout_error(homedir, mocker, session, session_maker, - reply_status_codes): + reply_status_codes, exception): ''' - Check that if the SendReplyJob api call fails because of a RequestTimeoutError that a - SendReplyJobTimeoutError is raised. + Check that if the SendReplyJob api call fails because of a RequestTimeoutError or + ServerConnectionError that a SendReplyJobTimeoutError is raised. ''' source = factory.Source() session.add(source) @@ -230,7 +233,7 @@ def test_send_reply_failure_timeout_error(homedir, mocker, session, session_make session.add(draft_reply) session.commit() api_client = mocker.MagicMock() - mocker.patch.object(api_client, 'reply_source', side_effect=sdclientapi.RequestTimeoutError) + mocker.patch.object(api_client, 'reply_source', side_effect=exception) gpg = GpgHelper(homedir, session_maker, is_qubes=False) encrypt_fn = mocker.patch.object(gpg, 'encrypt_to_source') job = SendReplyJob(source.uuid, 'mock_reply_uuid', 'mock_message', gpg) diff --git a/tests/test_logic.py b/tests/test_logic.py index 6252cb07b..097335110 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -7,7 +7,7 @@ import pytest from PyQt5.QtCore import Qt -from sdclientapi import RequestTimeoutError +from sdclientapi import RequestTimeoutError, ServerConnectionError from tests import factory from securedrop_client import db @@ -1430,13 +1430,14 @@ def test_Controller_resume_queues(homedir, mocker, session_maker): co.api_job_queue.resume_queues.assert_called_once_with() -def test_APICallRunner_api_call_timeout(mocker): +@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError]) +def test_APICallRunner_api_call_timeout(mocker, exception): """ - Ensure that if a RequestTimeoutError is raised, both the failure and timeout signals are - emitted. + Ensure that if a RequestTimeoutError or ServerConnectionError is raised, both + the failure and timeout signals are emitted. """ mock_api = mocker.MagicMock() - mock_api.fake_request = mocker.MagicMock(side_effect=RequestTimeoutError()) + mock_api.fake_request = mocker.MagicMock(side_effect=exception()) runner = APICallRunner(mock_api.fake_request) diff --git a/tests/test_queue.py b/tests/test_queue.py index df6bb02c6..d7117709f 100644 --- a/tests/test_queue.py +++ b/tests/test_queue.py @@ -1,8 +1,10 @@ ''' Testing for the ApiJobQueue and related classes. ''' +import pytest + from queue import Queue -from sdclientapi import RequestTimeoutError +from sdclientapi import RequestTimeoutError, ServerConnectionError from securedrop_client.api_jobs.downloads import FileDownloadJob from securedrop_client.api_jobs.base import ApiInaccessibleError, PauseQueueJob @@ -44,19 +46,21 @@ def test_RunnableQueue_happy_path(mocker): assert queue.queue.empty() -def test_RunnableQueue_job_timeout(mocker): +@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError]) +def test_RunnableQueue_job_timeout(mocker, exception): ''' Add two jobs to the queue. The first times out, and then gets resubmitted for the next pass through the loop. ''' queue = RunnableQueue(mocker.MagicMock(), mocker.MagicMock()) queue.pause = mocker.MagicMock() - job_cls = factory.dummy_job_factory(mocker, RequestTimeoutError(), remaining_attempts=5) + job_cls = factory.dummy_job_factory(mocker, exception(), remaining_attempts=5) job1 = job_cls() job2 = job_cls() queue.JOB_PRIORITIES = {PauseQueueJob: 0, job_cls: 1} - # RequestTimeoutError will cause the queue to pause, use our fake pause method instead + # RequestTimeoutError or ServerConnectionError will cause the queue to pause, + # use our fake pause method instead def fake_pause() -> None: queue.add_job(PauseQueueJob()) queue.pause.emit = fake_pause diff --git a/tests/test_sync.py b/tests/test_sync.py index d69ea7221..cf6fe93b2 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1,4 +1,6 @@ -from sdclientapi import RequestTimeoutError +import pytest + +from sdclientapi import RequestTimeoutError, ServerConnectionError from securedrop_client.api_jobs.base import ApiInaccessibleError from securedrop_client.sync import ApiSync @@ -153,7 +155,7 @@ def test_ApiSync_on_sync_success(mocker, session_maker, homedir): def test_ApiSync_on_sync_failure(mocker, session_maker, homedir): ''' Ensure failure handler emits failure signal that the Controller links to and does not fire - another sync for errors other than RequestTimeoutError + another sync for errors other than RequestTimeoutError or ServerConnectionError ''' api_sync = ApiSync(mocker.MagicMock(), session_maker, mocker.MagicMock(), homedir) sync_failure = mocker.patch.object(api_sync, 'sync_failure') @@ -167,15 +169,17 @@ def test_ApiSync_on_sync_failure(mocker, session_maker, homedir): singleShot_fn.assert_not_called() -def test_ApiSync_on_sync_failure_because_of_timeout(mocker, session_maker, homedir): +@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError]) +def test_ApiSync_on_sync_failure_because_of_timeout(mocker, session_maker, homedir, exception): ''' Ensure failure handler emits failure signal that the Controller links to and sets up timer to - fire another sync after 15 seconds if the failure reason is a RequestTimeoutError. + fire another sync after 15 seconds if the failure reason is a RequestTimeoutError or + ServerConnectionError. ''' api_sync = ApiSync(mocker.MagicMock(), session_maker, mocker.MagicMock(), homedir) sync_failure = mocker.patch.object(api_sync, 'sync_failure') singleShot_fn = mocker.patch('securedrop_client.sync.QTimer.singleShot') - error = RequestTimeoutError() + error = exception() api_sync.on_sync_failure(error)