diff --git a/client/securedrop_client/app.py b/client/securedrop_client/app.py index 74ecce28e..ff42f0c1e 100644 --- a/client/securedrop_client/app.py +++ b/client/securedrop_client/app.py @@ -194,7 +194,7 @@ def prevent_second_instance(app: QApplication, unique_name: str) -> None: def threads(count: int) -> Any: """Ensures that the thread is properly closed before its reference is dropped.""" threads = [] - for i in range(count): + for _i in range(count): threads.append(QThread()) yield threads diff --git a/client/securedrop_client/export.py b/client/securedrop_client/export.py index 44785ad83..534aab4bb 100644 --- a/client/securedrop_client/export.py +++ b/client/securedrop_client/export.py @@ -346,7 +346,7 @@ def print(self, filepaths: list[str]) -> None: self.print_failed.emit(ExportStatus.ERROR_PRINT) def _create_archive( - self, archive_dir: str, archive_fn: str, metadata: dict, filepaths: list[str] = [] + self, archive_dir: str, archive_fn: str, metadata: dict, filepaths: list[str] | None = None ) -> str: """ Create the archive to be sent to the Export VM. @@ -361,6 +361,8 @@ def _create_archive( str: The path to newly-created archive file. """ archive_path = os.path.join(archive_dir, archive_fn) + if filepaths is None: + filepaths = [] with tarfile.open(archive_path, "w:gz") as archive: self._add_virtual_file_to_archive(archive, self._METADATA_FN, metadata) diff --git a/client/securedrop_client/gui/base/misc.py b/client/securedrop_client/gui/base/misc.py index a459c1e5c..7fd1f070c 100644 --- a/client/securedrop_client/gui/base/misc.py +++ b/client/securedrop_client/gui/base/misc.py @@ -154,11 +154,13 @@ def __init__( self, text: str = "", parent: QWidget | None = None, - flags: Qt.WindowFlags | Qt.WindowType = Qt.WindowFlags(), + flags: Qt.WindowFlags | Qt.WindowType | None = None, wordwrap: bool = True, max_length: int = 0, with_tooltip: bool = False, ): + if flags is None: + flags = Qt.WindowFlags() super().__init__(parent, flags) self.wordwrap = wordwrap self.max_length = max_length diff --git a/client/tests/api_jobs/test_base.py b/client/tests/api_jobs/test_base.py index 751d7f1a3..32b84d9d1 100644 --- a/client/tests/api_jobs/test_base.py +++ b/client/tests/api_jobs/test_base.py @@ -182,7 +182,7 @@ def test_ApiJob_order_number_unset(mocker): api_job_2 = api_job_cls() with pytest.raises(ValueError): - api_job_1 < api_job_2 + assert api_job_1 < api_job_2 def test_SingleObjectApiJob_comparison_obj_without_uuid_attr(mocker): diff --git a/client/tests/gui/conversation/delete/test_dialog.py b/client/tests/gui/conversation/delete/test_dialog.py index cfa5f4df7..001e95326 100644 --- a/client/tests/gui/conversation/delete/test_dialog.py +++ b/client/tests/gui/conversation/delete/test_dialog.py @@ -18,9 +18,9 @@ def test_displays_accurate_source_information_by_default(self): assert "0 replies" in self.dialog.text() def test_displays_updated_source_information_when_shown(self): - for i in range(2): + for _i in range(2): factory.Reply(source=self._source) - for i in range(3): + for _i in range(3): factory.Message(source=self._source) QTimer.singleShot(300, self.dialog.close) diff --git a/client/tests/gui/test_main.py b/client/tests/gui/test_main.py index 7f48897b0..44063c4da 100644 --- a/client/tests/gui/test_main.py +++ b/client/tests/gui/test_main.py @@ -141,7 +141,7 @@ def test_show_main_window_without_username(mocker, homedir, session_maker): w.show.assert_called_once_with() w.showMaximized.assert_not_called() - w.set_logged_in_as.called is False + assert w.set_logged_in_as.called is False controller.qubes = False w.setup(controller) @@ -153,7 +153,7 @@ def test_show_main_window_without_username(mocker, homedir, session_maker): w.show.assert_not_called() w.showMaximized.assert_called_once_with() - w.set_logged_in_as.called is False + assert w.set_logged_in_as.called is False def test_show_main_window_without_username_when_already_showing(mocker, homedir, session_maker): @@ -169,7 +169,7 @@ def test_show_main_window_without_username_when_already_showing(mocker, homedir, w.show.assert_not_called() w.showMaximized.assert_not_called() - w.set_logged_in_as.called is False + assert w.set_logged_in_as.called is False controller.qubes = False w.setup(controller) @@ -181,7 +181,7 @@ def test_show_main_window_without_username_when_already_showing(mocker, homedir, w.show.assert_not_called() w.showMaximized.assert_not_called() - w.set_logged_in_as.called is False + assert w.set_logged_in_as.called is False def test_show_login(mocker): diff --git a/client/tests/gui/test_widgets.py b/client/tests/gui/test_widgets.py index 8bf90a623..90d0d98a6 100644 --- a/client/tests/gui/test_widgets.py +++ b/client/tests/gui/test_widgets.py @@ -451,7 +451,7 @@ def test_UserButton_setup(mocker): def test_UserButton_set_username(): ub = UserButton() ub.set_username("test_username") - ub.text() == "test_username" + assert ub.text() == "test_username" def test_UserButton_set_long_username(mocker): @@ -661,7 +661,7 @@ def test_MainView_on_source_changed_shows_correct_context(mocker, homedir, sessi """ # Build sourcelist sources = [] - for i in range(10): + for _i in range(10): s = factory.Source() sources.append(s) session.add(s) @@ -2645,7 +2645,7 @@ def test_SpeechBubble_init(mocker): sb = SpeechBubble("mock id", "hello", mock_update_signal, mock_download_error_signal, 0, 123) - sb.message.text() == "hello" + assert sb.message.text() == "hello" assert mock_update_connect.called assert mock_download_error_connect.called @@ -2673,7 +2673,7 @@ def test_SpeechBubble_init_with_error(mocker): failed_to_decrypt=True, ) - sb.message.text() == "hello" + assert sb.message.text() == "hello" assert mock_update_connect.called assert mock_download_error_connect.called @@ -3107,7 +3107,7 @@ def test_ReplyWidget__on_update_authenticated_user_updates_sender_when_changed(m user.username = "baz" reply_widget._on_update_authenticated_user(authenticated_user) - reply_widget.sender.username == "baz" + assert reply_widget.sender.username == "baz" assert reply_widget.sender_is_current_user assert reply_widget.sender_icon.is_current_user reply_widget._update_styles.assert_called_once_with() @@ -3272,7 +3272,7 @@ def test_FileWidget_event_handler_left_click(mocker, session, source): fw._on_left_click = mocker.MagicMock() fw.eventFilter(fw, test_event) - fw._on_left_click.call_count == 1 + assert fw._on_left_click.call_count == 1 def test_FileWidget_event_handler_hover(mocker, session, source): @@ -3362,8 +3362,8 @@ def test_FileWidget_on_left_click_downloading_in_progress(mocker, session, sourc fw.download_button = mocker.MagicMock() fw._on_left_click() - get_file.call_count == 0 - controller.on_submission_download.call_count == 0 + assert get_file.call_count == 0 + assert controller.on_submission_download.call_count == 0 def test_FileWidget_start_button_animation(mocker, session, source): diff --git a/client/tests/integration/test_styles_sdclient.py b/client/tests/integration/test_styles_sdclient.py index 299b99ca8..816e7956b 100644 --- a/client/tests/integration/test_styles_sdclient.py +++ b/client/tests/integration/test_styles_sdclient.py @@ -72,9 +72,9 @@ def test_class_name_matches_css_object_name(mocker, main_window): main_view.MULTI_SELECTED_INDEX, ]: view = main_view.view_layout.widget(index) - view.objectName() == "EmptyConversationView" - "EmptyConversationView" in view.objectName() - "EmptyConversationView" in view.objectName() + assert view.objectName() == "EmptyConversationView" + assert "EmptyConversationView" in view.objectName() + assert "EmptyConversationView" in view.objectName() source_list = main_view.source_list @@ -272,20 +272,20 @@ def test_styles_for_main_view(mocker, main_window): assert no_source_selected_spacer1.maximumSize().height() == 35 bullet1_bullet = no_source_selected.layout().itemAt(2).widget().layout().itemAt(0).widget() assert bullet1_bullet.getContentsMargins() == (0, 4, 0, 0) - bullet1_bullet.font().pixelSize() == 30 - QFont.Bold == bullet1_bullet.font().weight() + assert bullet1_bullet.font().pixelSize() == 30 + assert QFont.Bold == bullet1_bullet.font().weight() assert bullet1_bullet.font().family() == "Montserrat" assert bullet1_bullet.palette().color(QPalette.Foreground).name() == "#a5b3e9" bullet2_bullet = no_source_selected.layout().itemAt(3).widget().layout().itemAt(0).widget() assert bullet2_bullet.getContentsMargins() == (0, 4, 0, 0) - bullet2_bullet.font().pixelSize() == 30 - QFont.Bold == bullet2_bullet.font().weight() + assert bullet2_bullet.font().pixelSize() == 30 + assert QFont.Bold == bullet2_bullet.font().weight() assert bullet2_bullet.font().family() == "Montserrat" assert bullet2_bullet.palette().color(QPalette.Foreground).name() == "#a5b3e9" bullet3_bullet = no_source_selected.layout().itemAt(4).widget().layout().itemAt(0).widget() assert bullet3_bullet.getContentsMargins() == (0, 4, 0, 0) - bullet3_bullet.font().pixelSize() == 30 - QFont.Bold == bullet3_bullet.font().weight() + assert bullet3_bullet.font().pixelSize() == 30 + assert QFont.Bold == bullet3_bullet.font().weight() assert bullet3_bullet.font().family() == "Montserrat" assert bullet3_bullet.palette().color(QPalette.Foreground).name() == "#a5b3e9" no_source_selected_spacer2 = no_source_selected.layout().itemAt(5) @@ -298,23 +298,23 @@ def test_styles_source_list(mocker, main_window): source_widget = source_list.itemWidget(source_list.item(0)) preview = source_widget.preview assert preview.font().family() == "Source Sans Pro" - QFont.Normal == preview.font().weight() - preview.font().pixelSize() == 13 + assert QFont.Normal == preview.font().weight() + assert preview.font().pixelSize() == 13 assert preview.palette().color(QPalette.Foreground).name() == "#383838" deletion_indicator = source_widget.deletion_indicator assert deletion_indicator.font().family() == "Source Sans Pro" - QFont.Normal == deletion_indicator.font().weight() - deletion_indicator.font().pixelSize() == 13 + assert QFont.Normal == deletion_indicator.font().weight() + assert deletion_indicator.font().pixelSize() == 13 assert deletion_indicator.palette().color(QPalette.Foreground).name() == "#000000" name = source_widget.name assert name.font().family() == "Montserrat" - QFont.Normal == name.font().weight() - name.font().pixelSize() == 13 + assert QFont.Normal == name.font().weight() + assert name.font().pixelSize() == 13 assert name.palette().color(QPalette.Foreground).name() == "#2a319d" timestamp = source_widget.timestamp assert timestamp.font().family() == "Montserrat" - QFont.Normal == timestamp.font().weight() - timestamp.font().pixelSize() == 13 + assert QFont.Normal == timestamp.font().weight() + assert timestamp.font().pixelSize() == 13 assert timestamp.palette().color(QPalette.Foreground).name() == "#383838" diff --git a/client/tests/test_app.py b/client/tests/test_app.py index ecec3c935..80f4516be 100644 --- a/client/tests/test_app.py +++ b/client/tests/test_app.py @@ -205,7 +205,7 @@ def test_create_app_dir_permissions(tmpdir, mocker): mocker.patch("securedrop_client.app.make_session_maker", return_value=mock_session_maker) def func(): - start_app(mock_args, mock_qt_args) + start_app(mock_args, mock_qt_args) # noqa: B023 func() diff --git a/client/tests/test_logic.py b/client/tests/test_logic.py index ec7f65fa7..83ae78508 100644 --- a/client/tests/test_logic.py +++ b/client/tests/test_logic.py @@ -596,7 +596,7 @@ def test_Controller_on_sync_success_username_change(homedir, session, config, mo co.on_sync_success() - co.authenticated_user.username == "baz" + assert co.authenticated_user.username == "baz" assert len(update_authenticated_user_emissions) == 1 assert update_authenticated_user_emissions[0] == [co.authenticated_user] @@ -618,7 +618,7 @@ def test_Controller_on_sync_success_firstname_change(homedir, session, config, m co.on_sync_success() - co.authenticated_user.firstname == "baz" + assert co.authenticated_user.firstname == "baz" assert len(update_authenticated_user_emissions) == 1 assert update_authenticated_user_emissions[0] == [co.authenticated_user] @@ -640,7 +640,7 @@ def test_Controller_on_sync_success_lastname_change(homedir, session, config, mo co.on_sync_success() - co.authenticated_user.lastname == "baz" + assert co.authenticated_user.lastname == "baz" assert len(update_authenticated_user_emissions) == 1 assert update_authenticated_user_emissions[0] == [co.authenticated_user] @@ -1134,7 +1134,7 @@ def test_create_client_dir_permissions(tmpdir, mocker, session_maker): os.chmod(sdc_home, case["home_perms"]) def func() -> None: - Controller("http://localhost", mock_gui, session_maker, sdc_home, None) + Controller("http://localhost", mock_gui, session_maker, sdc_home, None) # noqa: B023 if case["should_pass"]: func() @@ -1391,9 +1391,10 @@ def test_Controller_on_file_downloaded_checksum_failure(homedir, config, mocker, # Job should get resubmitted and we should log this is happening assert co._submit_download_job.call_count == 1 - warning_logger.call_args_list[0][0][ - 0 - ] == f"Failure due to checksum mismatch, retrying {file_.uuid}" + assert ( + warning_logger.call_args_list[0][0][0] + == f"Failure due to checksum mismatch, retrying {file_.uuid}" + ) # No status will be set if it's a file corruption issue, the file just gets # re-downloaded. @@ -1425,7 +1426,7 @@ def test_Controller_on_file_decryption_failure(homedir, config, mocker, session, mock_update_error_status.assert_called_once_with("The file download failed. Please try again.") assert co._submit_download_job.call_count == 0 - error_logger.call_args_list[0][0][0] == f"Failed to decrypt {file_.uuid}" + assert error_logger.call_args_list[0][0][0] == f"Failed to decrypt {file_.uuid}" mock_set_status.assert_not_called() @@ -1660,9 +1661,10 @@ def test_Controller_on_reply_downloaded_checksum_failure(mocker, homedir, sessio # Job should get resubmitted and we should log this is happening assert co._submit_download_job.call_count == 1 - warning_logger.call_args_list[0][0][ - 0 - ] == f"Failure due to checksum mismatch, retrying {reply.uuid}" + assert ( + warning_logger.call_args_list[0][0][0] + == f"Failure due to checksum mismatch, retrying {reply.uuid}" + ) def test_Controller_on_reply_downloaded_decryption_failure(mocker, homedir, session_maker): @@ -1773,7 +1775,7 @@ def test_Controller_download_new_messages_skips_recent_failures( co.download_new_messages() assert len(add_job_emissions) == 0 - info_logger.call_args_list[0][0][0] == ( + assert info_logger.call_args_list[0][0][0] == ( f"Download of message {message.uuid} failed since client start; not retrying." ) @@ -1807,7 +1809,7 @@ def test_Controller_download_new_replies_skips_recent_failures( co.download_new_replies() assert len(add_job_emissions) == 0 - info_logger.call_args_list[0][0][0] == ( + assert info_logger.call_args_list[0][0][0] == ( f"Download of reply {reply.uuid} failed since client start; not retrying." ) diff --git a/client/tests/test_queue.py b/client/tests/test_queue.py index dee1010b7..d40cb9cd3 100644 --- a/client/tests/test_queue.py +++ b/client/tests/test_queue.py @@ -203,8 +203,7 @@ def test_RunnableQueue_duplicate_jobs(mocker): queue.add_job(dl_job) assert len(queue.queue.queue) == 1 - log_msg = f"Duplicate job {dl_job}, skipping" - debug_logger.call_args[1] == log_msg + assert debug_logger.call_args[1] == f"Duplicate job {dl_job}, skipping" # Now add a _different_ job with the same arguments (same uuid). queue.add_job(msg_dl_job) diff --git a/client/tests/test_storage.py b/client/tests/test_storage.py index 6f3082098..716fb8bcf 100644 --- a/client/tests/test_storage.py +++ b/client/tests/test_storage.py @@ -312,8 +312,8 @@ def test_update_local_storage_sanitizes_remote_data(mocker, homedir): ) sanitize_sources.assert_called_once_with([remote_source]) - sanitize_submissions_or_replies.call_args_list[0][0][0] == [remote_submissions] - sanitize_submissions_or_replies.call_args_list[1][0][0] == [remote_reply] + assert sanitize_submissions_or_replies.call_args_list[0][0][0] == [remote_submissions] + assert sanitize_submissions_or_replies.call_args_list[1][0][0] == [remote_reply] def test_sync_delete_race(homedir, mocker, session_maker, session): @@ -1677,7 +1677,7 @@ def test_delete_single_submission_or_reply_race_guard(homedir, mocker): mock_remove = mocker.patch("os.remove", side_effect=FileNotFoundError) delete_single_submission_or_reply_on_disk(test_obj, homedir) - mock_remove.call_count == 1 + assert mock_remove.call_count == 1 def test_delete_single_submission_or_reply_single_file(homedir, mocker): diff --git a/export/securedrop_export/disk/service.py b/export/securedrop_export/disk/service.py index 9a8034ee5..4b5a45a19 100644 --- a/export/securedrop_export/disk/service.py +++ b/export/securedrop_export/disk/service.py @@ -16,8 +16,11 @@ class Service: This is the "API" portion of the export workflow. """ - def __init__(self, submission: Archive, cli: CLI = CLI()): - self.cli = cli + def __init__(self, submission: Archive, cli: CLI | None = None): + if cli is not None: + self.cli = cli + else: + self.cli = CLI() self.submission = submission def scan_all_devices(self) -> Status: diff --git a/export/securedrop_export/print/service.py b/export/securedrop_export/print/service.py index 830820349..ca3e4c887 100644 --- a/export/securedrop_export/print/service.py +++ b/export/securedrop_export/print/service.py @@ -287,7 +287,9 @@ def _get_supported_mimetypes_libreoffice(self, desktop_dir: Path): for line in f.readlines(): if line.startswith("MimeType="): # Semicolon-separated list; don't leave empty element at the end - supported_mimetypes.update(line.strip("MimeType=").split(";")[:-1]) + supported_mimetypes.update( + line.removeprefix("MimeType=").split(";")[:-1] + ) return supported_mimetypes diff --git a/log/log_server/log_saver.py b/log/log_server/log_saver.py index 2199b5058..65c2fdf92 100755 --- a/log/log_server/log_saver.py +++ b/log/log_server/log_saver.py @@ -46,7 +46,7 @@ def main(): except Exception as e: print(e, file=sys.stderr) # Clean up all open files - for k, v in openfiles: + for _k, v in openfiles: v.close() sys.exit(1) diff --git a/pyproject.toml b/pyproject.toml index 3bfc3d2d2..5dbdfbed0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,8 @@ extend-include = ["log/securedrop-{log,log-saver,redis-log}"] [tool.ruff.lint] select = [ + # flake8-bugbear + "B", # pycodestyle errors "E", # pyflakes @@ -53,6 +55,10 @@ select = [ "RUF100", ] ignore = [ + # `pytest.raises(Exception)` is evil yes, sorry + "B017", + # we have too many cases of "raise Exception from err" + "B904", # code complexity checks that we fail "PLR0912", "PLR0913", "PLR0915", # magic-value-comparison, too many violations for now