Integration tests and unit test fixes

client/securedrop_client/export_status.py

@@ -31,8 +31,9 @@ class ExportStatus(Enum):
     SUCCESS_EXPORT = "SUCCESS_EXPORT"
     ERROR_EXPORT = "ERROR_EXPORT"  # Could not write to disk
 
-    # Export succeeds but drives were not properly unmounted
+    # Export succeeds but drives were not properly closed
     ERROR_EXPORT_CLEANUP = "ERROR_EXPORT_CLEANUP"
+    ERROR_UNMOUNT_VOLUME_BUSY = "ERROR_UNMOUNT_VOLUME_BUSY"
 
     DEVICE_ERROR = "DEVICE_ERROR"  # Something went wrong while trying to check the device
 

client/securedrop_client/gui/actions.py

@@ -15,8 +15,8 @@
 from securedrop_client import state
 from securedrop_client.conversation import Transcript as ConversationTranscript
 from securedrop_client.db import Source
+from securedrop_client.export import Export
 from securedrop_client.gui.base import ModalDialog
-from securedrop_client.gui.conversation import ExportDevice
 from securedrop_client.gui.conversation import (
     PrintTranscriptDialog as PrintConversationTranscriptDialog,
 )
@@ -184,7 +184,7 @@ def _on_triggered(self) -> None:
         # out of scope, any pending file removal will be performed
         # by the operating system.
         with open(file_path, "r") as f:
-            export = ExportDevice()
+            export = Export()
             dialog = PrintConversationTranscriptDialog(
                 export, TRANSCRIPT_FILENAME, [str(file_path)]
             )
@@ -234,7 +234,7 @@ def _on_triggered(self) -> None:
         # out of scope, any pending file removal will be performed
         # by the operating system.
         with open(file_path, "r") as f:
-            export_device = ExportDevice()
+            export_device = Export()
             wizard = ExportWizard(export_device, TRANSCRIPT_FILENAME, [str(file_path)])
             wizard.exec()
 
@@ -320,7 +320,7 @@ def _prepare_to_export(self) -> None:
         # out of scope, any pending file removal will be performed
         # by the operating system.
         with ExitStack() as stack:
-            export_device = ExportDevice()
+            export_device = Export()
             files = [
                 stack.enter_context(open(file_location, "r")) for file_location in file_locations
             ]

client/securedrop_client/gui/conversation/__init__.py

@@ -3,7 +3,6 @@
 """
 # Import classes here to make possible to import them from securedrop_client.gui.conversation
 from .delete import DeleteConversationDialog  # noqa: F401
-from .export import Export as ExportDevice  # noqa: F401
 from .export import ExportWizard as ExportWizard  # noqa: F401
-from .export import PrintDialog as PrintFileDialog  # noqa: F401
+from .export import PrintDialog  # noqa: F401
 from .export import PrintTranscriptDialog  # noqa: F401

client/securedrop_client/gui/conversation/export/__init__.py

@@ -1,4 +1,3 @@
-from ....export import Export  # noqa: F401
 from .export_wizard import ExportWizard  # noqa: F401
 from .print_dialog import PrintDialog  # noqa: F401
 from .print_transcript_dialog import PrintTranscriptDialog  # noqa: F401

client/securedrop_client/gui/conversation/export/export_wizard_page.py

@@ -85,6 +85,7 @@ def _build_layout(self) -> QVBoxLayout:
         """
         Create parent layout, draw elements, return parent layout
         """
+        self.setObjectName("QWizard_export_page")
         self.setStyleSheet(self.WIZARD_CSS)
         parent_layout = QVBoxLayout(self)
         parent_layout.setContentsMargins(self.MARGIN, self.MARGIN, self.MARGIN, self.MARGIN)

client/securedrop_client/gui/conversation/export/print_dialog.py

@@ -97,7 +97,7 @@ def _print_file(self) -> None:
         self._device.print(self.filepaths)
         self.close()
 
-    @pyqtSlot()
+    @pyqtSlot(object)
     def _on_print_preflight_check_succeeded(self, status: ExportStatus) -> None:
         # We don't use the ExportStatus for now for "success" status,
         # but in future work we will migrate towards a wizard-style dialog, where

client/securedrop_client/gui/conversation/export/wizard.css

@@ -1,9 +1,13 @@
 #QWizard_export {
     min-width: 800px;
     max-width: 800px;
-    min-height: 300px;
+    min-height: 500px;
     max-height: 800px;
-    background-color: #fff;
+    background: #ffffff;
+}
+
+#QWizard_export_page {
+    background: #ffffff;
 }
 
 #QWizard_header_icon, #QWizard_header_spinner {

client/securedrop_client/gui/widgets.py

@@ -70,6 +70,7 @@
     Source,
     User,
 )
+from securedrop_client.export import Export
 from securedrop_client.gui import conversation
 from securedrop_client.gui.actions import (
     DeleteConversationAction,
@@ -81,7 +82,6 @@
 )
 from securedrop_client.gui.base import SecureQLabel, SvgLabel, SvgPushButton, SvgToggleButton
 from securedrop_client.gui.conversation import DeleteConversationDialog
-from securedrop_client.gui.conversation.export import ExportWizard
 from securedrop_client.gui.datetime_helpers import format_datetime_local
 from securedrop_client.gui.source import DeleteSourceDialog
 from securedrop_client.logic import Controller
@@ -2460,9 +2460,11 @@ def _on_export_clicked(self) -> None:
             logger.debug("Clicked export but file not downloaded")
             return
 
-        export_device = conversation.ExportDevice()
+        export_device = Export()
 
-        self.export_wizard = ExportWizard(export_device, self.file.filename, [file_location])
+        self.export_wizard = conversation.ExportWizard(
+            export_device, self.file.filename, [file_location]
+        )
         self.export_wizard.show()
 
     @pyqtSlot()
@@ -2476,9 +2478,9 @@ def _on_print_clicked(self) -> None:
 
         filepath = self.file.location(self.controller.data_dir)
 
-        export_device = conversation.ExportDevice()
+        export_device = Export()
 
-        dialog = conversation.PrintFileDialog(export_device, self.file.filename, [filepath])
+        dialog = conversation.PrintDialog(export_device, self.file.filename, [filepath])
         dialog.exec()
 
     def _on_left_click(self) -> None:

client/tests/conftest.py

@@ -23,6 +23,7 @@
     Source,
     make_session_maker,
 )
+from securedrop_client.export import Export
 from securedrop_client.export_status import ExportStatus
 from securedrop_client.gui import conversation
 from securedrop_client.gui.main import Window
@@ -78,9 +79,9 @@ def lang(request):
 def print_dialog(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
-    dialog = conversation.PrintFileDialog(export_device, "file123.jpg", ["/mock/path/to/file"])
+    dialog = conversation.PrintDialog(export_device, "file123.jpg", ["/mock/path/to/file"])
 
     yield dialog
 
@@ -89,7 +90,7 @@ def print_dialog(mocker, homedir):
 def print_transcript_dialog(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     dialog = conversation.PrintTranscriptDialog(
         export_device, "transcript.txt", ["some/path/transcript.txt"]
@@ -102,7 +103,7 @@ def print_transcript_dialog(mocker, homedir):
 def export_wizard_multifile(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     wizard = conversation.ExportWizard(
         export_device,
@@ -117,7 +118,7 @@ def export_wizard_multifile(mocker, homedir):
 def export_wizard(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     dialog = conversation.ExportWizard(export_device, "file123.jpg", ["/mock/path/to/file"])
 
@@ -128,7 +129,7 @@ def export_wizard(mocker, homedir):
 def export_transcript_wizard(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     dialog = conversation.ExportWizard(
         export_device, "transcript.txt", ["/some/path/transcript.txt"]
@@ -177,7 +178,7 @@ def mock_export_locked():
         * "Export" clicked, export wizard launched
         * Passphrase successfully entered on first attempt (and export suceeeds)
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.NO_DEVICE_DETECTED
@@ -201,7 +202,7 @@ def mock_export_unlocked():
         * Export wizard launched
         * Export succeeds
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.DEVICE_WRITABLE
@@ -225,7 +226,7 @@ def mock_export_no_usb_then_bad_passphrase():
         * Correct passphrase
         * Export succeeds
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.NO_DEVICE_DETECTED
@@ -256,7 +257,7 @@ def mock_export_fail_early():
         * Unrecoverable error before export happens
           (eg, mount error)
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.DEVICE_LOCKED

client/tests/gui/conversation/export/test_export_wizard.py

@@ -1,7 +1,8 @@
 from unittest import mock
 
+from securedrop_client.export import Export
 from securedrop_client.export_status import ExportStatus
-from securedrop_client.gui.conversation.export import Export, ExportWizard
+from securedrop_client.gui.conversation.export import ExportWizard
 from securedrop_client.gui.conversation.export.export_wizard_constants import STATUS_MESSAGES, Pages
 from securedrop_client.gui.conversation.export.export_wizard_page import (
     ErrorPage,
@@ -134,6 +135,10 @@ def test_wizard_hides_error_details_on_success(self, qtbot):
         self.mock_export.export_state_changed.emit(ExportStatus.NO_DEVICE_DETECTED)
         self.wizard.next()
         assert isinstance(self.wizard.currentPage(), InsertUSBPage)
+
+        # Try to advance, but there's still no USB inserted
+        self.wizard.next()
+        assert isinstance(self.wizard.currentPage(), InsertUSBPage)
         assert self.wizard.currentPage().error_details.isVisible()
 
         self.mock_export.export_state_changed.emit(ExportStatus.DEVICE_LOCKED)

client/tests/gui/conversation/export/test_print_dialog.py

@@ -1,30 +1,30 @@
 from securedrop_client.export_status import ExportError, ExportStatus
-from securedrop_client.gui.conversation import PrintFileDialog
+from securedrop_client.gui.conversation import PrintDialog
 from tests.helper import app  # noqa: F401
 
 
-def test_PrintFileDialog_init(mocker):
+def test_PrintDialog_init(mocker):
     _show_starting_instructions_fn = mocker.patch(
-        "securedrop_client.gui.conversation.PrintFileDialog._show_starting_instructions"
+        "securedrop_client.gui.conversation.PrintDialog._show_starting_instructions"
     )
 
-    PrintFileDialog(mocker.MagicMock(), "mock.jpg", ["/mock/path/to/file"])
+    PrintDialog(mocker.MagicMock(), "mock.jpg", ["/mock/path/to/file"])
 
     _show_starting_instructions_fn.assert_called_once_with()
 
 
-def test_PrintFileDialog_init_sanitizes_filename(mocker):
+def test_PrintDialog_init_sanitizes_filename(mocker):
     secure_qlabel = mocker.patch(
         "securedrop_client.gui.conversation.export.print_dialog.SecureQLabel"
     )
     filename = '<script>alert("boom!");</script>'
 
-    PrintFileDialog(mocker.MagicMock(), filename, ["/mock/path/to/file"])
+    PrintDialog(mocker.MagicMock(), filename, ["/mock/path/to/file"])
 
     secure_qlabel.assert_any_call(filename, wordwrap=False, max_length=260)
 
 
-def test_PrintFileDialog__show_starting_instructions(mocker, print_dialog):
+def test_PrintDialog__show_starting_instructions(mocker, print_dialog):
     print_dialog._show_starting_instructions()
 
     # file123.jpg comes from the print_dialog fixture
@@ -55,7 +55,7 @@ def test_PrintFileDialog__show_starting_instructions(mocker, print_dialog):
     assert not print_dialog.cancel_button.isHidden()
 
 
-def test_PrintFileDialog__show_insert_usb_message(mocker, print_dialog):
+def test_PrintDialog__show_insert_usb_message(mocker, print_dialog):
     print_dialog._show_insert_usb_message()
 
     assert print_dialog.header.text() == "Connect USB printer"
@@ -68,7 +68,7 @@ def test_PrintFileDialog__show_insert_usb_message(mocker, print_dialog):
     assert not print_dialog.cancel_button.isHidden()
 
 
-def test_PrintFileDialog__show_generic_error_message(mocker, print_dialog):
+def test_PrintDialog__show_generic_error_message(mocker, print_dialog):
     print_dialog.error_status = "mock_error_status"
 
     print_dialog._show_generic_error_message()
@@ -83,15 +83,15 @@ def test_PrintFileDialog__show_generic_error_message(mocker, print_dialog):
     assert not print_dialog.cancel_button.isHidden()
 
 
-def test_PrintFileDialog__print_file(mocker, print_dialog):
+def test_PrintDialog__print_file(mocker, print_dialog):
     print_dialog.close = mocker.MagicMock()
 
     print_dialog._print_file()
 
     print_dialog.close.assert_called_once_with()
 
 
-def test_PrintFileDialog__on_print_preflight_check_succeeded(mocker, print_dialog):
+def test_PrintDialog__on_print_preflight_check_succeeded(mocker, print_dialog):
     print_dialog._print_file = mocker.MagicMock()
     print_dialog.continue_button = mocker.MagicMock()
     print_dialog.continue_button.clicked = mocker.MagicMock()
@@ -103,7 +103,7 @@ def test_PrintFileDialog__on_print_preflight_check_succeeded(mocker, print_dialo
     print_dialog.continue_button.clicked.connect.assert_called_once_with(print_dialog._print_file)
 
 
-def test_PrintFileDialog__on_print_preflight_check_succeeded_when_continue_enabled(
+def test_PrintDialog__on_print_preflight_check_succeeded_when_continue_enabled(
     mocker, print_dialog
 ):
     print_dialog._print_file = mocker.MagicMock()
@@ -114,23 +114,23 @@ def test_PrintFileDialog__on_print_preflight_check_succeeded_when_continue_enabl
     print_dialog._print_file.assert_called_once_with()
 
 
-def test_PrintFileDialog__on_print_preflight_check_succeeded_enabled_after_preflight_success(
+def test_PrintDialog__on_print_preflight_check_succeeded_enabled_after_preflight_success(
     mocker, print_dialog
 ):
     assert not print_dialog.continue_button.isEnabled()
     print_dialog._on_print_preflight_check_succeeded(ExportStatus.PRINT_PREFLIGHT_SUCCESS)
     assert print_dialog.continue_button.isEnabled()
 
 
-def test_PrintFileDialog__on_print_preflight_check_succeeded_enabled_after_preflight_failure(
+def test_PrintDialog__on_print_preflight_check_succeeded_enabled_after_preflight_failure(
     mocker, print_dialog
 ):
     assert not print_dialog.continue_button.isEnabled()
     print_dialog._on_print_preflight_check_failed(mocker.MagicMock())
     assert print_dialog.continue_button.isEnabled()
 
 
-def test_PrintFileDialog__on_print_preflight_check_failed_when_status_is_PRINTER_NOT_FOUND(
+def test_PrintDialog__on_print_preflight_check_failed_when_status_is_PRINTER_NOT_FOUND(
     mocker, print_dialog
 ):
     print_dialog._show_insert_usb_message = mocker.MagicMock()
@@ -150,7 +150,7 @@ def test_PrintFileDialog__on_print_preflight_check_failed_when_status_is_PRINTER
     print_dialog._show_insert_usb_message.assert_called_once_with()
 
 
-def test_PrintFileDialog__on_print_preflight_check_failed_when_status_is_ERROR_PRINTER_URI(
+def test_PrintDialog__on_print_preflight_check_failed_when_status_is_ERROR_PRINTER_URI(
     mocker, print_dialog
 ):
     print_dialog._show_generic_error_message = mocker.MagicMock()
@@ -172,7 +172,7 @@ def test_PrintFileDialog__on_print_preflight_check_failed_when_status_is_ERROR_P
     assert print_dialog.error_status == ExportStatus.ERROR_PRINTER_URI
 
 
-def test_PrintFileDialog__on_print_preflight_check_failed_when_status_is_CALLED_PROCESS_ERROR(
+def test_PrintDialog__on_print_preflight_check_failed_when_status_is_CALLED_PROCESS_ERROR(
     mocker, print_dialog
 ):
     print_dialog._show_generic_error_message = mocker.MagicMock()
@@ -194,9 +194,7 @@ def test_PrintFileDialog__on_print_preflight_check_failed_when_status_is_CALLED_
     assert print_dialog.error_status == ExportStatus.CALLED_PROCESS_ERROR
 
 
-def test_PrintFileDialog__on_print_preflight_check_failed_when_status_is_unknown(
-    mocker, print_dialog
-):
+def test_PrintDialog__on_print_preflight_check_failed_when_status_is_unknown(mocker, print_dialog):
     print_dialog._show_generic_error_message = mocker.MagicMock()
     print_dialog.continue_button = mocker.MagicMock()
     print_dialog.continue_button.clicked = mocker.MagicMock()