From 9e3b1d0a370974e737ac11475b2b5077437a52be Mon Sep 17 00:00:00 2001
From: Bart Feenstra <bart@mynameisbart.com>
Date: Wed, 21 Feb 2024 18:54:49 +0000
Subject: [PATCH] Fix a bug where cancelling site generation in the Graphical
 User Interface would cause a fatal error

---
 betty/gui/project.py            | 29 ++++++++++++++++++-----------
 betty/tests/gui/test_project.py | 18 ++++++++++--------
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/betty/gui/project.py b/betty/gui/project.py
index cf1191e86..3587e8019 100644
--- a/betty/gui/project.py
+++ b/betty/gui/project.py
@@ -6,13 +6,13 @@
 import asyncio
 import copy
 import re
-from asyncio import Task
+from asyncio import Task, CancelledError
 from contextlib import suppress
 from pathlib import Path
 from urllib.parse import urlparse
 
 from PyQt6.QtCore import Qt, QThread, QObject
-from PyQt6.QtGui import QAction
+from PyQt6.QtGui import QAction, QCloseEvent
 from PyQt6.QtWidgets import QFileDialog, QPushButton, QWidget, QVBoxLayout, QHBoxLayout, QMenu, QStackedLayout, \
     QGridLayout, QCheckBox, QFormLayout, QLabel, QLineEdit, QButtonGroup, QRadioButton, QFrame, QScrollArea, QSizePolicy
 from babel import Locale
@@ -598,7 +598,9 @@ def __init__(self, project: Project, generate_window: _GenerateWindow):
 
     @sync
     async def run(self) -> None:
-        self._task = asyncio.create_task(self._generate())
+        with suppress(CancelledError):
+            self._task = asyncio.create_task(self._generate())
+            await self._task
 
     async def _generate(self) -> None:
         with ExceptionCatcher(self._generate_window, close_parent=True):
@@ -638,6 +640,7 @@ def __init__(
         central_layout.addLayout(button_layout)
 
         self._cancel_button = QPushButton()
+        self._cancel_button.released.connect(self.close)
         button_layout.addWidget(self._cancel_button)
 
         self._serve_button = QPushButton()
@@ -646,9 +649,12 @@ def __init__(
         button_layout.addWidget(self._serve_button)
 
         self._logging_handler = LogRecordViewerHandler(self._log_record_viewer)
+        load.getLogger().addHandler(self._logging_handler)
+        generate.getLogger().addHandler(self._logging_handler)
+
         self._thread = _GenerateThread(self._app.project, self)
         self._thread.finished.connect(self._finish_generate)
-        self._cancel_button.released.connect(self._thread.cancel)
+        self._thread.start()
 
     @property
     def window_title(self) -> Localizable:
@@ -656,19 +662,20 @@ def window_title(self) -> Localizable:
 
     def _serve(self) -> None:
         with ExceptionCatcher(self):
-            serve_window = ServeProjectWindow(self._app, parent=self)
+            serve_window = ServeProjectWindow(self._app, parent=self.parent())
             serve_window.show()
 
-    def show(self) -> None:
-        super().show()
-        load.getLogger().addHandler(self._logging_handler)
-        generate.getLogger().addHandler(self._logging_handler)
-        self._thread.start()
+    def closeEvent(self, a0: QCloseEvent | None) -> None:
+        super().closeEvent(a0)
+        self._thread.cancel()
+        self._finalize()
 
     def _finish_generate(self) -> None:
         self._cancel_button.setDisabled(True)
         self._serve_button.setDisabled(False)
-        self.setWindowFlags(self.windowFlags() | Qt.WindowType.WindowCloseButtonHint)
+        self._finalize()
+
+    def _finalize(self) -> None:
         load.getLogger().removeHandler(self._logging_handler)
         generate.getLogger().removeHandler(self._logging_handler)
 
diff --git a/betty/tests/gui/test_project.py b/betty/tests/gui/test_project.py
index 13803133d..79245cf7c 100644
--- a/betty/tests/gui/test_project.py
+++ b/betty/tests/gui/test_project.py
@@ -1,4 +1,5 @@
 import json
+from asyncio import sleep
 from pathlib import Path
 
 import aiofiles
@@ -202,18 +203,19 @@ async def test_project_window_autowrite(self, betty_qtbot: BettyQtBot) -> None:
 
 
 class TestGenerateWindow:
-    async def test_cancel_button_should_close_window(
-        self,
-        betty_qtbot: BettyQtBot,
-    ) -> None:
+    async def test_cancel_button_should_close_window(self, mocker: MockerFixture, betty_qtbot: BettyQtBot) -> None:
+        async def _generate(app: App) -> None:
+            # Ensure this takes a very long time, longer than any timeout. That way,
+            # if cancellation fails, this test may never pass.
+            await sleep(999999999)
+        mocker.patch('betty.generate.generate', new_callable=lambda: _generate)
+
         async with App() as app:
             sut = _GenerateWindow(app)
             betty_qtbot.qtbot.addWidget(sut)
 
-            with betty_qtbot.qtbot.waitSignal(sut._thread.finished):
-                sut.show()
-                betty_qtbot.qtbot.mouseClick(sut._cancel_button, Qt.MouseButton.LeftButton)
-
+            sut.show()
+            betty_qtbot.mouse_click(sut._cancel_button)
             betty_qtbot.assert_not_window(sut)
 
     async def test_serve_button_should_open_serve_window(