From bc4bba4fa1db411208f336dca9a070bdd64e3ddd Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Tue, 25 Jul 2023 16:20:40 +0300 Subject: [PATCH] tests: Add full test coverage for updater checks Fully test the update check logic, by introducing several Qt tests. Also, improve the `UpdaterThread.get_letest_info()` method, that gets the latest version and changelog from GitHub, with several checks. These checks are also tested in our newly added tests. --- dangerzone/gui/updater.py | 30 +++- tests/gui/test_main_window.py | 303 +++++++++++++++++++++++++++++----- tests/gui/test_updater.py | 163 +++++++++++++++++- 3 files changed, 438 insertions(+), 58 deletions(-) diff --git a/dangerzone/gui/updater.py b/dangerzone/gui/updater.py index 411fd59..cd4453a 100644 --- a/dangerzone/gui/updater.py +++ b/dangerzone/gui/updater.py @@ -1,5 +1,6 @@ """A module that contains the logic for checking for updates.""" +import json import logging import platform import sys @@ -97,6 +98,7 @@ class UpdaterThread(QtCore.QThread): GH_RELEASE_URL = ( "https://api.github.com/repos/freedomofpress/dangerzone/releases/latest" ) + REQ_TIMEOUT = 15 def __init__(self, dangerzone: DangerzoneGui): super().__init__() @@ -201,14 +203,30 @@ class UpdaterThread(QtCore.QThread): to the users. """ try: - # TODO: Set timeout. - info = requests.get(self.GH_RELEASE_URL).json() + res = requests.get(self.GH_RELEASE_URL, timeout=self.REQ_TIMEOUT) except Exception as e: - # TODO:: Handle errors. - raise + raise RuntimeError( + f"Encountered an exception while querying {self.GH_RELEASE_URL}: {e}" + ) - version = info["tag_name"].lstrip("v") - changelog = markdown.markdown(info["body"]) + if res.status_code != 200: + raise RuntimeError( + f"Encountered an HTTP {res.status_code} error while querying" + f" {self.GH_RELEASE_URL}" + ) + + try: + info = res.json() + except json.JSONDecodeError as e: + raise ValueError(f"Received a non-JSON response from {self.GH_RELEASE_URL}") + + try: + version = info["tag_name"].lstrip("v") + changelog = markdown.markdown(info["body"]) + except KeyError as e: + raise ValueError( + f"Missing required fields in JSON response from {self.GH_RELEASE_URL}" + ) return UpdateReport(version=version, changelog=changelog) diff --git a/tests/gui/test_main_window.py b/tests/gui/test_main_window.py index 33f1ae2..d65d745 100644 --- a/tests/gui/test_main_window.py +++ b/tests/gui/test_main_window.py @@ -1,26 +1,22 @@ +import time +import typing + +from PySide6 import QtCore, QtWidgets from pytest import MonkeyPatch, fixture from pytest_mock import MockerFixture from pytestqt.qtbot import QtBot from dangerzone.gui import MainWindow -from dangerzone.gui import updater as updater_mod -from dangerzone.gui.main_window import * +from dangerzone.gui import main_window as main_window_module +from dangerzone.gui import updater as updater_module +from dangerzone.gui.logic import DangerzoneGui +from dangerzone.gui.main_window import ContentWidget from dangerzone.gui.updater import UpdateReport, UpdaterThread from dangerzone.util import get_version from .. import sample_doc, sample_pdf -from . import qt_updater, updater -from .test_updater import default_updater_settings - -# FIXME: See https://github.com/freedomofpress/dangerzone/issues/320 for more details. -if typing.TYPE_CHECKING: - from PySide2 import QtCore -else: - try: - from PySide6 import QtCore, QtGui, QtWidgets - except ImportError: - from PySide2 import QtCore, QtGui, QtWidgets - +from . import qt_updater as updater +from .test_updater import assert_report_equal, default_updater_settings ## ## Widget Fixtures @@ -28,7 +24,7 @@ else: @fixture -def content_widget(qtbot: QtBot, mocker: MockerFixture) -> QtWidgets.QWidget: +def content_widget(qtbot: QtBot, mocker: MockerFixture) -> ContentWidget: # Setup mock_app = mocker.MagicMock() dummy = mocker.MagicMock() @@ -38,63 +34,280 @@ def content_widget(qtbot: QtBot, mocker: MockerFixture) -> QtWidgets.QWidget: return w -def test_qt( +def test_default_menu( qtbot: QtBot, - qt_updater: UpdaterThread, + updater: UpdaterThread, +) -> None: + """Check that the default menu entries are in order.""" + updater.dangerzone.settings.set("updater_check", True) + + window = MainWindow(updater.dangerzone) + menu_actions = window.hamburger_button.menu().actions() + assert len(menu_actions) == 3 + + toggle_updates_action = menu_actions[0] + assert toggle_updates_action.text() == "Check for updates" + assert toggle_updates_action.isChecked() + + separator = menu_actions[1] + assert separator.isSeparator() + + exit_action = menu_actions[2] + assert exit_action.text() == "Exit" + + toggle_updates_action.trigger() + assert not toggle_updates_action.isChecked() + assert updater.dangerzone.settings.get("updater_check") == False + + +def test_no_update( + qtbot: QtBot, + updater: UpdaterThread, monkeypatch: MonkeyPatch, mocker: MockerFixture, ) -> None: - updater = qt_updater + """Test that when no update has been detected, the user is not alerted.""" + # Check that when no update is detected, e.g., due to update cooldown, an empty + # report is received that does not affect the menu entries. + curtime = int(time.time()) updater.dangerzone.settings.set("updater_check", True) - updater.dangerzone.settings.set("updater_last_check", 0) + updater.dangerzone.settings.set("updater_errors", 9) + updater.dangerzone.settings.set("updater_last_check", curtime) + expected_settings = default_updater_settings() expected_settings["updater_check"] = True - - mock_upstream_info = {"tag_name": f"v{get_version()}", "body": "changelog"} - - # Always assume that we can perform multiple update checks in a row. - monkeypatch.setattr(updater, "_should_postpone_update_check", lambda: False) - - # Make requests.get().json() return the above dictionary. - requests_mock = mocker.MagicMock() - requests_mock().json.return_value = mock_upstream_info - monkeypatch.setattr(updater_mod.requests, "get", requests_mock) + expected_settings["updater_errors"] = 0 # errors must be cleared + expected_settings["updater_last_check"] = curtime window = MainWindow(updater.dangerzone) window.register_update_handler(updater.finished) + handle_updates_spy = mocker.spy(window, "handle_updates") + + menu_actions_before = window.hamburger_button.menu().actions() + with qtbot.waitSignal(updater.finished) as blocker: updater.start() - assert len(window.hamburger_button.menu().actions()) == 3 + # Check that the callback function gets an empty report. + handle_updates_spy.assert_called_once() + assert_report_equal(handle_updates_spy.call_args.args[0], UpdateReport()) + + # Check that the menu entries remain exactly the same. + menu_actions_after = window.hamburger_button.menu().actions() + assert menu_actions_before == menu_actions_after + + # Check that any previous update errors are cleared. + assert updater.dangerzone.settings.get_updater_settings() == expected_settings + + +def test_update_detected( + qtbot: QtBot, + updater: UpdaterThread, + monkeypatch: MonkeyPatch, + mocker: MockerFixture, +) -> None: + """Test that a newly detected version leads to a notification to the user.""" + updater.dangerzone.settings.set("updater_check", True) + updater.dangerzone.settings.set("updater_last_check", 0) + updater.dangerzone.settings.set("updater_errors", 9) + + # Make requests.get().json() return the following dictionary. + mock_upstream_info = {"tag_name": "99.9.9", "body": "changelog"} + mocker.patch("dangerzone.gui.updater.requests.get") + requests_mock = updater_module.requests.get + requests_mock().status_code = 200 # type: ignore [call-arg] + requests_mock().json.return_value = mock_upstream_info # type: ignore [attr-defined, call-arg] + + window = MainWindow(updater.dangerzone) + window.register_update_handler(updater.finished) + handle_updates_spy = mocker.spy(window, "handle_updates") + load_svg_spy = mocker.spy(window, "load_svg_image") + + menu_actions_before = window.hamburger_button.menu().actions() + + with qtbot.waitSignal(updater.finished) as blocker: + updater.start() + + menu_actions_after = window.hamburger_button.menu().actions() + + # Check that the callback function gets an update report. + handle_updates_spy.assert_called_once() + assert_report_equal( + handle_updates_spy.call_args.args[0], UpdateReport("99.9.9", "

changelog

") + ) + + # Check that the settings have been updated properly. + expected_settings = default_updater_settings() + expected_settings["updater_check"] = True expected_settings["updater_last_check"] = updater.dangerzone.settings.get( "updater_last_check" ) - assert updater.dangerzone.settings.get_updater_settings() == expected_settings - - mock_upstream_info["tag_name"] = "v99.9.9" - mock_upstream_info["changelog"] = "changelog" expected_settings["updater_latest_version"] = "99.9.9" expected_settings["updater_latest_changelog"] = "

changelog

" + expected_settings["updater_errors"] = 0 + assert updater.dangerzone.settings.get_updater_settings() == expected_settings + + # Check that the hamburger icon has changed with the expected SVG image. + assert load_svg_spy.call_count == 2 + assert load_svg_spy.call_args_list[0].args[0] == "hamburger_menu_update_success.svg" + assert ( + load_svg_spy.call_args_list[1].args[0] == "hamburger_menu_update_available.svg" + ) + + # Check that new menu entries have been added. + menu_actions_after = window.hamburger_button.menu().actions() + assert len(menu_actions_after) == 5 + assert menu_actions_after[2:] == menu_actions_before + + success_action = menu_actions_after[0] + assert success_action.text() == "New version available" + + separator = menu_actions_after[1] + assert separator.isSeparator() + + # Check that clicking in the new menu entry, opens a dialog. + update_dialog_spy = mocker.spy(main_window_module, "UpdateDialog") + + def check_dialog() -> None: + dialog = updater.dangerzone.app.activeWindow() + + update_dialog_spy.assert_called_once() + kwargs = update_dialog_spy.call_args.kwargs + assert "99.9.9" in kwargs["title"] + assert "dangerzone.rocks" in kwargs["intro_msg"] + assert not kwargs["middle_widget"].toggle_button.isChecked() + collapsible_box = kwargs["middle_widget"] + text_browser = ( + collapsible_box.layout().itemAt(1).widget().layout().itemAt(0).widget() + ) + assert collapsible_box.toggle_button.text() == "What's New?" + assert text_browser.toPlainText() == "changelog" + + height_initial = dialog.sizeHint().height() + width_initial = dialog.sizeHint().width() + + # Collapse the "What's New" section and ensure that the dialog's height + # increases. + with qtbot.waitSignal(collapsible_box.toggle_animation.finished) as blocker: + collapsible_box.toggle_button.click() + + assert dialog.sizeHint().height() > height_initial + assert dialog.sizeHint().width() == width_initial + + # Uncollapse the "What's New" section, and ensure that the dialog's height gets + # back to the original value. + with qtbot.waitSignal(collapsible_box.toggle_animation.finished) as blocker: + collapsible_box.toggle_button.click() + + assert dialog.sizeHint().height() == height_initial + assert dialog.sizeHint().width() == width_initial + + dialog.close() + + QtCore.QTimer.singleShot(500, check_dialog) + success_action.trigger() + + # FIXME: We should check the content of the dialog here. + + +def test_update_error( + qtbot: QtBot, + updater: UpdaterThread, + monkeypatch: MonkeyPatch, + mocker: MockerFixture, +) -> None: + """Test that an error during an update check leads to a notification to the user.""" + # Test 1 - Check that the first error does not notify the user. + updater.dangerzone.settings.set("updater_check", True) + updater.dangerzone.settings.set("updater_last_check", 0) + updater.dangerzone.settings.set("updater_errors", 0) + + # Make requests.get() return an errorthe following dictionary. + mocker.patch("dangerzone.gui.updater.requests.get") + requests_mock = updater_module.requests.get + requests_mock.side_effect = Exception("failed") # type: ignore [attr-defined] + + window = MainWindow(updater.dangerzone) + window.register_update_handler(updater.finished) + handle_updates_spy = mocker.spy(window, "handle_updates") + load_svg_spy = mocker.spy(window, "load_svg_image") + + menu_actions_before = window.hamburger_button.menu().actions() with qtbot.waitSignal(updater.finished) as blocker: updater.start() - # The separator and install updates button has been added - assert len(window.hamburger_button.menu().actions()) == 5 + menu_actions_after = window.hamburger_button.menu().actions() + + # Check that the callback function gets an update report. + handle_updates_spy.assert_called_once() + assert "failed" in handle_updates_spy.call_args.args[0].error + + # Check that the settings have been updated properly. + expected_settings = default_updater_settings() + expected_settings["updater_check"] = True + expected_settings["updater_last_check"] = updater.dangerzone.settings.get( + "updater_last_check" + ) + expected_settings["updater_errors"] += 1 + assert updater.dangerzone.settings.get_updater_settings() == expected_settings + + # Check that the hamburger icon has not changed. + assert load_svg_spy.call_count == 0 + + # Check that no menu entries have been added. + assert menu_actions_before == menu_actions_after + + # Test 2 - Check that the second error does not notify the user either. + updater.dangerzone.settings.set("updater_last_check", 0) + with qtbot.waitSignal(updater.finished) as blocker: + updater.start() + + assert load_svg_spy.call_count == 0 + + # Check that the settings have been updated properly. + expected_settings["updater_errors"] += 1 expected_settings["updater_last_check"] = updater.dangerzone.settings.get( "updater_last_check" ) assert updater.dangerzone.settings.get_updater_settings() == expected_settings - # TODO: - # 1. Test that hamburger icon changes according to the update status. - # 2. Check that the dialogs for new updates / update errors have the expected - # content. - # 3. Check that a user can toggle updates from the hamburger menu. - # 4. Check that errors are cleared from the settings, whenever we have a successful - # update check. - # 5. Check that latest version/changelog, as well as update errors, are cached in - # the settings + # Check that no menu entries have been added. + assert menu_actions_before == menu_actions_after + + # Test 3 - Check that a third error shows a new menu entry. + updater.dangerzone.settings.set("updater_last_check", 0) + with qtbot.waitSignal(updater.finished) as blocker: + updater.start() + + menu_actions_after = window.hamburger_button.menu().actions() + assert len(menu_actions_after) == 5 + assert menu_actions_after[2:] == menu_actions_before + + error_action = menu_actions_after[0] + assert error_action.text() == "Update error" + + separator = menu_actions_after[1] + assert separator.isSeparator() + + # Check that clicking in the new menu entry, opens a dialog. + update_dialog_spy = mocker.spy(main_window_module, "UpdateDialog") + + def check_dialog() -> None: + dialog = updater.dangerzone.app.activeWindow() + + update_dialog_spy.assert_called_once() + kwargs = update_dialog_spy.call_args.kwargs + assert kwargs["title"] == "Update check error" + assert "Something went wrong" in kwargs["intro_msg"] + assert "Encountered an exception" in kwargs["middle_widget"].toPlainText() + assert "failed" in kwargs["middle_widget"].toPlainText() + assert "dangerzone.rocks" in kwargs["epilogue_msg"] + + dialog.close() + + QtCore.QTimer.singleShot(500, check_dialog) + error_action.trigger() ## diff --git a/tests/gui/test_updater.py b/tests/gui/test_updater.py index fa0acc2..b26d61e 100644 --- a/tests/gui/test_updater.py +++ b/tests/gui/test_updater.py @@ -3,9 +3,11 @@ import os import platform import sys import time +import typing from pathlib import Path import pytest +from PySide6 import QtCore from pytest import MonkeyPatch from pytest_mock import MockerFixture from pytestqt.qtbot import QtBot @@ -16,7 +18,7 @@ from dangerzone.gui import updater as updater_module from dangerzone.gui.updater import UpdateReport, UpdaterThread from dangerzone.util import get_version -from . import generate_isolated_updater, updater +from . import generate_isolated_updater, qt_updater, updater def default_updater_settings() -> dict: @@ -191,9 +193,10 @@ def test_update_checks( mock_upstream_info = {"tag_name": f"v{get_version()}", "body": "changelog"} # Make requests.get().json() return the above dictionary. - requests_mock = mocker.MagicMock() - requests_mock().json.return_value = mock_upstream_info - monkeypatch.setattr(updater_module.requests, "get", requests_mock) + mocker.patch("dangerzone.gui.updater.requests.get") + requests_mock = updater_module.requests.get + requests_mock().status_code = 200 # type: ignore [call-arg] + requests_mock().json.return_value = mock_upstream_info # type: ignore [attr-defined, call-arg] # Always assume that we can perform multiple update checks in a row. monkeypatch.setattr(updater, "_should_postpone_update_check", lambda: False) @@ -211,9 +214,12 @@ def test_update_checks( ) # Test 3 - Check that HTTP errors are converted to error reports. - requests_mock.side_effect = Exception("failed") + requests_mock.side_effect = Exception("failed") # type: ignore [attr-defined] report = updater.check_for_updates() - assert_report_equal(report, UpdateReport(error="failed")) + error_msg = ( + f"Encountered an exception while querying {updater.GH_RELEASE_URL}: failed" + ) + assert_report_equal(report, UpdateReport(error=error_msg)) # Test 4 - Check that cached version/changelog info do not trigger an update check. updater.dangerzone.settings.set("updater_latest_version", "99.9.9") @@ -238,6 +244,7 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - # # Make requests.get().json() return the version info that we want. mock_upstream_info = {"tag_name": "99.9.9", "body": "changelog"} + requests_mock().status_code = 200 # type: ignore [call-arg] requests_mock().json.return_value = mock_upstream_info # type: ignore [attr-defined, call-arg] # Test 1: The first time Dangerzone checks for updates, the cooldown period should @@ -289,4 +296,146 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) - report = updater.check_for_updates() assert cooldown_spy.spy_return == False assert updater.dangerzone.settings.get("updater_last_check") == curtime - assert_report_equal(report, UpdateReport(error="failed")) + error_msg = ( + f"Encountered an exception while querying {updater.GH_RELEASE_URL}: failed" + ) + assert_report_equal(report, UpdateReport(error=error_msg)) + + +def test_update_errors( + updater: UpdaterThread, monkeypatch: MonkeyPatch, mocker: MockerFixture +) -> None: + """Test update check errors.""" + # Mock requests.get(). + mocker.patch("dangerzone.gui.updater.requests.get") + requests_mock = updater_module.requests.get + + # Always assume that we can perform multiple update checks in a row. + monkeypatch.setattr(updater, "_should_postpone_update_check", lambda: False) + + # Test 1 - Check that request exceptions are being detected as errors. + requests_mock.side_effect = Exception("bad url") # type: ignore [attr-defined] + report = updater.check_for_updates() + assert report.error is not None + assert "bad url" in report.error + assert "Encountered an exception" in report.error + + # Test 2 - Check that non HTTP 200 responses are detected as errors. + class MockResponse500: + status_code = 500 + + requests_mock.return_value = MockResponse500() # type: ignore [attr-defined] + requests_mock.side_effect = None # type: ignore [attr-defined] + report = updater.check_for_updates() + assert report.error is not None + assert "Encountered an HTTP 500 error" in report.error + + # Test 3 - Check that non JSON responses are detected as errors. + class MockResponseBadJSON: + status_code = 200 + + def json(self) -> dict: + return json.loads("bad json") + + requests_mock.return_value = MockResponseBadJSON() # type: ignore [attr-defined] + report = updater.check_for_updates() + assert report.error is not None + assert "Received a non-JSON response" in report.error + + # Test 4 - Check that missing fields in JSON are detected as errors. + class MockResponseEmpty: + status_code = 200 + + def json(self) -> dict: + return {} + + requests_mock.return_value = MockResponseEmpty() # type: ignore [attr-defined] + report = updater.check_for_updates() + assert report.error is not None + assert "Missing required fields in JSON" in report.error + + # Test 5 - Check invalid versions are reported + class MockResponseBadVersion: + status_code = 200 + + def json(self) -> dict: + return {"tag_name": "vbad_version", "body": "changelog"} + + requests_mock.return_value = MockResponseBadVersion() # type: ignore [attr-defined] + report = updater.check_for_updates() + assert report.error is not None + assert "Invalid version" in report.error + + # Test 6 - Check invalid markdown is reported + class MockResponseBadMarkdown: + status_code = 200 + + def json(self) -> dict: + return {"tag_name": "v99.9.9", "body": ["bad", "markdown"]} + + requests_mock.return_value = MockResponseBadMarkdown() # type: ignore [attr-defined] + report = updater.check_for_updates() + assert report.error is not None + + # Test 7 - Check that a valid response passes. + class MockResponseValid: + status_code = 200 + + def json(self) -> dict: + return {"tag_name": "v99.9.9", "body": "changelog"} + + requests_mock.return_value = MockResponseValid() # type: ignore [attr-defined] + report = updater.check_for_updates() + assert_report_equal(report, UpdateReport("99.9.9", "

changelog

")) + + +def test_update_check_prompt( + qtbot: QtBot, + qt_updater: UpdaterThread, + monkeypatch: MonkeyPatch, + mocker: MockerFixture, +) -> None: + """Test that the prompt to enable update checks works properly.""" + # Force Dangerzone to check immediately for updates + qt_updater.dangerzone.settings.set("updater_last_check", 0) + + # Test 1 - Check that on the second run of Dangerzone, the user is prompted to + # choose if they want to enable update checks. By clicking on, we store this + # decision in the settings. + def click_ok() -> None: + dialog = qt_updater.dangerzone.app.activeWindow() + dialog.ok_button.click() # type: ignore [attr-defined] + + QtCore.QTimer.singleShot(500, click_ok) + res = qt_updater.should_check_for_updates() + + assert res == True + assert qt_updater.check == True + + # Test 2 - Same as the previous test, but check that clicking on cancel stores the + # opposite decision. + qt_updater.check = None + + def click_cancel() -> None: + dialog = qt_updater.dangerzone.app.activeWindow() + dialog.cancel_button.click() # type: ignore [attr-defined] + + QtCore.QTimer.singleShot(500, click_cancel) + res = qt_updater.should_check_for_updates() + + assert res == False + assert qt_updater.check == False + + # Test 3 - Same as the previous test, but check that clicking on "X" does not store + # any decision. + qt_updater.check = None + + def click_x() -> None: + dialog = qt_updater.dangerzone.app.activeWindow() + dialog.close() + + QtCore.QTimer.singleShot(500, click_x) + res = qt_updater.should_check_for_updates() + + assert res == False + assert qt_updater.check == None