SIENTIAPDE-1307: Refactor: Remove redundant patching of SientiaMonitoring and Postgres init in tests, and add shutdown mock to metrics controller.
This commit is contained in:
Bruno Domingues
@@ -21,7 +21,14 @@ def mock_notification_handler():
|
||||
@pytest.fixture
|
||||
def mock_metrics_controller():
|
||||
"""Create a mock metrics controller."""
|
||||
return MagicMock()
|
||||
controller = MagicMock()
|
||||
|
||||
# Make shutdown an async coroutine
|
||||
async def mock_shutdown():
|
||||
pass
|
||||
|
||||
controller.shutdown = mock_shutdown
|
||||
return controller
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -38,9 +45,8 @@ def db_config():
|
||||
}
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_experiment_tracking_init(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test ExperimentTracking initialization."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking
|
||||
@@ -58,23 +64,9 @@ def test_experiment_tracking_init(
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
|
||||
mock_postgres_init.assert_called_once_with(
|
||||
host=db_config['host'],
|
||||
port=db_config['port'],
|
||||
user=db_config['user'],
|
||||
password=db_config['password'],
|
||||
dbname=db_config['dbname'],
|
||||
min_connections=db_config['min_connections'],
|
||||
max_connections=db_config['max_connections'],
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_experiment_tracking_del_without_engine(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test __del__ when engine attribute does not exist."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking
|
||||
@@ -98,9 +90,8 @@ def test_experiment_tracking_del_without_engine(
|
||||
et.__del__()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_experiment_tracking_del_with_engine(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test __del__ when engine exists."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking
|
||||
@@ -128,9 +119,8 @@ def test_experiment_tracking_del_with_engine(
|
||||
et.__del__()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_experiment_tracking_del_with_engine_exception(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test __del__ catches exceptions."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking
|
||||
@@ -163,9 +153,8 @@ def test_experiment_tracking_del_with_engine_exception(
|
||||
et.__del__()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_execute_update_success(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test _execute_update executes query successfully."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking
|
||||
@@ -197,9 +186,8 @@ def test_execute_update_success(
|
||||
mock_connection.execute.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_status_success(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with STATUS update type."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -243,9 +231,8 @@ def test_update_experiment_run_status_success(
|
||||
et.info.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_status_missing_status(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with STATUS but missing status parameter."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -277,9 +264,8 @@ def test_update_experiment_run_status_missing_status(
|
||||
et.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_status_with_error_success(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with STATUS_WITH_ERROR update type."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -324,9 +310,8 @@ def test_update_experiment_run_status_with_error_success(
|
||||
et.info.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_status_with_error_truncate_message(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run truncates error message if too long."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -368,9 +353,8 @@ def test_update_experiment_run_status_with_error_truncate_message(
|
||||
assert len(call_args[0][1]['error_message']) == 1024
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_status_with_error_missing_error_message(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with STATUS_WITH_ERROR but missing error_message."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -403,9 +387,8 @@ def test_update_experiment_run_status_with_error_missing_error_message(
|
||||
et.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_model_saved_success(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with MODEL_SAVED update type."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -450,9 +433,8 @@ def test_update_experiment_run_model_saved_success(
|
||||
et.info.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_model_saved_missing_run_name(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with MODEL_SAVED but missing run_name."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -485,9 +467,8 @@ def test_update_experiment_run_model_saved_missing_run_name(
|
||||
et.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_invalid_update_type(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with invalid update_type."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking
|
||||
@@ -519,9 +500,8 @@ def test_update_experiment_run_invalid_update_type(
|
||||
et.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_no_rows_updated(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run raises error when no rows are updated."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -558,9 +538,8 @@ def test_update_experiment_run_no_rows_updated(
|
||||
et.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_status_with_error_missing_status(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with STATUS_WITH_ERROR but missing status - covers line 179."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -593,9 +572,8 @@ def test_update_experiment_run_status_with_error_missing_status(
|
||||
et.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_update_experiment_run_model_saved_missing_status(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test update_experiment_run with MODEL_SAVED but missing status - covers line 204."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking, UpdateType
|
||||
@@ -628,9 +606,8 @@ def test_update_experiment_run_model_saved_missing_status(
|
||||
et.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.experiment_tracking.Postgres.__init__', return_value=None)
|
||||
def test_experiment_tracking_del_with_engine_no_super_del(
|
||||
mock_postgres_init, db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
db_config, mock_logger, mock_notification_handler, mock_metrics_controller
|
||||
):
|
||||
"""Test __del__ when engine exists but super has no __del__ - covers line 103."""
|
||||
from model_manager.activities.experiment_tracking import ExperimentTracking
|
||||
|
||||
@@ -22,7 +22,14 @@ def mock_notification_handler():
|
||||
@pytest.fixture
|
||||
def mock_metrics_controller():
|
||||
"""Create a mock metrics controller."""
|
||||
return MagicMock()
|
||||
controller = MagicMock()
|
||||
|
||||
# Make shutdown an async coroutine
|
||||
async def mock_shutdown():
|
||||
pass
|
||||
|
||||
controller.shutdown = mock_shutdown
|
||||
return controller
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -50,11 +57,9 @@ def mock_train_params():
|
||||
return params
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
def test_training_init(
|
||||
mock_training_repo,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -72,21 +77,17 @@ def test_training_init(
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
|
||||
mock_base_init.assert_called_once_with(
|
||||
mock_logger, mock_notification_handler, mock_metrics_controller, set_error_counter=True
|
||||
)
|
||||
mock_training_repo.assert_called_once_with(mock_logger)
|
||||
assert training.model_repository is mock_model_repository
|
||||
assert training.storage_repository is mock_storage_repository
|
||||
assert training.metrics_controller is mock_metrics_controller
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
@patch('model_manager.activities.training.TrainModelParams')
|
||||
def test_validate_train_params_success(
|
||||
mock_train_params_class,
|
||||
mock_training_repo,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -122,13 +123,11 @@ def test_validate_train_params_success(
|
||||
training.info.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
@patch('model_manager.activities.training.TrainModelParams')
|
||||
def test_validate_train_params_value_error(
|
||||
mock_train_params_class,
|
||||
mock_training_repo,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -160,13 +159,11 @@ def test_validate_train_params_value_error(
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
@patch('model_manager.activities.training.TrainModelParams')
|
||||
def test_validate_train_params_type_error(
|
||||
mock_train_params_class,
|
||||
mock_training_repo,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -198,13 +195,11 @@ def test_validate_train_params_type_error(
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
@patch('model_manager.activities.training.TrainModelParams')
|
||||
def test_validate_train_params_key_error(
|
||||
mock_train_params_class,
|
||||
mock_training_repo,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -236,11 +231,9 @@ def test_validate_train_params_key_error(
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
def test_train_model_success_with_params_object(
|
||||
mock_training_repo_class,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -286,13 +279,11 @@ def test_train_model_success_with_params_object(
|
||||
mock_model_repository.save_model.assert_called_once_with(mock_train_result)
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
@patch('model_manager.activities.training.TrainModelParams')
|
||||
def test_train_model_success_with_params_dict(
|
||||
mock_train_params_class,
|
||||
mock_training_repo_class,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -334,11 +325,9 @@ def test_train_model_success_with_params_dict(
|
||||
mock_train_params_class.from_dict.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
def test_train_model_training_fails(
|
||||
mock_training_repo_class,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -376,11 +365,9 @@ def test_train_model_training_fails(
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
def test_train_model_save_fails(
|
||||
mock_training_repo_class,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -422,11 +409,9 @@ def test_train_model_save_fails(
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
def test_cleanup_resources_success(
|
||||
mock_training_repo_class,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -457,11 +442,9 @@ def test_cleanup_resources_success(
|
||||
mock_storage_repository.delete_file.assert_called_once_with('test-bucket', 'test-file.csv')
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
def test_cleanup_resources_cleanup_fails(
|
||||
mock_training_repo_class,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
@@ -495,11 +478,9 @@ def test_cleanup_resources_cleanup_fails(
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.SientiaMonitoring.__init__', return_value=None)
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
def test_cleanup_resources_with_empty_values(
|
||||
mock_training_repo_class,
|
||||
mock_base_init,
|
||||
mock_model_repository,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
|
||||
Reference in New Issue
Block a user