fixes

Author: aw-was-here

conftest.py

@@ -17,7 +17,6 @@
 
 import nowplaying.bootstrap
 import nowplaying.config
-import nowplaying.db
 import nowplaying.apicache
 
 # if sys.platform == 'darwin':
@@ -98,43 +97,53 @@ def bootstrap(getroot):  # pylint: disable=redefined-outer-name
 # don't stick around
 #
 @pytest.fixture(autouse=True, scope="function")
-def clear_old_testsuite():
+def clear_old_testsuite(request):
     ''' clear out old testsuite configs '''
+    # Skip Qt app creation for tests that need their own QApplication (like singleinstance tests)
+    skip_qt_cleanup = 'qapp' in request.fixturenames
+
     if sys.platform == "win32":
         qsettingsformat = QSettings.IniFormat
     else:
         qsettingsformat = QSettings.NativeFormat
 
     nowplaying.bootstrap.set_qt_names(appname='testsuite')
-    config = QSettings(qsettingsformat, QSettings.SystemScope, QCoreApplication.organizationName(),
-                       QCoreApplication.applicationName())
-    config.clear()
-    config.sync()
-
-    cachedir = pathlib.Path(QStandardPaths.standardLocations(QStandardPaths.CacheLocation)[0])
-    if 'testsuite' in cachedir.name and cachedir.exists():
-        logging.info('Removing %s', cachedir)
-        shutil.rmtree(cachedir)
-
-    config = QSettings(qsettingsformat, QSettings.UserScope, QCoreApplication.organizationName(),
-                       QCoreApplication.applicationName())
-    config.clear()
-    config.sync()
-    filename = pathlib.Path(config.fileName())
-    del config
-    if filename.exists():
+
+    # Only create QCoreApplication if test doesn't need QApplication
+    if not skip_qt_cleanup:
+        config = QSettings(qsettingsformat, QSettings.SystemScope, QCoreApplication.organizationName(),
+                           QCoreApplication.applicationName())
+        config.clear()
+        config.sync()
+
+        cachedir = pathlib.Path(QStandardPaths.standardLocations(QStandardPaths.CacheLocation)[0])
+        if 'testsuite' in cachedir.name and cachedir.exists():
+            logging.info('Removing %s', cachedir)
+            shutil.rmtree(cachedir)
+
+        config = QSettings(qsettingsformat, QSettings.UserScope, QCoreApplication.organizationName(),
+                           QCoreApplication.applicationName())
+        config.clear()
+        config.sync()
+        filename = pathlib.Path(config.fileName())
+        del config
+    else:
+        # For Qt app tests, just set up the filename for cleanup
+        filename = None
+
+    if filename and filename.exists():
         filename.unlink()
     reboot_macosx_prefs()
-    if filename.exists():
+    if filename and filename.exists():
         filename.unlink()
     reboot_macosx_prefs()
-    if filename.exists():
+    if filename and filename.exists():
         logging.error('Still exists, wtf?')
     yield filename
-    if filename.exists():
+    if filename and filename.exists():
         filename.unlink()
     reboot_macosx_prefs()
-    if filename.exists():
+    if filename and filename.exists():
         filename.unlink()
     reboot_macosx_prefs()
 
@@ -155,8 +164,8 @@ async def temp_api_cache():
                 await cache.close()
 
 
-@pytest.fixture(autouse=True)
-def auto_temp_api_cache_for_artistextras(request, temp_api_cache):  # pylint: disable=redefined-outer-name
+@pytest_asyncio.fixture(autouse=True)
+async def auto_temp_api_cache_for_artistextras(request, temp_api_cache):  # pylint: disable=redefined-outer-name
     """Automatically use temp API cache for artistextras tests to prevent random CI failures."""
     # Only auto-apply to artistextras tests, not apicache tests
     if 'test_artistextras' in request.module.__name__:

nowplaying/subprocesses.py

@@ -79,8 +79,8 @@ def _start_process(self, processname):
                 ))
             self.processes[processname]['process'].start()
 
-    def _stop_process_parallel(self, processname):
-        ''' Stop a process - designed for parallel execution '''
+    def _shutdown_process_common(self, processname: str, graceful_timeout: int = 8) -> None:
+        ''' Common shutdown logic for both parallel and sequential process stopping '''
         if not self.processes[processname]['process']:
             return
 
@@ -92,11 +92,13 @@ def _stop_process_parallel(self, processname):
             try:
                 func = getattr(self.processes[processname]['module'], 'stop')
                 func(process.pid)
-            except Exception as error:  # pylint: disable=broad-exception-caught
+            except (AttributeError, TypeError) as error:
                 logging.error('Error calling stop function for %s: %s', processname, error)
+            except Exception as error:  # pylint: disable=broad-exception-caught
+                logging.exception('Unexpected error calling stop function for %s: %s', processname, error)
 
-        # Wait for graceful shutdown (reduced since we're parallel)
-        process.join(8)
+        # Wait for graceful shutdown
+        process.join(graceful_timeout)
 
         # Force termination if still alive
         if process.is_alive():
@@ -108,19 +110,25 @@ def _stop_process_parallel(self, processname):
         # Cleanup - be defensive on Windows
         try:
             process.close()
-        except Exception as error:  # pylint: disable=broad-exception-caught
+        except (OSError, AttributeError) as error:
             logging.debug('Error closing process %s: %s', processname, error)
+        except Exception as error:  # pylint: disable=broad-exception-caught
+            logging.exception('Unexpected error closing process %s: %s', processname, error)
 
         del self.processes[processname]['process']
         self.processes[processname]['process'] = None
         logging.debug('%s stopped successfully', processname)
 
-    def _stop_process(self, processname):
+    def _stop_process_parallel(self, processname: str) -> None:
+        ''' Stop a process - designed for parallel execution '''
+        self._shutdown_process_common(processname, graceful_timeout=8)
+
+    def _stop_process(self, processname: str) -> None:
         ''' Stop a process - sequential version for individual stops '''
         if self.processes[processname]['process']:
             logging.debug('Notifying %s', processname)
             self.processes[processname]['stopevent'].set()
-            self._stop_process_parallel(processname)
+            self._shutdown_process_common(processname, graceful_timeout=15)
         logging.debug('%s should be stopped', processname)
 
     def start_process(self, processname):

nowplaying/systemtray.py

@@ -2,11 +2,12 @@
 ''' system tray '''
 
 import logging
+import sqlite3
 
 from PySide6.QtWidgets import (  # pylint: disable=no-name-in-module
     QApplication, QErrorMessage, QMenu, QMessageBox, QSystemTrayIcon)
 from PySide6.QtGui import QAction, QActionGroup, QIcon  # pylint: disable=no-name-in-module
-from PySide6.QtCore import QFileSystemWatcher  # pylint: disable=no-name-in-module
+from PySide6.QtCore import QFileSystemWatcher, QThread  # pylint: disable=no-name-in-module
 
 import nowplaying.apicache
 import nowplaying.config
@@ -19,6 +20,49 @@
 LASTANNOUNCED: dict[str,str|None] = {'artist': None, 'title': None}
 
 
+class DatabaseVacuumThread(QThread):  # pylint: disable=too-few-public-methods
+    ''' Background thread for database vacuum operations '''
+
+    def __init__(self, requestswindow=None) -> None:
+        super().__init__()
+        self.requestswindow = requestswindow
+
+    def run(self) -> None:
+        ''' Perform database vacuum operations in background '''
+        logging.debug("Starting database vacuum operations in background thread")
+
+        # Vacuum API cache database
+        try:
+            nowplaying.apicache.APIResponseCache.vacuum_database_file()
+            logging.debug("API cache database vacuumed successfully")
+        except (sqlite3.Error, OSError) as error:
+            logging.error("Error vacuuming API cache: %s", error)
+        except Exception as error:  # pylint: disable=broad-exception-caught
+            logging.exception("Unexpected error vacuuming API cache: %s", error)
+
+        # Vacuum metadata database
+        try:
+            metadb = nowplaying.db.MetadataDB()
+            metadb.vacuum_database()
+            logging.debug("Metadata database vacuumed successfully")
+        except (sqlite3.Error, OSError) as error:
+            logging.error("Error vacuuming metadata database: %s", error)
+        except Exception as error:  # pylint: disable=broad-exception-caught
+            logging.exception("Unexpected error vacuuming metadata database: %s", error)
+
+        # Vacuum requests database (if available)
+        try:
+            if self.requestswindow:
+                self.requestswindow.vacuum_database()
+                logging.debug("Requests database vacuumed successfully")
+        except (sqlite3.Error, OSError) as error:
+            logging.error("Error vacuuming requests database: %s", error)
+        except Exception as error:  # pylint: disable=broad-exception-caught
+            logging.exception("Unexpected error vacuuming requests database: %s", error)
+
+        logging.debug("Database vacuum operations completed in background thread")
+
+
 class Tray:  # pylint: disable=too-many-instance-attributes
     ''' System Tray object '''
 
@@ -43,14 +87,17 @@ def __init__(self, beam: bool = False) -> None:  #pylint: disable=too-many-state
         self.about_action.setEnabled(True)
         self.about_action.triggered.connect(self.aboutwindow.show)
 
-        # Vacuum databases on startup to reclaim space from previous session
-        self._vacuum_databases_on_startup()
+        # Vacuum databases will be started in background after requestswindow is created
 
         self.subprocesses = nowplaying.subprocesses.SubprocessManager(self.config)
         try:
             self.settingswindow = nowplaying.settingsui.SettingsUI(tray=self, beam=beam)
+        except (FileNotFoundError, ImportError) as error:
+            logging.error("Failed to create settings window - missing UI files: %s", error)
+            self._show_installation_error('settings UI files')
+            return
         except Exception as error:  # pylint: disable=broad-exception-caught
-            logging.error("Failed to create settings window: %s", error)
+            logging.exception("Unexpected error creating settings window: %s", error)
             self._show_installation_error('settings UI files')
             return
 
@@ -99,6 +146,9 @@ def __init__(self, beam: bool = False) -> None:  #pylint: disable=too-many-state
         self.requestswindow = None
         self._configure_twitchrequests()
 
+        # Start database vacuum operations in background thread now that requestswindow is created
+        self._vacuum_databases_on_startup()
+
     def _configure_beamstatus(self, beam: bool) -> None:
         self.config.cparser.setValue('control/beam', beam)
         # these will get filled in by their various subsystems as required
@@ -125,38 +175,16 @@ def _show_installation_error(ui_file: str) -> None:
             app.exit(1)
 
     def _vacuum_databases_on_startup(self) -> None:
-        ''' Vacuum databases on startup to reclaim space from previous session '''
-        logging.debug("Starting database vacuum operations on startup")
+        ''' Start database vacuum operations in background thread '''
+        logging.debug("Starting database vacuum operations in background thread")
 
-        # Vacuum API cache database
-        try:
-            nowplaying.apicache.APIResponseCache.vacuum_database_file()
-            logging.debug("API cache database vacuumed successfully")
-        except Exception as error:  # pylint: disable=broad-exception-caught
-            logging.error("Error vacuuming API cache: %s", error)
-
-        # Vacuum metadata database
-        try:
-            metadb = nowplaying.db.MetadataDB()
-            metadb.vacuum_database()
-            logging.debug("Metadata database vacuumed successfully")
-        except Exception as error:  # pylint: disable=broad-exception-caught
-            logging.error("Error vacuuming metadata database: %s", error)
-
-        # Vacuum requests database (will be created later, so check if available)
-        try:
-            if hasattr(self, 'requestswindow') and self.requestswindow:
-                self.requestswindow.vacuum_database()
-                logging.debug("Requests database vacuumed successfully")
-        except Exception as error:  # pylint: disable=broad-exception-caught
-            logging.error("Error vacuuming requests database: %s", error)
-
-        logging.debug("Database vacuum operations completed")
+        # Create and start vacuum thread
+        self.vacuum_thread = DatabaseVacuumThread(requestswindow=self.requestswindow)
+        self.vacuum_thread.start()
 
     def _requestswindow(self) -> None:
-        if self.config.cparser.value('settings/requests', type=bool):
-            if self.requestswindow:
-                self.requestswindow.raise_window()
+        if self.config.cparser.value('settings/requests', type=bool) and self.requestswindow:
+            self.requestswindow.raise_window()
 
     def _configure_newold_menu(self) -> None:
         self.action_newestmode.setCheckable(True)

pyproject.toml

@@ -89,6 +89,7 @@ testpaths = ["tests-qt", "tests"]
 markers = [
   "seratosettings: custom settings for serato ",
   "templatesettings: custom settings for templates",
+  "performance: performance-sensitive tests that may be flaky in CI",
 ]
 qt_api = "pyside6"
 #log_cli = true