fixes

Author: aw-was-here

conftest.py

@@ -111,7 +111,8 @@ def clear_old_testsuite(request):
 
     # Only create QCoreApplication if test doesn't need QApplication
     if not skip_qt_cleanup:
-        config = QSettings(qsettingsformat, QSettings.SystemScope, QCoreApplication.organizationName(),
+        config = QSettings(qsettingsformat, QSettings.SystemScope,
+                           QCoreApplication.organizationName(),
                            QCoreApplication.applicationName())
         config.clear()
         config.sync()
@@ -121,7 +122,8 @@ def clear_old_testsuite(request):
             logging.info('Removing %s', cachedir)
             shutil.rmtree(cachedir)
 
-        config = QSettings(qsettingsformat, QSettings.UserScope, QCoreApplication.organizationName(),
+        config = QSettings(qsettingsformat, QSettings.UserScope,
+                           QCoreApplication.organizationName(),
                            QCoreApplication.applicationName())
         config.clear()
         config.sync()

tests-qt/test_systemtray.py

@@ -164,12 +164,18 @@ def mock_dependencies():
          patch('nowplaying.settingsui.about_version_text'), \
          patch('nowplaying.trackrequests.Requests', MockTrackrequests), \
          patch('nowplaying.db.MetadataDB', MockMetadataDB), \
-         patch('nowplaying.apicache.APIResponseCache.vacuum_database_file') as mock_api_vacuum:
+         patch('nowplaying.apicache.APIResponseCache.vacuum_database_file') as mock_api_vacuum, \
+         patch('nowplaying.systemtray.DatabaseVacuumThread') as mock_vacuum_thread:
 
         mock_load_ui.return_value = MagicMock()
+        # Configure vacuum thread mock to prevent actual thread creation
+        mock_vacuum_thread_instance = MagicMock()
+        mock_vacuum_thread.return_value = mock_vacuum_thread_instance
         yield {
             'load_ui': mock_load_ui,
             'api_vacuum': mock_api_vacuum,
+            'vacuum_thread': mock_vacuum_thread,
+            'vacuum_thread_instance': mock_vacuum_thread_instance,
         }
 
 
@@ -247,9 +253,9 @@ def test_vacuum_databases_on_startup_method(qtbot, mock_dependencies):  # pylint
     # Verify the vacuum method exists and was called
     assert hasattr(tray, '_vacuum_databases_on_startup')
     assert hasattr(tray, 'vacuum_thread')
-
-    # Vacuum operations run in background thread, so we can't assert on timing
-    # but we can verify the thread was created and started
+    # Verify the vacuum thread was created and started
+    mock_dependencies['vacuum_thread'].assert_called_once_with(requestswindow=tray.requestswindow)
+    mock_dependencies['vacuum_thread_instance'].start.assert_called_once()
 
 
 def test_settings_window_integration(qtbot, mock_dependencies):  # pylint: disable=unused-argument,redefined-outer-name

tests/test_subprocesses.py

@@ -28,23 +28,24 @@ def mock_config():
 
 
 @pytest.fixture
-def subprocess_manager(mock_config):
+def subprocess_manager(mock_config):  # pylint: disable=redefined-outer-name
     ''' Create SubprocessManager for testing '''
     with patch('importlib.import_module'):
         manager = nowplaying.subprocesses.SubprocessManager(config=mock_config, testmode=True)
         # Mock the process modules
-        for name in manager.processes:
-            manager.processes[name]['module'] = MagicMock()
+        for _, process_info in manager.processes.items():
+            process_info['module'] = MagicMock()
         yield manager
 
 
-def test_subprocess_manager_init(mock_config):
+def test_subprocess_manager_init(mock_config):  # pylint: disable=redefined-outer-name
     ''' Test SubprocessManager initialization '''
-    with patch('importlib.import_module') as mock_import:
+    with patch('importlib.import_module'):
         manager = nowplaying.subprocesses.SubprocessManager(config=mock_config, testmode=True)
 
         # Should import all expected modules
-        expected_processes = ['trackpoll', 'obsws', 'twitchbot', 'discordbot', 'webserver', 'kickbot']
+        expected_processes = ['trackpoll', 'obsws', 'twitchbot', 'discordbot',
+                              'webserver', 'kickbot']
         assert len(manager.processes) == len(expected_processes)
 
         for process_name in expected_processes:
@@ -54,7 +55,7 @@ def test_subprocess_manager_init(mock_config):
             assert 'stopevent' in manager.processes[process_name]
 
 
-def test_subprocess_manager_beam_mode(mock_config):
+def test_subprocess_manager_beam_mode(mock_config):  # pylint: disable=redefined-outer-name
     ''' Test SubprocessManager in beam mode '''
     mock_config.cparser.value.side_effect = lambda key, **kwargs: {
         'control/beam': True,
@@ -70,7 +71,7 @@ def test_subprocess_manager_beam_mode(mock_config):
             assert process_name in manager.processes
 
 
-def test_start_process_with_conditions(subprocess_manager):
+def test_start_process_with_conditions(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test conditional process starting '''
     manager = subprocess_manager
 
@@ -82,7 +83,7 @@ def test_start_process_with_conditions(subprocess_manager):
         mock_start.assert_called_once_with('twitchbot')
 
 
-def test_start_process_disabled(subprocess_manager):
+def test_start_process_disabled(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test that disabled processes don't start '''
     manager = subprocess_manager
     manager.config.cparser.value.side_effect = lambda key, **kwargs: {
@@ -94,7 +95,7 @@ def test_start_process_disabled(subprocess_manager):
         mock_start.assert_not_called()
 
 
-class MockProcess:
+class MockProcess:  # pylint: disable=too-many-instance-attributes
     ''' Mock multiprocessing.Process for testing '''
 
     def __init__(self, target=None, name=None, args=None):
@@ -108,9 +109,11 @@ def __init__(self, target=None, name=None, args=None):
         self._closed = False
 
     def start(self):
+        ''' Mock process start '''
         self._started = True
 
     def join(self, timeout=None):
+        ''' Mock process join '''
         # Simulate quick shutdown for most processes
         if self.name in ['webserver', 'obsws']:
             self._alive = False
@@ -119,19 +122,22 @@ def join(self, timeout=None):
             self._alive = False
 
     def terminate(self):
+        ''' Mock process terminate '''
         self._terminated = True
         self._alive = False
 
     def is_alive(self):
+        ''' Mock process is_alive check '''
         return self._alive
 
     def close(self):
+        ''' Mock process close '''
         self._closed = True
 
 
 # Ensure test doesn't hang - using manual timing instead of pytest-timeout
 @pytest.mark.performance
-def test_parallel_shutdown_performance(subprocess_manager):
+def test_parallel_shutdown_performance(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test that parallel shutdown is faster than sequential '''
     manager = subprocess_manager
 
@@ -158,57 +164,57 @@ def test_parallel_shutdown_performance(subprocess_manager):
         assert manager.processes[name]['process'] is None
 
 
-def test_stop_process_parallel_graceful_shutdown(subprocess_manager):
+def test_stop_process_parallel_graceful_shutdown(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test graceful shutdown in parallel method '''
     manager = subprocess_manager
 
     mock_process = MockProcess(name='webserver')
-    mock_process._alive = True  # Start alive
+    mock_process._alive = True  # Start alive  # pylint: disable=protected-access
     manager.processes['webserver']['process'] = mock_process
 
     # Mock join to simulate quick shutdown
     with patch.object(mock_process, 'join') as mock_join:
         mock_join.side_effect = lambda timeout: setattr(mock_process, '_alive', False)
 
-        manager._stop_process_parallel('webserver')
+        manager._stop_process_parallel('webserver')  # pylint: disable=protected-access
 
         # Should call join with 8 second timeout
         mock_join.assert_called_with(8)
         assert manager.processes['webserver']['process'] is None
 
 
-def test_stop_process_parallel_forced_termination(subprocess_manager):
+def test_stop_process_parallel_forced_termination(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test forced termination when graceful shutdown fails '''
     manager = subprocess_manager
 
     mock_process = MockProcess(name='trackpoll')
     # Simulate a stuck process that doesn't respond to graceful shutdown
-    mock_process._alive = True
+    mock_process._alive = True  # pylint: disable=protected-access
     manager.processes['trackpoll']['process'] = mock_process
 
     with patch.object(mock_process, 'join') as mock_join, \
          patch.object(mock_process, 'terminate') as mock_terminate:
 
         # Mock join to keep process alive on first call, then die on second
-        def mock_join_behavior(timeout):
+        def mock_join_behavior(timeout):  # pylint: disable=unused-argument
             if mock_join.call_count == 1:
                 # First call (graceful) - process stays alive
                 pass
             else:
                 # Second call (after terminate) - process dies
-                mock_process._alive = False
+                mock_process._alive = False  # pylint: disable=protected-access
 
         mock_join.side_effect = mock_join_behavior
 
-        manager._stop_process_parallel('trackpoll')
+        manager._stop_process_parallel('trackpoll')  # pylint: disable=protected-access
 
         # Should try graceful first, then terminate
         assert mock_join.call_count == 2
         mock_terminate.assert_called_once()
         assert manager.processes['trackpoll']['process'] is None
 
 
-def test_stop_process_parallel_twitchbot_special_handling(subprocess_manager):
+def test_stop_process_parallel_twitchbot_special_handling(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test special handling for twitchbot process '''
     manager = subprocess_manager
 
@@ -222,13 +228,13 @@ def test_stop_process_parallel_twitchbot_special_handling(subprocess_manager):
     with patch.object(mock_process, 'join') as mock_join:
         mock_join.side_effect = lambda timeout: setattr(mock_process, '_alive', False)
 
-        manager._stop_process_parallel('twitchbot')
+        manager._stop_process_parallel('twitchbot')  # pylint: disable=protected-access
 
         # Should call the special stop function with PID
         mock_stop_func.assert_called_once_with(mock_process.pid)
 
 
-def test_stop_process_parallel_error_handling(subprocess_manager):
+def test_stop_process_parallel_error_handling(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test error handling during parallel shutdown '''
     manager = subprocess_manager
 
@@ -242,13 +248,13 @@ def test_stop_process_parallel_error_handling(subprocess_manager):
         mock_join.side_effect = lambda timeout: setattr(mock_process, '_alive', False)
 
         # Should not raise exception, just log and continue
-        manager._stop_process_parallel('webserver')
+        manager._stop_process_parallel('webserver')  # pylint: disable=protected-access
 
         # Process should still be cleaned up
         assert manager.processes['webserver']['process'] is None
 
 
-def test_stop_all_processes_with_timeout(subprocess_manager):
+def test_stop_all_processes_with_timeout(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test that stop_all_processes handles timeouts gracefully '''
     manager = subprocess_manager
 
@@ -270,7 +276,7 @@ def test_stop_all_processes_with_timeout(subprocess_manager):
         assert manager.processes[name]['process'] is None
 
 
-def test_stop_all_processes_with_no_running_processes(subprocess_manager):
+def test_stop_all_processes_with_no_running_processes(subprocess_manager):  # pylint: disable=redefined-outer-name
     '''Test that stop_all_processes does not raise errors when all process slots are None'''
     manager = subprocess_manager
 
@@ -286,7 +292,7 @@ def test_stop_all_processes_with_no_running_processes(subprocess_manager):
         assert manager.processes[name]['process'] is None
 
 
-def test_stop_all_processes_signals_all_first(subprocess_manager):
+def test_stop_all_processes_signals_all_first(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test that stop_all_processes signals all processes before joining '''
     manager = subprocess_manager
 
@@ -299,7 +305,10 @@ def test_stop_all_processes_signals_all_first(subprocess_manager):
 
         # Track when stopevent.set() is called
         original_set = manager.processes[name]['stopevent'].set
-        manager.processes[name]['stopevent'].set = lambda n=name: signal_order.append(n) or original_set()
+        # Create closure to capture name properly
+        def make_set_wrapper(process_name, orig_set):
+            return lambda: signal_order.append(process_name) or orig_set()
+        manager.processes[name]['stopevent'].set = make_set_wrapper(name, original_set)
 
     manager.stop_all_processes()
 
@@ -310,7 +319,7 @@ def test_stop_all_processes_signals_all_first(subprocess_manager):
         assert expected in signal_order
 
 
-def test_legacy_methods(subprocess_manager):
+def test_legacy_methods(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test that legacy methods still work '''
     manager = subprocess_manager
 
@@ -343,7 +352,7 @@ def test_legacy_methods(subprocess_manager):
 
 
 @pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific timeout test")
-def test_windows_process_termination_timeout(subprocess_manager):
+def test_windows_process_termination_timeout(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test that Windows gets longer termination timeouts '''
     manager = subprocess_manager
 
@@ -359,7 +368,7 @@ def join(self, timeout=None):
             if self.join_call_count == 1:
                 return  # Process stays alive
             # Second call (after terminate) - now it dies with Windows timeout
-            elif self.join_call_count == 2 and timeout and timeout >= 7:
+            if self.join_call_count == 2 and timeout and timeout >= 7:
                 self._alive = False
 
     mock_process = WindowsSlowProcess(name='trackpoll')
@@ -368,7 +377,7 @@ def join(self, timeout=None):
     with patch.object(mock_process, 'join', wraps=mock_process.join) as mock_join, \
          patch.object(mock_process, 'terminate') as mock_terminate:
 
-        manager._stop_process_parallel('trackpoll')
+        manager._stop_process_parallel('trackpoll')  # pylint: disable=protected-access
 
         # Should call join twice: first for graceful (8s), then for force (7s)
         assert mock_join.call_count == 2
@@ -378,7 +387,7 @@ def join(self, timeout=None):
         mock_join.assert_any_call(7)
 
 
-def test_cross_platform_timeout_behavior(subprocess_manager):
+def test_cross_platform_timeout_behavior(subprocess_manager):  # pylint: disable=redefined-outer-name
     ''' Test timeout behavior across platforms '''
     manager = subprocess_manager
 
@@ -388,10 +397,10 @@ def test_cross_platform_timeout_behavior(subprocess_manager):
     with patch.object(mock_process, 'join') as mock_join:
         mock_join.side_effect = lambda timeout: setattr(mock_process, '_alive', False)
 
-        manager._stop_process_parallel('webserver')
+        manager._stop_process_parallel('webserver')  # pylint: disable=protected-access
 
         # Should use 8 second graceful timeout on all platforms
         mock_join.assert_any_call(8)
 
         # If termination was needed, should use 7 second timeout
-        # (increased from 5 to accommodate Windows)
+        # (increased from 5 to accommodate Windows)