From a258036397cf928c94a7cf393c6ce824600f931d Mon Sep 17 00:00:00 2001 From: Bruno Domingues Date: Fri, 31 Oct 2025 17:15:59 -0300 Subject: [PATCH] SIENTIAPDE-1307: Refactor: Remove redundant patching of SientiaMonitoring and Postgres init in tests, and add shutdown mock to metrics controller. --- tests/activities/test_experiment_tracking.py | 73 +++++++------------- tests/activities/test_training.py | 37 +++------- 2 files changed, 34 insertions(+), 76 deletions(-) diff --git a/tests/activities/test_experiment_tracking.py b/tests/activities/test_experiment_tracking.py index 837c3d3..2528c68 100644 --- a/tests/activities/test_experiment_tracking.py +++ b/tests/activities/test_experiment_tracking.py @@ -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 diff --git a/tests/activities/test_training.py b/tests/activities/test_training.py index 1f83ff8..c9013c2 100644 --- a/tests/activities/test_training.py +++ b/tests/activities/test_training.py @@ -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,