From a3a5504487f673e3d40ad28b211dbcc3708d97d9 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Tue, 26 Nov 2024 12:39:22 -0800 Subject: [PATCH] WIP: Display download progress for file downloads Display a vanilla progress bar for file downloads that simply shows how much of the file has been downloaded so far. A new FileDownloadProgressBar widget replaces the existing animated spinner, and we inject a signal through to the SDK to pass the current progress back to the widget. The widget both displays the overall total progress as a percentage and also calculates the download speed by using an exponential moving average to smooth it out. A timer runs every 100ms to recalculate the speed. A new utils.humanize_speed() is used to translate the raw bytes/second into a human-readable version with a focus on keeping the length of the string roughly consistent so there's less visual shifting. Once the download has finished, an indeterminate progress bar is shown while decrypting since we don't (currently) have a way to report specific progress on that. TODO: * tests? Fixes #1104. --- .../securedrop_client/api_jobs/downloads.py | 10 +- client/securedrop_client/gui/base/__init__.py | 2 + client/securedrop_client/gui/base/progress.py | 114 ++++++++++++++++++ client/securedrop_client/gui/widgets.py | 33 +++-- client/securedrop_client/logic.py | 21 +++- client/securedrop_client/sdk/__init__.py | 25 +++- client/securedrop_client/utils.py | 30 +++++ client/tests/api_jobs/test_downloads.py | 25 +++- client/tests/gui/test_widgets.py | 36 ++---- .../test_styles_file_download_button.py | 26 ---- client/tests/sdk/utils.py | 2 + client/tests/test_utils.py | 16 +++ 12 files changed, 255 insertions(+), 85 deletions(-) create mode 100644 client/securedrop_client/gui/base/progress.py diff --git a/client/securedrop_client/api_jobs/downloads.py b/client/securedrop_client/api_jobs/downloads.py index 7499bd69b..c9bede8bd 100644 --- a/client/securedrop_client/api_jobs/downloads.py +++ b/client/securedrop_client/api_jobs/downloads.py @@ -12,6 +12,7 @@ from securedrop_client.api_jobs.base import SingleObjectApiJob from securedrop_client.crypto import CryptoError, GpgHelper from securedrop_client.db import DownloadError, DownloadErrorCodes, File, Message, Reply +from securedrop_client.gui.base.progress import ProgressProxy from securedrop_client.sdk import API, BaseError from securedrop_client.sdk import Reply as SdkReply from securedrop_client.sdk import Submission as SdkSubmission @@ -56,6 +57,7 @@ class DownloadJob(SingleObjectApiJob): def __init__(self, data_dir: str, uuid: str) -> None: super().__init__(uuid) self.data_dir = data_dir + self.progress: ProgressProxy | None = None def _get_realistic_timeout(self, size_in_bytes: int) -> int: """ @@ -177,6 +179,8 @@ def _decrypt(self, filepath: str, db_object: File | Message | Reply, session: Se """ Decrypt the file located at the given filepath and mark it as decrypted. """ + if self.progress: + self.progress.set_decrypting() try: original_filename = self.call_decrypt(filepath, session) db_object.download_error = None @@ -260,7 +264,7 @@ def call_download_api(self, api: API, db_object: Reply) -> tuple[str, str]: # will want to pass the default request timeout to download_reply instead of setting it on # the api object directly. api.default_request_timeout = 20 - return api.download_reply(sdk_object) + return api.download_reply(sdk_object, progress=self.progress) def call_decrypt(self, filepath: str, session: Session | None = None) -> str: """ @@ -316,7 +320,7 @@ def call_download_api(self, api: API, db_object: Message) -> tuple[str, str]: sdk_object.source_uuid = db_object.source.uuid sdk_object.filename = db_object.filename return api.download_submission( - sdk_object, timeout=self._get_realistic_timeout(db_object.size) + sdk_object, timeout=self._get_realistic_timeout(db_object.size), progress=self.progress ) def call_decrypt(self, filepath: str, session: Session | None = None) -> str: @@ -375,7 +379,7 @@ def call_download_api(self, api: API, db_object: File) -> tuple[str, str]: sdk_object.source_uuid = db_object.source.uuid sdk_object.filename = db_object.filename return api.download_submission( - sdk_object, timeout=self._get_realistic_timeout(db_object.size) + sdk_object, timeout=self._get_realistic_timeout(db_object.size), progress=self.progress ) def call_decrypt(self, filepath: str, session: Session | None = None) -> str: diff --git a/client/securedrop_client/gui/base/__init__.py b/client/securedrop_client/gui/base/__init__.py index 0b924e79b..1dbd3e074 100644 --- a/client/securedrop_client/gui/base/__init__.py +++ b/client/securedrop_client/gui/base/__init__.py @@ -26,8 +26,10 @@ SvgPushButton, SvgToggleButton, ) +from securedrop_client.gui.base.progress import FileDownloadProgressBar __all__ = [ + "FileDownloadProgressBar", "ModalDialog", "PasswordEdit", "SDPushButton", diff --git a/client/securedrop_client/gui/base/progress.py b/client/securedrop_client/gui/base/progress.py new file mode 100644 index 000000000..3886af7f5 --- /dev/null +++ b/client/securedrop_client/gui/base/progress.py @@ -0,0 +1,114 @@ +import math +import time + +from PyQt5.QtCore import QTimer, pyqtBoundSignal, pyqtSignal +from PyQt5.QtWidgets import QProgressBar + +from securedrop_client.utils import humanize_speed + +SMOOTHING_FACTOR = 0.3 + + +class FileDownloadProgressBar(QProgressBar): + """ + A progress bar for file downloads. + + It receives progress updates from the SDK and updates the total % downloaded, + as well as calculating the current speed. + + We use an exponential moving average to smooth out the speed as suggested by + https://stackoverflow.com/a/3841706; the reported speed is 30% of the current + speed and 70% of the previous speed. It is updated every 100ms. + """ + + size_signal = pyqtSignal(int) + decrypting_signal = pyqtSignal(bool) + + def __init__(self, file_size: int) -> None: + super().__init__() + self.setObjectName("FileDownloadProgressBar") + self.setMaximum(file_size) + # n.b. we only update the bar's value and let the text get updated by + # the timer in update_speed + self.size_signal.connect(self.setValue) + self.decrypting_signal.connect(self.handle_decrypting) + self.timer = QTimer(self) + self.timer.setInterval(100) + self.timer.timeout.connect(self.update_speed) + self.timer.start() + # The most recently calculated speed + self.speed = 0.0 + # The last time we updated the speed + self.last_total_time = 0.0 + # The number of bytes downloaded the last time we updated the speed + self.last_total_bytes = 0 + + def handle_decrypting(self, decrypting: bool) -> None: + if decrypting: + # Stop the speed timer and then switch to an indeterminate progress bar + self.timer.stop() + self.setMaximum(0) + self.setValue(0) + + def update_display(self) -> None: + """Update the text displayed in the progress bar.""" + # Use math.floor so we don't show 100% until we're actually done + maximum = self.maximum() + if maximum == 0: + # Race condition: we've likely switched to the indeterminate progress bar + # which has a maximum of 0. Treat it like 100% even though it won't show up + # just to avoid the DivisionByZero error. + percentage = 100 + else: + percentage = math.floor((self.value() / maximum) * 100) + formatted_speed = humanize_speed(self.speed) + # TODO: localize this? + if percentage in (0, 100): + # If haven't started or have finished, don't display a 0B/s speed + self.setFormat(f"{percentage}%") + else: + self.setFormat(f"{percentage}% | {formatted_speed}") + + def update_speed(self) -> None: + """Calculate the new speed and trigger updating the display.""" + now = time.monotonic() + value = self.value() + + # If this is the first update we report the speed as 0 + if self.last_total_time == 0: + self.last_total_time = now + self.last_total_bytes = value + self.speed = 0 + return + + time_diff = now - self.last_total_time + bytes_diff = value - self.last_total_bytes + if time_diff > 0: + self.speed = ( + 1 - SMOOTHING_FACTOR + ) * self.speed + SMOOTHING_FACTOR * bytes_diff / time_diff + + self.last_total_time = now + self.last_total_bytes = value + self.update_display() + + def proxy(self) -> "ProgressProxy": + """Get a proxy that updates this widget.""" + return ProgressProxy(self.size_signal, self.decrypting_signal) + + +class ProgressProxy: + """ + Relay the current download progress over to the UI; see + the FileDownloadProgressBar widget. + """ + + def __init__(self, size_signal: pyqtBoundSignal, decrypting_signal: pyqtBoundSignal) -> None: + self.size_signal = size_signal + self.decrypting_signal = decrypting_signal + + def set_value(self, value: int) -> None: + self.size_signal.emit(value) + + def set_decrypting(self) -> None: + self.decrypting_signal.emit(True) diff --git a/client/securedrop_client/gui/widgets.py b/client/securedrop_client/gui/widgets.py index 1ab4524c3..d785d8572 100644 --- a/client/securedrop_client/gui/widgets.py +++ b/client/securedrop_client/gui/widgets.py @@ -83,7 +83,13 @@ ExportConversationTranscriptAction, PrintConversationAction, ) -from securedrop_client.gui.base import SecureQLabel, SvgLabel, SvgPushButton, SvgToggleButton +from securedrop_client.gui.base import ( + FileDownloadProgressBar, + SecureQLabel, + SvgLabel, + SvgPushButton, + SvgToggleButton, +) from securedrop_client.gui.conversation import DeleteConversationDialog from securedrop_client.gui.datetime_helpers import format_datetime_local from securedrop_client.gui.shortcuts import Shortcuts @@ -2579,7 +2585,8 @@ def __init__( self.download_button.setIcon(load_icon("download_file.svg")) self.download_button.setFont(self.file_buttons_font) self.download_button.setCursor(QCursor(Qt.PointingHandCursor)) - self.download_animation = load_movie("download_file.gif") + self.download_progress = FileDownloadProgressBar(self.file.size) + self.download_progress.hide() self.export_button = QPushButton(_("EXPORT")) self.export_button.setObjectName("FileWidget_export_print") self.export_button.setFont(self.file_buttons_font) @@ -2590,6 +2597,9 @@ def __init__( self.print_button.setFont(self.file_buttons_font) self.print_button.setCursor(QCursor(Qt.PointingHandCursor)) file_options_layout.addWidget(self.download_button) + file_options_layout.addWidget(self.download_progress) + # Add a bit of padding after the progress bar + file_options_layout.addSpacing(5) file_options_layout.addWidget(self.export_button) file_options_layout.addWidget(self.middot) file_options_layout.addWidget(self.print_button) @@ -2675,6 +2685,7 @@ def _set_file_state(self) -> None: logger.debug(f"Changing file {self.uuid} state to decrypted/downloaded") self._set_file_name() self.download_button.hide() + self.download_progress.hide() self.no_file_name.hide() self.export_button.show() self.middot.show() @@ -2693,6 +2704,7 @@ def _set_file_state(self) -> None: self.download_button.setFont(self.file_buttons_font) self.download_button.show() + self.download_progress.hide() # Reset stylesheet self.download_button.setStyleSheet("") @@ -2793,15 +2805,17 @@ def _on_left_click(self) -> None: if self.controller.api: self.start_button_animation() # Download the file. - self.controller.on_submission_download(File, self.uuid) + self.controller.on_submission_download( + File, self.uuid, self.download_progress.proxy() + ) def start_button_animation(self) -> None: """ Update the download button to the animated "downloading" state. """ self.downloading = True - self.download_animation.frameChanged.connect(self.set_button_animation_frame) - self.download_animation.start() + self.download_progress.setValue(0) + self.download_progress.show() self.download_button.setText(_(" DOWNLOADING ")) # Reset widget stylesheet @@ -2809,18 +2823,11 @@ def start_button_animation(self) -> None: self.download_button.setObjectName("FileWidget_download_button_animating") self.download_button.setStyleSheet(self.DOWNLOAD_BUTTON_CSS) - def set_button_animation_frame(self, frame_number: int) -> None: - """ - Sets the download button's icon to the current frame of the spinner - animation. - """ - self.download_button.setIcon(QIcon(self.download_animation.currentPixmap())) - def stop_button_animation(self) -> None: """ Stops the download animation and restores the button to its default state. """ - self.download_animation.stop() + self.download_progress.hide() file = self.controller.get_file(self.file.uuid) if file is None: self.deleteLater() diff --git a/client/securedrop_client/logic.py b/client/securedrop_client/logic.py index dc5b04275..e24f15638 100644 --- a/client/securedrop_client/logic.py +++ b/client/securedrop_client/logic.py @@ -59,8 +59,13 @@ SendReplyJobTimeoutError, ) from securedrop_client.crypto import GpgHelper +from securedrop_client.gui.base.progress import ProgressProxy from securedrop_client.queue import ApiJobQueue -from securedrop_client.sdk import AuthError, RequestTimeoutError, ServerConnectionError +from securedrop_client.sdk import ( + AuthError, + RequestTimeoutError, + ServerConnectionError, +) from securedrop_client.sync import ApiSync from securedrop_client.utils import check_dir_permissions @@ -825,7 +830,11 @@ def set_status(self, message: str, duration: int = 5000) -> None: @login_required def _submit_download_job( - self, object_type: type[db.Reply] | type[db.Message] | type[db.File], uuid: str + self, + object_type: type[db.Reply] | type[db.Message] | type[db.File], + uuid: str, + # progress is only reported for file downloads + progress: ProgressProxy | None = None, ) -> None: if object_type == db.Reply: job: ReplyDownloadJob | MessageDownloadJob | FileDownloadJob = ReplyDownloadJob( @@ -842,6 +851,7 @@ def _submit_download_job( job.success_signal.connect(self.on_file_download_success) job.failure_signal.connect(self.on_file_download_failure) + job.progress = progress self.add_job.emit(job) def download_new_messages(self) -> None: @@ -974,12 +984,15 @@ def on_file_open(self, file: db.File) -> None: @login_required def on_submission_download( - self, submission_type: type[db.File] | type[db.Message], submission_uuid: str + self, + submission_type: type[db.File] | type[db.Message], + submission_uuid: str, + progress: ProgressProxy | None = None, ) -> None: """ Download the file associated with the Submission (which may be a File or Message). """ - self._submit_download_job(submission_type, submission_uuid) + self._submit_download_job(submission_type, submission_uuid, progress) def on_file_download_success(self, uuid: str) -> None: """ diff --git a/client/securedrop_client/sdk/__init__.py b/client/securedrop_client/sdk/__init__.py index 8034bc020..4ce7685e9 100644 --- a/client/securedrop_client/sdk/__init__.py +++ b/client/securedrop_client/sdk/__init__.py @@ -10,6 +10,7 @@ from typing import Any from securedrop_client.config import Config +from securedrop_client.gui.base.progress import ProgressProxy from .sdlocalobjects import ( AuthError, @@ -131,7 +132,10 @@ def _rpc_target(self) -> list: return [f"{target_directory}/debug/securedrop-proxy"] def _streaming_download( - self, data: dict[str, Any], env: dict + self, + data: dict[str, Any], + env: dict, + progress: ProgressProxy | None = None, ) -> StreamedResponse | JSONResponse: fobj = tempfile.TemporaryFile("w+b") # noqa: SIM115 @@ -184,6 +188,8 @@ def _streaming_download( fobj.write(chunk) bytes_written += len(chunk) + if progress is not None: + progress.set_value(bytes_written) logger.debug(f"Retry {retry}, bytes written: {bytes_written:,}") # Wait for the process to end @@ -309,6 +315,7 @@ def _send_json_request( body: str | None = None, headers: dict[str, str] | None = None, timeout: int | None = None, + progress: ProgressProxy | None = None, ) -> StreamedResponse | JSONResponse: """Build a JSON-serialized request to pass to the proxy. Handle the JSON or streamed response back, plus translate HTTP error statuses @@ -336,7 +343,7 @@ def _send_json_request( # Streaming if stream: - return self._streaming_download(data, env) + return self._streaming_download(data, env, progress) # Not streaming data_str = json.dumps(data).encode() @@ -366,6 +373,8 @@ def _send_json_request( logger.error("Internal proxy error (non-streaming)") raise BaseError(f"Internal proxy error: {error_desc}") + if progress is not None: + progress.set_value(len(response.stdout)) return self._handle_json_response(response.stdout) def authenticate(self, totp: str | None = None) -> bool: @@ -689,7 +698,11 @@ def delete_submission(self, submission: Submission) -> bool: return False def download_submission( - self, submission: Submission, path: str | None = None, timeout: int | None = None + self, + submission: Submission, + path: str | None = None, + timeout: int | None = None, + progress: ProgressProxy | None = None, ) -> tuple[str, str]: """ Returns a tuple of etag (format is algorithm:checksum) and file path for @@ -718,6 +731,7 @@ def download_submission( stream=True, headers=self.build_headers(), timeout=timeout or self.default_download_timeout, + progress=progress, ) if isinstance(response, JSONResponse): @@ -915,7 +929,9 @@ def get_all_replies(self) -> list[Reply]: return result - def download_reply(self, reply: Reply, path: str | None = None) -> tuple[str, str]: + def download_reply( + self, reply: Reply, path: str | None = None, progress: ProgressProxy | None = None + ) -> tuple[str, str]: """ Returns a tuple of etag (format is algorithm:checksum) and file path for a given Reply object. This method requires a directory path @@ -942,6 +958,7 @@ def download_reply(self, reply: Reply, path: str | None = None) -> tuple[str, st stream=True, headers=self.build_headers(), timeout=self.default_request_timeout, + progress=progress, ) if isinstance(response, JSONResponse): diff --git a/client/securedrop_client/utils.py b/client/securedrop_client/utils.py index ae384e1d6..a3d550175 100644 --- a/client/securedrop_client/utils.py +++ b/client/securedrop_client/utils.py @@ -194,6 +194,36 @@ def humanize_filesize(filesize: int) -> str: return f"{math.floor(filesize / 1024**2)}MB" +def humanize_speed(speed: float, length: int = 2) -> str: + """ + Returns a human readable string of a speed, with an input unit of + bytes/second. + + length controls how it should be rounded, e.g. length=3 will + give you 100KB/s, 4.02MB/s, 62.3KB/s, etc. + """ + + def adjust(x: float) -> float: + if x < 1: + # Less than 1B/s, just round to 0 + return 0 + if x >= 10**length: + return math.floor(x) + # Calculate digits ahead of decimal point + digits = math.ceil(math.log10(x)) + if digits >= length: + return round(x) + # Otherwise keep a few digits after the decimal + return round(x, length - digits) + + if speed < 1024: + return f"{adjust(speed)}B/s" + elif speed < 1024 * 1024: + return f"{adjust(speed / 1024)}KB/s" + else: + return f"{adjust(speed / 1024**2)}MB/s" + + @contextmanager def chronometer(logger: logging.Logger, description: str) -> Generator: """ diff --git a/client/tests/api_jobs/test_downloads.py b/client/tests/api_jobs/test_downloads.py index bd1c5cf77..46d449354 100644 --- a/client/tests/api_jobs/test_downloads.py +++ b/client/tests/api_jobs/test_downloads.py @@ -12,6 +12,7 @@ ReplyDownloadJob, ) from securedrop_client.crypto import CryptoError, GpgHelper +from securedrop_client.gui.base.progress import ProgressProxy from securedrop_client.sdk import BaseError from securedrop_client.sdk import Submission as SdkSubmission from tests import factory @@ -351,7 +352,9 @@ def test_FileDownloadJob_happy_path_no_etag(mocker, homedir, session, session_ma gpg = GpgHelper(homedir, session_maker, is_qubes=False) mock_decrypt = patch_decrypt(mocker, homedir, gpg, file_.filename) - def fake_download(sdk_obj: SdkSubmission, timeout: int) -> tuple[str, str]: + def fake_download( + sdk_obj: SdkSubmission, timeout: int, progress: ProgressProxy | None + ) -> tuple[str, str]: """ :return: (etag, path_to_dl) """ @@ -389,7 +392,9 @@ def test_FileDownloadJob_happy_path_sha256_etag( gpg = GpgHelper(homedir, session_maker, is_qubes=False) mock_decrypt = patch_decrypt(mocker, homedir, gpg, file_.filename) - def fake_download(sdk_obj: SdkSubmission, timeout: int) -> tuple[str, str]: + def fake_download( + sdk_obj: SdkSubmission, timeout: int, progress: ProgressProxy + ) -> tuple[str, str]: """ :return: (etag, path_to_dl) """ @@ -426,7 +431,9 @@ def test_FileDownloadJob_bad_sha256_etag( gpg = GpgHelper(homedir, session_maker, is_qubes=False) - def fake_download(sdk_obj: SdkSubmission, timeout: int) -> tuple[str, str]: + def fake_download( + sdk_obj: SdkSubmission, timeout: int, progress: ProgressProxy + ) -> tuple[str, str]: """ :return: (etag, path_to_dl) """ @@ -455,7 +462,9 @@ def test_FileDownloadJob_happy_path_unknown_etag(mocker, homedir, session, sessi gpg = GpgHelper(homedir, session_maker, is_qubes=False) - def fake_download(sdk_obj: SdkSubmission, timeout: int) -> tuple[str, str]: + def fake_download( + sdk_obj: SdkSubmission, timeout: int, progress: ProgressProxy + ) -> tuple[str, str]: """ :return: (etag, path_to_dl) """ @@ -494,7 +503,9 @@ def test_FileDownloadJob_decryption_error( gpg = GpgHelper(homedir, session_maker, is_qubes=False) mock_decrypt = mocker.patch.object(gpg, "decrypt_submission_or_reply", side_effect=CryptoError) - def fake_download(sdk_obj: SdkSubmission, timeout: int) -> tuple[str, str]: + def fake_download( + sdk_obj: SdkSubmission, timeout: int, progress: ProgressProxy + ) -> tuple[str, str]: """ :return: (etag, path_to_dl) """ @@ -535,7 +546,9 @@ def test_FileDownloadJob_raises_on_path_traversal_attack( api_client = mocker.MagicMock() download_fn = mocker.patch.object(api_client, "download_reply") - def fake_download(sdk_obj: SdkSubmission, timeout: int) -> tuple[str, str]: + def fake_download( + sdk_obj: SdkSubmission, timeout: int, progress: ProgressProxy + ) -> tuple[str, str]: """ :return: (etag, path-to-download) """ diff --git a/client/tests/gui/test_widgets.py b/client/tests/gui/test_widgets.py index 8bf90a623..efd6d5a4a 100644 --- a/client/tests/gui/test_widgets.py +++ b/client/tests/gui/test_widgets.py @@ -18,6 +18,7 @@ from securedrop_client import db, logic, storage from securedrop_client.app import threads +from securedrop_client.gui.base.progress import ProgressProxy from securedrop_client.gui.datetime_helpers import format_datetime_local from securedrop_client.gui.source import DeleteSourceDialog from securedrop_client.gui.widgets import ( @@ -3334,7 +3335,10 @@ def test_FileWidget_on_left_click_download(mocker, session, source): fw._on_left_click() get_file.assert_called_once_with(file_.uuid) - controller.on_submission_download.assert_called_once_with(db.File, file_.uuid) + assert controller.on_submission_download.call_args[0][0] == db.File + assert controller.on_submission_download.call_args[0][1] == file_.uuid + # ProgressProxy is created dynamically so just assert the type + assert isinstance(controller.on_submission_download.call_args[0][2], ProgressProxy) def test_FileWidget_on_left_click_downloading_in_progress(mocker, session, source): @@ -3386,10 +3390,10 @@ def test_FileWidget_start_button_animation(mocker, session, source): 0, 123, ) - fw.download_button = mocker.MagicMock() + fw.download_progress = mocker.MagicMock() fw.start_button_animation() # Check indicators of activity have been updated. - assert fw.download_button.setIcon.call_count == 1 + assert fw.download_progress.show.call_count == 1 def test_FileWidget_on_left_click_open(mocker, session, source): @@ -3416,31 +3420,6 @@ def test_FileWidget_on_left_click_open(mocker, session, source): fw.controller.on_file_open.assert_called_once_with(file_) -def test_FileWidget_set_button_animation_frame(mocker, session, source): - """ - Left click on download when file is not downloaded should trigger - a download. - """ - file_ = factory.File(source=source["source"], is_downloaded=False, is_decrypted=None) - session.add(file_) - session.commit() - - controller = mocker.MagicMock() - - fw = FileWidget( - file_, - controller, - mocker.MagicMock(), - mocker.MagicMock(), - mocker.MagicMock(), - 0, - 123, - ) - fw.download_button = mocker.MagicMock() - fw.set_button_animation_frame(1) - assert fw.download_button.setIcon.call_count == 1 - - def test_FileWidget_update(mocker, session, source): """ The update method should show/hide widgets if file is downloaded @@ -3632,7 +3611,6 @@ def test_FileWidget_on_file_missing_show_download_button_when_uuid_matches( assert fw.print_button.isHidden() assert not fw.no_file_name.isHidden() assert fw.file_name.isHidden() - assert fw.download_animation.state() == QMovie.NotRunning def test_FileWidget_on_file_missing_does_not_show_download_button_when_uuid_does_not_match( diff --git a/client/tests/integration/test_styles_file_download_button.py b/client/tests/integration/test_styles_file_download_button.py index 58ca631fc..0ecf8625d 100644 --- a/client/tests/integration/test_styles_file_download_button.py +++ b/client/tests/integration/test_styles_file_download_button.py @@ -25,29 +25,3 @@ def test_styles(mocker, main_window): expected_image = load_icon("download_file.svg").pixmap(20, 20).toImage() assert download_button.icon().pixmap(20, 20).toImage() == expected_image assert download_button.palette().color(QPalette.Foreground).name() == "#2a319d" - - -def test_styles_animated(mocker, main_window): - wrapper = main_window.main_view.view_layout.widget(main_window.main_view.CONVERSATION_INDEX) - conversation_scrollarea = wrapper.conversation_view._scroll - file_widget = conversation_scrollarea.widget().layout().itemAt(0).widget() - download_button = file_widget.download_button - - file_widget.start_button_animation() - - expected_image = load_icon("download_file.gif").pixmap(20, 20).toImage() - assert download_button.icon().pixmap(20, 20).toImage() == expected_image - assert download_button.font().family() == "Source Sans Pro" - assert QFont.Bold == download_button.font().weight() - assert download_button.font().pixelSize() == 13 - assert download_button.palette().color(QPalette.Foreground).name() == "#05a6fe" - - file_widget.eventFilter(download_button, QEvent(QEvent.HoverEnter)) - expected_image = load_icon("download_file.gif").pixmap(20, 20).toImage() - assert download_button.icon().pixmap(20, 20).toImage() == expected_image - assert download_button.palette().color(QPalette.Foreground).name() == "#05a6fe" - - file_widget.eventFilter(download_button, QEvent(QEvent.HoverLeave)) - expected_image = load_icon("download_file.gif").pixmap(20, 20).toImage() - assert download_button.icon().pixmap(20, 20).toImage() == expected_image - assert download_button.palette().color(QPalette.Foreground).name() == "#05a6fe" diff --git a/client/tests/sdk/utils.py b/client/tests/sdk/utils.py index 8ae2ab23e..0449c6852 100644 --- a/client/tests/sdk/utils.py +++ b/client/tests/sdk/utils.py @@ -4,6 +4,7 @@ import vcr from vcr.request import Request +from securedrop_client.gui.base.progress import ProgressProxy from securedrop_client.sdk import API, JSONResponse, StreamedResponse VCR = vcr.VCR(cassette_library_dir="tests/sdk/data/") @@ -40,6 +41,7 @@ def _send_json_request( body: str | None = None, headers: dict[str, str] | None = None, timeout: int | None = None, + progress: ProgressProxy | None = None, ) -> StreamedResponse | JSONResponse: """If the cassette contains a VCR.py `Request` object corresponding to this request, play back the response. If it's an exception, raise it to diff --git a/client/tests/test_utils.py b/client/tests/test_utils.py index 6e523d843..98d6359fe 100644 --- a/client/tests/test_utils.py +++ b/client/tests/test_utils.py @@ -9,6 +9,7 @@ check_dir_permissions, check_path_traversal, humanize_filesize, + humanize_speed, relative_filepath, safe_mkdir, ) @@ -32,6 +33,21 @@ def test_humanize_file_size_megabytes(): assert expected_humanized_filesize == actual_humanized_filesize +@pytest.mark.parametrize( + ("input", "expected"), + [ + (0, "0B/s"), + (0.1, "0B/s"), + (1, "1B/s"), + (1234, "1.2KB/s"), + (678_123, "662KB/s"), + (12_345_678, "12MB/s"), + ], +) +def test_humanize_speed(input, expected): + assert expected == humanize_speed(input) + + def test_safe_mkdir_with_unsafe_path(homedir): """ Ensure an error is raised if the path contains path traversal string.