feat: enhance E2E testing setup and model reporting
- Added a new fixture to manage runtime report artifacts in a writable temp directory during E2E tests, addressing permission issues in local CI/dev environments. - Updated `conftest.py` to include a requirements.txt file in the model packaging path for training activities. - Refactored existing fixtures to use `pytest.fixture` instead of `pytest_asyncio.fixture` for better compatibility. - Enhanced the `Reports` class to include a target alias for report metrics, ensuring compatibility with Evidently's reporting requirements. - Introduced new test scenarios to validate the handling of missing and whitespace-only `date_column` inputs in the training workflow. These changes improve the robustness of the E2E testing framework and enhance the clarity of model reporting metrics.
This commit is contained in:
@@ -125,7 +125,7 @@ def test_cleanup_temp_directories_nonexistent_path(
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = MagicMock()
|
||||
cleanup._emit_metrics = MagicMock() # type: ignore[method-assign]
|
||||
cleanup.warning = MagicMock()
|
||||
|
||||
cleanup.cleanup_temp_directories({'temp_path': '/nonexistent/path', 'metadata': {}})
|
||||
@@ -152,7 +152,7 @@ def test_cleanup_temp_directories_success_with_deletions(
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = MagicMock()
|
||||
cleanup._emit_metrics = MagicMock() # type: ignore[method-assign]
|
||||
|
||||
old_time = (datetime.now() - timedelta(hours=48)).strftime('%Y%m%d_%H%M%S_000000')
|
||||
old_dir = os.path.join(temp_dir, f'old_dir_{old_time}')
|
||||
@@ -187,7 +187,7 @@ def test_cleanup_temp_directories_dry_run(
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = MagicMock()
|
||||
cleanup._emit_metrics = MagicMock() # type: ignore[method-assign]
|
||||
|
||||
old_time = (datetime.now() - timedelta(hours=48)).strftime('%Y%m%d_%H%M%S_000000')
|
||||
old_dir = os.path.join(temp_dir, f'old_dir_{old_time}')
|
||||
@@ -217,7 +217,7 @@ def test_cleanup_temp_directories_delete_error(
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = MagicMock()
|
||||
cleanup._emit_metrics = MagicMock() # type: ignore[method-assign]
|
||||
cleanup.error = MagicMock()
|
||||
|
||||
old_time = (datetime.now() - timedelta(hours=48)).strftime('%Y%m%d_%H%M%S_000000')
|
||||
@@ -276,7 +276,7 @@ def test_cleanup_temp_directories_with_files_and_unmatched_dirs(
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = MagicMock()
|
||||
cleanup._emit_metrics = MagicMock() # type: ignore[method-assign]
|
||||
cleanup.debug = MagicMock()
|
||||
|
||||
# Create a file and a directory with a non-matching name
|
||||
@@ -310,7 +310,7 @@ def test_cleanup_temp_directories_invalid_timestamp_format(
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = MagicMock()
|
||||
cleanup._emit_metrics = MagicMock() # type: ignore[method-assign]
|
||||
cleanup.error = MagicMock()
|
||||
|
||||
# Create a directory with a malformed timestamp that matches the regex but fails parsing
|
||||
@@ -340,7 +340,7 @@ def test_cleanup_temp_directories_generic_exception(
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = MagicMock()
|
||||
cleanup._emit_metrics = MagicMock() # type: ignore[method-assign]
|
||||
cleanup.send_notification = MagicMock()
|
||||
|
||||
with patch('os.listdir', side_effect=Exception('Unexpected OS Error')):
|
||||
|
||||
@@ -136,6 +136,11 @@ def test_experiment_tracking_del_with_engine_exception(
|
||||
et.engine = MagicMock()
|
||||
|
||||
class MockSuperWithError:
|
||||
_should_raise: bool
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._should_raise = False
|
||||
|
||||
def __del__(self):
|
||||
# Only raise error if not being cleaned up by garbage collector
|
||||
# This prevents the PytestUnraisableExceptionWarning
|
||||
@@ -215,7 +220,7 @@ def test_update_experiment_run_status_success(
|
||||
mock_execute(*args, **kwargs)
|
||||
return {'rowcount': 1}
|
||||
|
||||
et._execute_update = mock_execute_update
|
||||
et._execute_update = mock_execute_update # type: ignore[method-assign]
|
||||
et.info = MagicMock()
|
||||
|
||||
input_data = {
|
||||
@@ -293,7 +298,7 @@ def test_update_experiment_run_status_with_error_success(
|
||||
mock_execute(*args, **kwargs)
|
||||
return {'rowcount': 1}
|
||||
|
||||
et._execute_update = mock_execute_update
|
||||
et._execute_update = mock_execute_update # type: ignore[method-assign]
|
||||
et.info = MagicMock()
|
||||
|
||||
input_data = {
|
||||
@@ -339,7 +344,7 @@ def test_update_experiment_run_status_with_error_truncate_message(
|
||||
mock_execute(*args, **kwargs)
|
||||
return {'rowcount': 1}
|
||||
|
||||
et._execute_update = mock_execute_update
|
||||
et._execute_update = mock_execute_update # type: ignore[method-assign]
|
||||
et.info = MagicMock()
|
||||
|
||||
long_error = 'x' * 2000
|
||||
@@ -416,7 +421,7 @@ def test_update_experiment_run_model_saved_success(
|
||||
mock_execute(*args, **kwargs)
|
||||
return {'rowcount': 1}
|
||||
|
||||
et._execute_update = mock_execute_update
|
||||
et._execute_update = mock_execute_update # type: ignore[method-assign]
|
||||
et.info = MagicMock()
|
||||
|
||||
input_data = {
|
||||
@@ -526,7 +531,7 @@ def test_update_experiment_run_no_rows_updated(
|
||||
def mock_execute_update(*args, **kwargs):
|
||||
return {'rowcount': 0}
|
||||
|
||||
et._execute_update = mock_execute_update
|
||||
et._execute_update = mock_execute_update # type: ignore[method-assign]
|
||||
et.send_notification = MagicMock()
|
||||
|
||||
input_data = {
|
||||
|
||||
@@ -131,13 +131,15 @@ def test_train_model_success_serializes_result(mock_mlflow, training):
|
||||
|
||||
training.minio_repository.download_file = MagicMock(return_value=b'csv')
|
||||
training.data_manager_repository.prepare_training_data = MagicMock(return_value=tmr)
|
||||
|
||||
def _set_metrics(x, _w, **_kw):
|
||||
x.mse_val = 0.1
|
||||
x.mae_val = 0.2
|
||||
x.r2_val = 0.9
|
||||
return x
|
||||
|
||||
training.data_manager_repository.compute_regression_metrics = MagicMock(
|
||||
side_effect=lambda x, _w, **_kw: (
|
||||
setattr(x, 'mse_val', 0.1),
|
||||
setattr(x, 'mae_val', 0.2),
|
||||
setattr(x, 'r2_val', 0.9),
|
||||
x,
|
||||
)[-1]
|
||||
side_effect=_set_metrics
|
||||
)
|
||||
|
||||
def _fill_report(x, **_kw):
|
||||
|
||||
@@ -21,7 +21,7 @@ def _stub_evidently() -> None:
|
||||
sys.modules['evidently'] = ev
|
||||
|
||||
mp = ModuleType('evidently.metric_preset')
|
||||
mp.DataDriftPreset = _make_dummy('DataDriftPreset')
|
||||
mp.DataDriftPreset = _make_dummy('DataDriftPreset') # type: ignore[attr-defined]
|
||||
sys.modules['evidently.metric_preset'] = mp
|
||||
|
||||
metrics = ModuleType('evidently.metrics')
|
||||
@@ -47,22 +47,22 @@ def _stub_evidently() -> None:
|
||||
def generate_column_metrics(*_a, **_k):
|
||||
return []
|
||||
|
||||
base.generate_column_metrics = generate_column_metrics
|
||||
base.generate_column_metrics = generate_column_metrics # type: ignore[attr-defined]
|
||||
sys.modules['evidently.metrics.base_metric'] = base
|
||||
|
||||
opt = ModuleType('evidently.options')
|
||||
opt.ColorOptions = _make_dummy('ColorOptions')
|
||||
opt.ColorOptions = _make_dummy('ColorOptions') # type: ignore[attr-defined]
|
||||
sys.modules['evidently.options'] = opt
|
||||
|
||||
pipeline = ModuleType('evidently.pipeline')
|
||||
sys.modules['evidently.pipeline'] = pipeline
|
||||
|
||||
colmap = ModuleType('evidently.pipeline.column_mapping')
|
||||
colmap.ColumnMapping = _make_dummy('ColumnMapping')
|
||||
colmap.ColumnMapping = _make_dummy('ColumnMapping') # type: ignore[attr-defined]
|
||||
sys.modules['evidently.pipeline.column_mapping'] = colmap
|
||||
|
||||
rep = ModuleType('evidently.report')
|
||||
rep.Report = _make_dummy('Report')
|
||||
rep.Report = _make_dummy('Report') # type: ignore[attr-defined]
|
||||
sys.modules['evidently.report'] = rep
|
||||
|
||||
|
||||
@@ -82,9 +82,9 @@ def pytest_configure(config) -> None: # noqa: ARG001
|
||||
def treat_nan(input_data, *_a, **_k):
|
||||
return input_data
|
||||
|
||||
df_pre.create_features = create_features
|
||||
df_pre.limit_dataset = limit_dataset
|
||||
df_pre.treat_nan = treat_nan
|
||||
df_pre.create_features = create_features # type: ignore[attr-defined]
|
||||
df_pre.limit_dataset = limit_dataset # type: ignore[attr-defined]
|
||||
df_pre.treat_nan = treat_nan # type: ignore[attr-defined]
|
||||
sys.modules['sientia_do.operations.df_preprocessor'] = df_pre
|
||||
|
||||
sys.modules.setdefault('sientia_do.operations', ModuleType('sientia_do.operations'))
|
||||
@@ -97,7 +97,7 @@ def pytest_configure(config) -> None: # noqa: ARG001
|
||||
|
||||
pass
|
||||
|
||||
ts_an.TimeSeriesDiscontinuityAnalyzer = TimeSeriesDiscontinuityAnalyzer
|
||||
ts_an.TimeSeriesDiscontinuityAnalyzer = TimeSeriesDiscontinuityAnalyzer # type: ignore[attr-defined]
|
||||
sys.modules['sientia_do.timeseries.analyzer'] = ts_an
|
||||
|
||||
sys.modules.setdefault('sientia_do.timeseries', ModuleType('sientia_do.timeseries'))
|
||||
|
||||
@@ -128,7 +128,10 @@ def test_add_data_quality_section_with_run(monkeypatch, tmp_path, stub_color_opt
|
||||
ReportMock.assert_called_once_with(
|
||||
metrics=[summary, column_metrics, conflict, correlations], options=report.options
|
||||
)
|
||||
report_instance.run.assert_called_once_with(reference_data='ref', current_data='cur')
|
||||
run_kwargs = report_instance.run.call_args.kwargs
|
||||
assert run_kwargs['reference_data'] == 'ref'
|
||||
assert run_kwargs['current_data'] == 'cur'
|
||||
assert run_kwargs['column_mapping'].target == 'target'
|
||||
report_instance.save_html.assert_called_once_with(
|
||||
os.path.join(str(tmp_path), 'data_quality.html')
|
||||
)
|
||||
@@ -160,6 +163,34 @@ def test_add_data_quality_section_run_without_base_path(monkeypatch, stub_color_
|
||||
report_instance.save_html.assert_not_called()
|
||||
|
||||
|
||||
def test_add_data_quality_section_non_default_target_keeps_conflict_metric(
|
||||
monkeypatch, stub_color_options
|
||||
):
|
||||
summary = object()
|
||||
column_metrics = object()
|
||||
conflict = object()
|
||||
correlations = object()
|
||||
|
||||
monkeypatch.setattr(reports, 'DatasetSummaryMetric', lambda: summary)
|
||||
monkeypatch.setattr(
|
||||
reports,
|
||||
'generate_column_metrics',
|
||||
lambda *args, **kwargs: column_metrics,
|
||||
)
|
||||
monkeypatch.setattr(reports, 'ConflictTargetMetric', lambda: conflict)
|
||||
monkeypatch.setattr(reports, 'DatasetCorrelationsMetric', lambda: correlations)
|
||||
|
||||
report = reports.Reports(reference_data='ref', current_data='cur', target_name='sales')
|
||||
report.add_data_quality_section(columns=['c1'], run=False)
|
||||
|
||||
assert report.metrics[-4:] == [
|
||||
summary,
|
||||
column_metrics,
|
||||
conflict,
|
||||
correlations,
|
||||
]
|
||||
|
||||
|
||||
def test_add_data_drift_section_paths(monkeypatch, tmp_path, stub_color_options):
|
||||
drift_instances = [object(), object(), object()]
|
||||
DataDriftPresetMock = MagicMock(side_effect=drift_instances)
|
||||
@@ -180,7 +211,10 @@ def test_add_data_drift_section_paths(monkeypatch, tmp_path, stub_color_options)
|
||||
report.add_data_drift_section(columns=['c1'], run=True)
|
||||
assert report.sections['data_drift'] == {'result': 'data_drift'}
|
||||
ReportMock.assert_called_with(metrics=[drift_instances[2]], options=report.options)
|
||||
report_instance.run.assert_called_with(reference_data='ref', current_data='cur')
|
||||
run_kwargs = report_instance.run.call_args.kwargs
|
||||
assert run_kwargs['reference_data'] == 'ref'
|
||||
assert run_kwargs['current_data'] == 'cur'
|
||||
assert run_kwargs['column_mapping'].target == 'target'
|
||||
report_instance.save_html.assert_called_with(os.path.join(str(tmp_path), 'data_drift.html'))
|
||||
|
||||
|
||||
@@ -273,10 +307,12 @@ def test_set_color_options_appends(monkeypatch):
|
||||
report = reports.Reports(reference_data='ref', current_data='cur', target_name='target')
|
||||
report.set_color_options(primary_color='#111', secondary_color='#222')
|
||||
|
||||
assert len(report.options) == 2
|
||||
options = report.options
|
||||
assert options is not None
|
||||
assert len(options) == 2
|
||||
assert calls[0]['primary_color'] == '#0F4C81'
|
||||
assert calls[1]['primary_color'] == '#111'
|
||||
assert report.options[1]['secondary_color'] == '#222'
|
||||
assert options[1]['secondary_color'] == '#222'
|
||||
|
||||
|
||||
def test_save_all_sections_html_requires_base_path(stub_color_options):
|
||||
|
||||
@@ -46,7 +46,7 @@ def test_experiment_status_comparison():
|
||||
assert ExperimentStatus.ORCHESTRATOR_VALIDATION_ERROR == 'ORCHESTRATOR_VALIDATION_ERROR'
|
||||
assert ExperimentStatus.ORCHESTRATOR_WAITING_PROC == 'ORCHESTRATOR_WAITING_PROC'
|
||||
assert ExperimentStatus.TRAINING_SUCCESS == 'TRAINING_SUCCESS'
|
||||
assert ExperimentStatus.TRAINING_ERROR != 'TRAINING_SUCCESS'
|
||||
assert str(ExperimentStatus.TRAINING_ERROR) != 'TRAINING_SUCCESS'
|
||||
|
||||
|
||||
def test_experiment_status_access_by_name():
|
||||
|
||||
@@ -476,6 +476,35 @@ def test_generate_report_success(tmp_path):
|
||||
json.load(f)
|
||||
|
||||
|
||||
def test_generate_report_adds_target_alias_for_reports(tmp_path):
|
||||
repo = dmr.DataManagerRepository(MagicMock())
|
||||
p = TrainModelParams.from_dict(_minimal_dict_for_prepare())
|
||||
tmr = TrainModelResult(
|
||||
params=p,
|
||||
train_data=pd.DataFrame({'v1': [1.0, 2.0], 't': [1.0, 2.0]}),
|
||||
val_data=pd.DataFrame({'v1': [1.0, 2.0], 't': [1.0, 2.0]}),
|
||||
y_train_pred=pd.DataFrame({'t': [1.0, 2.0]}),
|
||||
y_pred=pd.DataFrame({'t': [1.0, 2.0]}),
|
||||
run_name='testrun',
|
||||
)
|
||||
|
||||
with (
|
||||
patch.object(repo, '_get_reports_directory', return_value=str(tmp_path)),
|
||||
patch('model_manager.utils.repository.data_manager_repository.Reports') as mrep,
|
||||
):
|
||||
instance = mrep.return_value
|
||||
instance.save_all_sections_html = Mock()
|
||||
repo.generate_report(tmr, {})
|
||||
|
||||
kwargs = mrep.call_args.kwargs
|
||||
reference_data = kwargs['reference_data']
|
||||
current_data = kwargs['current_data']
|
||||
assert 'target' in reference_data.columns
|
||||
assert 'target' in current_data.columns
|
||||
assert reference_data['target'].equals(reference_data['t'])
|
||||
assert current_data['target'].equals(current_data['t'])
|
||||
|
||||
|
||||
def test_generate_report_skips_equation_file_when_not_linear(tmp_path):
|
||||
repo = dmr.DataManagerRepository(MagicMock())
|
||||
p = TrainModelParams.from_dict(_minimal_dict_for_prepare())
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
"""Unit tests for the CleanupFiles workflow."""
|
||||
|
||||
import os
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
@@ -18,8 +17,7 @@ async def test_cleanup_files_workflow(mock_workflow_module):
|
||||
|
||||
# Instantiate and run the workflow
|
||||
workflow_instance = CleanupFiles()
|
||||
with patch.dict(os.environ, {'POD_ID': 'temporal-pod'}):
|
||||
await workflow_instance.run({})
|
||||
await workflow_instance.run({})
|
||||
|
||||
# Verify that the activities were called with the correct parameters
|
||||
calls = mock_workflow_module.execute_activity_method.call_args_list
|
||||
@@ -28,7 +26,5 @@ async def test_cleanup_files_workflow(mock_workflow_module):
|
||||
# Check cleanup_temp_directories call
|
||||
local_call_args = calls[0][0][1]
|
||||
assert local_call_args['temp_path'] == REPORTS_TEMP_DIR
|
||||
assert local_call_args['metadata'] == {
|
||||
'pod_id': 'temporal-pod',
|
||||
'workflow_name': 'cleanup_files',
|
||||
}
|
||||
assert local_call_args['metadata']['workflow_name'] == 'cleanup_files'
|
||||
assert 'pod_id' in local_call_args['metadata']
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
from unittest.mock import AsyncMock, Mock, patch
|
||||
|
||||
import pytest
|
||||
from temporalio.exceptions import ApplicationError
|
||||
|
||||
from model_manager.utils.models.experiment_status import ExperimentStatus
|
||||
from model_manager.utils.models.train_model_params import TrainModelParams
|
||||
@@ -295,7 +296,7 @@ async def test_run_missing_experiment_run_id(mock_wf):
|
||||
from model_manager.workflows.train_model import TrainModel
|
||||
|
||||
mock_wf.logger = Mock()
|
||||
with pytest.raises(ValueError, match='experiment_run_id is required'):
|
||||
with pytest.raises(ApplicationError, match='experiment_run_id is required'):
|
||||
await TrainModel().run({})
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user