Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable ruff's flake8-bugbear ruleset #2341

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/securedrop_client/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion client/securedrop_client/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion client/securedrop_client/gui/base/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion client/tests/api_jobs/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions client/tests/gui/conversation/delete/test_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions client/tests/gui/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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):
Expand All @@ -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)
Expand All @@ -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):
Expand Down
16 changes: 8 additions & 8 deletions client/tests/gui/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
34 changes: 17 additions & 17 deletions client/tests/integration/test_styles_sdclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand All @@ -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"


Expand Down
2 changes: 1 addition & 1 deletion client/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
28 changes: 15 additions & 13 deletions client/tests/test_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand All @@ -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]

Expand All @@ -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]

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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."
)

Expand Down Expand Up @@ -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."
)

Expand Down
3 changes: 1 addition & 2 deletions client/tests/test_queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions client/tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 5 additions & 2 deletions export/securedrop_export/disk/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion export/securedrop_export/print/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion log/log_server/log_saver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading
Loading