fix(test): repair test infrastructure and mock fixtures (#281)

* fix(test): repair test infrastructure and mock fixtures

- Add test/__init__.py for proper test collection
- Fix ConfigManager instantiation to use config_path parameter
- Route schedule config through config_service mock
- Update mock to match get_raw_file_content endpoint change

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(test): correct get_main_config assertion per CodeRabbit review

The endpoint calls load_config(), not get_raw_file_content('main').
Also set up load_config mock return value in the fixture so the
test's data assertions pass correctly.

Co-Authored-By: 5ymb01 <noreply@github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(test): correct plugin config test mock structure and schema returns

- Plugin configs live at top-level keys, not under 'plugins' subkey
- Mock schema_manager.generate_default_config to return a dict
- Mock schema_manager.merge_with_defaults to merge dicts (not MagicMock)
- Fixes test_get_plugin_config returning 500 due to non-serializable MagicMock

Co-Authored-By: 5ymb01 <noreply@github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(test): use patch.object for config_service.get_config in schedule tests

config_service.get_config is a real method, not a mock — can't set
return_value on it directly. Use patch.object context manager instead.

Co-Authored-By: 5ymb01 <noreply@github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: 5ymb01 <5ymb01@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: 5ymb01 <noreply@github.com>

src/base_classes/baseball.py

@@ -329,7 +329,7 @@ class Baseball(SportsCore):
             return
         
         series_summary = game.get("series_summary", "")
-        font = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)
+        font = self.fonts.get('detail', ImageFont.load_default())
         bbox = draw_overlay.textbbox((0, 0), series_summary, font=self.fonts['time'])
         height = bbox[3] - bbox[1]
         shots_y = (self.display_height - height) // 2

src/base_classes/basketball.py

@@ -201,14 +201,7 @@ class BasketballLive(Basketball, SportsLive):
 
             # Draw records or rankings if enabled
             if self.show_records or self.show_ranking:
-                try:
-                    record_font = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)
-                    self.logger.debug(f"Loaded 6px record font successfully")
-                except IOError:
-                    record_font = ImageFont.load_default()
-                    self.logger.warning(
-                        f"Failed to load 6px font, using default font (size: {record_font.size})"
-                    )
+                record_font = self.fonts.get('detail', ImageFont.load_default())
 
                 # Get team abbreviations
                 away_abbr = game.get("away_abbr", "")

src/base_classes/football.py

@@ -308,13 +308,8 @@ class FootballLive(Football, SportsLive):
 
             # Draw records or rankings if enabled
             if self.show_records or self.show_ranking:
-                try:
-                    record_font = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)
-                    self.logger.debug(f"Loaded 6px record font successfully")
-                except IOError:
-                    record_font = ImageFont.load_default()
-                    self.logger.warning(f"Failed to load 6px font, using default font (size: {record_font.size})")
-                
+                record_font = self.fonts.get('detail', ImageFont.load_default())
+
                 # Get team abbreviations
                 away_abbr = game.get('away_abbr', '')
                 home_abbr = game.get('home_abbr', '')

src/base_classes/hockey.py

@@ -255,7 +255,7 @@ class HockeyLive(Hockey, SportsLive):
 
             # Shots on Goal
             if self.show_shots_on_goal:
-                shots_font = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)
+                shots_font = self.fonts.get('detail', ImageFont.load_default())
                 home_shots = str(game.get("home_shots", "0"))
                 away_shots = str(game.get("away_shots", "0"))
                 shots_text = f"{away_shots}   SHOTS   {home_shots}"
@@ -276,14 +276,7 @@ class HockeyLive(Hockey, SportsLive):
 
             # Draw records or rankings if enabled
             if self.show_records or self.show_ranking:
-                try:
-                    record_font = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)
-                    self.logger.debug(f"Loaded 6px record font successfully")
-                except IOError:
-                    record_font = ImageFont.load_default()
-                    self.logger.warning(
-                        f"Failed to load 6px font, using default font (size: {record_font.size})"
-                    )
+                record_font = self.fonts.get('detail', ImageFont.load_default())
 
                 # Get team abbreviations
                 away_abbr = game.get("away_abbr", "")

src/base_classes/sports.py

@@ -863,13 +863,8 @@ class SportsUpcoming(SportsCore):
 
             # Draw records or rankings if enabled
             if self.show_records or self.show_ranking:
-                try:
-                    record_font = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)
-                    self.logger.debug(f"Loaded 6px record font successfully")
-                except IOError:
-                    record_font = ImageFont.load_default()
-                    self.logger.warning(f"Failed to load 6px font, using default font (size: {record_font.size})")
-                
+                record_font = self.fonts.get('detail', ImageFont.load_default())
+
                 # Get team abbreviations
                 away_abbr = game.get('away_abbr', '')
                 home_abbr = game.get('home_abbr', '')
@@ -1172,13 +1167,8 @@ class SportsRecent(SportsCore):
 
             # Draw records or rankings if enabled
             if self.show_records or self.show_ranking:
-                try:
-                    record_font = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6)
-                    self.logger.debug(f"Loaded 6px record font successfully")
-                except IOError:
-                    record_font = ImageFont.load_default()
-                    self.logger.warning(f"Failed to load 6px font, using default font (size: {record_font.size})")
-                
+                record_font = self.fonts.get('detail', ImageFont.load_default())
+
                 # Get team abbreviations
                 away_abbr = game.get('away_abbr', '')
                 home_abbr = game.get('home_abbr', '')

src/layout_manager.py

@@ -238,7 +238,7 @@ class LayoutManager:
         # Format the text
         try:
             text = format_str.format(value=value)
-        except:
+        except (ValueError, TypeError, KeyError, IndexError):
             text = str(value)
         
         self.display_manager.draw_text(text, x, y, color)

src/logo_downloader.py

@@ -237,7 +237,7 @@ class LogoDownloader:
                 logger.error(f"Downloaded file for {team_abbreviation} is not a valid image or conversion failed: {e}")
                 try:
                     os.remove(filepath)  # Remove invalid file
-                except:
+                except OSError:
                     pass
                 return False
             
@@ -642,10 +642,10 @@ class LogoDownloader:
             # Try to load a font, fallback to default
             try:
                 font = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 12)
-            except:
+            except (OSError, IOError):
                 try:
                     font = ImageFont.load_default()
-                except:
+                except (OSError, IOError):
                     font = None
             
             # Draw team abbreviation

test/__init__.py

@@ -0,0 +1 @@
+# Test package

test/test_config_validation_edge_cases.py

@@ -23,6 +23,7 @@ if str(project_root) not in sys.path:
     sys.path.insert(0, str(project_root))
 
 from src.config_manager import ConfigManager
+from src.exceptions import ConfigError
 from src.plugin_system.schema_manager import SchemaManager
 
 
@@ -30,35 +31,31 @@ class TestInvalidJson:
     """Test handling of invalid JSON in config files."""
 
     def test_invalid_json_syntax(self, tmp_path):
-        """Config with invalid JSON syntax should be handled gracefully."""
+        """Config with invalid JSON syntax should raise ConfigError."""
         config_file = tmp_path / "config.json"
         config_file.write_text("{ invalid json }")
 
-        with patch.object(ConfigManager, '_get_config_path', return_value=str(config_file)):
-            config_manager = ConfigManager(config_dir=str(tmp_path))
-            # Should not raise, should return empty or default config
-            config = config_manager.load_config()
-            assert isinstance(config, dict)
+        config_manager = ConfigManager(config_path=str(config_file))
+        with pytest.raises(ConfigError):
+            config_manager.load_config()
 
     def test_truncated_json(self, tmp_path):
-        """Config with truncated JSON should be handled gracefully."""
+        """Config with truncated JSON should raise ConfigError."""
         config_file = tmp_path / "config.json"
         config_file.write_text('{"plugin": {"enabled": true')  # Missing closing braces
 
-        with patch.object(ConfigManager, '_get_config_path', return_value=str(config_file)):
-            config_manager = ConfigManager(config_dir=str(tmp_path))
-            config = config_manager.load_config()
-            assert isinstance(config, dict)
+        config_manager = ConfigManager(config_path=str(config_file))
+        with pytest.raises(ConfigError):
+            config_manager.load_config()
 
     def test_empty_config_file(self, tmp_path):
-        """Empty config file should be handled gracefully."""
+        """Empty config file should raise ConfigError."""
         config_file = tmp_path / "config.json"
         config_file.write_text("")
 
-        with patch.object(ConfigManager, '_get_config_path', return_value=str(config_file)):
-            config_manager = ConfigManager(config_dir=str(tmp_path))
-            config = config_manager.load_config()
-            assert isinstance(config, dict)
+        config_manager = ConfigManager(config_path=str(config_file))
+        with pytest.raises(ConfigError):
+            config_manager.load_config()
 
 
 class TestTypeValidation:

test/test_display_controller.py

@@ -209,49 +209,47 @@ class TestDisplayControllerSchedule:
     def test_schedule_disabled(self, test_display_controller):
         """Test when schedule is disabled."""
         controller = test_display_controller
-        controller.config = {"schedule": {"enabled": False}}
-        
-        controller._check_schedule()
-        assert controller.is_display_active is True
-        
+        schedule_config = {"schedule": {"enabled": False}}
+        with patch.object(controller.config_service, 'get_config', return_value=schedule_config):
+            controller._check_schedule()
+            assert controller.is_display_active is True
+
     def test_active_hours(self, test_display_controller):
         """Test active hours check."""
         controller = test_display_controller
-        # Mock datetime to be within active hours
         with patch('src.display_controller.datetime') as mock_datetime:
             mock_datetime.now.return_value.strftime.return_value.lower.return_value = "monday"
             mock_datetime.now.return_value.time.return_value = datetime.strptime("12:00", "%H:%M").time()
             mock_datetime.strptime = datetime.strptime
-            
-            controller.config = {
+
+            schedule_config = {
                 "schedule": {
                     "enabled": True,
                     "start_time": "09:00",
                     "end_time": "17:00"
                 }
             }
-            
-            controller._check_schedule()
-            assert controller.is_display_active is True
+            with patch.object(controller.config_service, 'get_config', return_value=schedule_config):
+                controller._check_schedule()
+                assert controller.is_display_active is True
 
     def test_inactive_hours(self, test_display_controller):
         """Test inactive hours check."""
         controller = test_display_controller
-        # Mock datetime to be outside active hours
         with patch('src.display_controller.datetime') as mock_datetime:
             mock_datetime.now.return_value.strftime.return_value.lower.return_value = "monday"
             mock_datetime.now.return_value.time.return_value = datetime.strptime("20:00", "%H:%M").time()
             mock_datetime.strptime = datetime.strptime
-            
-            controller.config = {
+
+            schedule_config = {
                 "schedule": {
                     "enabled": True,
                     "start_time": "09:00",
                     "end_time": "17:00"
                 }
             }
-            
-            controller._check_schedule()
-            assert controller.is_display_active is False
+            with patch.object(controller.config_service, 'get_config', return_value=schedule_config):
+                controller._check_schedule()
+                assert controller.is_display_active is False
             
 from datetime import datetime

test/test_web_api.py

@@ -29,7 +29,13 @@ def mock_config_manager():
     }
     mock.get_config_path.return_value = 'config/config.json'
     mock.get_secrets_path.return_value = 'config/config_secrets.json'
-    mock.get_raw_file_content.return_value = {'weather': {'api_key': 'test'}}
+    mock_config = {
+        'display': {'brightness': 50},
+        'plugins': {},
+        'timezone': 'UTC'
+    }
+    mock.load_config.return_value = mock_config
+    mock.get_raw_file_content.return_value = mock_config
     mock.save_config_atomic.return_value = MagicMock(
         status=MagicMock(value='success'),
         message=None
@@ -385,17 +391,21 @@ class TestPluginsAPI:
     
     def test_get_plugin_config(self, client, mock_config_manager):
         """Test getting plugin configuration."""
+        # Plugin configs live at top-level keys (not under 'plugins')
         mock_config_manager.load_config.return_value = {
-            'plugins': {
-                'weather': {
-                    'enabled': True,
-                    'api_key': 'test_key'
-                }
+            'weather': {
+                'enabled': True,
+                'api_key': 'test_key'
             }
         }
-        
+
+        # Ensure schema manager returns serializable values
+        from web_interface.blueprints.api_v3 import api_v3
+        api_v3.schema_manager.generate_default_config.return_value = {'enabled': False}
+        api_v3.schema_manager.merge_with_defaults.side_effect = lambda config, defaults: {**defaults, **config}
+
         response = client.get('/api/v3/plugins/config?plugin_id=weather')
-        
+
         assert response.status_code == 200
         data = json.loads(response.data)
         assert 'enabled' in data or 'config' in data or 'data' in data

web_interface/app.py

@@ -442,7 +442,7 @@ def system_status_generator():
                 try:
                     with open('/sys/class/thermal/thermal_zone0/temp', 'r') as f:
                         cpu_temp = round(float(f.read()) / 1000.0, 1)
-                except:
+                except (OSError, ValueError):
                     pass
                     
             except ImportError:
@@ -456,7 +456,7 @@ def system_status_generator():
                 result = subprocess.run(['systemctl', 'is-active', 'ledmatrix'], 
                                       capture_output=True, text=True, timeout=2)
                 service_active = result.stdout.strip() == 'active'
-            except:
+            except (subprocess.SubprocessError, OSError):
                 pass
             
             status = {
@@ -492,7 +492,7 @@ def display_preview_generator():
         parallel = main_config.get('display', {}).get('hardware', {}).get('parallel', 1)
         width = cols * chain_length
         height = rows * parallel
-    except:
+    except (KeyError, TypeError, ValueError):
         width = 128
         height = 64
     

web_interface/blueprints/api_v3.py

@@ -406,14 +406,11 @@ def save_schedule_config():
 
         return success_response(message='Schedule configuration saved successfully')
     except Exception as e:
-        import logging
-        import traceback
-        error_msg = f"Error saving schedule config: {str(e)}\n{traceback.format_exc()}"
-        logging.error(error_msg)
+        logger.exception("[ScheduleConfig] Failed to save schedule configuration")
         return error_response(
             ErrorCode.CONFIG_SAVE_FAILED,
-            f"Error saving schedule configuration: {str(e)}",
-            details=traceback.format_exc(),
+            "Error saving schedule configuration",
+            details="Internal server error - check server logs",
             status_code=500
         )
 
@@ -627,14 +624,11 @@ def save_dim_schedule_config():
 
         return success_response(message='Dim schedule configuration saved successfully')
     except Exception as e:
-        import logging
-        import traceback
-        error_msg = f"Error saving dim schedule config: {str(e)}\n{traceback.format_exc()}"
-        logging.error(error_msg)
+        logger.exception("[DimScheduleConfig] Failed to save dim schedule configuration")
         return error_response(
             ErrorCode.CONFIG_SAVE_FAILED,
-            f"Error saving dim schedule configuration: {str(e)}",
-            details=traceback.format_exc(),
+            "Error saving dim schedule configuration",
+            details="Internal server error - check server logs",
             status_code=500
         )
 
@@ -978,14 +972,11 @@ def save_main_config():
 
         return success_response(message='Configuration saved successfully')
     except Exception as e:
-        import logging
-        import traceback
-        error_msg = f"Error saving config: {str(e)}\n{traceback.format_exc()}"
-        logging.error(error_msg)
+        logger.exception("[Config] Failed to save configuration")
         return error_response(
             ErrorCode.CONFIG_SAVE_FAILED,
-            f"Error saving configuration: {e}",
-            details=traceback.format_exc(),
+            "Error saving configuration",
+            details="Internal server error - check server logs",
             status_code=500
         )
 
@@ -1021,32 +1012,20 @@ def save_raw_main_config():
     except json.JSONDecodeError as e:
         return jsonify({'status': 'error', 'message': f'Invalid JSON: {str(e)}'}), 400
     except Exception as e:
-        import logging
-        import traceback
         from src.exceptions import ConfigError
-
-        # Log the full error for debugging
-        error_msg = f"Error saving raw main config: {str(e)}\n{traceback.format_exc()}"
-        logging.error(error_msg)
-
-        # Extract more specific error message if it's a ConfigError
+        logger.exception("[RawConfig] Failed to save raw main config")
         if isinstance(e, ConfigError):
-            error_message = str(e)
-            if hasattr(e, 'config_path') and e.config_path:
-                error_message = f"{error_message} (config_path: {e.config_path})"
             return error_response(
                 ErrorCode.CONFIG_SAVE_FAILED,
-                error_message,
-                details=traceback.format_exc(),
-                context={'config_path': e.config_path} if hasattr(e, 'config_path') and e.config_path else None,
+                "Error saving raw main configuration",
+                details="Internal server error - check server logs",
                 status_code=500
             )
         else:
-            error_message = str(e) if str(e) else "An unexpected error occurred while saving the configuration"
             return error_response(
                 ErrorCode.UNKNOWN_ERROR,
-                error_message,
-                details=traceback.format_exc(),
+                "An unexpected error occurred while saving the configuration",
+                details="Internal server error - check server logs",
                 status_code=500
             )
 
@@ -1072,24 +1051,22 @@ def save_raw_secrets_config():
     except json.JSONDecodeError as e:
         return jsonify({'status': 'error', 'message': f'Invalid JSON: {str(e)}'}), 400
     except Exception as e:
-        import logging
-        import traceback
         from src.exceptions import ConfigError
-
-        # Log the full error for debugging
-        error_msg = f"Error saving raw secrets config: {str(e)}\n{traceback.format_exc()}"
-        logging.error(error_msg)
-
-        # Extract more specific error message if it's a ConfigError
+        logger.exception("[RawSecrets] Failed to save raw secrets config")
         if isinstance(e, ConfigError):
-            # ConfigError has a message attribute and may have context
-            error_message = str(e)
-            if hasattr(e, 'config_path') and e.config_path:
-                error_message = f"{error_message} (config_path: {e.config_path})"
+            return error_response(
+                ErrorCode.CONFIG_SAVE_FAILED,
+                "Error saving raw secrets configuration",
+                details="Internal server error - check server logs",
+                status_code=500
+            )
         else:
-            error_message = str(e) if str(e) else "An unexpected error occurred while saving the configuration"
-
-        return jsonify({'status': 'error', 'message': error_message}), 500
+            return error_response(
+                ErrorCode.UNKNOWN_ERROR,
+                "An unexpected error occurred while saving the configuration",
+                details="Internal server error - check server logs",
+                status_code=500
+            )
 
 @api_v3.route('/system/status', methods=['GET'])
 def get_system_status():
@@ -1470,10 +1447,8 @@ def execute_system_action():
 
     except Exception as e:
         import traceback
-        error_details = traceback.format_exc()
-        print(f"Error in execute_system_action: {str(e)}")
-        print(error_details)
-        return jsonify({'status': 'error', 'message': str(e), 'details': error_details}), 500
+        logger.exception("[SystemAction] Unexpected error")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 @api_v3.route('/display/current', methods=['GET'])
 def get_display_current():
@@ -1882,10 +1857,8 @@ def get_installed_plugins():
         return jsonify({'status': 'success', 'data': {'plugins': plugins}})
     except Exception as e:
         import traceback
-        error_details = traceback.format_exc()
-        print(f"Error in get_installed_plugins: {str(e)}")
-        print(error_details)
-        return jsonify({'status': 'error', 'message': str(e), 'details': error_details}), 500
+        logger.exception("[Plugins] Error listing installed plugins")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 @api_v3.route('/plugins/health', methods=['GET'])
 def get_plugin_health():
@@ -6068,8 +6041,8 @@ def upload_plugin_asset():
         })
 
     except Exception as e:
-        import traceback
-        return jsonify({'status': 'error', 'message': str(e), 'traceback': traceback.format_exc()}), 500
+        logger.exception("[API] Unexpected error")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 @api_v3.route('/plugins/of-the-day/json/upload', methods=['POST'])
 def upload_of_the_day_json():
@@ -6218,8 +6191,8 @@ def upload_of_the_day_json():
         })
 
     except Exception as e:
-        import traceback
-        return jsonify({'status': 'error', 'message': str(e), 'traceback': traceback.format_exc()}), 500
+        logger.exception("[API] Unexpected error")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 @api_v3.route('/plugins/of-the-day/json/delete', methods=['POST'])
 def delete_of_the_day_json():
@@ -6265,8 +6238,8 @@ def delete_of_the_day_json():
         })
 
     except Exception as e:
-        import traceback
-        return jsonify({'status': 'error', 'message': str(e), 'traceback': traceback.format_exc()}), 500
+        logger.exception("[API] Unexpected error")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 @api_v3.route('/plugins/<plugin_id>/static/<path:file_path>', methods=['GET'])
 def serve_plugin_static(plugin_id, file_path):
@@ -6286,7 +6259,9 @@ def serve_plugin_static(plugin_id, file_path):
         requested_file = (plugin_dir / file_path).resolve()
 
         # Security check: ensure file is within plugin directory
-        if not str(requested_file).startswith(str(plugin_dir)):
+        try:
+            requested_file.relative_to(plugin_dir)
+        except ValueError:
             return jsonify({'status': 'error', 'message': 'Invalid file path'}), 403
 
         # Check if file exists
@@ -6311,8 +6286,8 @@ def serve_plugin_static(plugin_id, file_path):
         return Response(content, mimetype=content_type)
 
     except Exception as e:
-        import traceback
-        return jsonify({'status': 'error', 'message': str(e), 'traceback': traceback.format_exc()}), 500
+        logger.exception("[API] Unexpected error")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 
 @api_v3.route('/plugins/calendar/upload-credentials', methods=['POST'])
@@ -6483,8 +6458,8 @@ def delete_plugin_asset():
         return jsonify({'status': 'success', 'message': 'Image deleted successfully'})
 
     except Exception as e:
-        import traceback
-        return jsonify({'status': 'error', 'message': str(e), 'traceback': traceback.format_exc()}), 500
+        logger.exception("[API] Unexpected error")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 @api_v3.route('/plugins/assets/list', methods=['GET'])
 def list_plugin_assets():
@@ -6511,8 +6486,8 @@ def list_plugin_assets():
         return jsonify({'status': 'success', 'data': {'assets': assets}})
 
     except Exception as e:
-        import traceback
-        return jsonify({'status': 'error', 'message': str(e), 'traceback': traceback.format_exc()}), 500
+        logger.exception("[API] Unexpected error")
+        return jsonify({'status': 'error', 'message': 'Internal server error - check server logs'}), 500
 
 @api_v3.route('/logs', methods=['GET'])
 def get_logs():

web_interface/static/v3/js/plugins/api_client.js

@@ -329,7 +329,7 @@ const PluginAPI = {
      * @returns {Promise<Array>} List of available plugins
      */
     async getPluginStore() {
-        const response = await this.request('/plugins/store');
+        const response = await this.request('/plugins/store/list');
         return response.data || [];
     },