SIENTIAPDE-1255: Add comprehensive tests for __del__ methods in Activities and ExperimentTracking to ensure proper resource cleanup and exception handling.
This commit is contained in:
Bruno Domingues
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user