STILL DONT MERGE

Makefile

@@ -3,7 +3,7 @@ SHELL := /bin/bash
 
 .PHONY: clean
 clean:  ## Clean the workspace of generated resources
-	@rm -rf build dist *.egg-info .coverage .eggs docs/_build .pytest_cache lib htmlcov && \
+	@rm -rf build dist *.egg-info .coverage .eggs docs/_build .pytest_cache lib htmlcov .cache && \
 		find . \( -name '*.py[co]' -o -name dropin.cache \) -delete && \
 		find . \( -name '*.bak' -o -name dropin.cache \) -delete && \
 		find . \( -name '*.tgz' -o -name dropin.cache \) -delete && \
@@ -34,7 +34,7 @@ check: clean lint test ## Run the full CI test suite
 # 6. Format columns with colon as delimiter.
 .PHONY: help
 help: ## Print this message and exit.
-	@printf "Makefile for developing and testing SecureDrop.\n"
+	@printf "Makefile for developing and testing the SecureDrop client.\n"
 	@printf "Subcommands:\n\n"
 	@awk 'BEGIN {FS = ":.*?## "} /^[0-9a-zA-Z_-]+:.*?## / {printf "\033[36m%s\033[0m : %s\n", $$1, $$2}' $(MAKEFILE_LIST) \
 		| sort \

securedrop_client/gui/main.py

@@ -177,7 +177,6 @@ def show_conversation_for(self, source):
 
         if conversation_container is None:
             conversation = ConversationView(source, self.sdc_home, self.controller, parent=self)
-            conversation.setup(self.controller)
 
             conversation_container = QWidget()
             layout = QVBoxLayout()

securedrop_client/gui/widgets.py

@@ -498,10 +498,10 @@ def __init__(self, message_id: str, text: str, update_signal) -> None:
 
         layout.addWidget(self.message)
 
-        update_signal.connect(self.__update_text)
+        update_signal.connect(self._update_text)
 
     @pyqtSlot(str, str)
-    def __update_text(self, message_id: str, text: str) -> None:
+    def _update_text(self, message_id: str, text: str) -> None:
         """
         Conditionally update this SpeechBubble's text if and only if the message_id of the emitted
         signal matche the message_id of this speech bubble.
@@ -552,10 +552,10 @@ class MessageWidget(ConversationWidget):
     Represents an incoming message from the source.
     """
 
-    def __init__(self, message_id: str, message: str, controller: Client) -> None:
+    def __init__(self, message_id: str, message: str, update_signal) -> None:
         super().__init__(message_id,
                          message,
-                         controller.message_sync.message_downloaded,
+                         update_signal,
                          align="left")
         self.setStyleSheet("""
         background-color: #EEE;
@@ -567,10 +567,10 @@ class ReplyWidget(ConversationWidget):
     Represents a reply to a source.
     """
 
-    def __init__(self, message_id: str, message: str, controller: Client) -> None:
+    def __init__(self, message_id: str, message: str, update_signal) -> None:
         super().__init__(message_id,
                          message,
-                         controller.reply_sync.reply_downloaded,
+                         update_signal,
                          align="right")
         self.setStyleSheet("""
         background-color: #2299EE;
@@ -637,7 +637,7 @@ class ConversationView(QWidget):
     Renders a conversation.
     """
 
-    def __init__(self, source_db_object, sdc_home: str, controller, parent=None):
+    def __init__(self, source_db_object, sdc_home: str, controller: Client, parent=None):
         super().__init__(parent)
         self.source = source_db_object
         self.sdc_home = sdc_home
@@ -669,7 +669,7 @@ def update_conversation(self, collection: list) -> None:
         # clear all old items
         while True:
             w = self.conversation_layout.takeAt(0)
-            if w:
+            if w:  # pragma: no cover
                 del w
             else:
                 break
@@ -696,12 +696,6 @@ def add_item_content_or(self, adder, item, default):
         else:
             adder(item.uuid, get_data(self.sdc_home, item.filename))
 
-    def setup(self, controller):
-        """
-        Ensure there's a reference to program logic.
-        """
-        self.controller = controller
-
     def add_file(self, source_db_object, submission_db_object):
         """
         Add a file from the source.
@@ -721,13 +715,15 @@ def add_message(self, message_id: str, message: str) -> None:
         """
         Add a message from the source.
         """
-        self.conversation_layout.addWidget(MessageWidget(message_id, message, self.controller))
+        self.conversation_layout.addWidget(
+            MessageWidget(message_id, message, self.controller.message_sync.message_downloaded))
 
     def add_reply(self, message_id: str, reply: str, files=None) -> None:
         """
         Add a reply from a journalist.
         """
-        self.conversation_layout.addWidget(ReplyWidget(message_id, reply, self.controller))
+        self.conversation_layout.addWidget(
+            ReplyWidget(message_id, reply, self.controller.reply_sync.reply_downloaded))
 
 
 class DeleteSourceAction(QAction):

tests/gui/test_main.py

@@ -5,6 +5,7 @@
 from securedrop_client.gui.main import Window
 from securedrop_client.resources import load_icon
 from securedrop_client.db import Submission
+from uuid import uuid4
 
 
 app = QApplication([])
@@ -233,7 +234,8 @@ def test_conversation_pending_message(mocker):
     mock_source = mocker.MagicMock()
     mock_source.journalistic_designation = 'Testy McTestface'
 
-    submission = Submission(source=mock_source, uuid="test", size=123,
+    msg_uuid = str(uuid4())
+    submission = Submission(source=mock_source, uuid=msg_uuid, size=123,
                             filename="test.msg.gpg",
                             download_url='http://test/test')
 
@@ -248,7 +250,7 @@ def test_conversation_pending_message(mocker):
     w.show_conversation_for(mock_source)
 
     assert mocked_add_message.call_count == 1
-    assert mocked_add_message.call_args == mocker.call("<Message not yet downloaded>")
+    assert mocked_add_message.call_args == mocker.call(msg_uuid, "<Message not yet downloaded>")
 
 
 def test_set_status(mocker):

tests/gui/test_widgets.py

@@ -542,9 +542,33 @@ def test_SpeechBubble_init(mocker):
     mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel')
     mocker.patch('securedrop_client.gui.widgets.QVBoxLayout')
     mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout')
+    mock_signal = mocker.Mock()
+    mock_connect = mocker.Mock()
+    mock_signal.connect = mock_connect
 
-    SpeechBubble('hello')
+    SpeechBubble('mock id', 'hello', mock_signal)
     mock_label.assert_called_once_with('hello')
+    assert mock_connect.called
+
+
+def test_SpeechBubble_update_text(mocker):
+    """
+    Check that the calling the slot updates the text.
+    """
+    mocker.patch('securedrop_client.gui.widgets.QVBoxLayout')
+    mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout')
+    mock_signal = mocker.MagicMock()
+
+    msg_id = 'abc123'
+    sb = SpeechBubble(msg_id, 'hello', mock_signal)
+
+    new_msg = 'new message'
+    sb._update_text(msg_id, new_msg)
+    assert sb.message.text() == new_msg
+
+    newer_msg = 'an even newer message'
+    sb._update_text(msg_id + 'xxxxx', newer_msg)
+    assert sb.message.text() == new_msg
 
 
 def test_SpeechBubble_html_init(mocker):
@@ -555,8 +579,9 @@ def test_SpeechBubble_html_init(mocker):
     mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel')
     mocker.patch('securedrop_client.gui.widgets.QVBoxLayout')
     mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout')
+    mock_signal = mocker.MagicMock()
 
-    SpeechBubble('<b>hello</b>')
+    SpeechBubble('mock id', '<b>hello</b>', mock_signal)
     mock_label.assert_called_once_with('&lt;b&gt;hello&lt;/b&gt;')
 
 
@@ -565,48 +590,73 @@ def test_SpeechBubble_with_apostrophe_in_text(mocker):
     mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel')
     mocker.patch('securedrop_client.gui.widgets.QVBoxLayout')
     mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout')
+    mock_signal = mocker.MagicMock()
 
     message = "I'm sure, you are reading my message."
-    SpeechBubble(message)
+    SpeechBubble('mock id', message, mock_signal)
     mock_label.assert_called_once_with(message)
 
 
-def test_ConversationWidget_init_left():
+def test_ConversationWidget_init_left(mocker):
     """
     Check the ConversationWidget is configured correctly for align-left.
     """
-    cw = ConversationWidget('hello', align='left')
+    mock_signal = mocker.Mock()
+    mock_connect = mocker.Mock()
+    mock_signal.connect = mock_connect
+
+    cw = ConversationWidget('mock id', 'hello', mock_signal, align='left')
     layout = cw.layout()
+
     assert isinstance(layout.takeAt(0), QWidgetItem)
     assert isinstance(layout.takeAt(0), QSpacerItem)
+    assert mock_connect.called
 
 
-def test_ConversationWidget_init_right():
+def test_ConversationWidget_init_right(mocker):
     """
     Check the ConversationWidget is configured correctly for align-left.
     """
-    cw = ConversationWidget('hello', align='right')
+    mock_signal = mocker.Mock()
+    mock_connect = mocker.Mock()
+    mock_signal.connect = mock_connect
+
+    cw = ConversationWidget('mock id', 'hello', mock_signal, align='right')
     layout = cw.layout()
+
     assert isinstance(layout.takeAt(0), QSpacerItem)
     assert isinstance(layout.takeAt(0), QWidgetItem)
+    assert mock_connect.called
 
 
-def test_MessageWidget_init():
+def test_MessageWidget_init(mocker):
     """
     Check the CSS is set as expected.
     """
-    mw = MessageWidget('hello')
+    mock_signal = mocker.Mock()
+    mock_connected = mocker.Mock()
+    mock_signal.connect = mock_connected
+
+    mw = MessageWidget('mock id', 'hello', mock_signal)
     ss = mw.styleSheet()
+
     assert 'background-color' in ss
+    assert mock_connected.called
 
 
-def test_ReplyWidget_init():
+def test_ReplyWidget_init(mocker):
     """
     Check the CSS is set as expected.
     """
-    rw = ReplyWidget('hello')
+    mock_signal = mocker.Mock()
+    mock_connected = mocker.Mock()
+    mock_signal.connect = mock_connected
+
+    rw = ReplyWidget('mock id', 'hello', mock_signal)
     ss = rw.styleSheet()
+
     assert 'background-color' in ss
+    assert mock_connected.called
 
 
 def test_FileWidget_init_left(mocker):
@@ -685,28 +735,21 @@ def test_ConversationView_init(mocker, homedir):
     Ensure the conversation view has a layout to add widgets to.
     """
     mocked_source = mocker.MagicMock()
-    cv = ConversationView(mocked_source, homedir)
+    mocked_controller = mocker.MagicMock()
+    cv = ConversationView(mocked_source, homedir, mocked_controller)
     assert isinstance(cv.conversation_layout, QVBoxLayout)
 
 
-def test_ConversationView_setup(mocker, homedir):
-    """
-    Ensure the controller is set
-    """
-    mocked_source = mocker.MagicMock()
-    cv = ConversationView(mocked_source, homedir)
-    mock_controller = mocker.MagicMock()
-    cv.setup(mock_controller)
-    assert cv.controller == mock_controller
-
-
 def test_ConversationView_move_to_bottom(mocker, homedir):
     """
     Check the signal handler sets the correct value for the scrollbar to be
     the maximum possible value.
     """
     mocked_source = mocker.MagicMock()
-    cv = ConversationView(mocked_source, homedir)
+    mocked_controller = mocker.MagicMock()
+
+    cv = ConversationView(mocked_source, homedir, mocked_controller)
+
     cv.scroll = mocker.MagicMock()
     cv.move_to_bottom(0, 6789)
     cv.scroll.verticalScrollBar().setValue.assert_called_once_with(6789)
@@ -717,11 +760,12 @@ def test_ConversationView_add_message(mocker, homedir):
     Adding a message results in a new MessageWidget added to the layout.
     """
     mocked_source = mocker.MagicMock()
-    cv = ConversationView(mocked_source, homedir)
-    cv.controller = mocker.MagicMock()
+    mocked_controller = mocker.MagicMock()
+
+    cv = ConversationView(mocked_source, homedir, mocked_controller)
     cv.conversation_layout = mocker.MagicMock()
 
-    cv.add_message('hello')
+    cv.add_message('mock id', 'hello')
     assert cv.conversation_layout.addWidget.call_count == 1
 
     cal = cv.conversation_layout.addWidget.call_args_list
@@ -733,11 +777,12 @@ def test_ConversationView_add_reply(mocker, homedir):
     Adding a reply results in a new ReplyWidget added to the layout.
     """
     mocked_source = mocker.MagicMock()
-    cv = ConversationView(mocked_source, homedir)
-    cv.controller = mocker.MagicMock()
+    mocked_controller = mocker.MagicMock()
+
+    cv = ConversationView(mocked_source, homedir, mocked_controller)
     cv.conversation_layout = mocker.MagicMock()
 
-    cv.add_reply('hello')
+    cv.add_reply('mock id', 'hello')
     assert cv.conversation_layout.addWidget.call_count == 1
 
     cal = cv.conversation_layout.addWidget.call_args_list
@@ -750,8 +795,9 @@ def test_ConversationView_add_downloaded_file(mocker, homedir):
     proper QLabel.
     """
     mocked_source = mocker.MagicMock()
-    cv = ConversationView(mocked_source, homedir)
-    cv.controller = mocker.MagicMock()
+    mocked_controller = mocker.MagicMock()
+
+    cv = ConversationView(mocked_source, homedir, mocked_controller)
     cv.conversation_layout = mocker.MagicMock()
 
     mock_source = mocker.MagicMock()
@@ -775,8 +821,9 @@ def test_ConversationView_add_not_downloaded_file(mocker, homedir):
     proper QLabel.
     """
     mocked_source = mocker.MagicMock()
-    cv = ConversationView(mocked_source, homedir)
-    cv.controller = mocker.MagicMock()
+    mocked_controller = mocker.MagicMock()
+
+    cv = ConversationView(mocked_source, homedir, mocked_controller)
     cv.conversation_layout = mocker.MagicMock()
 
     mock_source = mocker.MagicMock()

tests/test_app.py

@@ -234,7 +234,7 @@ def fake_known_args():
     mock_start_app.assert_called_once_with(mock_args, mock_qt_args)
 
 
-def test_signal_interception(mocker):
+def test_signal_interception(mocker, homedir):
     # check that initializing an app calls configure_signal_handlers
     mocker.patch('securedrop_client.app.QApplication')
     mocker.patch('securedrop_client.app.prevent_second_instance')
@@ -245,8 +245,10 @@ def test_signal_interception(mocker):
     mocker.patch('securedrop_client.logic.GpgHelper')
     mocker.patch('securedrop_client.app.configure_logging')
     mock_signal_handlers = mocker.patch('securedrop_client.app.configure_signal_handlers')
+    mock_args = mocker.Mock()
+    mock_args.sdc_home = homedir
 
-    start_app(mocker.MagicMock(), [])
+    start_app(mock_args, [])
     assert mock_signal_handlers.called
 
     # check that a signal interception calls quit on the app