Skip to content

Commit

Permalink
test: ensure ServerConnectionError treated as RequestTimeoutError
Browse files Browse the repository at this point in the history
these two exceptions should behave the same from the perspective
of the client. At some later point, we may want to customize
behavior (e.g. since the ServerConnectionError will be raised
much faster than a RequestTimeoutError).
  • Loading branch information
redshiftzero authored and sssoleileraaa committed Feb 13, 2020
1 parent 9fad57a commit 6cb78d4
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 29 deletions.
28 changes: 17 additions & 11 deletions tests/api_jobs/test_base.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -140,19 +145,20 @@ 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()

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
Expand Down
11 changes: 7 additions & 4 deletions tests/api_jobs/test_uploads.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,22 @@ 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)
draft_reply = factory.DraftReply(uuid='mock_reply_uuid')
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)
Expand Down
11 changes: 6 additions & 5 deletions tests/test_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
12 changes: 8 additions & 4 deletions tests/test_queue.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions tests/test_sync.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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')
Expand All @@ -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)

Expand Down

0 comments on commit 6cb78d4

Please sign in to comment.