diff --git a/tests/activities/test_activities.py b/tests/activities/test_activities.py index 82481c4..d9850d3 100644 --- a/tests/activities/test_activities.py +++ b/tests/activities/test_activities.py @@ -142,3 +142,394 @@ async def test_shutdown(_mock_mlflow_init, mock_experiment_tracking_init): await activities.shutdown() mock_experiment_tracking_init.close.assert_called_once() + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__') +@patch('model_manager.activities.activities.MLFlow.__init__') +@patch('model_manager.activities.activities.MinIO.__init__') +@patch('model_manager.activities.activities.Training.__init__') +def test___del___with_engine( + mock_training_init, + mock_minio_init, + mock_mlflow_init, + mock_experiment_tracking_init, +): + """Test __del__ calls parent destructor when engine attribute exists.""" + postgres_config = { + 'host': 'localhost', + 'port': 5432, + 'user': 'postgres', + 'password': 'postgres', + 'dbname': 'postgres', + 'min_connections': 1, + 'max_connections': 10, + } + + mlflow_config = {'host': 'localhost', 'port': 5000, 'username': 'mlflow', 'password': 'mlflow'} + + minio_config = { + 'endpoint_url': 'http://localhost:9000', + 'access_key': 'minioadmin', + 'secret_key': 'minioadmin', + 'region': 'us-east-1', + 'use_ssl': False, + 'max_retry_attempts': 3, + 'retry_mode': 'adaptive', + 'connect_timeout': 10, + 'read_timeout': 60, + } + + logger = MagicMock() + notification_handler = MagicMock() + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=logger, + notification_handler=notification_handler, + ) + + # Add engine attribute to simulate Postgres initialization + activities.engine = MagicMock() + + # Create a mock __del__ that will be detected by hasattr + mock_parent_del = MagicMock() + + # Patch both the class and the instance to ensure super().__del__ exists and is callable + with patch.object(ExperimentTracking, '__del__', mock_parent_del, create=True): + # Trigger __del__ + activities.__del__() + + # Verify parent __del__ was called + mock_parent_del.assert_called_once() + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__') +@patch('model_manager.activities.activities.MLFlow.__init__') +@patch('model_manager.activities.activities.MinIO.__init__') +@patch('model_manager.activities.activities.Training.__init__') +def test___del___without_engine( + mock_training_init, + mock_minio_init, + mock_mlflow_init, + mock_experiment_tracking_init, +): + """Test __del__ does not call parent destructor when engine attribute is missing.""" + postgres_config = { + 'host': 'localhost', + 'port': 5432, + 'user': 'postgres', + 'password': 'postgres', + 'dbname': 'postgres', + 'min_connections': 1, + 'max_connections': 10, + } + + mlflow_config = {'host': 'localhost', 'port': 5000, 'username': 'mlflow', 'password': 'mlflow'} + + minio_config = { + 'endpoint_url': 'http://localhost:9000', + 'access_key': 'minioadmin', + 'secret_key': 'minioadmin', + 'region': 'us-east-1', + 'use_ssl': False, + 'max_retry_attempts': 3, + 'retry_mode': 'adaptive', + 'connect_timeout': 10, + 'read_timeout': 60, + } + + logger = MagicMock() + notification_handler = MagicMock() + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=logger, + notification_handler=notification_handler, + ) + + # Ensure engine attribute does NOT exist + if hasattr(activities, 'engine'): + delattr(activities, 'engine') + + # Mock super().__del__ to track if it's called + with patch.object(ExperimentTracking, '__del__', MagicMock()) as mock_parent_del: + # Trigger __del__ + activities.__del__() + + # Verify parent __del__ was NOT called + mock_parent_del.assert_not_called() + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__') +@patch('model_manager.activities.activities.MLFlow.__init__') +@patch('model_manager.activities.activities.MinIO.__init__') +@patch('model_manager.activities.activities.Training.__init__') +def test___del___handles_exception_gracefully( + mock_training_init, + mock_minio_init, + mock_mlflow_init, + mock_experiment_tracking_init, +): + """Test __del__ handles exceptions from parent destructor gracefully.""" + postgres_config = { + 'host': 'localhost', + 'port': 5432, + 'user': 'postgres', + 'password': 'postgres', + 'dbname': 'postgres', + 'min_connections': 1, + 'max_connections': 10, + } + + mlflow_config = {'host': 'localhost', 'port': 5000, 'username': 'mlflow', 'password': 'mlflow'} + + minio_config = { + 'endpoint_url': 'http://localhost:9000', + 'access_key': 'minioadmin', + 'secret_key': 'minioadmin', + 'region': 'us-east-1', + 'use_ssl': False, + 'max_retry_attempts': 3, + 'retry_mode': 'adaptive', + 'connect_timeout': 10, + 'read_timeout': 60, + } + + logger = MagicMock() + notification_handler = MagicMock() + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=logger, + notification_handler=notification_handler, + ) + + # Add engine attribute + activities.engine = MagicMock() + + # Mock super().__del__ to raise an exception + mock_parent_del = MagicMock(side_effect=RuntimeError('Cleanup failed')) + + with patch.object(ExperimentTracking, '__del__', mock_parent_del): + # Trigger __del__ - should not raise exception + try: + activities.__del__() + # Test passes if no exception is raised + except Exception as e: + # Test fails if exception propagates + raise AssertionError(f'__del__ should not raise exception, but raised: {e}') from e + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__') +@patch('model_manager.activities.activities.MLFlow.__init__') +@patch('model_manager.activities.activities.MinIO.__init__') +@patch('model_manager.activities.activities.Training.__init__') +def test___del___when_parent_has_no_del( + mock_training_init, + mock_minio_init, + mock_mlflow_init, + mock_experiment_tracking_init, +): + """Test __del__ handles case when parent class has no __del__ method.""" + postgres_config = { + 'host': 'localhost', + 'port': 5432, + 'user': 'postgres', + 'password': 'postgres', + 'dbname': 'postgres', + 'min_connections': 1, + 'max_connections': 10, + } + + mlflow_config = {'host': 'localhost', 'port': 5000, 'username': 'mlflow', 'password': 'mlflow'} + + minio_config = { + 'endpoint_url': 'http://localhost:9000', + 'access_key': 'minioadmin', + 'secret_key': 'minioadmin', + 'region': 'us-east-1', + 'use_ssl': False, + 'max_retry_attempts': 3, + 'retry_mode': 'adaptive', + 'connect_timeout': 10, + 'read_timeout': 60, + } + + logger = MagicMock() + notification_handler = MagicMock() + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=logger, + notification_handler=notification_handler, + ) + + # Add engine attribute + activities.engine = MagicMock() + + # Remove __del__ from parent to simulate it not existing + with patch.object(ExperimentTracking, '__del__', create=False): + # Trigger __del__ - should not raise exception + try: + activities.__del__() + # Test passes if no exception is raised + except Exception as e: + # Test fails if exception propagates + raise AssertionError( + f'__del__ should handle missing parent __del__, but raised: {e}' + ) from e + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__') +@patch('model_manager.activities.activities.MLFlow.__init__') +@patch('model_manager.activities.activities.MinIO.__init__') +@patch('model_manager.activities.activities.Training.__init__') +def test___del___calls_super_successfully( + mock_training_init, + mock_minio_init, + mock_mlflow_init, + mock_experiment_tracking_init, +): + """Test __del__ successfully calls super().__del__() when it exists - covers line 118.""" + postgres_config = { + 'host': 'localhost', + 'port': 5432, + 'user': 'postgres', + 'password': 'postgres', + 'dbname': 'postgres', + 'min_connections': 1, + 'max_connections': 10, + } + + mlflow_config = {'host': 'localhost', 'port': 5000, 'username': 'mlflow', 'password': 'mlflow'} + + minio_config = { + 'endpoint_url': 'http://localhost:9000', + 'access_key': 'minioadmin', + 'secret_key': 'minioadmin', + 'region': 'us-east-1', + 'use_ssl': False, + 'max_retry_attempts': 3, + 'retry_mode': 'adaptive', + 'connect_timeout': 10, + 'read_timeout': 60, + } + + logger = MagicMock() + notification_handler = MagicMock() + + # Mock all parent __init__ methods to return None + mock_experiment_tracking_init.return_value = None + mock_mlflow_init.return_value = None + mock_minio_init.return_value = None + mock_training_init.return_value = None + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=logger, + notification_handler=notification_handler, + ) + + # Add engine attribute to simulate Postgres initialization + activities.engine = MagicMock() + + # Track if super().__del__() was actually called + super_del_called = [] + + def mock_super_del(self): + """Mock parent __del__ that tracks when it's called.""" + super_del_called.append(True) + + # Patch ExperimentTracking.__del__ to exist and be callable + with patch.object(ExperimentTracking, '__del__', mock_super_del, create=True): + # Trigger __del__ - this should execute line 118: super().__del__() + activities.__del__() + + # Verify that super().__del__() was actually called (line 118 executed) + assert len(super_del_called) == 1, 'super().__del__() should have been called once' + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__') +@patch('model_manager.activities.activities.MLFlow.__init__') +@patch('model_manager.activities.activities.MinIO.__init__') +@patch('model_manager.activities.activities.Training.__init__') +def test___del___when_super_has_no_del_method( + mock_training_init, + mock_minio_init, + mock_mlflow_init, + mock_experiment_tracking_init, +): + """Test __del__ handles case when hasattr(super(), '__del__') returns False - covers line 118 false branch.""" + postgres_config = { + 'host': 'localhost', + 'port': 5432, + 'user': 'postgres', + 'password': 'postgres', + 'dbname': 'postgres', + 'min_connections': 1, + 'max_connections': 10, + } + + mlflow_config = {'host': 'localhost', 'port': 5000, 'username': 'mlflow', 'password': 'mlflow'} + + minio_config = { + 'endpoint_url': 'http://localhost:9000', + 'access_key': 'minioadmin', + 'secret_key': 'minioadmin', + 'region': 'us-east-1', + 'use_ssl': False, + 'max_retry_attempts': 3, + 'retry_mode': 'adaptive', + 'connect_timeout': 10, + 'read_timeout': 60, + } + + logger = MagicMock() + notification_handler = MagicMock() + + # Mock all __init__ methods to return None + mock_experiment_tracking_init.return_value = None + mock_mlflow_init.return_value = None + mock_minio_init.return_value = None + mock_training_init.return_value = None + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=logger, + notification_handler=notification_handler, + ) + + # Add engine attribute to pass the first hasattr check (line 115) + activities.engine = MagicMock() + + # Create a mock class without __del__ method to simulate super() not having __del__ + class MockSuperWithoutDel: + """Mock class that explicitly does not have __del__ method.""" + + pass + + # Patch super() to return an instance that doesn't have __del__ + mock_super_instance = MockSuperWithoutDel() + + with patch('builtins.super', return_value=mock_super_instance): + # Trigger __del__ - should handle the case when hasattr(super(), '__del__') is False + try: + activities.__del__() + # Test passes - the false branch of line 118 was executed without error + except Exception as e: + # Test fails if exception propagates + raise AssertionError( + f'__del__ should handle super() without __del__ method, but raised: {e}' + ) from e diff --git a/tests/activities/test_experiment_tracking.py b/tests/activities/test_experiment_tracking.py index 1be5aec..cb16efe 100644 --- a/tests/activities/test_experiment_tracking.py +++ b/tests/activities/test_experiment_tracking.py @@ -383,3 +383,182 @@ def test_update_type_enum_values(): assert UpdateType.STATUS == 'status' assert UpdateType.STATUS_WITH_ERROR == 'status_with_error' assert UpdateType.MODEL_SAVED == 'model_saved' + + +# Tests for __del__ method - 100% coverage + + +@patch('model_manager.activities.experiment_tracking.Postgres.__init__') +def test___del___with_engine_and_parent_del_exists(mock_postgres_init): + """Test __del__ calls parent destructor when engine exists and parent has __del__.""" + mock_postgres_init.return_value = None + + tracking = ExperimentTracking( + host='localhost', + port=5432, + user='test', + password='test', + dbname='test', + min_connections=1, + max_connections=10, + logger=MagicMock(), + notification_handler=MagicMock(), + ) + + # Add engine attribute to simulate Postgres initialization + tracking.engine = MagicMock() + + # Track if parent __del__ was called + parent_del_called = [] + + def mock_parent_del(self): + """Mock parent __del__ that tracks when it's called.""" + parent_del_called.append(True) + + # Patch parent class to have __del__ method + with patch.object(type(tracking).__bases__[0], '__del__', mock_parent_del, create=True): + # Trigger __del__ - should call parent __del__ (line 103) + tracking.__del__() + + # Verify parent __del__ was called (line 103 executed) + assert len(parent_del_called) == 1, 'Parent __del__ should have been called once' + + +@patch('model_manager.activities.experiment_tracking.Postgres.__init__') +def test___del___without_engine(mock_postgres_init): + """Test __del__ does not call parent destructor when engine attribute is missing.""" + mock_postgres_init.return_value = None + + tracking = ExperimentTracking( + host='localhost', + port=5432, + user='test', + password='test', + dbname='test', + min_connections=1, + max_connections=10, + logger=MagicMock(), + notification_handler=MagicMock(), + ) + + # Ensure engine attribute does not exist + if hasattr(tracking, 'engine'): + delattr(tracking, 'engine') + + # Mock parent __del__ to track if it's called + mock_parent_del = MagicMock() + + with patch.object(type(tracking).__bases__[0], '__del__', mock_parent_del, create=True): + # Trigger __del__ - should NOT call parent __del__ (line 100 is False) + tracking.__del__() + + # Verify parent __del__ was NOT called + mock_parent_del.assert_not_called() + + +@patch('model_manager.activities.experiment_tracking.Postgres.__init__') +def test___del___when_parent_has_no_del_method(mock_postgres_init): + """Test __del__ handles case when parent class has no __del__ method - covers line 102 false branch.""" + mock_postgres_init.return_value = None + + tracking = ExperimentTracking( + host='localhost', + port=5432, + user='test', + password='test', + dbname='test', + min_connections=1, + max_connections=10, + logger=MagicMock(), + notification_handler=MagicMock(), + ) + + # Add engine attribute to pass the first hasattr check (line 100) + tracking.engine = MagicMock() + + # Create a mock class without __del__ method + class MockSuperWithoutDel: + """Mock class that explicitly does not have __del__ method.""" + + pass + + # Patch super() to return an instance without __del__ + mock_super_instance = MockSuperWithoutDel() + + with patch('builtins.super', return_value=mock_super_instance): + # Trigger __del__ - should handle the case when hasattr(super(), '__del__') is False (line 102) + try: + tracking.__del__() + # Test passes - the false branch of line 102 was executed without error + except Exception as e: + raise AssertionError( + f'__del__ should handle super() without __del__ method, but raised: {e}' + ) from e + + +@patch('model_manager.activities.experiment_tracking.Postgres.__init__') +def test___del___handles_exception_from_parent_del(mock_postgres_init): + """Test __del__ handles exceptions from parent destructor gracefully - covers line 104.""" + mock_postgres_init.return_value = None + + tracking = ExperimentTracking( + host='localhost', + port=5432, + user='test', + password='test', + dbname='test', + min_connections=1, + max_connections=10, + logger=MagicMock(), + notification_handler=MagicMock(), + ) + + # Add engine attribute + tracking.engine = MagicMock() + + # Mock parent __del__ to raise an exception + mock_parent_del = MagicMock(side_effect=RuntimeError('Cleanup failed')) + + with patch.object(type(tracking).__bases__[0], '__del__', mock_parent_del, create=True): + # Trigger __del__ - should catch exception and not propagate it (line 104-106) + try: + tracking.__del__() + # Test passes if no exception is raised + except Exception as e: + raise AssertionError( + f'__del__ should handle exceptions gracefully, but raised: {e}' + ) from e + + +@patch('model_manager.activities.experiment_tracking.Postgres.__init__') +def test___del___handles_attribute_error_from_parent_del(mock_postgres_init): + """Test __del__ handles AttributeError from parent destructor - covers line 104 exception handling.""" + mock_postgres_init.return_value = None + + tracking = ExperimentTracking( + host='localhost', + port=5432, + user='test', + password='test', + dbname='test', + min_connections=1, + max_connections=10, + logger=MagicMock(), + notification_handler=MagicMock(), + ) + + # Add engine attribute + tracking.engine = MagicMock() + + # Mock parent __del__ to raise AttributeError + mock_parent_del = MagicMock(side_effect=AttributeError('engine not found')) + + with patch.object(type(tracking).__bases__[0], '__del__', mock_parent_del, create=True): + # Trigger __del__ - should catch AttributeError and not propagate it + try: + tracking.__del__() + # Test passes if no exception is raised + except Exception as e: + raise AssertionError( + f'__del__ should handle AttributeError gracefully, but raised: {e}' + ) from e