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.
This commit is contained in:
Bruno Domingues
302
tests/activities/test_activities.py
Normal file
302
tests/activities/test_activities.py
Normal file
@@ -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__()
|
||||
111
validate.sh
111
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
|
||||
|
||||
Reference in New Issue
Block a user