diff --git a/.semgrep/custom-rules.yml b/.semgrep/custom-rules.yml index 94db7976f..0385449c8 100644 --- a/.semgrep/custom-rules.yml +++ b/.semgrep/custom-rules.yml @@ -165,4 +165,18 @@ rules: - pattern-regex: |- _\(\s*".*"\s*\.format\( - pattern-regex: |- - _\(\s*f"(.*) \ No newline at end of file + _\(\s*f"(.*) + +- id: unsafe-assert + languages: + - python + severity: ERROR + message: Assert is unsafe unless used for type narrowing - see https://mypy.readthedocs.io/en/stable/type_narrowing.html + patterns: + # Exempt assertions that are used for type narrowing + - pattern-not-regex: | + assert isinstance\(.*\) + + # Forbid any other use of assert + - pattern-regex: | + assert.* diff --git a/requirements/dev-bookworm-requirements.txt b/requirements/dev-bookworm-requirements.txt index 6eec2c40c..f793e81c6 100644 --- a/requirements/dev-bookworm-requirements.txt +++ b/requirements/dev-bookworm-requirements.txt @@ -501,6 +501,10 @@ pyqt5-sip==12.9.0 \ # via # -r requirements/dev-bookworm-requirements.in # pyqt5 +pyqt5-stubs==5.15.6.0 \ + --hash=sha256:7fb8177c72489a8911f021b7bd7c33f12c87f6dba92dcef3fdcdb5d9400f0f3f \ + --hash=sha256:91270ac23ebf38a1dc04cd97aa852cd08af82dc839100e5395af1447e3e99707 + # via -r requirements/dev-sdw-requirements.in pyrect==0.2.0 \ --hash=sha256:f65155f6df9b929b67caffbd57c0947c5ae5449d3b580d178074bffb47a09b78 # via pygetwindow diff --git a/requirements/dev-bullseye-requirements.txt b/requirements/dev-bullseye-requirements.txt index 902371386..665d615ae 100644 --- a/requirements/dev-bullseye-requirements.txt +++ b/requirements/dev-bullseye-requirements.txt @@ -501,6 +501,10 @@ pyqt5-sip==12.8.1 \ # via # -r requirements/dev-bullseye-requirements.in # pyqt5 +pyqt5-stubs==5.15.6.0 \ + --hash=sha256:7fb8177c72489a8911f021b7bd7c33f12c87f6dba92dcef3fdcdb5d9400f0f3f \ + --hash=sha256:91270ac23ebf38a1dc04cd97aa852cd08af82dc839100e5395af1447e3e99707 + # via -r requirements/dev-sdw-requirements.in pyrect==0.2.0 \ --hash=sha256:f65155f6df9b929b67caffbd57c0947c5ae5449d3b580d178074bffb47a09b78 # via pygetwindow diff --git a/requirements/dev-sdw-requirements.in b/requirements/dev-sdw-requirements.in index ca0fef6c0..d527b126f 100644 --- a/requirements/dev-sdw-requirements.in +++ b/requirements/dev-sdw-requirements.in @@ -11,6 +11,7 @@ pip-tools>=6.8.0 # for jazzband/piptools#1617 PyAutoGUI pyobjc-core;platform_system=="Darwin" pyobjc;platform_system=="Darwin" +pyqt5-stubs pytest>=7.2.0 # CVE-2022-42969 pytest-cov pytest-mock diff --git a/requirements/dev-sdw-requirements.txt b/requirements/dev-sdw-requirements.txt index 73c6d68e6..115588f94 100644 --- a/requirements/dev-sdw-requirements.txt +++ b/requirements/dev-sdw-requirements.txt @@ -469,6 +469,10 @@ pyparsing==3.0.9 \ pyperclip==1.8.2 \ --hash=sha256:105254a8b04934f0bc84e9c24eb360a591aaf6535c9def5f29d92af107a9bf57 # via mouseinfo +pyqt5-stubs==5.15.6.0 \ + --hash=sha256:7fb8177c72489a8911f021b7bd7c33f12c87f6dba92dcef3fdcdb5d9400f0f3f \ + --hash=sha256:91270ac23ebf38a1dc04cd97aa852cd08af82dc839100e5395af1447e3e99707 + # via -r requirements/dev-sdw-requirements.in pyrect==0.2.0 \ --hash=sha256:f65155f6df9b929b67caffbd57c0947c5ae5449d3b580d178074bffb47a09b78 # via pygetwindow diff --git a/securedrop_client/api_jobs/base.py b/securedrop_client/api_jobs/base.py index 407169933..0670ef5b0 100644 --- a/securedrop_client/api_jobs/base.py +++ b/securedrop_client/api_jobs/base.py @@ -23,9 +23,10 @@ def __init__(self, message: Optional[str] = None) -> None: class QueueJob(QObject): - def __init__(self) -> None: + def __init__(self, remaining_attempts: int = DEFAULT_NUM_ATTEMPTS) -> None: super().__init__() self.order_number = None # type: Optional[int] + self.remaining_attempts = remaining_attempts def __lt__(self, other: QueueJobType) -> bool: """ @@ -60,8 +61,7 @@ class ApiJob(QueueJob): failure_signal = pyqtSignal(Exception) def __init__(self, remaining_attempts: int = DEFAULT_NUM_ATTEMPTS) -> None: - super().__init__() - self.remaining_attempts = remaining_attempts + super().__init__(remaining_attempts) def _do_call_api(self, api_client: API, session: Session) -> None: if not api_client: diff --git a/securedrop_client/app.py b/securedrop_client/app.py index f87649061..d4165f0bc 100644 --- a/securedrop_client/app.py +++ b/securedrop_client/app.py @@ -170,9 +170,11 @@ def prevent_second_instance(app: QApplication, unique_name: str) -> None: IDENTIFIER = "\0" + app.applicationName() + unique_name ALREADY_BOUND_ERRNO = 98 - app.instance_binding = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM) + app.instance_binding = socket.socket( # type: ignore [attr-defined] + socket.AF_UNIX, socket.SOCK_DGRAM + ) try: - app.instance_binding.bind(IDENTIFIER) + app.instance_binding.bind(IDENTIFIER) # type: ignore [attr-defined] except OSError as e: if e.errno == ALREADY_BOUND_ERRNO: err_dialog = QMessageBox() diff --git a/securedrop_client/export.py b/securedrop_client/export.py index 3f142f201..62abdbd84 100644 --- a/securedrop_client/export.py +++ b/securedrop_client/export.py @@ -10,7 +10,7 @@ from tempfile import TemporaryDirectory from typing import List, Optional -from PyQt5.QtCore import QObject, Qt, pyqtSignal, pyqtSlot +from PyQt5.QtCore import QObject, pyqtBoundSignal, pyqtSignal, pyqtSlot logger = logging.getLogger(__name__) @@ -80,10 +80,10 @@ class Export(QObject): def __init__( self, - export_preflight_check_requested: Optional[pyqtSignal] = None, - export_requested: Optional[pyqtSignal] = None, - print_preflight_check_requested: Optional[pyqtSignal] = None, - print_requested: Optional[pyqtSignal] = None, + export_preflight_check_requested: Optional[pyqtBoundSignal] = None, + export_requested: Optional[pyqtBoundSignal] = None, + print_preflight_check_requested: Optional[pyqtBoundSignal] = None, + print_requested: Optional[pyqtBoundSignal] = None, ) -> None: super().__init__() @@ -96,26 +96,22 @@ def __init__( def connect_signals( self, - export_preflight_check_requested: Optional[pyqtSignal] = None, - export_requested: Optional[pyqtSignal] = None, - print_preflight_check_requested: Optional[pyqtSignal] = None, - print_requested: Optional[pyqtSignal] = None, + export_preflight_check_requested: Optional[pyqtBoundSignal] = None, + export_requested: Optional[pyqtBoundSignal] = None, + print_preflight_check_requested: Optional[pyqtBoundSignal] = None, + print_requested: Optional[pyqtBoundSignal] = None, ) -> None: # This instance can optionally react to events to prevent # coupling it to dependent code. if export_preflight_check_requested is not None: - export_preflight_check_requested.connect( - self.run_preflight_checks, type=Qt.QueuedConnection - ) + export_preflight_check_requested.connect(self.run_preflight_checks) if export_requested is not None: - export_requested.connect(self.send_file_to_usb_device, type=Qt.QueuedConnection) + export_requested.connect(self.send_file_to_usb_device) if print_requested is not None: - print_requested.connect(self.print, type=Qt.QueuedConnection) + print_requested.connect(self.print) if print_preflight_check_requested is not None: - print_preflight_check_requested.connect( - self.run_printer_preflight, type=Qt.QueuedConnection - ) + print_preflight_check_requested.connect(self.run_printer_preflight) def _export_archive(cls, archive_path: str) -> Optional[ExportStatus]: """ diff --git a/securedrop_client/gui/actions.py b/securedrop_client/gui/actions.py index 474979c61..37a5082b8 100644 --- a/securedrop_client/gui/actions.py +++ b/securedrop_client/gui/actions.py @@ -74,9 +74,9 @@ def __init__( ) -> None: self.source = source self.controller = controller - self.text = _("Delete Source Account") + text = _("Delete Source Account") - super().__init__(self.text, parent) + super().__init__(text, parent) self._confirmation_dialog = confirmation_dialog(self.source) self._confirmation_dialog.accepted.connect( @@ -105,9 +105,9 @@ def __init__( self.source = source self.controller = controller self._state = app_state - self.text = _("Delete All Files and Messages") + text = _("Delete All Files and Messages") - super().__init__(self.text, parent) + super().__init__(text, parent) self._confirmation_dialog = confirmation_dialog(self.source) self._confirmation_dialog.accepted.connect(lambda: self._on_confirmation_dialog_accepted()) diff --git a/securedrop_client/gui/base/checkbox.py b/securedrop_client/gui/base/checkbox.py index b7692d123..73e397348 100644 --- a/securedrop_client/gui/base/checkbox.py +++ b/securedrop_client/gui/base/checkbox.py @@ -26,10 +26,10 @@ def __init__(self) -> None: font = QFont() font.setLetterSpacing(QFont.AbsoluteSpacing, self.PASSPHRASE_LABEL_SPACING) - self.layout = QHBoxLayout() - self.layout.setContentsMargins(0, 0, 0, 0) - self.layout.setSpacing(0) - self.setLayout(self.layout) + self._layout = QHBoxLayout() + self._layout.setContentsMargins(0, 0, 0, 0) + self._layout.setSpacing(0) + self.setLayout(self._layout) self.frame = QFrame() self.frame.setLayout(QHBoxLayout()) @@ -40,7 +40,7 @@ def __init__(self) -> None: self.label = QLabel(_("Show Passphrase")) self.label.setFont(font) - self.layout.addWidget(self.frame) + self._layout.addWidget(self.frame) self.frame.layout().addWidget(self.checkbox) self.frame.layout().addWidget(self.label) self.frame.setCursor(QCursor(Qt.PointingHandCursor)) diff --git a/securedrop_client/gui/base/inputs.py b/securedrop_client/gui/base/inputs.py index ff2167fac..2fbbc49d5 100644 --- a/securedrop_client/gui/base/inputs.py +++ b/securedrop_client/gui/base/inputs.py @@ -25,8 +25,7 @@ class PasswordEdit(QLineEdit): """ def __init__(self, parent: QDialog) -> None: - self.parent = parent - super().__init__(self.parent) + super().__init__(parent) self.setEchoMode(QLineEdit.Password) self.password_shown = False diff --git a/securedrop_client/gui/base/misc.py b/securedrop_client/gui/base/misc.py index 20296c841..d8cf6e24c 100644 --- a/securedrop_client/gui/base/misc.py +++ b/securedrop_client/gui/base/misc.py @@ -19,6 +19,7 @@ from typing import Optional, Union from PyQt5.QtCore import QSize, Qt +from PyQt5.QtGui import QIcon from PyQt5.QtWidgets import QHBoxLayout, QLabel, QPushButton, QWidget from securedrop_client.resources import load_icon, load_svg @@ -39,7 +40,7 @@ class SvgToggleButton(QPushButton): The display size of the SVG, defaults to filling the entire size of the widget. """ - def __init__(self, on: str, off: str, svg_size: Optional[str] = None): + def __init__(self, on: str, off: str, svg_size: Optional[QSize] = None): super().__init__() # Set layout @@ -51,16 +52,16 @@ def __init__(self, on: str, off: str, svg_size: Optional[str] = None): layout.setSpacing(0) # Add SVG icon and set its size - self.icon = load_icon(normal=on, normal_off=off) - self.setIcon(self.icon) + self._icon = load_icon(normal=on, normal_off=off) + self.setIcon(self._icon) self.setIconSize(svg_size) if svg_size else self.setIconSize(QSize()) # Make this a toggle button self.setCheckable(True) def set_icon(self, on: str, off: str) -> None: - self.icon = load_icon(normal=on, normal_off=off) - self.setIcon(self.icon) + self._icon = load_icon(normal=on, normal_off=off) + self.setIcon(self._icon) class SvgPushButton(QPushButton): @@ -88,7 +89,7 @@ def __init__( disabled: Optional[str] = None, active: Optional[str] = None, selected: Optional[str] = None, - svg_size: Optional[str] = None, + svg_size: Optional[QSize] = None, ) -> None: super().__init__() @@ -101,7 +102,7 @@ def __init__( layout.setSpacing(0) # Add SVG icon and set its size - self._icon = load_icon( + self._icon: QIcon = load_icon( normal=normal, disabled=disabled, active=active, @@ -124,7 +125,7 @@ class SvgLabel(QLabel): The display size of the SVG, defaults to filling the entire size of the widget. """ - def __init__(self, filename: str, svg_size: Optional[str] = None) -> None: + def __init__(self, filename: str, svg_size: Optional[QSize] = None) -> None: super().__init__() # Remove margins and spacing @@ -138,7 +139,7 @@ def __init__(self, filename: str, svg_size: Optional[str] = None) -> None: self.svg.setFixedSize(svg_size) if svg_size else self.svg.setFixedSize(QSize()) layout.addWidget(self.svg) - def update_image(self, filename: str, svg_size: Optional[str] = None) -> None: + def update_image(self, filename: str, svg_size: Optional[QSize] = None) -> None: self.svg = load_svg(filename) self.svg.setFixedSize(svg_size) if svg_size else self.svg.setFixedSize(QSize()) child = self.layout().takeAt(0) diff --git a/securedrop_client/gui/conversation/delete/dialog.py b/securedrop_client/gui/conversation/delete/dialog.py index 82fcd214d..e609982d0 100644 --- a/securedrop_client/gui/conversation/delete/dialog.py +++ b/securedrop_client/gui/conversation/delete/dialog.py @@ -87,7 +87,7 @@ def make_body_text(self) -> str: source=source, ) - def exec(self) -> None: + def exec(self) -> int: # Refresh counters self.body.setText(self.make_body_text()) - super().exec() + return super().exec() diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 553412efa..b3a2880c9 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -16,6 +16,8 @@ You should have received a copy of the GNU Affero General Public License along with this program. If not, see . """ +from __future__ import annotations + import html import logging from datetime import datetime @@ -485,14 +487,14 @@ def __init__(self) -> None: self.setLayoutDirection(Qt.RightToLeft) - self.menu = UserMenu() - self.setMenu(self.menu) + self._menu = UserMenu() + self.setMenu(self._menu) # Set cursor. self.setCursor(QCursor(Qt.PointingHandCursor)) def setup(self, controller: Controller) -> None: - self.menu.setup(controller) + self._menu.setup(controller) def set_username(self, username: str) -> None: formatted_name = _("{}").format(html.escape(username)) @@ -549,13 +551,13 @@ def setup(self, window) -> None: # type: ignore [no-untyped-def] """ Store a reference to the GUI window object. """ - self.window = window + self._window = window def _on_clicked(self) -> None: """ Called when the login button is clicked. """ - self.window.show_login() + self._window.show_login() class MainView(QWidget): @@ -566,7 +568,7 @@ class MainView(QWidget): def __init__( self, - parent: QObject, + parent: Optional[QWidget], app_state: Optional[state.State] = None, export_service: Optional[export.Service] = None, ) -> None: @@ -579,12 +581,12 @@ def __init__( self.setObjectName("MainView") # Set layout - self.layout = QHBoxLayout(self) - self.setLayout(self.layout) + self._layout = QHBoxLayout(self) + self.setLayout(self._layout) # Set margins and spacing - self.layout.setContentsMargins(0, 0, 0, 0) - self.layout.setSpacing(0) + self._layout.setContentsMargins(0, 0, 0, 0) + self._layout.setSpacing(0) # Create SourceList widget self.source_list = SourceList() @@ -606,8 +608,8 @@ def __init__( self.view_layout.addWidget(self.empty_conversation_view) # Add widgets to layout - self.layout.addWidget(self.source_list, stretch=1) - self.layout.addWidget(self.view_holder, stretch=2) + self._layout.addWidget(self.source_list, stretch=1) + self._layout.addWidget(self.view_holder, stretch=2) # Note: We should not delete SourceConversationWrapper when its source is unselected. This # is a temporary solution to keep copies of our objects since we do delete them. @@ -644,7 +646,7 @@ def show_sources(self, sources: List[Source]) -> None: if not self.source_list.source_items: self.source_list.initial_update(sources) else: - deleted_sources = self.source_list.update(sources) + deleted_sources = self.source_list.update_sources(sources) for source_uuid in deleted_sources: # Then call the function to remove the wrapper and its children. self.delete_conversation(source_uuid) @@ -668,7 +670,9 @@ def on_source_changed(self) -> None: # Get or create the SourceConversationWrapper if source.uuid in self.source_conversations: conversation_wrapper = self.source_conversations[source.uuid] - conversation_wrapper.conversation_view.update_conversation(source.collection) + conversation_wrapper.conversation_view.update_conversation( # type: ignore [has-type] # noqa: E501 + source.collection + ) else: conversation_wrapper = SourceConversationWrapper( source, self.controller, self._state, self._export_service @@ -694,7 +698,9 @@ def refresh_source_conversations(self) -> None: self.controller.session.refresh(source) self.controller.mark_seen(source) conversation_wrapper = self.source_conversations[source.uuid] - conversation_wrapper.conversation_view.update_conversation(source.collection) + conversation_wrapper.conversation_view.update_conversation( # type: ignore [has-type] + source.collection + ) except sqlalchemy.exc.InvalidRequestError as e: logger.debug("Error refreshing source conversations: %s", e) @@ -825,6 +831,8 @@ def __lt__(self, other: "SourceListWidgetItem") -> bool: me = lw.itemWidget(self) them = lw.itemWidget(other) if me and them: + assert isinstance(me, SourceWidget) + assert isinstance(them, SourceWidget) my_ts = arrow.get(me.last_updated) other_ts = arrow.get(them.last_updated) return my_ts < other_ts @@ -880,7 +888,7 @@ def setup(self, controller: Controller) -> None: self.controller.message_download_failed.connect(self.set_snippet) self.controller.reply_download_failed.connect(self.set_snippet) - def update(self, sources: List[Source]) -> List[str]: + def update_sources(self, sources: List[Source]) -> List[str]: """ Update the list with the passed in list of sources. """ @@ -907,6 +915,7 @@ def update(self, sources: List[Source]) -> List[str]: source_widget = self.itemWidget(source_item) self.takeItem(self.row(source_item)) + assert isinstance(source_widget, SourceWidget) if source_widget.source_uuid in self.source_items: del self.source_items[source_widget.source_uuid] @@ -920,7 +929,8 @@ def update(self, sources: List[Source]) -> List[str]: if not source_widget: continue - source_widget.update() + assert isinstance(source_widget, SourceWidget) + source_widget.reload() # Add widgets for new sources for uuid in sources_to_add: @@ -994,23 +1004,27 @@ def get_selected_source(self) -> Optional[Source]: source_item = self.selectedItems()[0] source_widget = self.itemWidget(source_item) + assert isinstance(source_widget, SourceWidget) if source_widget and source_exists(self.controller.session, source_widget.source_uuid): return source_widget.source return None # pragma: nocover - def get_source_widget(self, source_uuid: str) -> Optional[QListWidget]: + def get_source_widget(self, source_uuid: str) -> Optional[SourceWidget]: """ First try to get the source widget from the cache, then look for it in the SourceList. """ try: source_item = self.source_items[source_uuid] - return self.itemWidget(source_item) + source_widget = self.itemWidget(source_item) + assert isinstance(source_widget, SourceWidget) + return source_widget except KeyError: pass for i in range(self.count()): list_item = self.item(i) source_widget = self.itemWidget(list_item) + assert isinstance(source_widget, SourceWidget) if source_widget and source_widget.source_uuid == source_uuid: return source_widget @@ -1201,8 +1215,8 @@ def __init__( self, controller: Controller, source: Source, - source_selected_signal: pyqtSignal, - adjust_preview: pyqtSignal, + source_selected_signal: pyqtBoundSignal, + adjust_preview: pyqtBoundSignal, ): super().__init__() @@ -1219,10 +1233,10 @@ def __init__( source_selected_signal.connect(self._on_source_selected) adjust_preview.connect(self._on_adjust_preview) - self.source = source + self.source: Source = source self.seen = self.source.seen - self.source_uuid = self.source.uuid - self.last_updated = self.source.last_updated + self.source_uuid: str = self.source.uuid + self.last_updated: sqlalchemy.DateTime = self.source.last_updated self.selected = False self.deletion_scheduled_timestamp = datetime.utcnow() self.sync_started_timestamp = datetime.utcnow() @@ -1293,14 +1307,14 @@ def __init__( layout.setSpacing(0) layout.addWidget(self.source_widget) - self.update() + self.reload() @pyqtSlot(int) def _on_adjust_preview(self, width: int) -> None: self.setFixedWidth(width) self.preview.adjust_preview(width) - def update(self) -> None: + def reload(self) -> None: """ Updates the displayed values with the current values from self.source. """ @@ -1563,7 +1577,7 @@ def enable_toggle(self) -> None: self.setCheckable(True) self.set_icon(on="star_on.svg", off="star_off.svg") # Undo icon change from disable_toggle - def eventFilter(self, obj: QObject, event: QEvent) -> None: + def eventFilter(self, obj: QObject, event: QEvent) -> bool: """ If the button is checkable then we show a hover state. """ @@ -1908,7 +1922,7 @@ def update_seen_by_list(self, usernames: Dict[str, User]) -> None: self.check_mark.setToolTip(",\n".join(username for username in self.seen_by.keys())) - def eventFilter(self, obj: QObject, event: QEvent) -> None: + def eventFilter(self, obj: QObject, event: QEvent) -> bool: t = event.type() if t == QEvent.HoverEnter: self.check_mark.setIcon(load_icon("checkmark_hover.svg")) @@ -2307,9 +2321,9 @@ def __init__( layout.addWidget(self.file_size) # Connect signals to slots - file_download_started.connect(self._on_file_download_started, type=Qt.QueuedConnection) - file_ready_signal.connect(self._on_file_downloaded, type=Qt.QueuedConnection) - file_missing.connect(self._on_file_missing, type=Qt.QueuedConnection) + file_download_started.connect(self._on_file_download_started) + file_ready_signal.connect(self._on_file_downloaded) + file_missing.connect(self._on_file_missing) def adjust_width(self, container_width: int) -> None: """ @@ -2321,9 +2335,10 @@ def adjust_width(self, container_width: int) -> None: else: self.setFixedWidth(int(container_width * self.WIDTH_TO_CONTAINER_WIDTH_RATIO)) - def eventFilter(self, obj: QObject, event: QEvent) -> None: + def eventFilter(self, obj: QObject, event: QEvent) -> bool: t = event.type() if t == QEvent.MouseButtonPress: + assert isinstance(event, QMouseEvent) if event.button() == Qt.LeftButton: self._on_left_click() elif t == QEvent.HoverEnter and not self.downloading: @@ -2514,8 +2529,8 @@ def resizeEvent(self, event: QResizeEvent) -> None: super().resizeEvent(event) self.widget().setFixedWidth(event.size().width()) - for widget in self.findChildren(FileWidget): - widget.adjust_width(self.widget().width()) + for file_widget in self.findChildren(FileWidget): + file_widget.adjust_width(self.widget().width()) for widget in self.findChildren(SpeechBubble): widget.adjust_width(self.widget().width()) @@ -2638,7 +2653,7 @@ def __init__( ) # To hold currently displayed messages. - self.current_messages = {} # type: Dict[str, QWidget] + self.current_messages = {} # type: Dict[str, Union[FileWidget, MessageWidget, ReplyWidget]] self.deletion_scheduled_timestamp = datetime.utcnow() self.sync_started_timestamp = datetime.utcnow() @@ -2658,16 +2673,16 @@ def __init__( main_layout.addWidget(self.deleted_conversation_items_marker) main_layout.addWidget(self.deleted_conversation_marker) - self.scroll = ConversationScrollArea() + self._scroll = ConversationScrollArea() # Flag to show if the current user has sent a reply. See issue #61. self.reply_flag = False # Completely unintuitive way to ensure the view remains scrolled to the bottom. - sb = self.scroll.verticalScrollBar() + sb = self._scroll.verticalScrollBar() sb.rangeChanged.connect(self.update_conversation_position) - main_layout.addWidget(self.scroll) + main_layout.addWidget(self._scroll) try: self.update_conversation(self.source.collection) @@ -2700,11 +2715,11 @@ def _on_conversation_deletion_successful(self, source_uuid: str, timestamp: date # If a draft reply exists then show the tear pattern above the draft replies. # Otherwise, show that the entire conversation is deleted. if draft_reply_exists: - self.scroll.show() + self._scroll.show() self.deleted_conversation_items_marker.show() self.deleted_conversation_marker.hide() else: - self.scroll.hide() + self._scroll.hide() self.deleted_conversation_items_marker.hide() self.deleted_conversation_marker.show() except sqlalchemy.exc.InvalidRequestError as e: @@ -2712,12 +2727,12 @@ def _on_conversation_deletion_successful(self, source_uuid: str, timestamp: date def update_deletion_markers(self) -> None: if self.source.collection: - self.scroll.show() + self._scroll.show() if self.source.collection[0].file_counter > 1: self.deleted_conversation_marker.hide() self.deleted_conversation_items_marker.show() elif self.source.interaction_count > 0: - self.scroll.hide() + self._scroll.hide() self.deleted_conversation_items_marker.hide() self.deleted_conversation_marker.show() @@ -2749,16 +2764,19 @@ def update_conversation(self, collection: list) -> None: for index, conversation_item in enumerate(collection): item_widget = current_conversation.get(conversation_item.uuid) if item_widget: + # FIXME: Item types cannot be defines as (FileWidget, MessageWidget, ReplyWidget) + # because one test mocks MessageWidget. + assert isinstance(item_widget, (FileWidget, SpeechBubble)) current_conversation.pop(conversation_item.uuid) if item_widget.index != index: # The existing widget is out of order. # Remove / re-add it and update index details. - self.scroll.remove_widget_from_conversation(item_widget) + self._scroll.remove_widget_from_conversation(item_widget) item_widget.index = index if isinstance(item_widget, ReplyWidget): - self.scroll.add_widget_to_conversation(index, item_widget, Qt.AlignRight) + self._scroll.add_widget_to_conversation(index, item_widget, Qt.AlignRight) else: - self.scroll.add_widget_to_conversation(index, item_widget, Qt.AlignLeft) + self._scroll.add_widget_to_conversation(index, item_widget, Qt.AlignLeft) # Check if text in item has changed, then update the # widget to reflect this change. if not isinstance(item_widget, FileWidget): @@ -2795,7 +2813,7 @@ def update_conversation(self, collection: list) -> None: logger.debug("Deleting item: {}".format(item_widget.uuid)) self.current_messages.pop(item_widget.uuid) item_widget.deleteLater() - self.scroll.remove_widget_from_conversation(item_widget) + self._scroll.remove_widget_from_conversation(item_widget) self.update_deletion_markers() self.conversation_updated.emit() @@ -2812,10 +2830,10 @@ def add_file(self, file: File, index: int) -> None: self.controller.file_ready, self.controller.file_missing, index, - self.scroll.widget().width(), + self._scroll.widget().width(), self._export_service, ) - self.scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignLeft) + self._scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignLeft) self.current_messages[file.uuid] = conversation_item self.conversation_updated.emit() @@ -2825,7 +2843,7 @@ def update_conversation_position(self, min_val: int, max_val: int) -> None: it's scrolled to the bottom and thus visible. """ if self.reply_flag and max_val > 0: - self.scroll.verticalScrollBar().setValue(max_val) + self._scroll.verticalScrollBar().setValue(max_val) self.reply_flag = False def add_message(self, message: Message, index: int) -> None: @@ -2838,7 +2856,7 @@ def add_message(self, message: Message, index: int) -> None: self.controller.message_ready, self.controller.message_download_failed, index, - self.scroll.widget().width(), + self._scroll.widget().width(), self.controller.authenticated_user, message.download_error is not None, ) @@ -2849,7 +2867,7 @@ def add_message(self, message: Message, index: int) -> None: ) # Retrieve the list of usernames of the users who have seen the message. conversation_item.update_seen_by_list(message.seen_by_list) - self.scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignLeft) + self._scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignLeft) self.current_messages[message.uuid] = conversation_item self.conversation_updated.emit() @@ -2880,7 +2898,7 @@ def add_reply(self, reply: Union[DraftReply, Reply], sender: User, index: int) - self.controller.reply_succeeded, self.controller.reply_failed, index, - self.scroll.widget().width(), + self._scroll.widget().width(), sender, sender_is_current_user, self.controller.authenticated_user, @@ -2892,7 +2910,7 @@ def add_reply(self, reply: Union[DraftReply, Reply], sender: User, index: int) - ) # Retrieve the list of usernames of the users who have seen the reply. conversation_item.update_seen_by_list(reply.seen_by_list) - self.scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignRight) + self._scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignRight) self.current_messages[reply.uuid] = conversation_item def on_reply_sent(self, source_uuid: str) -> None: @@ -3400,8 +3418,8 @@ def __init__( self.setIcon(load_icon("ellipsis.svg")) self.setIconSize(QSize(22, 33)) # Make it taller than the svg viewBox to increase hitbox - self.menu = SourceMenu(self.source, self.controller, app_state) - self.setMenu(self.menu) + menu = SourceMenu(self.source, self.controller, app_state) + self.setMenu(menu) self.setPopupMode(QToolButton.InstantPopup) # Set cursor. diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index feca86147..451f112e2 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -29,7 +29,7 @@ import arrow import sdclientapi import sqlalchemy.orm.exc -from PyQt5.QtCore import QObject, QProcess, Qt, QThread, QTimer, pyqtSignal, pyqtSlot +from PyQt5.QtCore import QObject, QProcess, QThread, QTimer, pyqtSignal, pyqtSlot from sdclientapi import AuthError, RequestTimeoutError, ServerConnectionError from sqlalchemy.orm.session import sessionmaker @@ -398,9 +398,9 @@ def __init__( # type: ignore [no-untyped-def] self.api_sync = ApiSync( self.api, self.session_maker, self.gpg, self.data_dir, self.sync_thread, state ) - self.api_sync.sync_started.connect(self.on_sync_started, type=Qt.QueuedConnection) - self.api_sync.sync_success.connect(self.on_sync_success, type=Qt.QueuedConnection) - self.api_sync.sync_failure.connect(self.on_sync_failure, type=Qt.QueuedConnection) + self.api_sync.sync_started.connect(self.on_sync_started) + self.api_sync.sync_success.connect(self.on_sync_success) + self.api_sync.sync_failure.connect(self.on_sync_failure) # Create a timer to show the time since the last sync self.show_last_sync_timer = QTimer() @@ -755,8 +755,8 @@ def mark_seen(self, source: db.Source) -> None: return job = SeenJob(files, messages, replies) - job.success_signal.connect(self.on_seen_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_seen_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_seen_success) + job.failure_signal.connect(self.on_seen_failure) self.add_job.emit(job) except sqlalchemy.exc.InvalidRequestError as e: logger.debug(e) @@ -784,8 +784,8 @@ def update_star(self, source_uuid: str, is_starred: bool) -> None: Star or unstar. """ job = UpdateStarJob(source_uuid, is_starred) - job.success_signal.connect(self.on_update_star_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_update_star_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_update_star_success) + job.failure_signal.connect(self.on_update_star_failure) self.add_job.emit(job) def logout(self) -> None: @@ -835,16 +835,16 @@ def _submit_download_job( job = ReplyDownloadJob( uuid, self.data_dir, self.gpg ) # type: Union[ReplyDownloadJob, MessageDownloadJob, FileDownloadJob] - job.success_signal.connect(self.on_reply_download_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_reply_download_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_reply_download_success) + job.failure_signal.connect(self.on_reply_download_failure) elif object_type == db.Message: job = MessageDownloadJob(uuid, self.data_dir, self.gpg) - job.success_signal.connect(self.on_message_download_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_message_download_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_message_download_success) + job.failure_signal.connect(self.on_message_download_failure) elif object_type == db.File: job = FileDownloadJob(uuid, self.data_dir, self.gpg) - job.success_signal.connect(self.on_file_download_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_file_download_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_file_download_success) + job.failure_signal.connect(self.on_file_download_failure) self.add_job.emit(job) @@ -1037,8 +1037,8 @@ def delete_source(self, source: db.Source) -> None: the failure handler will display an error. """ job = DeleteSourceJob(source.uuid) - job.success_signal.connect(self.on_delete_source_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_delete_source_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_delete_source_success) + job.failure_signal.connect(self.on_delete_source_failure) self.add_job.emit(job) self.source_deleted.emit(source.uuid) @@ -1054,8 +1054,8 @@ def delete_conversation(self, source: db.Source) -> None: handler will display an error. """ job = DeleteConversationJob(source.uuid) - job.success_signal.connect(self.on_delete_conversation_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_delete_conversation_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_delete_conversation_success) + job.failure_signal.connect(self.on_delete_conversation_failure) self.add_job.emit(job) self.conversation_deleted.emit(source.uuid) @@ -1066,8 +1066,8 @@ def download_conversation(self, id: state.ConversationId) -> None: for file in files: if not file.is_downloaded: job = FileDownloadJob(str(file.id), self.data_dir, self.gpg) - job.success_signal.connect(self.on_file_download_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_file_download_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_file_download_success) + job.failure_signal.connect(self.on_file_download_failure) self.add_job.emit(job) self.file_download_started.emit(file.id) @@ -1107,8 +1107,8 @@ def send_reply(self, source_uuid: str, reply_uuid: str, message: str) -> None: self.session.commit() job = SendReplyJob(source_uuid, reply_uuid, message, self.gpg) - job.success_signal.connect(self.on_reply_success, type=Qt.QueuedConnection) - job.failure_signal.connect(self.on_reply_failure, type=Qt.QueuedConnection) + job.success_signal.connect(self.on_reply_success) + job.failure_signal.connect(self.on_reply_failure) self.add_job.emit(job) diff --git a/securedrop_client/sync.py b/securedrop_client/sync.py index f70635109..e92688f2f 100644 --- a/securedrop_client/sync.py +++ b/securedrop_client/sync.py @@ -1,7 +1,7 @@ import logging from typing import Optional -from PyQt5.QtCore import QObject, Qt, QThread, QTimer, pyqtSignal +from PyQt5.QtCore import QObject, QThread, QTimer, pyqtBoundSignal, pyqtSignal from sdclientapi import API from sqlalchemy.orm import scoped_session @@ -110,7 +110,7 @@ def __init__( # type: ignore [no-untyped-def] session_maker: scoped_session, gpg: GpgHelper, data_dir: str, - sync_started: pyqtSignal, + sync_started: pyqtBoundSignal, on_sync_success, on_sync_failure, app_state: Optional[state.State] = None, @@ -126,8 +126,8 @@ def __init__( # type: ignore [no-untyped-def] self.on_sync_failure = on_sync_failure self.job = MetadataSyncJob(self.data_dir, app_state) - self.job.success_signal.connect(self.on_sync_success, type=Qt.QueuedConnection) - self.job.failure_signal.connect(self.on_sync_failure, type=Qt.QueuedConnection) + self.job.success_signal.connect(self.on_sync_success) + self.job.failure_signal.connect(self.on_sync_failure) def sync(self) -> None: """ diff --git a/tests/conftest.py b/tests/conftest.py index 86dceed5d..f6dbc4b98 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -203,7 +203,7 @@ def functional_test_offline_context(functional_test_logged_in_context, qtbot): # Trigger log out # Note: The qtbot object cannot interact with QAction items (as used in the logout button/menu), # so we programatically logout rather than using the GUI via qtbot - gui.left_pane.user_profile.user_button.menu.logout.trigger() + gui.left_pane.user_profile.user_button._menu.logout.trigger() def check_login_button(): assert gui.left_pane.user_profile.login_button.isVisible() diff --git a/tests/functional/test_logout.py b/tests/functional/test_logout.py index 5c90012f3..bc7ea428a 100644 --- a/tests/functional/test_logout.py +++ b/tests/functional/test_logout.py @@ -22,7 +22,7 @@ def test_logout_as_journalist(functional_test_logged_in_context, qtbot, mocker): # Trigger log out # Note: The qtbot object cannot interact with QAction items (as used in the logout button/menu), # so we programatically logout rather than using the GUI via qtbot - gui.left_pane.user_profile.user_button.menu.logout.trigger() + gui.left_pane.user_profile.user_button._menu.logout.trigger() def login_button_is_visible(): assert gui.left_pane.user_profile.login_button.isVisible() diff --git a/tests/functional/test_user_profile_menu.py b/tests/functional/test_user_profile_menu.py index 2601c41ad..55fdd170c 100644 --- a/tests/functional/test_user_profile_menu.py +++ b/tests/functional/test_user_profile_menu.py @@ -27,7 +27,7 @@ def test_user_icon_click(qtbot, mocker, functional_test_logged_in_context): pyautogui.click(cursor_x, cursor_y) def user_menu_is_visible(): - assert gui.left_pane.user_profile.user_button.menu.logout.isVisible() + assert gui.left_pane.user_profile.user_button._menu.logout.isVisible() # Ensure user menu is visible after clicking on the user icon qtbot.waitUntil(user_menu_is_visible, timeout=TIME_CLICK_ACTION) diff --git a/tests/gui/base/test_misc.py b/tests/gui/base/test_misc.py index 6cd486d5d..90ef01a2b 100644 --- a/tests/gui/base/test_misc.py +++ b/tests/gui/base/test_misc.py @@ -52,7 +52,7 @@ def test_SvgToggleButton_set_icon(mocker): load_icon_fn.assert_called_with(normal="mock_on", normal_off="mock_off") setIcon_fn.assert_called_with(icon) - assert stb.icon == icon + assert stb._icon == icon def test_SvgPushButton_init(mocker): diff --git a/tests/gui/conversation/export/test_device.py b/tests/gui/conversation/export/test_device.py index 28800e782..710ce59af 100644 --- a/tests/gui/conversation/export/test_device.py +++ b/tests/gui/conversation/export/test_device.py @@ -2,7 +2,6 @@ from PyQt5.QtTest import QSignalSpy -from securedrop_client import export from securedrop_client.app import threads from securedrop_client.gui.conversation.export import Device from securedrop_client.gui.main import Window @@ -14,7 +13,7 @@ def no_session(): pass -def test_Device_run_printer_preflight_checks(homedir, mocker, source): +def test_Device_run_printer_preflight_checks(homedir, mocker, source, export_service): gui = mocker.MagicMock(spec=Window) with threads(3) as [sync_thread, main_queue_thread, file_download_queue_thread]: controller = Controller( @@ -27,7 +26,6 @@ def test_Device_run_printer_preflight_checks(homedir, mocker, source): main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - export_service = export.Service() device = Device(controller, export_service) print_preflight_check_requested_emissions = QSignalSpy( device.print_preflight_check_requested @@ -38,7 +36,7 @@ def test_Device_run_printer_preflight_checks(homedir, mocker, source): assert len(print_preflight_check_requested_emissions) == 1 -def test_Device_run_print_file(mocker, homedir): +def test_Device_run_print_file(mocker, homedir, export_service): gui = mocker.MagicMock(spec=Window) with threads(3) as [sync_thread, main_queue_thread, file_download_queue_thread]: controller = Controller( @@ -51,7 +49,6 @@ def test_Device_run_print_file(mocker, homedir): main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - export_service = export.Service() device = Device(controller, export_service) print_requested_emissions = QSignalSpy(device.print_requested) file = factory.File(source=factory.Source()) @@ -67,7 +64,7 @@ def test_Device_run_print_file(mocker, homedir): assert len(print_requested_emissions) == 1 -def test_Device_print_file_file_missing(homedir, mocker, session): +def test_Device_print_file_file_missing(homedir, mocker, session, export_service): """ If the file is missing from the data dir, is_downloaded should be set to False and the failure should be communicated to the user. @@ -84,7 +81,6 @@ def test_Device_print_file_file_missing(homedir, mocker, session): main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - export_service = export.Service() device = Device(controller, export_service) file = factory.File(source=factory.Source()) mocker.patch("securedrop_client.logic.Controller.get_file", return_value=file) @@ -98,7 +94,9 @@ def test_Device_print_file_file_missing(homedir, mocker, session): warning_logger.assert_called_once_with(log_msg) -def test_Device_print_file_when_orig_file_already_exists(homedir, config, mocker, source): +def test_Device_print_file_when_orig_file_already_exists( + homedir, config, mocker, source, export_service +): """ The signal `print_requested` should still be emitted if the original file already exists. """ @@ -114,7 +112,6 @@ def test_Device_print_file_when_orig_file_already_exists(homedir, config, mocker main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - export_service = export.Service() device = Device(controller, export_service) file = factory.File(source=factory.Source()) print_requested_emissions = QSignalSpy(device.print_requested) @@ -127,7 +124,7 @@ def test_Device_print_file_when_orig_file_already_exists(homedir, config, mocker controller.get_file.assert_called_with(file.uuid) -def test_Device_run_export_preflight_checks(homedir, mocker, source): +def test_Device_run_export_preflight_checks(homedir, mocker, source, export_service): gui = mocker.MagicMock(spec=Window) with threads(3) as [sync_thread, main_queue_thread, file_download_queue_thread]: controller = Controller( @@ -140,7 +137,7 @@ def test_Device_run_export_preflight_checks(homedir, mocker, source): main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - device = Device(controller, mocker.MagicMock(spec=export.Service)) + device = Device(controller, export_service) export_preflight_check_requested_emissions = QSignalSpy( device.export_preflight_check_requested ) @@ -152,7 +149,7 @@ def test_Device_run_export_preflight_checks(homedir, mocker, source): assert len(export_preflight_check_requested_emissions) == 1 -def test_Device_export_file_to_usb_drive(homedir, mocker): +def test_Device_export_file_to_usb_drive(homedir, mocker, export_service): """ The signal `export_requested` should be emitted during export_file_to_usb_drive. """ @@ -168,7 +165,6 @@ def test_Device_export_file_to_usb_drive(homedir, mocker): main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - export_service = export.Service() device = Device(controller, export_service) export_requested_emissions = QSignalSpy(device.export_requested) file = factory.File(source=factory.Source()) @@ -184,7 +180,7 @@ def test_Device_export_file_to_usb_drive(homedir, mocker): assert len(export_requested_emissions) == 1 -def test_Device_export_file_to_usb_drive_file_missing(homedir, mocker, session): +def test_Device_export_file_to_usb_drive_file_missing(homedir, mocker, session, export_service): """ If the file is missing from the data dir, is_downloaded should be set to False and the failure should be communicated to the user. @@ -201,7 +197,6 @@ def test_Device_export_file_to_usb_drive_file_missing(homedir, mocker, session): main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - export_service = export.Service() device = Device(controller, export_service) file = factory.File(source=factory.Source()) mocker.patch("securedrop_client.logic.Controller.get_file", return_value=file) @@ -216,7 +211,7 @@ def test_Device_export_file_to_usb_drive_file_missing(homedir, mocker, session): def test_Device_export_file_to_usb_drive_when_orig_file_already_exists( - homedir, config, mocker, source + homedir, config, mocker, source, export_service ): """ The signal `export_requested` should still be emitted if the original file already exists. @@ -233,7 +228,6 @@ def test_Device_export_file_to_usb_drive_when_orig_file_already_exists( main_queue_thread=main_queue_thread, file_download_queue_thread=file_download_queue_thread, ) - export_service = export.Service() device = Device(controller, export_service) export_requested_emissions = QSignalSpy(device.export_requested) file = factory.File(source=factory.Source()) diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 86a89b7a5..d53aa25b4 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -10,8 +10,8 @@ import arrow import sqlalchemy import sqlalchemy.orm.exc -from PyQt5.QtCore import QEvent, QSize, Qt -from PyQt5.QtGui import QFocusEvent, QMovie, QResizeEvent +from PyQt5.QtCore import QEvent, QPointF, QSize, Qt +from PyQt5.QtGui import QFocusEvent, QMouseEvent, QMovie, QResizeEvent from PyQt5.QtTest import QTest from PyQt5.QtWidgets import QVBoxLayout, QWidget from sqlalchemy.orm import attributes, scoped_session, sessionmaker @@ -438,12 +438,12 @@ def test_UserIconLabel_clicks(mocker): def test_UserButton_setup(mocker): ub = UserButton() - ub.menu = mocker.MagicMock() + ub._menu = mocker.MagicMock() controller = mocker.MagicMock() ub.setup(controller) - ub.menu.setup.assert_called_once_with(controller) + ub._menu.setup.assert_called_once_with(controller) def test_UserButton_set_username(): @@ -498,11 +498,11 @@ def test_LoginButton_setup(mocker): lb.window = window -def test_Loginbutton_on_clicked(mocker): +def test_LoginButton_on_clicked(mocker): lb = LoginButton() - lb.window = mocker.MagicMock() + lb._window = mocker.MagicMock() lb._on_clicked() - lb.window.show_login.assert_called_once_with() + lb._window.show_login.assert_called_once_with() def test_MainView_init(): @@ -539,13 +539,13 @@ def test_MainView_show_sources_with_none_selected(mocker): source_item = SourceListWidgetItem(mv.source_list) mv.source_list.setItemWidget(source_item, source_widget) mv.source_list.source_items["stub_uuid"] = source_item - mocker.patch.object(mv.source_list, "update") + mocker.patch.object(mv.source_list, "update_sources") mv.empty_conversation_view = mocker.MagicMock() mv.show_sources([1, 2, 3]) - mv.source_list.update.assert_called_once_with([1, 2, 3]) + mv.source_list.update_sources.assert_called_once_with([1, 2, 3]) mv.empty_conversation_view.show_no_source_selected_message.assert_called_once_with() mv.empty_conversation_view.show.assert_called_once_with() @@ -574,7 +574,7 @@ def test_MainView_show_sources_with_no_sources_at_all(mocker): mv.show_sources([]) - mv.source_list.update.assert_called_once_with([]) + mv.source_list.update_sources.assert_called_once_with([]) mv.empty_conversation_view.show_no_sources_message.assert_called_once_with() mv.empty_conversation_view.show.assert_called_once_with() @@ -586,12 +586,12 @@ def test_MainView_show_sources_when_sources_are_deleted(mocker): mv = MainView(None) mv.source_list = mocker.MagicMock() mv.empty_conversation_view = mocker.MagicMock() - mv.source_list.update = mocker.MagicMock(return_value=[]) + mv.source_list.update_sources = mocker.MagicMock(return_value=[]) mv.delete_conversation = mocker.MagicMock() mv.show_sources([1, 2, 3, 4]) - mv.source_list.update = mocker.MagicMock(return_value=[4]) + mv.source_list.update_sources = mocker.MagicMock(return_value=[4]) mv.show_sources([1, 2, 3]) @@ -816,7 +816,7 @@ def test_MainView_refresh_source_conversations(homedir, mocker, qtbot, session_m controller.api = mocker.MagicMock() mv.setup(controller) - mv.source_list.update(sources) + mv.source_list.update_sources(sources) mv.show() # get the conversations created @@ -905,7 +905,7 @@ def test_MainView_refresh_source_conversations_with_deleted( sources = [source1, source2] - mv.source_list.update(sources) + mv.source_list.update_sources(sources) mv.show() def collection_error(): @@ -965,7 +965,7 @@ def test_SourceList_get_selected_source(mocker): sl = SourceList() sl.controller = mocker.MagicMock() sources = [factory.Source(), factory.Source()] - sl.update(sources) + sl.update_sources(sources) assert sl.get_selected_source() is None @@ -996,7 +996,7 @@ def test_SourceList_update_adds_new_sources(mocker): mocker.patch("securedrop_client.gui.widgets.SourceListWidgetItem", mock_lwi) sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock()] - sl.update(sources) + sl.update_sources(sources) assert mock_sw.call_count == len(sources) assert mock_lwi.call_count == len(sources) @@ -1050,7 +1050,7 @@ def test_SourceList_update_when_source_deleted(mocker, session, session_maker, h # add it to the SourceList sl = SourceList() sl.setup(controller) - deleted_uuids = sl.update([oss]) + deleted_uuids = sl.update_sources([oss]) assert not deleted_uuids assert len(sl.source_items) == 1 @@ -1060,13 +1060,13 @@ def test_SourceList_update_when_source_deleted(mocker, session, session_maker, h # now verify that updating does not raise an exception, and that the SourceWidget still exists # since the sync will end up calling update again and delete it - deleted_uuids = sl.update([source]) + deleted_uuids = sl.update_sources([source]) assert len(deleted_uuids) == 0 assert not deleted_uuids assert len(sl.source_items) == 1 # finish sync simulation where a local source is deleted - deleted_uuids = sl.update([]) + deleted_uuids = sl.update_sources([]) assert len(deleted_uuids) == 1 assert source.uuid in deleted_uuids assert len(sl.source_items) == 0 @@ -1122,17 +1122,19 @@ def test_SourceList_update_does_not_raise_exc(mocker): sl = SourceList() sl.controller = mocker.MagicMock() source = DeletedSource() - sl.update([source]) + sl.update_sources([source]) def test_SourceList_update_does_not_raise_exc_when_itemWidget_is_none(mocker): sl = SourceList() sl.controller = mocker.MagicMock() source = factory.Source() - sl.update([source]) # first add the source so that the next time the same source is updated + sl.update_sources( + [source] + ) # first add the source so that the next time the same source is updated sl.itemWidget = mocker.MagicMock(return_value=None) - sl.update([source]) # does not raise exception + sl.update_sources([source]) # does not raise exception def test_SourceList_update_maintains_selection(mocker): @@ -1142,16 +1144,16 @@ def test_SourceList_update_maintains_selection(mocker): sl = SourceList() sl.controller = mocker.MagicMock() sources = [factory.Source(), factory.Source()] - sl.update(sources) + sl.update_sources(sources) sl.setCurrentItem(sl.itemAt(0, 0)) # select the first source - sl.update(sources) + sl.update_sources(sources) assert sl.currentItem() assert sl.itemWidget(sl.currentItem()).source.id == sources[0].id sl.setCurrentItem(sl.itemAt(1, 0)) # select the second source - sl.update(sources) + sl.update_sources(sources) assert sl.currentItem() assert sl.itemWidget(sl.currentItem()).source.id == sources[1].id @@ -1163,12 +1165,12 @@ def test_SourceList_update_with_pre_selected_source_maintains_selection(mocker): """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update([factory.Source(), factory.Source()]) + sl.update_sources([factory.Source(), factory.Source()]) second_item = sl.itemAt(1, 0) sl.setCurrentItem(second_item) # select the second source mocker.patch.object(second_item, "isSelected", return_value=True) - sl.update([factory.Source(), factory.Source()]) + sl.update_sources([factory.Source(), factory.Source()]) assert second_item.isSelected() is True @@ -1179,10 +1181,10 @@ def test_SourceList_update_removes_selected_item_results_in_no_current_selection """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update([factory.Source(uuid="new"), factory.Source(uuid="newer")]) + sl.update_sources([factory.Source(uuid="new"), factory.Source(uuid="newer")]) sl.setCurrentItem(sl.itemAt(0, 0)) # select source with uuid='newer' - sl.update([factory.Source(uuid="new")]) # delete source with uuid='newer' + sl.update_sources([factory.Source(uuid="new")]) # delete source with uuid='newer' assert not sl.currentItem() @@ -1193,11 +1195,11 @@ def test_SourceList_update_removes_item_from_end_of_list(mocker): """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update( + sl.update_sources( [factory.Source(uuid="new"), factory.Source(uuid="newer"), factory.Source(uuid="newest")] ) assert sl.count() == 3 - sl.update([factory.Source(uuid="newer"), factory.Source(uuid="newest")]) + sl.update_sources([factory.Source(uuid="newer"), factory.Source(uuid="newest")]) assert sl.count() == 2 assert sl.itemWidget(sl.item(0)).source.uuid == "newest" assert sl.itemWidget(sl.item(1)).source.uuid == "newer" @@ -1210,11 +1212,11 @@ def test_SourceList_update_removes_item_from_middle_of_list(mocker): """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update( + sl.update_sources( [factory.Source(uuid="new"), factory.Source(uuid="newer"), factory.Source(uuid="newest")] ) assert sl.count() == 3 - sl.update([factory.Source(uuid="new"), factory.Source(uuid="newest")]) + sl.update_sources([factory.Source(uuid="new"), factory.Source(uuid="newest")]) assert sl.count() == 2 assert sl.itemWidget(sl.item(0)).source.uuid == "newest" assert sl.itemWidget(sl.item(1)).source.uuid == "new" @@ -1227,11 +1229,11 @@ def test_SourceList_update_removes_item_from_beginning_of_list(mocker): """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update( + sl.update_sources( [factory.Source(uuid="new"), factory.Source(uuid="newer"), factory.Source(uuid="newest")] ) assert sl.count() == 3 - sl.update([factory.Source(uuid="new"), factory.Source(uuid="newer")]) + sl.update_sources([factory.Source(uuid="new"), factory.Source(uuid="newer")]) assert sl.count() == 2 assert sl.itemWidget(sl.item(0)).source.uuid == "newer" assert sl.itemWidget(sl.item(1)).source.uuid == "new" @@ -1330,7 +1332,7 @@ def test_SourceList_set_snippet(mocker): def test_SourceList_get_source_widget(mocker): sl = SourceList() sl.controller = mocker.MagicMock() - sl.update([factory.Source(uuid="mock_uuid")]) + sl.update_sources([factory.Source(uuid="mock_uuid")]) sl.source_items = {} source_widget = sl.get_source_widget("mock_uuid") @@ -1343,7 +1345,7 @@ def test_SourceList_get_source_widget_does_not_exist(mocker): sl = SourceList() sl.controller = mocker.MagicMock() mock_source = factory.Source(uuid="mock_uuid") - sl.update([mock_source]) + sl.update_sources([mock_source]) sl.source_items = {} source_widget = sl.get_source_widget("uuid_for_source_not_in_list") @@ -1636,7 +1638,7 @@ def test_SourceWidget_html_init(mocker): sw.summary_layout = mocker.MagicMock() mocker.patch("securedrop_client.gui.base.SvgLabel") - sw.update() + sw.reload() sw.name.setText.assert_called_once_with("foo bar baz") @@ -1826,12 +1828,12 @@ def test_SourceWidget_update_attachment_icon(mocker): mark_seen_signal = mocker.MagicMock() sw = SourceWidget(controller, source, mark_seen_signal, mocker.MagicMock()) - sw.update() + sw.reload() assert not sw.paperclip.isHidden() source.document_count = 0 - sw.update() + sw.reload() assert sw.paperclip.isHidden() @@ -1848,7 +1850,7 @@ def test_SourceWidget_update_does_not_raise_exception(mocker): controller.session.refresh.side_effect = ex mock_logger = mocker.MagicMock() mocker.patch("securedrop_client.gui.widgets.logger", mock_logger) - sw.update() + sw.reload() assert mock_logger.debug.call_count == 1 @@ -1859,7 +1861,7 @@ def test_SourceWidget_update_skips_setting_snippet_if_deletion_in_progress(mocke sw = SourceWidget(mocker.MagicMock(), factory.Source(), mocker.MagicMock(), mocker.MagicMock()) sw.deleting = True sw.set_snippet = mocker.MagicMock() - sw.update() + sw.reload() sw.set_snippet.assert_not_called() @@ -1871,7 +1873,7 @@ def test_SourceWidget_update_skips_setting_snippet_if_sync_is_stale(mocker): sw.sync_started_timestamp = datetime.now() sw.deletion_scheduled_timestamp = datetime.now() sw.set_snippet = mocker.MagicMock() - sw.update() + sw.reload() sw.set_snippet.assert_not_called() @@ -1882,7 +1884,7 @@ def test_SourceWidget_update_skips_setting_snippet_if_conversation_deletion_in_p sw = SourceWidget(mocker.MagicMock(), factory.Source(), mocker.MagicMock(), mocker.MagicMock()) sw.deleting_conversation = True sw.set_snippet = mocker.MagicMock() - sw.update() + sw.reload() sw.set_snippet.assert_not_called() @@ -1948,7 +1950,7 @@ def test_SourceWidget_update_truncate_latest_msg(mocker): mark_seen_signal = mocker.MagicMock() sw = SourceWidget(controller, source, mark_seen_signal, mocker.MagicMock()) - sw.update() + sw.reload() assert sw.preview.text().endswith("...") @@ -2074,7 +2076,7 @@ def test_SourceWidget_preview_after_conversation_deleted(mocker, session, i18n): controller = mocker.MagicMock() sw = SourceWidget(controller, source, mocker.MagicMock(), mocker.MagicMock()) - sw.update() + sw.reload() assert sw.preview.property("class") == "conversation_deleted" assert sw.preview.text() == _("\u2014 All files and messages deleted for this source \u2014") @@ -2137,12 +2139,12 @@ def test_SourceWidget_uses_SecureQLabel(mocker): mark_seen_signal = mocker.MagicMock() sw = SourceWidget(controller, source, mark_seen_signal, mocker.MagicMock()) - sw.update() + sw.reload() assert isinstance(sw.preview, SecureQLabel) sw.preview.setTextFormat = mocker.MagicMock() sw.preview.setText("bad text") - sw.update() + sw.reload() sw.preview.setTextFormat.assert_called_with(Qt.PlainText) @@ -3160,8 +3162,9 @@ def test_FileWidget_event_handler_left_click(mocker, session, source): get_file = mocker.MagicMock(return_value=file_) controller = mocker.MagicMock(get_file=get_file) - test_event = QEvent(QEvent.MouseButtonPress) - test_event.button = mocker.MagicMock(return_value=Qt.LeftButton) + test_event = QMouseEvent( + QEvent.MouseButtonPress, QPointF(), Qt.LeftButton, Qt.NoButton, Qt.NoModifier + ) fw = FileWidget( file_.uuid, @@ -3894,7 +3897,7 @@ def test_ConversationView_init(mocker, homedir): user = factory.User() mocked_controller = mocker.MagicMock(authenticated_user=user) cv = ConversationView(mocked_source, mocked_controller) - assert isinstance(cv.scroll.conversation_layout, QVBoxLayout) + assert isinstance(cv._scroll.conversation_layout, QVBoxLayout) def test_ConversationView_init_with_deleted_source(mocker, homedir, session): @@ -3941,12 +3944,12 @@ def test_ConversationView_ConversationScrollArea_resize(mocker): file_widget_adjust_width = mocker.patch("securedrop_client.gui.widgets.FileWidget.adjust_width") cv.setFixedWidth(800) - event = QResizeEvent(cv.scroll.size(), QSize(123_456_789, 123_456_789)) - cv.scroll.resizeEvent(event) + event = QResizeEvent(cv._scroll.size(), QSize(123_456_789, 123_456_789)) + cv._scroll.resizeEvent(event) - assert cv.scroll.widget().width() == cv.scroll.width() - speech_bubble_adjust_width.assert_called_with(cv.scroll.widget().width()) - file_widget_adjust_width.assert_called_with(cv.scroll.widget().width()) + assert cv._scroll.widget().width() == cv._scroll.width() + speech_bubble_adjust_width.assert_called_with(cv._scroll.widget().width()) + file_widget_adjust_width.assert_called_with(cv._scroll.widget().width()) def test_ConversationView__on_sync_started(mocker, session): @@ -3969,7 +3972,7 @@ def test_ConversationView__on_conversation_deletion_successful(mocker, session): cv._on_conversation_deletion_successful(cv.source.uuid, timestamp) assert cv.deletion_scheduled_timestamp == timestamp - assert cv.scroll.isHidden() + assert cv._scroll.isHidden() assert cv.deleted_conversation_items_marker.isHidden() assert not cv.deleted_conversation_marker.isHidden() assert cv.current_messages[message.uuid].isHidden() @@ -3983,13 +3986,13 @@ def test_ConversationView__on_conversation_deletion_successful_with_mismatched_s source = factory.Source(uuid="abc123") cv = ConversationView(source, mocker.MagicMock()) - assert not cv.scroll.isHidden() + assert not cv._scroll.isHidden() assert cv.deleted_conversation_items_marker.isHidden() assert cv.deleted_conversation_marker.isHidden() cv._on_conversation_deletion_successful("notabc123", datetime.now()) - assert not cv.scroll.isHidden() + assert not cv._scroll.isHidden() assert cv.deleted_conversation_items_marker.isHidden() assert cv.deleted_conversation_marker.isHidden() @@ -4013,7 +4016,7 @@ def test_ConversationView__on_conversation_deletion_successful_does_not_hide_dra cv._on_conversation_deletion_successful(cv.source.uuid, datetime.now()) - assert not cv.scroll.isHidden() + assert not cv._scroll.isHidden() assert not cv.deleted_conversation_items_marker.isHidden() assert cv.deleted_conversation_marker.isHidden() assert cv.current_messages[message.uuid].isHidden() @@ -4039,13 +4042,13 @@ def test_ConversationView_update_conversation_position_follow(mocker, homedir): # Flag to denote a reply was sent by the user. cv.reply_flag = True - cv.scroll.verticalScrollBar().value = mocker.MagicMock(return_value=5900) - cv.scroll.viewport().height = mocker.MagicMock(return_value=500) - cv.scroll.verticalScrollBar().setValue = mocker.MagicMock() + cv._scroll.verticalScrollBar().value = mocker.MagicMock(return_value=5900) + cv._scroll.viewport().height = mocker.MagicMock(return_value=500) + cv._scroll.verticalScrollBar().setValue = mocker.MagicMock() cv.update_conversation_position(0, 6000) - cv.scroll.verticalScrollBar().setValue.assert_called_once_with(6000) + cv._scroll.verticalScrollBar().setValue.assert_called_once_with(6000) def test_ConversationView_update_conversation_position_stay_fixed(mocker, homedir): @@ -4062,13 +4065,13 @@ def test_ConversationView_update_conversation_position_stay_fixed(mocker, homedi cv = ConversationView(mocked_source, mocked_controller) - cv.scroll.verticalScrollBar().value = mocker.MagicMock(return_value=5500) - cv.scroll.viewport().height = mocker.MagicMock(return_value=500) - cv.scroll.verticalScrollBar().setValue = mocker.MagicMock() + cv._scroll.verticalScrollBar().value = mocker.MagicMock(return_value=5500) + cv._scroll.viewport().height = mocker.MagicMock(return_value=500) + cv._scroll.verticalScrollBar().setValue = mocker.MagicMock() cv.update_conversation_position(0, 6000) - cv.scroll.verticalScrollBar().setValue.assert_not_called() + cv._scroll.verticalScrollBar().setValue.assert_not_called() def test_ConversationView_add_message(mocker, session, session_maker, homedir): @@ -4088,7 +4091,7 @@ def test_ConversationView_add_message(mocker, session, session_maker, homedir): cv.add_message(message=message, index=0) # Check that we added the correct widget to the layout - message_widget = cv.scroll.conversation_layout.itemAt(1).widget() + message_widget = cv._scroll.conversation_layout.itemAt(1).widget() assert isinstance(message_widget, MessageWidget) assert message_widget.message.text() == ">^..^<" @@ -4116,7 +4119,7 @@ def test_ConversationView_add_message_no_content(mocker, session, session_maker, cv.add_message(message=message, index=0) # Check that we added the correct widget to the layout - message_widget = cv.scroll.conversation_layout.itemAt(1).widget() + message_widget = cv._scroll.conversation_layout.itemAt(1).widget() assert isinstance(message_widget, MessageWidget) assert message_widget.message.text() == "" @@ -4197,7 +4200,7 @@ def test_ConversationView_add_reply(mocker, homedir, session, session_maker): cv.add_reply(reply=reply, sender=sender, index=0) # Check that we added the correct widget to the layout - reply_widget = cv.scroll.conversation_layout.itemAt(1).widget() + reply_widget = cv._scroll.conversation_layout.itemAt(1).widget() assert isinstance(reply_widget, ReplyWidget) assert reply_widget.uuid == "abc123" @@ -4232,7 +4235,7 @@ def test_ConversationView_add_reply_no_content(mocker, homedir, session_maker, s cv.add_reply(reply=reply, sender=sender, index=0) # Check that we added the correct widget to the layout - reply_widget = cv.scroll.conversation_layout.itemAt(1).widget() + reply_widget = cv._scroll.conversation_layout.itemAt(1).widget() assert isinstance(reply_widget, ReplyWidget) assert reply_widget.uuid == "abc123" @@ -4267,7 +4270,7 @@ def test_ConversationView_add_reply_that_has_current_user_as_sender( cv.add_reply(reply=reply, sender=authenticated_user, index=0) # Check that we added the correct widget to the layout - reply_widget = cv.scroll.conversation_layout.itemAt(1).widget() + reply_widget = cv._scroll.conversation_layout.itemAt(1).widget() assert isinstance(reply_widget, ReplyWidget) assert reply_widget.uuid == "abc123" @@ -4306,7 +4309,7 @@ def test_ConversationView_add_downloaded_file(mocker, homedir, source, session): mock_label.assert_called_with("123B") # default factory filesize assert cv.conversation_updated.emit.call_count == 1 - file_widget = cv.scroll.conversation_layout.itemAt(1).widget() + file_widget = cv._scroll.conversation_layout.itemAt(1).widget() assert isinstance(file_widget, FileWidget) @@ -4334,7 +4337,7 @@ def test_ConversationView_add_not_downloaded_file(mocker, homedir, source, sessi mock_label.assert_called_with("123B") # default factory filesize assert cv.conversation_updated.emit.call_count == 1 - file_widget = cv.scroll.conversation_layout.itemAt(1).widget() + file_widget = cv._scroll.conversation_layout.itemAt(1).widget() assert isinstance(file_widget, FileWidget) @@ -4850,11 +4853,11 @@ def test_update_conversation_maintains_old_items(mocker, session): controller = mocker.MagicMock(get_file=get_file, authenticated_user=user) cv = ConversationView(source, controller) - assert cv.scroll.conversation_layout.count() == 3 + assert cv._scroll.conversation_layout.count() == 3 cv.update_conversation(cv.source.collection) - assert cv.scroll.conversation_layout.count() == 3 + assert cv._scroll.conversation_layout.count() == 3 def test_update_conversation_does_not_remove_pending_draft_items(mocker, session): @@ -4881,7 +4884,7 @@ def test_update_conversation_does_not_remove_pending_draft_items(mocker, session controller = mocker.MagicMock(get_file=get_file, authenticated_user=user) cv = ConversationView(source, controller) - assert cv.scroll.conversation_layout.count() == 3 # precondition with draft + assert cv._scroll.conversation_layout.count() == 3 # precondition with draft # add the new message and persist new_message = factory.Message(filename="4-source-msg.gpg", source=source) @@ -4890,7 +4893,7 @@ def test_update_conversation_does_not_remove_pending_draft_items(mocker, session # New message added, draft message persists. cv.update_conversation(cv.source.collection) - assert cv.scroll.conversation_layout.count() == 4 + assert cv._scroll.conversation_layout.count() == 4 def test_update_conversation_does_remove_successful_draft_items(mocker, session): @@ -4917,7 +4920,7 @@ def test_update_conversation_does_remove_successful_draft_items(mocker, session) controller = mocker.MagicMock(get_file=get_file, authenticated_user=user) cv = ConversationView(source, controller) - assert cv.scroll.conversation_layout.count() == 3 # precondition with draft + assert cv._scroll.conversation_layout.count() == 3 # precondition with draft # add the new message and persist new_message = factory.Message(filename="4-source-msg.gpg", source=source) @@ -4930,7 +4933,7 @@ def test_update_conversation_does_remove_successful_draft_items(mocker, session) # New message added, draft message is gone. cv.update_conversation(cv.source.collection) - assert cv.scroll.conversation_layout.count() == 3 + assert cv._scroll.conversation_layout.count() == 3 def test_update_conversation_keeps_failed_draft_items(mocker, session): @@ -4957,7 +4960,7 @@ def test_update_conversation_keeps_failed_draft_items(mocker, session): controller = mocker.MagicMock(get_file=get_file, authenticated_user=user) cv = ConversationView(source, controller) - assert cv.scroll.conversation_layout.count() == 3 # precondition with draft + assert cv._scroll.conversation_layout.count() == 3 # precondition with draft # add the new message and persist new_message = factory.Message(filename="4-source-msg.gpg", source=source) @@ -4966,7 +4969,7 @@ def test_update_conversation_keeps_failed_draft_items(mocker, session): # New message added, draft message retained. cv.update_conversation(cv.source.collection) - assert cv.scroll.conversation_layout.count() == 4 + assert cv._scroll.conversation_layout.count() == 4 def test_update_conversation_adds_new_items(mocker, session): @@ -4990,7 +4993,7 @@ def test_update_conversation_adds_new_items(mocker, session): controller = mocker.MagicMock(get_file=get_file, authenticated_user=user) cv = ConversationView(source, controller) - assert cv.scroll.conversation_layout.count() == 3 # precondition + assert cv._scroll.conversation_layout.count() == 3 # precondition # add the new message and persist new_message = factory.Message(filename="4-source-msg.gpg", source=source) @@ -4998,7 +5001,7 @@ def test_update_conversation_adds_new_items(mocker, session): session.commit() cv.update_conversation(cv.source.collection) - assert cv.scroll.conversation_layout.count() == 4 + assert cv._scroll.conversation_layout.count() == 4 def test_update_conversation_position_updates(mocker, session): @@ -5022,7 +5025,7 @@ def test_update_conversation_position_updates(mocker, session): controller = mocker.MagicMock(get_file=get_file, authenticated_user=user) cv = ConversationView(source, controller) - assert cv.scroll.conversation_layout.count() == 3 # precondition + assert cv._scroll.conversation_layout.count() == 3 # precondition # Change the position of the Reply. reply_widget = cv.current_messages[reply.uuid] @@ -5034,7 +5037,7 @@ def test_update_conversation_position_updates(mocker, session): session.commit() cv.update_conversation(cv.source.collection) - assert cv.scroll.conversation_layout.count() == 4 + assert cv._scroll.conversation_layout.count() == 4 assert reply_widget.index == 2 # re-ordered. @@ -5059,11 +5062,15 @@ def test_update_conversation_content_updates(mocker, session): cv.update_deletion_markers = mocker.MagicMock() cv.current_messages = {} - cv.scroll.conversation_layout.removeWidget = mocker.MagicMock() - mocker.patch.object(cv.scroll, "add_widget_to_conversation") + cv._scroll.conversation_layout.removeWidget = mocker.MagicMock() + mocker.patch.object(cv._scroll, "add_widget_to_conversation") # this is the MessageWidget that __init__() would return - mock_msg_widget_res = mocker.MagicMock() + # FIXME: It is a sign that something isn't quite right that index and message must + # be manually added to the MessageWidget spec. + mock_msg_widget_res = mocker.MagicMock( + spec=MessageWidget, index=None, message=mocker.MagicMock(text=lambda: None) + ) # mock MessageWidget so we can inspect the __init__ call to see what content # is in the widget. mock_msg_widget = mocker.patch( diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 00702ab35..3a2411345 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -36,7 +36,7 @@ def main_window(mocker, homedir): # Create a source widget source_list = gui.main_view.source_list source = factory.Source() - source_list.update([source]) + source_list.update_sources([source]) # Create a file widget, message widget, and reply widget mocker.patch("securedrop_client.gui.widgets.humanize_filesize", return_value="100") @@ -88,7 +88,7 @@ def main_window_no_key(mocker, homedir): # Create a source widget source_list = gui.main_view.source_list source = factory.Source(public_key=None) - source_list.update([source]) + source_list.update_sources([source]) # Create a file widget, message widget, and reply widget mocker.patch("securedrop_client.gui.widgets.humanize_filesize", return_value="100") diff --git a/tests/integration/test_styles_file_download_button.py b/tests/integration/test_styles_file_download_button.py index 5d657195f..7a428b597 100644 --- a/tests/integration/test_styles_file_download_button.py +++ b/tests/integration/test_styles_file_download_button.py @@ -6,7 +6,7 @@ def test_styles(mocker, main_window): wrapper = main_window.main_view.view_layout.itemAt(0).widget() - conversation_scrollarea = wrapper.conversation_view.scroll + conversation_scrollarea = wrapper.conversation_view._scroll file_widget = conversation_scrollarea.widget().layout().itemAt(0).widget() download_button = file_widget.download_button @@ -29,7 +29,7 @@ def test_styles(mocker, main_window): def test_styles_animated(mocker, main_window): wrapper = main_window.main_view.view_layout.itemAt(0).widget() - conversation_scrollarea = wrapper.conversation_view.scroll + conversation_scrollarea = wrapper.conversation_view._scroll file_widget = conversation_scrollarea.widget().layout().itemAt(0).widget() download_button = file_widget.download_button diff --git a/tests/integration/test_styles_reply_status_bar.py b/tests/integration/test_styles_reply_status_bar.py index 1114be147..215b4da2d 100644 --- a/tests/integration/test_styles_reply_status_bar.py +++ b/tests/integration/test_styles_reply_status_bar.py @@ -3,7 +3,7 @@ def test_styles(mocker, main_window): wrapper = main_window.main_view.view_layout.itemAt(0).widget() - conversation_scrollarea = wrapper.conversation_view.scroll + conversation_scrollarea = wrapper.conversation_view._scroll reply_widget = conversation_scrollarea.widget().layout().itemAt(2).widget() reply_widget.sender_is_current_user = False @@ -26,7 +26,7 @@ def test_styles(mocker, main_window): def test_styles_for_replies_from_authenticated_user(mocker, main_window): wrapper = main_window.main_view.view_layout.itemAt(0).widget() - conversation_scrollarea = wrapper.conversation_view.scroll + conversation_scrollarea = wrapper.conversation_view._scroll reply_widget = conversation_scrollarea.widget().layout().itemAt(2).widget() reply_widget.sender_is_current_user = True diff --git a/tests/integration/test_styles_sdclient.py b/tests/integration/test_styles_sdclient.py index 85455eedd..8d9aab2fe 100644 --- a/tests/integration/test_styles_sdclient.py +++ b/tests/integration/test_styles_sdclient.py @@ -112,7 +112,7 @@ def test_class_name_matches_css_object_name(mocker, main_window): assert "LastUpdatedLabel" in last_updated_label.objectName() title = conversation_title_bar.layout().itemAt(0).widget().layout().itemAt(0).widget() assert "TitleLabel" in title.objectName() - conversation_scroll_area = wrapper.conversation_view.scroll + conversation_scroll_area = wrapper.conversation_view._scroll assert "ConversationScrollArea" == conversation_scroll_area.__class__.__name__ assert "ConversationScrollArea" in conversation_scroll_area.widget().objectName() file_widget = conversation_scroll_area.widget().layout().itemAt(0).widget() @@ -365,7 +365,7 @@ def test_styles_for_conversation_view(mocker, main_window): assert 24 == title.font().pixelSize() assert "#2a319d" == title.palette().color(QPalette.Foreground).name() - conversation_scrollarea = wrapper.conversation_view.scroll + conversation_scrollarea = wrapper.conversation_view._scroll assert "#f9f9ff" == conversation_scrollarea.palette().color(QPalette.Background).name() assert "#f9f9ff" == conversation_scrollarea.widget().palette().color(QPalette.Background).name() file_widget = conversation_scrollarea.widget().layout().itemAt(0).widget() diff --git a/tests/integration/test_styles_speech_bubble_message.py b/tests/integration/test_styles_speech_bubble_message.py index 1355c7c9d..faa111d70 100644 --- a/tests/integration/test_styles_speech_bubble_message.py +++ b/tests/integration/test_styles_speech_bubble_message.py @@ -3,7 +3,7 @@ def test_styles(mocker, main_window): wrapper = main_window.main_view.view_layout.itemAt(0).widget() - conversation_scrollarea = wrapper.conversation_view.scroll + conversation_scrollarea = wrapper.conversation_view._scroll speech_bubble = conversation_scrollarea.widget().layout().itemAt(1).widget() assert 400 == speech_bubble.speech_bubble.minimumSize().width() diff --git a/tests/integration/test_styles_speech_bubble_status_bar.py b/tests/integration/test_styles_speech_bubble_status_bar.py index 41da34cdd..5b1585f85 100644 --- a/tests/integration/test_styles_speech_bubble_status_bar.py +++ b/tests/integration/test_styles_speech_bubble_status_bar.py @@ -3,7 +3,7 @@ def test_styles(mocker, main_window): wrapper = main_window.main_view.view_layout.itemAt(0).widget() - conversation_scrollarea = wrapper.conversation_view.scroll + conversation_scrollarea = wrapper.conversation_view._scroll speech_bubble = conversation_scrollarea.widget().layout().itemAt(1).widget() assert 5 == speech_bubble.color_bar.minimumSize().height() diff --git a/tests/test_logic.py b/tests/test_logic.py index b71f8070e..cbefb76ef 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -12,7 +12,6 @@ import arrow import pytest import sqlalchemy.orm.exc -from PyQt5.QtCore import Qt from PyQt5.QtTest import QSignalSpy from sdclientapi import AuthError, RequestTimeoutError, ServerConnectionError from sqlalchemy.orm import attributes @@ -710,8 +709,8 @@ def test_Controller_mark_seen(homedir, config, mocker, session, session_maker): assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [job] - job.success_signal.connect.assert_called_once_with(co.on_seen_success, type=Qt.QueuedConnection) - job.failure_signal.connect.assert_called_once_with(co.on_seen_failure, type=Qt.QueuedConnection) + job.success_signal.connect.assert_called_once_with(co.on_seen_success) + job.failure_signal.connect.assert_called_once_with(co.on_seen_failure) def test_Controller_mark_seen_with_unseen_item_of_each_type( @@ -1236,13 +1235,13 @@ def test_Controller_download_conversation(homedir, config, session, mocker, sess assert add_job_emissions[0] == [job] assert add_job_emissions[1] == [job] expected = [ - call(co.on_file_download_success, type=Qt.QueuedConnection), - call(co.on_file_download_success, type=Qt.QueuedConnection), + call(co.on_file_download_success), + call(co.on_file_download_success), ] assert job_success_signal.connect.mock_calls == expected expected = [ - call(co.on_file_download_failure, type=Qt.QueuedConnection), - call(co.on_file_download_failure, type=Qt.QueuedConnection), + call(co.on_file_download_failure), + call(co.on_file_download_failure), ] assert job_failure_signal.connect.mock_calls == expected @@ -1319,12 +1318,8 @@ def test_Controller_on_file_download_Submission(homedir, config, session, mocker mock_job_cls.assert_called_once_with(file_.uuid, co.data_dir, co.gpg) assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [mock_job] - mock_success_signal.connect.assert_called_once_with( - co.on_file_download_success, type=Qt.QueuedConnection - ) - mock_failure_signal.connect.assert_called_once_with( - co.on_file_download_failure, type=Qt.QueuedConnection - ) + mock_success_signal.connect.assert_called_once_with(co.on_file_download_success) + mock_failure_signal.connect.assert_called_once_with(co.on_file_download_failure) def test_Controller_on_file_download_Submission_no_auth( @@ -1644,12 +1639,8 @@ def test_Controller_download_new_replies_with_new_reply(mocker, session, session assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [job] - success_signal.connect.assert_called_once_with( - co.on_reply_download_success, type=Qt.QueuedConnection - ) - failure_signal.connect.assert_called_once_with( - co.on_reply_download_failure, type=Qt.QueuedConnection - ) + success_signal.connect.assert_called_once_with(co.on_reply_download_success) + failure_signal.connect.assert_called_once_with(co.on_reply_download_failure) def test_Controller_download_new_replies_without_replies(mocker, session, session_maker, homedir): @@ -1770,12 +1761,8 @@ def test_Controller_download_new_messages_with_new_message(mocker, session, sess assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [job] - success_signal.connect.assert_called_once_with( - co.on_message_download_success, type=Qt.QueuedConnection - ) - failure_signal.connect.assert_called_once_with( - co.on_message_download_failure, type=Qt.QueuedConnection - ) + success_signal.connect.assert_called_once_with(co.on_message_download_success) + failure_signal.connect.assert_called_once_with(co.on_message_download_failure) # fake APIJobQueue's emitting main_queue_updated when the job is queued and dequeued co.api_job_queue.main_queue_updated.emit(1) @@ -2020,12 +2007,8 @@ def test_Controller_delete_source(homedir, config, mocker, session_maker, sessio mock_job_cls.assert_called_once_with(source.uuid) assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [mock_job] - mock_success_signal.connect.assert_called_once_with( - co.on_delete_source_success, type=Qt.QueuedConnection - ) - mock_failure_signal.connect.assert_called_once_with( - co.on_delete_source_failure, type=Qt.QueuedConnection - ) + mock_success_signal.connect.assert_called_once_with(co.on_delete_source_success) + mock_failure_signal.connect.assert_called_once_with(co.on_delete_source_failure) def test_Controller_on_delete_conversation_success(mocker, homedir): @@ -2100,12 +2083,8 @@ def test_Controller_delete_conversation(homedir, config, mocker, session_maker, mock_job_cls.assert_called_once_with(source.uuid) assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [mock_job] - mock_success_signal.connect.assert_called_once_with( - co.on_delete_conversation_success, type=Qt.QueuedConnection - ) - mock_failure_signal.connect.assert_called_once_with( - co.on_delete_conversation_failure, type=Qt.QueuedConnection - ) + mock_success_signal.connect.assert_called_once_with(co.on_delete_conversation_success) + mock_failure_signal.connect.assert_called_once_with(co.on_delete_conversation_failure) def test_Controller_send_reply_success( @@ -2141,12 +2120,8 @@ def test_Controller_send_reply_success( assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [mock_job] - mock_success_signal.connect.assert_called_once_with( - co.on_reply_success, type=Qt.QueuedConnection - ) - mock_failure_signal.connect.assert_called_once_with( - co.on_reply_failure, type=Qt.QueuedConnection - ) + mock_success_signal.connect.assert_called_once_with(co.on_reply_success) + mock_failure_signal.connect.assert_called_once_with(co.on_reply_failure) def test_Controller_send_reply_does_not_send_if_not_authenticated( @@ -2392,12 +2367,8 @@ def test_Controller_call_update_star_success(homedir, config, mocker, session_ma assert len(add_job_emissions) == 1 assert add_job_emissions[0] == [mock_job] assert star_update_successful.connect.call_count == 1 - star_update_failed.connect.assert_called_once_with( - co.on_update_star_failure, type=Qt.QueuedConnection - ) - star_update_successful.connect.assert_called_once_with( - co.on_update_star_success, type=Qt.QueuedConnection - ) + star_update_failed.connect.assert_called_once_with(co.on_update_star_failure) + star_update_successful.connect.assert_called_once_with(co.on_update_star_success) def test_get_file(mocker, session, homedir):