From fc2fe33b595f995b41d33be7a8c245203c6f9d6a Mon Sep 17 00:00:00 2001 From: Bruno Domingues Date: Wed, 22 Oct 2025 23:38:01 -0300 Subject: [PATCH] SIENTIAPDE-1241: Add unit tests for Activities class and enhance validation script with fix and test options This commit introduces a new test suite for the Activities class, achieving 100% coverage. Additionally, the validation script (validate.sh) is enhanced with new options: - --fix: Automatically fixes code formatting and linting issues using Ruff. - --only-tests: Runs only the unit tests, skipping other validation steps. The validation script now also supports skipping tests and provides more informative output. --- tests/activities/test_activities.py | 302 ++++++++++++++++++++++++++++ validate.sh | 111 +++++++--- 2 files changed, 384 insertions(+), 29 deletions(-) create mode 100644 tests/activities/test_activities.py diff --git a/tests/activities/test_activities.py b/tests/activities/test_activities.py new file mode 100644 index 0000000..43fb648 --- /dev/null +++ b/tests/activities/test_activities.py @@ -0,0 +1,302 @@ +"""Unit tests for Activities class with 100% coverage.""" + +import asyncio +from unittest.mock import MagicMock, patch + +import pytest + + +@pytest.fixture +def mock_logger(): + """Create a mock logger.""" + return MagicMock() + + +@pytest.fixture +def mock_notification_handler(): + """Create a mock notification handler.""" + return MagicMock() + + +@pytest.fixture +def postgres_config(): + """Create a valid PostgreSQL configuration.""" + return { + 'host': 'localhost', + 'port': 5432, + 'user': 'testuser', + 'password': 'testpass', + 'dbname': 'testdb', + 'min_connections': 1, + 'max_connections': 10, + } + + +@pytest.fixture +def mlflow_config(): + """Create a valid MLFlow configuration.""" + return { + 'url': 'http://mlflow:5080', + 'username': 'aignosi', + 'password': 'aignosi', + } + + +@pytest.fixture +def minio_config(): + """Create a valid MinIO configuration.""" + return { + 'endpoint_url': 'http://minio:9000', + 'access_key': 'minioadmin', + 'secret_key': 'minioadmin', + 'region': 'us-east-1', + 'use_ssl': False, + 'max_retry_attempts': 3, + 'retry_mode': 'standard', + 'connect_timeout': 5, + 'read_timeout': 5, + } + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__', return_value=None) +@patch('model_manager.activities.activities.Training.__init__', return_value=None) +@patch('model_manager.activities.activities.ModelRepository') +@patch('model_manager.activities.activities.StorageRepository') +def test_activities_init_success( + mock_storage_repo, + mock_model_repo, + mock_training_init, + mock_et_init, + postgres_config, + mlflow_config, + minio_config, + mock_logger, + mock_notification_handler, +): + """Test successful initialization of Activities.""" + from model_manager.activities.activities import Activities + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=mock_logger, + notification_handler=mock_notification_handler, + ) + + mock_et_init.assert_called_once() + assert mock_et_init.call_args[1]['host'] == postgres_config['host'] + assert mock_et_init.call_args[1]['port'] == postgres_config['port'] + assert mock_et_init.call_args[1]['user'] == postgres_config['user'] + assert mock_et_init.call_args[1]['password'] == postgres_config['password'] + assert mock_et_init.call_args[1]['dbname'] == postgres_config['dbname'] + assert mock_et_init.call_args[1]['min_connections'] == postgres_config['min_connections'] + assert mock_et_init.call_args[1]['max_connections'] == postgres_config['max_connections'] + assert mock_et_init.call_args[1]['logger'] is mock_logger + assert mock_et_init.call_args[1]['notification_handler'] is mock_notification_handler + + mock_model_repo.assert_called_once_with( + url=mlflow_config['url'], + username=mlflow_config['username'], + password=mlflow_config['password'], + logger=mock_logger, + ) + + mock_storage_repo.assert_called_once_with( + endpoint_url=minio_config['endpoint_url'], + access_key=minio_config['access_key'], + secret_key=minio_config['secret_key'], + region=minio_config['region'], + use_ssl=minio_config['use_ssl'], + max_retry_attempts=minio_config['max_retry_attempts'], + retry_mode=minio_config['retry_mode'], + connect_timeout=minio_config['connect_timeout'], + read_timeout=minio_config['read_timeout'], + logger=mock_logger, + ) + + mock_training_init.assert_called_once() + assert mock_training_init.call_args[1]['model_repository'] is mock_model_repo.return_value + assert mock_training_init.call_args[1]['storage_repository'] is mock_storage_repo.return_value + assert mock_training_init.call_args[1]['logger'] is mock_logger + assert mock_training_init.call_args[1]['notification_handler'] is mock_notification_handler + + assert hasattr(activities, 'model_repository') + assert hasattr(activities, 'storage_repository') + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__', return_value=None) +@patch('model_manager.activities.activities.ExperimentTracking.close') +@patch('model_manager.activities.activities.Training.__init__', return_value=None) +@patch('model_manager.activities.activities.ModelRepository') +@patch('model_manager.activities.activities.StorageRepository') +def test_activities_shutdown( + mock_storage_repo, + mock_model_repo, + mock_training_init, + mock_et_close, + mock_et_init, + postgres_config, + mlflow_config, + minio_config, + mock_logger, + mock_notification_handler, +): + """Test Activities.shutdown() calls ExperimentTracking.close().""" + from model_manager.activities.activities import Activities + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=mock_logger, + notification_handler=mock_notification_handler, + ) + + asyncio.run(activities.shutdown()) + + mock_et_close.assert_called_once_with(activities) + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__', return_value=None) +@patch('model_manager.activities.activities.Training.__init__', return_value=None) +@patch('model_manager.activities.activities.ModelRepository') +@patch('model_manager.activities.activities.StorageRepository') +def test_activities_del_without_engine( + mock_storage_repo, + mock_model_repo, + mock_training_init, + mock_et_init, + postgres_config, + mlflow_config, + minio_config, + mock_logger, + mock_notification_handler, +): + """Test __del__ when engine attribute does not exist.""" + from model_manager.activities.activities import Activities + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=mock_logger, + notification_handler=mock_notification_handler, + ) + + if hasattr(activities, 'engine'): + delattr(activities, 'engine') + + activities.__del__() + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__', return_value=None) +@patch('model_manager.activities.activities.Training.__init__', return_value=None) +@patch('model_manager.activities.activities.ModelRepository') +@patch('model_manager.activities.activities.StorageRepository') +def test_activities_del_with_engine_no_super_del( + mock_storage_repo, + mock_model_repo, + mock_training_init, + mock_et_init, + postgres_config, + mlflow_config, + minio_config, + mock_logger, + mock_notification_handler, +): + """Test __del__ when engine exists but super has no __del__.""" + from model_manager.activities.activities import Activities + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=mock_logger, + notification_handler=mock_notification_handler, + ) + + activities.engine = MagicMock() + + with patch('builtins.super') as mock_super: + mock_super_instance = MagicMock() + del mock_super_instance.__del__ + mock_super.return_value = mock_super_instance + + activities.__del__() + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__', return_value=None) +@patch('model_manager.activities.activities.Training.__init__', return_value=None) +@patch('model_manager.activities.activities.ModelRepository') +@patch('model_manager.activities.activities.StorageRepository') +def test_activities_del_with_engine_and_super_del( + mock_storage_repo, + mock_model_repo, + mock_training_init, + mock_et_init, + postgres_config, + mlflow_config, + minio_config, + mock_logger, + mock_notification_handler, +): + """Test __del__ when engine exists and super has __del__.""" + from model_manager.activities.activities import Activities + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=mock_logger, + notification_handler=mock_notification_handler, + ) + + activities.engine = MagicMock() + + mock_super_del = MagicMock() + + class MockSuper: + def __del__(self): + mock_super_del() + + with patch('builtins.super', return_value=MockSuper()): + activities.__del__() + + mock_super_del.assert_called_once() + + +@patch('model_manager.activities.activities.ExperimentTracking.__init__', return_value=None) +@patch('model_manager.activities.activities.Training.__init__', return_value=None) +@patch('model_manager.activities.activities.ModelRepository') +@patch('model_manager.activities.activities.StorageRepository') +def test_activities_del_with_engine_exception_caught( + mock_storage_repo, + mock_model_repo, + mock_training_init, + mock_et_init, + postgres_config, + mlflow_config, + minio_config, + mock_logger, + mock_notification_handler, +): + """Test __del__ catches exceptions when super().__del__() raises.""" + from model_manager.activities.activities import Activities + + activities = Activities( + postgres_config=postgres_config, + mlflow_config=mlflow_config, + minio_config=minio_config, + logger=mock_logger, + notification_handler=mock_notification_handler, + ) + + activities.engine = MagicMock() + + class MockSuperWithError: + def __del__(self): + raise RuntimeError('Test error') + + with patch('builtins.super', return_value=MockSuperWithError()): + activities.__del__() diff --git a/validate.sh b/validate.sh index d4bbc15..96beac1 100755 --- a/validate.sh +++ b/validate.sh @@ -6,22 +6,38 @@ # ./validate.sh # Run all checks including tests (default) # ./validate.sh --no-tests # Skip unit tests # ./validate.sh --skip-tests # Skip unit tests (alias) +# ./validate.sh --only-tests # Run only unit tests +# ./validate.sh --fix # Auto-fix formatting and linting, then run validations (no tests) set -e # Exit on any error # Parse command line arguments RUN_TESTS=true +ONLY_TESTS=false +FIX_MODE=false + for arg in "$@"; do case $arg in --no-tests|--skip-tests) RUN_TESTS=false shift ;; + --only-tests) + ONLY_TESTS=true + shift + ;; + --fix) + FIX_MODE=true + RUN_TESTS=false + shift + ;; --help|-h) echo "Usage: $0 [OPTIONS]" echo "" echo "Options:" echo " --no-tests, --skip-tests Skip unit tests (default: run tests)" + echo " --only-tests Run only unit tests" + echo " --fix Auto-fix formatting and linting, then run validations (no tests)" echo " --help, -h Show this help message" echo "" exit 0 @@ -46,7 +62,36 @@ echo -e "${BLUE}║ Model Manager - Code Validation Suite ║${N echo -e "${BLUE}╚════════════════════════════════════════════════════════╝${NC}" echo "" -if [ "$RUN_TESTS" = false ]; then +# Handle --fix mode +if [ "$FIX_MODE" = true ]; then + echo -e "${BLUE}🔧 Running auto-fix mode...${NC}" + echo "" + + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${BLUE}▶ Auto-fixing code formatting (Ruff)${NC}" + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + ruff format model_manager/ tests/ + echo -e "${GREEN}✅ Code formatting applied${NC}" + echo "" + + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${BLUE}▶ Auto-fixing linting issues (Ruff)${NC}" + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + ruff check --fix model_manager/ tests/ + echo -e "${GREEN}✅ Linting fixes applied${NC}" + echo "" + + echo -e "${YELLOW}ℹ️ Now running validations (without tests)...${NC}" + echo "" +fi + +# Handle --only-tests mode +if [ "$ONLY_TESTS" = true ]; then + echo -e "${BLUE}🧪 Running only unit tests...${NC}" + echo "" +fi + +if [ "$RUN_TESTS" = false ] && [ "$ONLY_TESTS" = false ]; then echo -e "${YELLOW}ℹ️ Unit tests will be skipped${NC}" echo "" fi @@ -81,37 +126,45 @@ run_step() { # Track failures FAILED_STEPS=() -# Step 1: Code Formatting Check (Ruff) -if ! run_step "1. Code Formatting (Ruff)" "ruff format --check model_manager/ tests/"; then - FAILED_STEPS+=("Code Formatting") -fi - -# Step 2: Linting (Ruff) -if ! run_step "2. Code Linting (Ruff)" "ruff check model_manager/ tests/"; then - FAILED_STEPS+=("Linting") -fi - -# Step 3: Type Checking (mypy) -if ! run_step "3. Type Checking (mypy)" "mypy model_manager/"; then - FAILED_STEPS+=("Type Checking") -fi - -# Step 4: Security Analysis (Bandit) -if ! run_step "4. Security Analysis (Bandit)" "bandit -r model_manager/ -ll -q"; then - FAILED_STEPS+=("Security Analysis") -fi - -# Step 5: Unit Tests (pytest) -if [ "$RUN_TESTS" = true ]; then - if ! run_step "5. Unit Tests (pytest)" "pytest tests/ --cov=model_manager --cov-report=term-missing --cov-report=xml --cov-report=html --cov-fail-under=80 -q"; then +# Handle --only-tests mode +if [ "$ONLY_TESTS" = true ]; then + # Step 5: Unit Tests (pytest) + if ! run_step "Unit Tests (pytest)" "pytest tests/ --cov=model_manager --cov-report=term-missing --cov-report=xml --cov-report=html --cov-fail-under=80 -q"; then FAILED_STEPS+=("Unit Tests") fi else - echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" - echo -e "${BLUE}▶ 5. Unit Tests (pytest)${NC}" - echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" - echo -e "${YELLOW}⏭️ Unit Tests - SKIPPED${NC}" - echo "" + # Step 1: Code Formatting Check (Ruff) + if ! run_step "1. Code Formatting (Ruff)" "ruff format --check model_manager/ tests/"; then + FAILED_STEPS+=("Code Formatting") + fi + + # Step 2: Linting (Ruff) + if ! run_step "2. Code Linting (Ruff)" "ruff check model_manager/ tests/"; then + FAILED_STEPS+=("Linting") + fi + + # Step 3: Type Checking (mypy) + if ! run_step "3. Type Checking (mypy)" "mypy model_manager/"; then + FAILED_STEPS+=("Type Checking") + fi + + # Step 4: Security Analysis (Bandit) + if ! run_step "4. Security Analysis (Bandit)" "bandit -r model_manager/ -ll -q"; then + FAILED_STEPS+=("Security Analysis") + fi + + # Step 5: Unit Tests (pytest) + if [ "$RUN_TESTS" = true ]; then + if ! run_step "5. Unit Tests (pytest)" "pytest tests/ --cov=model_manager --cov-report=term-missing --cov-report=xml --cov-report=html --cov-fail-under=80 -q"; then + FAILED_STEPS+=("Unit Tests") + fi + else + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${BLUE}▶ 5. Unit Tests (pytest)${NC}" + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${YELLOW}⏭️ Unit Tests - SKIPPED${NC}" + echo "" + fi fi # Summary